Skip to content

Commit 93edf59

Browse files
committed
Remove severity
Instead of making the severity configurable, this patch removes severity all together and treats every finding as an error. Issue: swiftlang#879
1 parent eeb2850 commit 93edf59

27 files changed

+307
-136
lines changed

.gitignore

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
.swiftpm/
33
swift-format.xcodeproj/
44
Package.resolved
5-
5+
.index-build

Documentation/RuleDocumentation.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

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

Package.swift

+1
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ var targets: [Target] = [
113113
dependencies: [
114114
"SwiftFormat",
115115
"_SwiftFormatTestSupport",
116+
"swift-format",
116117
.product(name: "Markdown", package: "swift-markdown"),
117118
] + swiftSyntaxDependencies(["SwiftOperators", "SwiftParser", "SwiftSyntax", "SwiftSyntaxBuilder"])
118119
),

Sources/SwiftFormat/API/Finding.swift

-12
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,6 @@
1212

1313
/// A problem with the style or syntax of the source code discovered during linting or formatting.
1414
public struct Finding {
15-
/// The severity of a finding.
16-
public enum Severity {
17-
case warning
18-
case error
19-
case refactoring
20-
case convention
21-
}
2215

2316
/// The file path and location in that file where a finding was encountered.
2417
public struct Location {
@@ -83,9 +76,6 @@ public struct Finding {
8376
/// The finding's message.
8477
public let message: Message
8578

86-
/// The severity of the finding.
87-
public let severity: Severity
88-
8979
/// The optional location of the finding.
9080
public let location: Location?
9181

@@ -97,13 +87,11 @@ public struct Finding {
9787
init(
9888
category: FindingCategorizing,
9989
message: Message,
100-
severity: Finding.Severity,
10190
location: Location? = nil,
10291
notes: [Note] = []
10392
) {
10493
self.category = category
10594
self.message = message
106-
self.severity = severity
10795
self.location = location
10896
self.notes = notes
10997
}

Sources/SwiftFormat/API/FindingCategorizing.swift

+2-8
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,7 @@
1717
/// to be displayed as part of the diagnostic message when the finding is presented to the user.
1818
/// For example, the category `Indentation` in the message `[Indentation] Indent by 2 spaces`.
1919
public protocol FindingCategorizing: CustomStringConvertible {
20-
/// The default severity of findings emitted in this category.
21-
///
22-
/// By default, all findings are warnings. Individual categories may choose to override this to
23-
/// make the findings in those categories more severe.
24-
var defaultSeverity: Finding.Severity { get }
25-
}
2620

27-
extension FindingCategorizing {
28-
public var defaultSeverity: Finding.Severity { .warning }
21+
/// The name of the category.
22+
var name: String { get }
2923
}

Sources/SwiftFormat/Core/FindingEmitter.swift

-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ final class FindingEmitter {
5454
Finding(
5555
category: category,
5656
message: message,
57-
severity: category.defaultSeverity,
5857
location: location,
5958
notes: notes
6059
)

Sources/SwiftFormat/Core/Rule.swift

+1-2
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ extension Rule {
6262
public func diagnose<SyntaxType: SyntaxProtocol>(
6363
_ message: Finding.Message,
6464
on node: SyntaxType?,
65-
severity: Finding.Severity? = nil,
6665
anchor: FindingAnchor = .start,
6766
notes: [Finding.Note] = []
6867
) {
@@ -86,7 +85,7 @@ extension Rule {
8685
syntaxLocation = nil
8786
}
8887

89-
let category = RuleBasedFindingCategory(ruleType: type(of: self), severity: severity)
88+
let category = RuleBasedFindingCategory(ruleType: type(of: self))
9089
context.findingEmitter.emit(
9190
message,
9291
category: category,

Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ struct RuleBasedFindingCategory: FindingCategorizing {
2222

2323
var description: String { ruleType.ruleName }
2424

25-
var severity: Finding.Severity?
25+
var name: String {
26+
return description
27+
}
2628

2729
/// Creates a finding category that wraps the given rule type.
28-
init(ruleType: Rule.Type, severity: Finding.Severity? = nil) {
30+
init(ruleType: Rule.Type) {
2931
self.ruleType = ruleType
30-
self.severity = severity
3132
}
3233
}

Sources/SwiftFormat/Core/RuleRegistry+Generated.swift

+9
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,14 @@
5757
"UseTripleSlashForDocumentationComments": true,
5858
"UseWhereClausesInForLoops": false,
5959
"ValidateDocumentationComments": false,
60+
"AddLines": true,
61+
"EndOfLineComment": true,
62+
"Indentation": true,
63+
"LineLength": true,
64+
"RemoveLine": true,
65+
"Spacing": true,
66+
"SpacingCharacter": true,
67+
"TrailingComma": true,
68+
"TrailingWhitespace": true,
6069
]
6170
}

Sources/SwiftFormat/PrettyPrint/PrettyPrintFindingCategory.swift

+5
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,9 @@ enum PrettyPrintFindingCategory: FindingCategorizing {
2424
case .trailingComma: return "TrailingComma"
2525
}
2626
}
27+
28+
var name: String {
29+
self.description
30+
}
31+
2732
}

Sources/SwiftFormat/PrettyPrint/WhitespaceFindingCategory.swift

+4
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,8 @@ enum WhitespaceFindingCategory: FindingCategorizing {
4444
case .lineLength: return "LineLength"
4545
}
4646
}
47+
48+
var name: String {
49+
return self.description
50+
}
4751
}

Sources/SwiftFormat/Rules/OmitExplicitReturns.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
3434
}
3535

3636
funcDecl.body?.statements = rewrapReturnedExpression(returnStmt)
37-
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
37+
diagnose(.omitReturnStatement, on: returnStmt)
3838
return DeclSyntax(funcDecl)
3939
}
4040

@@ -78,7 +78,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
7878
}
7979

