Skip to content

Commit

Permalink
Allow to customize Rule severity
Browse files Browse the repository at this point in the history
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: swiftlang#879
  • Loading branch information
bkolb committed Nov 21, 2024
1 parent c7a8b75 commit 13fa278
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 25 deletions.
1 change: 1 addition & 0 deletions Sources/SwiftFormat/API/Configuration+Default.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions Sources/SwiftFormat/API/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -40,6 +45,7 @@ public struct Configuration: Codable, Equatable {
case fileScopedDeclarationPrivacy
case indentSwitchCaseLabels
case rules
case ruleSeverity
case spacesAroundRangeFormationOperators
case noAssignmentInExpressions
case multiElementCollectionTrailingCommas
Expand All @@ -60,6 +66,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

Expand Down Expand Up @@ -280,6 +290,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 {
Expand Down
12 changes: 12 additions & 0 deletions Sources/SwiftFormat/Core/Rule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,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,
Expand All @@ -90,3 +92,13 @@ extension Rule {
notes: notes)
}
}

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
}
}
}
4 changes: 4 additions & 0 deletions Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions Sources/_SwiftFormatTestSupport/DiagnosingTestCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ open class DiagnosingTestCase: XCTestCase {
file: file,
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(
Expand Down
8 changes: 7 additions & 1 deletion Sources/_SwiftFormatTestSupport/FindingSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
}

Expand Down
21 changes: 21 additions & 0 deletions Tests/SwiftFormatTests/API/ConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
#if os(Windows)
let path = #"C:\test.swift"#
Expand Down
12 changes: 6 additions & 6 deletions Tests/SwiftFormatTests/Rules/OmitReturnsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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)
])
}

Expand All @@ -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)
])
}

Expand Down Expand Up @@ -75,8 +75,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),
])
}

Expand Down Expand Up @@ -112,8 +112,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),
])
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,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),
]
)
}
Expand All @@ -37,8 +37,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),
]
)
}
Expand All @@ -64,9 +64,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),
]
)
}
Expand Down Expand Up @@ -108,14 +108,14 @@ 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("5️⃣", message: "rename the actor '_internalActor' using UpperCamelCase; for example, '_InternalActor'"),
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("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'", severity: .convention),
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),
]
)
}
Expand Down

0 comments on commit 13fa278

Please sign in to comment.