Skip to content

Output a warning for unknown configuration rules in .swift-format #612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@allevato, @_spi is done here.

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