Skip to content

Commit

Permalink
Output a warning for unknown config rules
Browse files Browse the repository at this point in the history
**Summary**:
- Helps developers surface any errors and typos in their `.swift-format`
  config files by showing a warning when running `swift-format` if the
  configuration includes invalid rules.
- The actual check is in `Frontend.swift`.
- The dictionary of rules for verification is in `RuleRegistry`, which
  is made `public` to be able to import it in `swift-format` target.

**Motivation**:
- Closes swiftlang#607
  • Loading branch information
natikgadzhi committed Sep 4, 2023
1 parent 0ebb3c8 commit cad7b65
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 5 deletions.
4 changes: 2 additions & 2 deletions Sources/SwiftFormatConfiguration/RuleRegistry+Generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

// This file is automatically generated with generate-pipeline. Do Not Edit!

enum RuleRegistry {
static let rules: [String: Bool] = [
public enum RuleRegistry {
public static let rules: [String: Bool] = [
"AllPublicDeclarationsHaveDocumentation": false,
"AlwaysUseLowerCamelCase": true,
"AmbiguousTrailingClosureOverload": true,
Expand Down
4 changes: 2 additions & 2 deletions Sources/generate-pipeline/RuleRegistryGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ final class RuleRegistryGenerator: FileGenerator {
// This file is automatically generated with generate-pipeline. Do Not Edit!
enum RuleRegistry {
static let rules: [String: Bool] = [
publc enum RuleRegistry {
public static let rules: [String: Bool] = [
"""
)
Expand Down
17 changes: 16 additions & 1 deletion Sources/swift-format/Frontend/Frontend.swift
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ class Frontend {
// loaded. (Do not try to fall back to a path inferred from the source file path.)
if let configurationFileURL = configurationFileURL {
do {
return try configurationLoader.configuration(at: configurationFileURL)
let configuration = try configurationLoader.configuration(at: configurationFileURL)
self.verifyConfigurationRules(for: configuration)
return configuration
} catch {
diagnosticsEngine.emitError("Unable to read configuration: \(error.localizedDescription)")
return nil
Expand All @@ -193,6 +195,7 @@ class Frontend {
if let swiftFileURL = swiftFileURL {
do {
if let configuration = try configurationLoader.configuration(forSwiftFileAt: swiftFileURL) {
self.verifyConfigurationRules(for: configuration)
return configuration
}
// Fall through to the default return at the end of the function.
Expand All @@ -208,4 +211,16 @@ class Frontend {
// default configuration.
return Configuration()
}

/// Checks if all the rules in the given configuration are supported by the registry.
/// If there are any rules that are not supported, they are emitted as a warning.
private func verifyConfigurationRules(for configuration: Configuration) -> Void {
// If any rules in the decoded configuration are not supported by the registry,
// emit them into the diagnosticsEngine as warnings.
// That way they will be printed out, but we'll continue execution on the valid rules.
let invalidRules = configuration.rules.filter { !RuleRegistry.rules.keys.contains($0.key) }
if !invalidRules.isEmpty {
diagnosticsEngine.emitWarning("Configuration contains an unsupported rule: \(invalidRules.keys.joined(separator: ", "))", location: nil)
}
}
}
14 changes: 14 additions & 0 deletions Sources/swift-format/Utilities/DiagnosticsEngine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ final class DiagnosticsEngine {
message: message))
}

/// Emits a generic warning message.
///
/// - Parameters:
/// - message: The message associated with the error.
/// - location: The location in the source code associated with the error, or nil if there is no
/// location associated with the error.
func emitWarning(_ message: String, location: SourceLocation? = nil) {
emit(
Diagnostic(
severity: .warning,
location: location.map(Diagnostic.Location.init),
message: message))
}

/// Emits a finding from the linter and any of its associated notes as diagnostics.
///
/// - Parameter finding: The finding that should be emitted.
Expand Down

0 comments on commit cad7b65

Please sign in to comment.