8080
closureExpr.statements = rewrapReturnedExpression(returnStmt)
81-
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
81+
diagnose(.omitReturnStatement, on: returnStmt)
8282
return ExprSyntax(closureExpr)
8383
}
8484

@@ -111,7 +111,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
111111

112112
getter.body?.statements = rewrapReturnedExpression(returnStmt)
113113

114-
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
114+
diagnose(.omitReturnStatement, on: returnStmt)
115115

116116
accessors[getterAt] = getter
117117
var newBlock = accessorBlock
@@ -123,7 +123,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
123123
return nil
124124
}
125125

126-
diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
126+
diagnose(.omitReturnStatement, on: returnStmt)
127127

128128
var newBlock = accessorBlock
129129
newBlock.accessors = .getter(rewrapReturnedExpression(returnStmt))

Sources/SwiftFormat/Rules/TypeNamesShouldBeCapitalized.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public final class TypeNamesShouldBeCapitalized: SyntaxLintRule {
6161
if let firstChar = name.text[leadingUnderscores.endIndex...].first,
6262
firstChar.uppercased() != String(firstChar)
6363
{
64-
diagnose(.capitalizeTypeName(name: name.text, kind: kind), on: name, severity: .convention)
64+
diagnose(.capitalizeTypeName(name: name.text, kind: kind), on: name)
6565
}
6666
}
6767
}

Sources/_SwiftFormatTestSupport/Configuration+Testing.swift

