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 #607

Apply suggestions from code review

Co-authored-by: Tony Allevato <tony.allevato@gmail.com>

Cleaning up based on PR review
  • Loading branch information
natikgadzhi committed Sep 6, 2023
1 parent b50fd28 commit 5d45493
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 7 deletions.
4 changes: 2 additions & 2 deletions Sources/SwiftFormat/Core/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] = [
@_spi(Internal) 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] = [
@_spi(Internal) public enum RuleRegistry {
public static let rules: [String: Bool] = [
"""
)
Expand Down
23 changes: 20 additions & 3 deletions Sources/swift-format/Frontend/Frontend.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//===----------------------------------------------------------------------===//

import Foundation
import SwiftFormat
@_spi(Internal) import SwiftFormat
import SwiftSyntax
import SwiftParser

Expand Down Expand Up @@ -161,17 +161,19 @@ class Frontend {
}

/// Returns the configuration that applies to the given `.swift` source file, when an explicit
/// configuration path is also perhaps provided.
/// configuration path is also perhaps provided. Checks for unrecognized rules within the configuration.
///
/// - Parameters:
/// - configurationFilePath: The path to a configuration file that will be loaded, or `nil` to
/// try to infer it from `swiftFilePath`.
/// - swiftFilePath: The path to a `.swift` file, which will be used to infer the path to the
/// configuration file if `configurationFilePath` is nil.
///
/// - Returns: If successful, the returned configuration is the one loaded from
/// `configurationFilePath` if it was provided, or by searching in paths inferred by
/// `swiftFilePath` if one exists, or the default configuration otherwise. If an error occurred
/// when reading the configuration, a diagnostic is emitted and `nil` is returned.
/// if neither `configurationFilePath` nor `swiftFilePath` were provided, a default `Configuration()` will be returned.
private func configuration(
at configurationFileURL: URL?,
orInferredFromSwiftFileAt swiftFileURL: URL?
Expand All @@ -180,7 +182,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.checkForUnrecognizedRules(in: configuration)
return configuration
} catch {
diagnosticsEngine.emitError("Unable to read configuration: \(error.localizedDescription)")
return nil
Expand All @@ -192,6 +196,7 @@ class Frontend {
if let swiftFileURL = swiftFileURL {
do {
if let configuration = try configurationLoader.configuration(forSwiftFileAt: swiftFileURL) {
self.checkForUnrecognizedRules(in: configuration)
return configuration
}
// Fall through to the default return at the end of the function.
Expand All @@ -207,4 +212,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 checkForUnrecognizedRules(in configuration: Configuration) {
// 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) }
for rule in invalidRules {
diagnosticsEngine.emitWarning("Configuration contains an unrecognized rule: \(rule.key)", 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 5d45493

Please sign in to comment.