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:
```
{
    "rules": {
        "AlwaysUseLowerCamelCase": "warning",
        "AmbiguousTrailingClosureOverload": "error",
        "UseLetInEveryBoundCaseVariable": "true", // use rule default
        "UseWhereClausesInForLoops": "false", // disabled
    }
}
```

In addition, one can now control how pretty-print violations
should be treated in the same way

Example:
```
{
    "rules": {
        "TrailingComma": "warning",
        "LineLength": "error",
        "Indentation": "true", // use rule default
        "TrailingWhitespace": "false", // disabled
    }
}
```

Issue: swiftlang#879
  • Loading branch information
bkolb committed Nov 25, 2024
1 parent 080535c commit 1f8e4f0
Show file tree
Hide file tree
Showing 24 changed files with 431 additions and 124 deletions.
2 changes: 1 addition & 1 deletion Documentation/RuleDocumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

Use the rules below in the `rules` block of your `.swift-format`
configuration file, as described in
[Configuration](Configuration.md). All of these rules can be
[Configuration](Documentation/Configuration.md). All of these rules can be
applied in the linter, but only some of them can format your source code
automatically.

Expand Down
1 change: 0 additions & 1 deletion Sources/SwiftFormat/API/Configuration+Default.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ 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
21 changes: 10 additions & 11 deletions Sources/SwiftFormat/API/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public struct Configuration: Codable, Equatable {
public enum RuleSeverity: String, Codable, CaseIterable, Equatable, Sendable {
case warning = "warning"
case error = "error"
case ruleDefault = "true"
case disabled = "false"
}

private enum CodingKeys: CodingKey {
Expand Down Expand Up @@ -59,7 +61,7 @@ public struct Configuration: Codable, Equatable {
/// names.
///
/// This value is generated by `generate-swift-format` based on the `isOptIn` value of each rule.
public static let defaultRuleEnablements: [String: Bool] = RuleRegistry.rules
public static let defaultRuleEnablements: [String: Configuration.RuleSeverity] = RuleRegistry.rules

/// The version of this configuration.
private var version: Int = highestSupportedConfigurationVersion
Expand All @@ -68,11 +70,7 @@ public struct Configuration: Codable, Equatable {

/// The dictionary containing the rule names that we wish to run on. A rule is not used if it is
/// 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]
public var rules: [String: Configuration.RuleSeverity]

/// The maximum number of consecutive blank lines that may appear in a file.
public var maximumBlankLines: Int
Expand Down Expand Up @@ -398,11 +396,8 @@ public struct Configuration: Codable, Equatable {
// default-initialized. To get an empty rules dictionary, one can explicitly
// set the `rules` key to `{}`.
self.rules =
try container.decodeIfPresent([String: Bool].self, forKey: .rules)
try container.decodeIfPresent([String: Configuration.RuleSeverity].self, forKey: .rules)
?? defaults.rules

self.ruleSeverity =
try container.decodeIfPresent([String: RuleSeverity].self, forKey: .ruleSeverity) ?? [:]
}

public func encode(to encoder: Encoder) throws {
Expand Down Expand Up @@ -515,10 +510,14 @@ fileprivate extension URL {
}

extension Configuration.RuleSeverity {
var findingSeverity: Finding.Severity {
func findingSeverity(ruleDefault: Finding.Severity) -> Finding.Severity {
switch self {
case .warning: return .warning
case .error: return .error
case .ruleDefault:
return ruleDefault
case .disabled:
return .disabled
}
}
}
1 change: 1 addition & 0 deletions Sources/SwiftFormat/API/Finding.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public struct Finding {
case error
case refactoring
case convention
case disabled
}

/// The file path and location in that file where a finding was encountered.
Expand Down
12 changes: 1 addition & 11 deletions Sources/SwiftFormat/API/FindingCategorizing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,9 @@ public protocol FindingCategorizing: CustomStringConvertible {
///
/// By default, all findings are warnings. Individual categories or configuration may choose to override this to
/// make the findings in those categories more severe.
func severity(configuration: Configuration) -> Finding.Severity
var severity: Finding.Severity { get }

/// The name of the category.
var name: String {get}
}

extension FindingCategorizing {
func severity(configuration: Configuration) -> Finding.Severity {
return severityFromConfig(configuration: configuration)
}

func severityFromConfig(configuration: Configuration) -> Finding.Severity {
guard let customSeverity = configuration.ruleSeverity[self.name] else { return .warning }
return customSeverity.findingSeverity
}
}
7 changes: 6 additions & 1 deletion Sources/SwiftFormat/Core/Context.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ public final class Context {
let ruleName = ruleNameCache[ObjectIdentifier(rule)] ?? R.ruleName
switch ruleMask.ruleState(ruleName, at: loc) {
case .default:
return configuration.rules[ruleName] ?? false
guard let configSeverity = configuration.rules[ruleName] else { return false }
if case .disabled = configSeverity {
return false
} else {
return true
}
case .disabled:
return false
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftFormat/Core/FindingEmitter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ final class FindingEmitter {
Finding(
category: category,
message: message,
severity: category.severity(configuration: context.configuration),
severity: category.severity,
location: location,
notes: notes
)
Expand Down
8 changes: 4 additions & 4 deletions Sources/SwiftFormat/Core/Rule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ extension Rule {
syntaxLocation = nil
}

let severity: Finding.Severity? = severity ?? context.configuration.findingSeverity(for: type(of: self))
let severity: Finding.Severity = severity ?? context.configuration.findingSeverity(for: type(of: self), defaultSeverity: .warning)

let category = RuleBasedFindingCategory(ruleType: type(of: self), severity: severity)
context.findingEmitter.emit(
Expand All @@ -100,8 +100,8 @@ extension Rule {
}

extension Configuration {
func findingSeverity(for rule: any Rule.Type) -> Finding.Severity? {
guard let severity = self.ruleSeverity[rule.ruleName] else { return nil }
return severity.findingSeverity
func findingSeverity(for rule: any Rule.Type, defaultSeverity: Finding.Severity) -> Finding.Severity {
guard let severity = self.rules[rule.ruleName] else { return defaultSeverity }
return severity.findingSeverity(ruleDefault: defaultSeverity)
}
}
11 changes: 2 additions & 9 deletions Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,15 @@ struct RuleBasedFindingCategory: FindingCategorizing {

var description: String { ruleType.ruleName }

var severity: Finding.Severity?
var severity: Finding.Severity

var name: String {
return description
}

/// Creates a finding category that wraps the given rule type.
init(ruleType: Rule.Type, severity: Finding.Severity? = nil) {
init(ruleType: Rule.Type, severity: Finding.Severity) {
self.ruleType = ruleType
self.severity = severity
}

func severity(configuration: Configuration) -> Finding.Severity {
if let severity = severity {
return severity
}
return severityFromConfig(configuration: configuration)
}
}
97 changes: 53 additions & 44 deletions Sources/SwiftFormat/Core/RuleRegistry+Generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,49 +13,58 @@
// This file is automatically generated with generate-swift-format. Do not edit!

@_spi(Internal) public enum RuleRegistry {
public static let rules: [String: Bool] = [
"AllPublicDeclarationsHaveDocumentation": false,
"AlwaysUseLiteralForEmptyCollectionInit": false,
"AlwaysUseLowerCamelCase": true,
"AmbiguousTrailingClosureOverload": true,
"AvoidRetroactiveConformances": true,
"BeginDocumentationCommentWithOneLineSummary": false,
"DoNotUseSemicolons": true,
"DontRepeatTypeInStaticProperties": true,
"FileScopedDeclarationPrivacy": true,
"FullyIndirectEnum": true,
"GroupNumericLiterals": true,
"IdentifiersMustBeASCII": true,
"NeverForceUnwrap": false,
"NeverUseForceTry": false,
"NeverUseImplicitlyUnwrappedOptionals": false,
"NoAccessLevelOnExtensionDeclaration": true,
"NoAssignmentInExpressions": true,
"NoBlockComments": true,
"NoCasesWithOnlyFallthrough": true,
"NoEmptyLinesOpeningClosingBraces": false,
"NoEmptyTrailingClosureParentheses": true,
"NoLabelsInCasePatterns": true,
"NoLeadingUnderscores": false,
"NoParensAroundConditions": true,
"NoPlaygroundLiterals": true,
"NoVoidReturnOnFunctionSignature": true,
"OmitExplicitReturns": false,
"OneCasePerLine": true,
"OneVariableDeclarationPerLine": true,
"OnlyOneTrailingClosureArgument": true,
"OrderedImports": true,
"ReplaceForEachWithForLoop": true,
"ReturnVoidInsteadOfEmptyTuple": true,
"TypeNamesShouldBeCapitalized": true,
"UseEarlyExits": false,
"UseExplicitNilCheckInConditions": true,
"UseLetInEveryBoundCaseVariable": true,
"UseShorthandTypeNames": true,
"UseSingleLinePropertyGetter": true,
"UseSynthesizedInitializer": true,
"UseTripleSlashForDocumentationComments": true,
"UseWhereClausesInForLoops": false,
"ValidateDocumentationComments": false,
public static let rules: [String: Configuration.RuleSeverity] = [
"AllPublicDeclarationsHaveDocumentation": .disabled,
"AlwaysUseLiteralForEmptyCollectionInit": .disabled,
"AlwaysUseLowerCamelCase": .ruleDefault,
"AmbiguousTrailingClosureOverload": .ruleDefault,
"AvoidRetroactiveConformances": .ruleDefault,
"BeginDocumentationCommentWithOneLineSummary": .disabled,
"DoNotUseSemicolons": .ruleDefault,
"DontRepeatTypeInStaticProperties": .ruleDefault,
"FileScopedDeclarationPrivacy": .ruleDefault,
"FullyIndirectEnum": .ruleDefault,
"GroupNumericLiterals": .ruleDefault,
"IdentifiersMustBeASCII": .ruleDefault,
"NeverForceUnwrap": .disabled,
"NeverUseForceTry": .disabled,
"NeverUseImplicitlyUnwrappedOptionals": .disabled,
"NoAccessLevelOnExtensionDeclaration": .ruleDefault,
"NoAssignmentInExpressions": .ruleDefault,
"NoBlockComments": .ruleDefault,
"NoCasesWithOnlyFallthrough": .ruleDefault,
"NoEmptyLinesOpeningClosingBraces": .disabled,
"NoEmptyTrailingClosureParentheses": .ruleDefault,
"NoLabelsInCasePatterns": .ruleDefault,
"NoLeadingUnderscores": .disabled,
"NoParensAroundConditions": .ruleDefault,
"NoPlaygroundLiterals": .ruleDefault,
"NoVoidReturnOnFunctionSignature": .ruleDefault,
"OmitExplicitReturns": .disabled,
"OneCasePerLine": .ruleDefault,
"OneVariableDeclarationPerLine": .ruleDefault,
"OnlyOneTrailingClosureArgument": .ruleDefault,
"OrderedImports": .ruleDefault,
"ReplaceForEachWithForLoop": .ruleDefault,
"ReturnVoidInsteadOfEmptyTuple": .ruleDefault,
"TypeNamesShouldBeCapitalized": .ruleDefault,
"UseEarlyExits": .disabled,
"UseExplicitNilCheckInConditions": .ruleDefault,
"UseLetInEveryBoundCaseVariable": .ruleDefault,
"UseShorthandTypeNames": .ruleDefault,
"UseSingleLinePropertyGetter": .ruleDefault,
"UseSynthesizedInitializer": .ruleDefault,
"UseTripleSlashForDocumentationComments": .ruleDefault,
"UseWhereClausesInForLoops": .disabled,
"ValidateDocumentationComments": .disabled,
"AddLines": .ruleDefault,
"EndOfLineComment": .ruleDefault,
"Indentation": .ruleDefault,
"LineLength": .ruleDefault,
"RemoveLine": .ruleDefault,
"Spacing": .ruleDefault,
"SpacingCharacter": .ruleDefault,
"TrailingComma": .ruleDefault,
"TrailingWhitespace": .ruleDefault,
]
}
15 changes: 12 additions & 3 deletions Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,11 @@ public class PrettyPrinter {

if wasEndOfLine {
if !(canFit(comment.length) || isBreakingSuppressed) {
diagnose(.moveEndOfLineComment, category: .endOfLineComment)
let category: PrettyPrintFindingCategory = .endOfLineComment()
let severity = configuration
.rules[category.name]?
.findingSeverity(ruleDefault: category.severity) ?? category.severity
diagnose(.moveEndOfLineComment, category: .endOfLineComment(severity))
}
}
outputBuffer.write(comment.print(indent: currentIndentation))
Expand Down Expand Up @@ -514,10 +518,14 @@ public class PrettyPrinter {
let shouldHaveTrailingComma =
startLineNumber != openCloseBreakCompensatingLineNumber && !isSingleElement
&& configuration.multiElementCollectionTrailingCommas
let category: PrettyPrintFindingCategory = .trailingComma()
let severity = configuration
.rules[category.name]?
.findingSeverity(ruleDefault: category.severity) ?? category.severity
if shouldHaveTrailingComma && !hasTrailingComma {
diagnose(.addTrailingComma, category: .trailingComma)
diagnose(.addTrailingComma, category: .trailingComma(severity))
} else if !shouldHaveTrailingComma && hasTrailingComma {
diagnose(.removeTrailingComma, category: .trailingComma)
diagnose(.removeTrailingComma, category: .trailingComma(severity))
}

let shouldWriteComma = whitespaceOnly ? hasTrailingComma : shouldHaveTrailingComma
Expand Down Expand Up @@ -814,6 +822,7 @@ public class PrettyPrinter {

/// Emits a finding with the given message and category at the current location in `outputBuffer`.
private func diagnose(_ message: Finding.Message, category: PrettyPrintFindingCategory) {
if case .disabled = category.severity { return }
// Add 1 since columns uses 1-based indices.
let column = outputBuffer.column + 1
context.findingEmitter.emit(
Expand Down
11 changes: 9 additions & 2 deletions Sources/SwiftFormat/PrettyPrint/PrettyPrintFindingCategory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
enum PrettyPrintFindingCategory: FindingCategorizing {

/// Finding related to an end-of-line comment.
case endOfLineComment
case endOfLineComment(Finding.Severity = .warning)

/// Findings related to the presence of absence of a trailing comma in collection literals.
case trailingComma
case trailingComma(Finding.Severity = .warning)

var description: String {
switch self {
Expand All @@ -30,4 +30,11 @@ enum PrettyPrintFindingCategory: FindingCategorizing {
self.description
}

var severity: Finding.Severity {
switch self {
case .endOfLineComment(let severity): return severity
case .trailingComma(let severity): return severity
}
}

}
26 changes: 19 additions & 7 deletions Sources/SwiftFormat/PrettyPrint/WhitespaceFindingCategory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,25 @@
/// Categories for findings emitted by the whitespace linter.
enum WhitespaceFindingCategory: FindingCategorizing {
/// Findings related to trailing whitespace on a line.
case trailingWhitespace
case trailingWhitespace(Finding.Severity = .warning)

/// Findings related to indentation (i.e., whitespace at the beginning of a line).
case indentation
case indentation(Finding.Severity = .warning)

/// Findings related to interior whitespace (i.e., neither leading nor trailing space).
case spacing
case spacing(Finding.Severity = .warning)

/// Findings related to specific characters used for interior whitespace.
case spacingCharacter
case spacingCharacter(Finding.Severity = .warning)

/// Findings related to the removal of line breaks.
case removeLine
case removeLine(Finding.Severity = .warning)

/// Findings related to the addition of line breaks.
case addLines
case addLines(Finding.Severity = .warning)

/// Findings related to the length of a line.
case lineLength
case lineLength(Finding.Severity = .warning)

var description: String {
switch self {
Expand All @@ -48,4 +48,16 @@ enum WhitespaceFindingCategory: FindingCategorizing {
var name: String {
return self.description
}

var severity: Finding.Severity {
switch self {
case .trailingWhitespace(let severity): return severity
case .indentation(let severity): return severity
case .spacing(let severity): return severity
case .spacingCharacter(let severity): return severity
case .removeLine(let severity): return severity
case .addLines(let severity): return severity
case .lineLength(let severity): return severity
}
}
}
Loading

0 comments on commit 1f8e4f0

Please sign in to comment.