+20
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,24 @@ extension Configuration {
4444
config.indentBlankLines = false
4545
return config
4646
}
47+
48+
public static func forTesting(enabledRule: String) -> Configuration {
49+
var config = Configuration.forTesting.disableAllRules()
50+
config.rules[enabledRule] = true
51+
return config
52+
}
53+
}
54+
55+
extension Configuration {
56+
public func disableAllRules() -> Self {
57+
var config = self
58+
config.rules = config.rules.mapValues({ _ in false })
59+
return config
60+
}
61+
62+
public func enable(_ rule: String, enabled: Bool) -> Self {
63+
var config = self
64+
config.rules[rule] = enabled
65+
return config
66+
}
4767
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import Foundation
14+
@_spi(Rules) import SwiftFormat
15+
import SwiftParser
16+
import SwiftSyntax
17+
18+
/// Collects information about rules in the formatter code base.
19+
final class PrettyPrintCollector {
20+
21+
/// A list of all the format-only pretty-print categories found in the code base.
22+
var allPrettyPrinterCategories = Set<String>()
23+
24+
/// Populates the internal collections with rules in the given directory.
25+
///
26+
/// - Parameter url: The file system URL that should be scanned for rules.
27+
func collect(from url: URL) throws {
28+
// For each file in the Rules directory, find types that either conform to SyntaxLintRule or
29+
// inherit from SyntaxFormatRule.
30+
let fm = FileManager.default
31+
guard let rulesEnumerator = fm.enumerator(atPath: url.path) else {
32+
fatalError("Could not list the directory \(url.path)")
33+
}
34+
35+
for baseName in rulesEnumerator {
36+
// Ignore files that aren't Swift source files.
37+
guard let baseName = baseName as? String, baseName.hasSuffix(".swift") else { continue }
38+
39+
let fileURL = url.appendingPathComponent(baseName)
40+
let fileInput = try String(contentsOf: fileURL)
41+
let sourceFile = Parser.parse(source: fileInput)
42+
43+
for statement in sourceFile.statements {
44+
let pp = self.detectPrettyPrintCategories(at: statement)
45+
allPrettyPrinterCategories.formUnion(pp)
46+
}
47+
}
48+
}
49+
50+
private func detectPrettyPrintCategories(at statement: CodeBlockItemSyntax) -> [String] {
51+
guard let enumDecl = statement.item.as(EnumDeclSyntax.self) else {
52+
return []
53+
}
54+
55+
if enumDecl.name.text == "PrettyPrintFindingCategory" {
56+
print("HIT")
57+
}
58+
59+
// Make sure it has an inheritance clause.
60+
guard let inheritanceClause = enumDecl.inheritanceClause else {
61+
return []
62+
}
63+
64+
// Scan through the inheritance clause to find one of the protocols/types we're interested in.
65+
for inheritance in inheritanceClause.inheritedTypes {
66+
guard let identifier = inheritance.type.as(IdentifierTypeSyntax.self) else {
67+
continue
68+
}
69+
70+
if identifier.name.text != "FindingCategorizing" {
71+
// Keep looking at the other inheritances.
72+
continue
73+
}
74+
75+
// Now that we know it's a pretty printing category, collect the `description` method and extract the name.
76+
for member in enumDecl.memberBlock.members {
77+
guard let varDecl = member.decl.as(VariableDeclSyntax.self) else { continue }
78+
guard
79+
let descriptionDecl = varDecl.bindings
80+
.first(where: {
81+
$0.pattern.as(IdentifierPatternSyntax.self)?.identifier.text == "description"
82+
})
83+
else { continue }
84+
let pp = PrettyPrintCategoryVisitor(viewMode: .sourceAccurate)
85+
_ = pp.walk(descriptionDecl)
86+
return pp.prettyPrintCategories
87+
}
88+
}
89+
90+
return []
91+
}
92+
}
93+
94+
final class PrettyPrintCategoryVisitor: SyntaxVisitor {
95+
96+
var prettyPrintCategories: [String] = []
97+
98+
override func visit(_ node: StringSegmentSyntax) -> SyntaxVisitorContinueKind {
99+
prettyPrintCategories.append(node.content.text)
100+
return .skipChildren
101+
}
102+
}

Sources/generate-swift-format/RuleRegistryGenerator.swift

+19-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@ final class RuleRegistryGenerator: FileGenerator {
1818
/// The rules collected by scanning the formatter source code.
1919
let ruleCollector: RuleCollector
2020

21+
/// The pretty-printing categories collected by scanning the formatter source code.
22+
let prettyPrintCollector: PrettyPrintCollector
23+
2124
/// Creates a new rule registry generator.
22-
init(ruleCollector: RuleCollector) {
25+
init(ruleCollector: RuleCollector, prettyPrintCollector: PrettyPrintCollector) {
2326
self.ruleCollector = ruleCollector
27+
self.prettyPrintCollector = prettyPrintCollector
2428
}
2529

2630
func write(into handle: FileHandle) throws {
@@ -41,14 +45,26 @@ final class RuleRegistryGenerator: FileGenerator {
4145
// This file is automatically generated with generate-swift-format. Do not edit!
4246
4347
@_spi(Internal) public enum RuleRegistry {
44-
public static let rules: [String: Bool] = [
48+
public static let rules: [String: Configuration.RuleSeverity] = [
4549
4650
"""
4751
)
4852

4953
for detectedRule in ruleCollector.allLinters.sorted(by: { $0.typeName < $1.typeName }) {
50-
handle.write(" \"\(detectedRule.typeName)\": \(!detectedRule.isOptIn),\n")
54+
handle.write(" \"\(detectedRule.typeName)\": \(severity(detectedRule.isOptIn)),\n")
55+
}
56+
57+
for ppCategory in prettyPrintCollector.allPrettyPrinterCategories.sorted(by: { $0 < $1 }) {
58+
handle.write(" \"\(ppCategory)\": .ruleDefault,\n")
5159
}
5260
handle.write(" ]\n}\n")
5361
}
62+
63+
func severity(_ isOptIn: Bool) -> String {
64+
if isOptIn {
65+
return "false"
66+
} else {
67+
return "true"
68+
}
69+
}
5470
}

Sources/generate-swift-format/main.swift

+8-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ let rulesDirectory =
2020
sourcesDirectory
2121
.appendingPathComponent("SwiftFormat")
2222
.appendingPathComponent("Rules")
23+
let prettyPrintDirectory =
24+
sourcesDirectory
25+
.appendingPathComponent("SwiftFormat")
26+
.appendingPathComponent("PrettyPrint")
2327
let pipelineFile =
2428
sourcesDirectory
2529
.appendingPathComponent("SwiftFormat")
@@ -46,12 +50,15 @@ let ruleDocumentationFile =
4650
var ruleCollector = RuleCollector()
4751
try ruleCollector.collect(from: rulesDirectory)
4852

53+
var prettyPrintCollector = PrettyPrintCollector()
54+
try prettyPrintCollector.collect(from: prettyPrintDirectory)
55+
4956
// Generate a file with extensions for the lint and format pipelines.
5057
let pipelineGenerator = PipelineGenerator(ruleCollector: ruleCollector)
5158
try pipelineGenerator.generateFile(at: pipelineFile)
5259

5360
// Generate the rule registry dictionary for configuration.
54-
let registryGenerator = RuleRegistryGenerator(ruleCollector: ruleCollector)
61+
let registryGenerator = RuleRegistryGenerator(ruleCollector: ruleCollector, prettyPrintCollector: prettyPrintCollector)
5562
try registryGenerator.generateFile(at: ruleRegistryFile)
5663

5764
// Generate the rule name cache.

0 commit comments

Comments
 (0)