From 089812655e87227a2ea7c7afb115cf5ac105e9f8 Mon Sep 17 00:00:00 2001 From: Bernd Kolb Date: Thu, 21 Nov 2024 16:54:09 +0100 Subject: [PATCH] Allow to customize Rule severity In order to customize the severity of rules, I added the possibility to do so via the configuration files. If no severity is specified, we use the one pre-determined by the Rule itself. Example: ``` { "ruleSeverity": { "AlwaysUseLowerCamelCase": "warning", "AmbiguousTrailingClosureOverload": "error", } } ``` Issue: #879 --- .../API/Configuration+Default.swift | 1 + Sources/SwiftFormat/API/Configuration.swift | 13 +++++++ Sources/SwiftFormat/Core/Rule.swift | 12 ++++++ .../Core/RuleBasedFindingCategory.swift | 4 ++ .../DiagnosingTestCase.swift | 12 ++++++ .../_SwiftFormatTestSupport/FindingSpec.swift | 8 +++- .../API/ConfigurationTests.swift | 21 +++++++++++ .../Rules/OmitReturnsTests.swift | 12 +++--- .../TypeNamesShouldBeCapitalizedTests.swift | 37 ++++++++++--------- 9 files changed, 95 insertions(+), 25 deletions(-) diff --git a/Sources/SwiftFormat/API/Configuration+Default.swift b/Sources/SwiftFormat/API/Configuration+Default.swift index 1af06a121..25a77f4cd 100644 --- a/Sources/SwiftFormat/API/Configuration+Default.swift +++ b/Sources/SwiftFormat/API/Configuration+Default.swift @@ -22,6 +22,7 @@ extension Configuration { /// the JSON will be populated from this default configuration. public init() { self.rules = Self.defaultRuleEnablements + self.ruleSeverity = [:] self.maximumBlankLines = 1 self.lineLength = 100 self.tabWidth = 8 diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index 20b491a07..966e29bc1 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -24,6 +24,11 @@ internal let highestSupportedConfigurationVersion = 1 /// Holds the complete set of configured values and defaults. public struct Configuration: Codable, Equatable { + public enum RuleSeverity: String, Codable, CaseIterable, Equatable, Sendable { + case warning = "warning" + case error = "error" + } + private enum CodingKeys: CodingKey { case version case maximumBlankLines @@ -42,6 +47,7 @@ public struct Configuration: Codable, Equatable { case fileScopedDeclarationPrivacy case indentSwitchCaseLabels case rules + case ruleSeverity case spacesAroundRangeFormationOperators case noAssignmentInExpressions case multiElementCollectionTrailingCommas @@ -64,6 +70,10 @@ public struct Configuration: Codable, Equatable { /// marked as `false`, or if it is missing from the dictionary. public var rules: [String: Bool] + /// The dictionary containing the severities for the rule names that we wish to run on. If a rule + /// is not listed here, the default severity is used. + public var ruleSeverity: [String: RuleSeverity] + /// The maximum number of consecutive blank lines that may appear in a file. public var maximumBlankLines: Int @@ -390,6 +400,9 @@ public struct Configuration: Codable, Equatable { self.rules = try container.decodeIfPresent([String: Bool].self, forKey: .rules) ?? defaults.rules + + self.ruleSeverity = + try container.decodeIfPresent([String: RuleSeverity].self, forKey: .ruleSeverity) ?? [:] } public func encode(to encoder: Encoder) throws { diff --git a/Sources/SwiftFormat/Core/Rule.swift b/Sources/SwiftFormat/Core/Rule.swift index 368c7087e..190e93b7e 100644 --- a/Sources/SwiftFormat/Core/Rule.swift +++ b/Sources/SwiftFormat/Core/Rule.swift @@ -86,6 +86,8 @@ extension Rule { syntaxLocation = nil } + let severity: Finding.Severity? = severity ?? context.configuration.findingSeverity(for: type(of: self)) + let category = RuleBasedFindingCategory(ruleType: type(of: self), severity: severity) context.findingEmitter.emit( message, @@ -95,3 +97,13 @@ extension Rule { ) } } + +extension Configuration { + func findingSeverity(for rule: any Rule.Type) -> Finding.Severity? { + guard let severity = self.ruleSeverity[rule.ruleName] else { return nil } + switch severity { + case .warning: return .warning + case .error: return .error + } + } +} diff --git a/Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift b/Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift index a43dade37..42521b236 100644 --- a/Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift +++ b/Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift @@ -24,6 +24,10 @@ struct RuleBasedFindingCategory: FindingCategorizing { var severity: Finding.Severity? + public var defaultSeverity: Finding.Severity { + return severity ?? .warning + } + /// Creates a finding category that wraps the given rule type. init(ruleType: Rule.Type, severity: Finding.Severity? = nil) { self.ruleType = ruleType diff --git a/Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift b/Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift index cb4e07267..ec560fb5f 100644 --- a/Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift +++ b/Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift @@ -145,6 +145,18 @@ open class DiagnosingTestCase: XCTestCase { line: line ) } + + XCTAssertEqual( + matchedFinding.severity, + findingSpec.severity, + """ + Finding emitted at marker '\(findingSpec.marker)' \ + (line:col \(markerLocation.line):\(markerLocation.column), offset \(utf8Offset)) \ + had the wrong severity + """, + file: file, + line: line + ) } private func assertAndRemoveNote( diff --git a/Sources/_SwiftFormatTestSupport/FindingSpec.swift b/Sources/_SwiftFormatTestSupport/FindingSpec.swift index e9751ede4..6691d810c 100644 --- a/Sources/_SwiftFormatTestSupport/FindingSpec.swift +++ b/Sources/_SwiftFormatTestSupport/FindingSpec.swift @@ -10,6 +10,8 @@ // //===----------------------------------------------------------------------===// +import SwiftFormat + /// A description of a `Finding` that can be asserted during tests. public struct FindingSpec { /// The marker that identifies the finding. @@ -21,11 +23,15 @@ public struct FindingSpec { /// A description of a `Note` that should be associated with this finding. public var notes: [NoteSpec] + /// A description of a `Note` that should be associated with this finding. + public var severity: Finding.Severity + /// Creates a new `FindingSpec` with the given values. - public init(_ marker: String = "1️⃣", message: String, notes: [NoteSpec] = []) { + public init(_ marker: String = "1️⃣", message: String, notes: [NoteSpec] = [], severity: Finding.Severity = .warning) { self.marker = marker self.message = message self.notes = notes + self.severity = severity } } diff --git a/Tests/SwiftFormatTests/API/ConfigurationTests.swift b/Tests/SwiftFormatTests/API/ConfigurationTests.swift index 86a9d8dd2..572b30c9a 100644 --- a/Tests/SwiftFormatTests/API/ConfigurationTests.swift +++ b/Tests/SwiftFormatTests/API/ConfigurationTests.swift @@ -18,6 +18,27 @@ final class ConfigurationTests: XCTestCase { XCTAssertEqual(defaultInitConfig, emptyJSONConfig) } + func testSeverityDecoding() { + var config = Configuration() + config.ruleSeverity["AlwaysUseLowerCamelCase"] = .warning + config.ruleSeverity["AmbiguousTrailingClosureOverload"] = .error + + let dictionaryData = + """ + { + "ruleSeverity": { + "AlwaysUseLowerCamelCase": "warning", + "AmbiguousTrailingClosureOverload": "error", + } + } + """.data(using: .utf8)! + let jsonDecoder = JSONDecoder() + let jsonConfig = + try! jsonDecoder.decode(Configuration.self, from: dictionaryData) + + XCTAssertEqual(config, jsonConfig) + } + func testMissingConfigurationFile() throws { #if os(Windows) #if compiler(<6.0.2) diff --git a/Tests/SwiftFormatTests/Rules/OmitReturnsTests.swift b/Tests/SwiftFormatTests/Rules/OmitReturnsTests.swift index ae0d84188..e30bddc64 100644 --- a/Tests/SwiftFormatTests/Rules/OmitReturnsTests.swift +++ b/Tests/SwiftFormatTests/Rules/OmitReturnsTests.swift @@ -16,7 +16,7 @@ final class OmitReturnsTests: LintOrFormatRuleTestCase { } """, findings: [ - FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression") + FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression", severity: .refactoring) ] ) } @@ -35,7 +35,7 @@ final class OmitReturnsTests: LintOrFormatRuleTestCase { } """, findings: [ - FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression") + FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression", severity: .refactoring) ] ) } @@ -76,8 +76,8 @@ final class OmitReturnsTests: LintOrFormatRuleTestCase { } """, findings: [ - FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression"), - FindingSpec("2️⃣", message: "'return' can be omitted because body consists of a single expression"), + FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression", severity: .refactoring), + FindingSpec("2️⃣", message: "'return' can be omitted because body consists of a single expression", severity: .refactoring), ] ) } @@ -114,8 +114,8 @@ final class OmitReturnsTests: LintOrFormatRuleTestCase { } """, findings: [ - FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression"), - FindingSpec("2️⃣", message: "'return' can be omitted because body consists of a single expression"), + FindingSpec("1️⃣", message: "'return' can be omitted because body consists of a single expression", severity: .refactoring), + FindingSpec("2️⃣", message: "'return' can be omitted because body consists of a single expression", severity: .refactoring), ] ) } diff --git a/Tests/SwiftFormatTests/Rules/TypeNamesShouldBeCapitalizedTests.swift b/Tests/SwiftFormatTests/Rules/TypeNamesShouldBeCapitalizedTests.swift index fa30409e4..bd6445668 100644 --- a/Tests/SwiftFormatTests/Rules/TypeNamesShouldBeCapitalizedTests.swift +++ b/Tests/SwiftFormatTests/Rules/TypeNamesShouldBeCapitalizedTests.swift @@ -17,11 +17,11 @@ final class TypeNamesShouldBeCapitalizedTests: LintOrFormatRuleTestCase { } """, findings: [ - FindingSpec("1️⃣", message: "rename the struct 'a' using UpperCamelCase; for example, 'A'"), - FindingSpec("2️⃣", message: "rename the class 'klassName' using UpperCamelCase; for example, 'KlassName'"), - FindingSpec("3️⃣", message: "rename the struct 'subType' using UpperCamelCase; for example, 'SubType'"), - FindingSpec("4️⃣", message: "rename the protocol 'myProtocol' using UpperCamelCase; for example, 'MyProtocol'"), - FindingSpec("5️⃣", message: "rename the struct 'innerType' using UpperCamelCase; for example, 'InnerType'"), + FindingSpec("1️⃣", message: "rename the struct 'a' using UpperCamelCase; for example, 'A'", severity: .convention), + FindingSpec("2️⃣", message: "rename the class 'klassName' using UpperCamelCase; for example, 'KlassName'", severity: .convention), + FindingSpec("3️⃣", message: "rename the struct 'subType' using UpperCamelCase; for example, 'SubType'", severity: .convention), + FindingSpec("4️⃣", message: "rename the protocol 'myProtocol' using UpperCamelCase; for example, 'MyProtocol'", severity: .convention), + FindingSpec("5️⃣", message: "rename the struct 'innerType' using UpperCamelCase; for example, 'InnerType'", severity: .convention), ] ) } @@ -36,8 +36,8 @@ final class TypeNamesShouldBeCapitalizedTests: LintOrFormatRuleTestCase { distributed actor DistGreeter {} """, findings: [ - FindingSpec("1️⃣", message: "rename the actor 'myActor' using UpperCamelCase; for example, 'MyActor'"), - FindingSpec("2️⃣", message: "rename the actor 'greeter' using UpperCamelCase; for example, 'Greeter'"), + FindingSpec("1️⃣", message: "rename the actor 'myActor' using UpperCamelCase; for example, 'MyActor'", severity: .convention), + FindingSpec("2️⃣", message: "rename the actor 'greeter' using UpperCamelCase; for example, 'Greeter'", severity: .convention), ] ) } @@ -63,9 +63,9 @@ final class TypeNamesShouldBeCapitalizedTests: LintOrFormatRuleTestCase { } """, findings: [ - FindingSpec("1️⃣", message: "rename the associated type 'kind' using UpperCamelCase; for example, 'Kind'"), - FindingSpec("2️⃣", message: "rename the type alias 'x' using UpperCamelCase; for example, 'X'"), - FindingSpec("3️⃣", message: "rename the type alias 'data' using UpperCamelCase; for example, 'Data'"), + FindingSpec("1️⃣", message: "rename the associated type 'kind' using UpperCamelCase; for example, 'Kind'", severity: .convention), + FindingSpec("2️⃣", message: "rename the type alias 'x' using UpperCamelCase; for example, 'X'", severity: .convention), + FindingSpec("3️⃣", message: "rename the type alias 'data' using UpperCamelCase; for example, 'Data'", severity: .convention), ] ) } @@ -107,17 +107,18 @@ final class TypeNamesShouldBeCapitalizedTests: LintOrFormatRuleTestCase { distributed actor __InternalGreeter {} """, findings: [ - FindingSpec("1️⃣", message: "rename the protocol '_p' using UpperCamelCase; for example, '_P'"), - FindingSpec("2️⃣", message: "rename the associated type '_value' using UpperCamelCase; for example, '_Value'"), - FindingSpec("3️⃣", message: "rename the struct '_data' using UpperCamelCase; for example, '_Data'"), - FindingSpec("4️⃣", message: "rename the type alias '_x' using UpperCamelCase; for example, '_X'"), + FindingSpec("1️⃣", message: "rename the protocol '_p' using UpperCamelCase; for example, '_P'", severity: .convention), + FindingSpec("2️⃣", message: "rename the associated type '_value' using UpperCamelCase; for example, '_Value'", severity: .convention), + FindingSpec("3️⃣", message: "rename the struct '_data' using UpperCamelCase; for example, '_Data'", severity: .convention), + FindingSpec("4️⃣", message: "rename the type alias '_x' using UpperCamelCase; for example, '_X'", severity: .convention), FindingSpec( "5️⃣", - message: "rename the actor '_internalActor' using UpperCamelCase; for example, '_InternalActor'" + message: "rename the actor '_internalActor' using UpperCamelCase; for example, '_InternalActor'", + severity: .convention ), - FindingSpec("6️⃣", message: "rename the enum '__e' using UpperCamelCase; for example, '__E'"), - FindingSpec("7️⃣", message: "rename the class '_myClass' using UpperCamelCase; for example, '_MyClass'"), - FindingSpec("8️⃣", message: "rename the actor '__greeter' using UpperCamelCase; for example, '__Greeter'"), + FindingSpec("6️⃣", message: "rename the enum '__e' using UpperCamelCase; for example, '__E'", severity: .convention), + FindingSpec("7️⃣", message: "rename the class '_myClass' using UpperCamelCase; for example, '_MyClass'", severity: .convention), + FindingSpec("8️⃣", message: "rename the actor '__greeter' using UpperCamelCase; for example, '__Greeter'", severity: .convention), ] ) }