Remove severity

Instead of making the severity configurable,
this patch removes severity all together and treats
every finding as an error.

Issue: #879

Author: bkolb

.gitignore

@@ -4,3 +4,4 @@
 swift-format.xcodeproj/
 Package.resolved
 /.vscode
+.index-build

Documentation/RuleDocumentation.md

@@ -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.
 

Sources/SwiftFormat/API/Finding.swift

@@ -12,13 +12,6 @@
 
 /// A problem with the style or syntax of the source code discovered during linting or formatting.
 public struct Finding {
-  /// The severity of a finding.
-  public enum Severity {
-    case warning
-    case error
-    case refactoring
-    case convention
-  }
 
   /// The file path and location in that file where a finding was encountered.
   public struct Location {
@@ -83,27 +76,22 @@ public struct Finding {
   /// The finding's message.
   public let message: Message
 
-  /// The severity of the finding.
-  public let severity: Severity
-
   /// The optional location of the finding.
   public let location: Location?
 
   /// Notes that provide additional detail about the finding.
   public let notes: [Note]
 
-  /// Creates a new finding with the given category, message, severity, optional location, and
+  /// Creates a new finding with the given category, message, optional location, and
   /// notes.
   init(
     category: FindingCategorizing,
     message: Message,
-    severity: Finding.Severity,
     location: Location? = nil,
     notes: [Note] = []
   ) {
     self.category = category
     self.message = message
-    self.severity = severity
     self.location = location
     self.notes = notes
   }

Sources/SwiftFormat/API/FindingCategorizing.swift

@@ -17,13 +17,5 @@
 /// to be displayed as part of the diagnostic message when the finding is presented to the user.
 /// For example, the category `Indentation` in the message `[Indentation] Indent by 2 spaces`.
 public protocol FindingCategorizing: CustomStringConvertible {
-  /// The default severity of findings emitted in this category.
-  ///
-  /// By default, all findings are warnings. Individual categories may choose to override this to
-  /// make the findings in those categories more severe.
-  var defaultSeverity: Finding.Severity { get }
-}
 
-extension FindingCategorizing {
-  public var defaultSeverity: Finding.Severity { .warning }
 }

Sources/SwiftFormat/Core/FindingEmitter.swift

@@ -48,13 +48,10 @@ final class FindingEmitter {
   ) {
     guard let consumer = self.consumer else { return }
 
-    // TODO: Provide a way via the formatter configuration for users to customize the severity of
-    // findings based on their category, falling back to the default if it isn't overridden.
     consumer(
       Finding(
         category: category,
         message: message,
-        severity: category.defaultSeverity,
         location: location,
         notes: notes
       )

Sources/SwiftFormat/Core/Parsing.swift

@@ -63,11 +63,9 @@ func parseAndEmitDiagnostics(
     for diagnostic in diagnostics {
       let location = diagnostic.location(converter: expectedConverter)
 
-      // Downgrade editor placeholders to warnings, because it is useful to support formatting
+      // Ignore editor placeholders, because it is useful to support formatting
       // in-progress files that contain those.
-      if diagnostic.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
-        parsingDiagnosticHandler(downgradedToWarning(diagnostic), location)
-      } else {
+      if diagnostic.diagnosticID != StaticTokenError.editorPlaceholder.diagnosticID {
         parsingDiagnosticHandler(diagnostic, location)
         hasErrors = true
       }
@@ -79,29 +77,3 @@ func parseAndEmitDiagnostics(
   }
   return sourceFile
 }
-
-// Wraps a `DiagnosticMessage` but forces its severity to be that of a warning instead of an error.
-struct DowngradedDiagnosticMessage: DiagnosticMessage {
-  var originalDiagnostic: DiagnosticMessage
-
-  var message: String { originalDiagnostic.message }
-
-  var diagnosticID: SwiftDiagnostics.MessageID { originalDiagnostic.diagnosticID }
-
-  var severity: DiagnosticSeverity { .warning }
-}
-
-/// Returns a new `Diagnostic` that is identical to the given diagnostic, except that its severity
-/// has been downgraded to a warning.
-func downgradedToWarning(_ diagnostic: Diagnostic) -> Diagnostic {
-  // `Diagnostic` is immutable, so create a new one with the same values except for the
-  // severity-downgraded message.
-  return Diagnostic(
-    node: diagnostic.node,
-    position: diagnostic.position,
-    message: DowngradedDiagnosticMessage(originalDiagnostic: diagnostic.diagMessage),
-    highlights: diagnostic.highlights,
-    notes: diagnostic.notes,
-    fixIts: diagnostic.fixIts
-  )
-}

Sources/SwiftFormat/Core/Rule.swift

@@ -62,7 +62,6 @@ extension Rule {
   public func diagnose<SyntaxType: SyntaxProtocol>(
     _ message: Finding.Message,
     on node: SyntaxType?,
-    severity: Finding.Severity? = nil,
     anchor: FindingAnchor = .start,
     notes: [Finding.Note] = []
   ) {
@@ -86,7 +85,7 @@ extension Rule {
       syntaxLocation = nil
     }
 
-    let category = RuleBasedFindingCategory(ruleType: type(of: self), severity: severity)
+    let category = RuleBasedFindingCategory(ruleType: type(of: self))
     context.findingEmitter.emit(
       message,
       category: category,

Sources/SwiftFormat/Core/RuleBasedFindingCategory.swift

@@ -22,11 +22,8 @@ struct RuleBasedFindingCategory: FindingCategorizing {
 
   var description: String { ruleType.ruleName }
 
-  var severity: Finding.Severity?
-
   /// Creates a finding category that wraps the given rule type.
-  init(ruleType: Rule.Type, severity: Finding.Severity? = nil) {
+  init(ruleType: Rule.Type) {
     self.ruleType = ruleType
-    self.severity = severity
   }
 }

Sources/SwiftFormat/Rules/OmitExplicitReturns.swift

@@ -34,7 +34,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
     }
 
     funcDecl.body?.statements = rewrapReturnedExpression(returnStmt)
-    diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
+    diagnose(.omitReturnStatement, on: returnStmt)
     return DeclSyntax(funcDecl)
   }
 
@@ -78,7 +78,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
     }
 
     closureExpr.statements = rewrapReturnedExpression(returnStmt)
-    diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
+    diagnose(.omitReturnStatement, on: returnStmt)
     return ExprSyntax(closureExpr)
   }
 
@@ -111,7 +111,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
 
       getter.body?.statements = rewrapReturnedExpression(returnStmt)
 
-      diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
+      diagnose(.omitReturnStatement, on: returnStmt)
 
       accessors[getterAt] = getter
       var newBlock = accessorBlock
@@ -123,7 +123,7 @@ public final class OmitExplicitReturns: SyntaxFormatRule {
         return nil
       }
 
-      diagnose(.omitReturnStatement, on: returnStmt, severity: .refactoring)
+      diagnose(.omitReturnStatement, on: returnStmt)
 
       var newBlock = accessorBlock
       newBlock.accessors = .getter(rewrapReturnedExpression(returnStmt))

Sources/SwiftFormat/Rules/TypeNamesShouldBeCapitalized.swift

@@ -61,7 +61,7 @@ public final class TypeNamesShouldBeCapitalized: SyntaxLintRule {
     if let firstChar = name.text[leadingUnderscores.endIndex...].first,
       firstChar.uppercased() != String(firstChar)
     {
-      diagnose(.capitalizeTypeName(name: name.text, kind: kind), on: name, severity: .convention)
+      diagnose(.capitalizeTypeName(name: name.text, kind: kind), on: name)
     }
   }
 }

Sources/_SwiftFormatTestSupport/Configuration+Testing.swift

@@ -44,4 +44,11 @@ extension Configuration {
     config.indentBlankLines = false
     return config
   }
+
+  public static func forTesting(enabledRule: String) -> Configuration {
+    var config = Configuration.forTesting
+    config.rules = config.rules.mapValues({ _ in false })
+    config.rules[enabledRule] = true
+    return config
+  }
 }

Sources/swift-format/Subcommands/Lint.swift

@@ -28,7 +28,7 @@ extension SwiftFormatCommand {
 
     @Flag(
       name: .shortAndLong,
-      help: "Fail on warnings."
+      help: "Fail on warnings. Deprecated: All findings are treated as errors now."
     )
     var strict: Bool = false
 
@@ -38,9 +38,17 @@ extension SwiftFormatCommand {
     func run() throws {
       try performanceMeasurementOptions.printingInstructionCountIfRequested {
         let frontend = LintFrontend(configurationOptions: configurationOptions, lintFormatOptions: lintOptions)
+
+        if strict {
+          frontend.diagnosticsEngine.emitWarning(
+            """
+            Running swift-format with --strict is deprecated and will be removed in the future.
+            """
+          )
+        }
         frontend.run()
 
-        if frontend.diagnosticsEngine.hasErrors || strict && frontend.diagnosticsEngine.hasWarnings {
+        if frontend.diagnosticsEngine.hasErrors {
           throw ExitCode.failure
         }
       }

Sources/swift-format/Utilities/DiagnosticsEngine.swift

@@ -134,15 +134,8 @@ final class DiagnosticsEngine {
   /// Converts a lint finding into a diagnostic message that can be used by the `TSCBasic`
   /// diagnostics engine and returns it.
   private func diagnosticMessage(for finding: Finding) -> Diagnostic {
-    let severity: Diagnostic.Severity
-    switch finding.severity {
-    case .error: severity = .error
-    case .warning: severity = .warning
-    case .refactoring: severity = .warning
-    case .convention: severity = .warning
-    }
     return Diagnostic(
-      severity: severity,
+      severity: .error,
       location: finding.location.map(Diagnostic.Location.init),
       category: "\(finding.category)",
       message: "\(finding.message.text)"

Tests/SwiftFormatTests/Rules/FileScopedDeclarationPrivacyTests.swift

@@ -179,7 +179,7 @@ final class FileScopedDeclarationPrivacyTests: LintOrFormatRuleTestCase {
     findingsProvider: (String, String) -> [FindingSpec]
   ) {
     for testConfig in testConfigurations {
-      var configuration = Configuration.forTesting
+      var configuration = Configuration.forTesting(enabledRule: FileScopedDeclarationPrivacy.self.ruleName)
       configuration.fileScopedDeclarationPrivacy.accessLevel = testConfig.desired
 
       let substitutedInput = source.replacingOccurrences(of: "$access$", with: testConfig.original)

Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift

@@ -49,8 +49,7 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase {
     var emittedFindings = [Finding]()
 
     // Force the rule to be enabled while we test it.
-    var configuration = Configuration.forTesting
-    configuration.rules[type.ruleName] = true
+    let configuration = Configuration.forTesting(enabledRule: type.ruleName)
     let context = makeContext(
       sourceFileSyntax: sourceFileSyntax,
       configuration: configuration,
@@ -59,8 +58,6 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase {
     )
 
     var emittedPipelineFindings = [Finding]()
-    // Disable default rules, so only select rule runs in pipeline
-    configuration.rules = [type.ruleName: true]
     let pipeline = SwiftLinter(
       configuration: configuration,
       findingConsumer: { emittedPipelineFindings.append($0) }
@@ -118,8 +115,8 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase {
     var emittedFindings = [Finding]()
 
     // Force the rule to be enabled while we test it.
-    var configuration = configuration ?? Configuration.forTesting
-    configuration.rules[formatType.ruleName] = true
+    let configuration = configuration ?? Configuration.forTesting(enabledRule: formatType.ruleName)
+
     let context = makeContext(
       sourceFileSyntax: sourceFileSyntax,
       configuration: configuration,
@@ -162,8 +159,6 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase {
     )
 
     var emittedPipelineFindings = [Finding]()
-    // Disable default rules, so only select rule runs in pipeline
-    configuration.rules = [formatType.ruleName: true]
     let pipeline = SwiftFormatter(
       configuration: configuration,
       findingConsumer: { emittedPipelineFindings.append($0) }

api-breakages.txt

@@ -7,3 +7,10 @@ API breakage: enumelement SwiftFormatError.unsupportedConfigurationVersion has b
 API breakage: class UseLetInEveryBoundCaseVariable has changed its super class from SwiftFormat.SyntaxLintRule to SwiftFormat.SyntaxFormatRule
 API breakage: func UseLetInEveryBoundCaseVariable.visit(_:) has return type change from SwiftSyntax.SyntaxVisitorContinueKind to SwiftSyntax.MatchingPatternConditionSyntax
 API breakage: func UseLetInEveryBoundCaseVariable.visit(_:) has parameter 0 type change from SwiftSyntax.ValueBindingPatternSyntax to SwiftSyntax.MatchingPatternConditionSyntax
+API breakage: func Rule.diagnose(_:on:severity:anchor:notes:) has parameter 2 type change from SwiftFormat.Finding.Severity? to SwiftFormat.FindingAnchor
+API breakage: func Rule.diagnose(_:on:severity:anchor:notes:) has parameter 3 type change from SwiftFormat.FindingAnchor to [SwiftFormat.Finding.Note]
+API breakage: enum Finding.Severity has been removed
+API breakage: var Finding.severity has been removed
+API breakage: var FindingCategorizing.defaultSeverity has been removed
+API breakage: var FindingCategorizing.defaultSeverity has been removed
+API breakage: func Rule.diagnose(_:on:severity:anchor:notes:) has been renamed to func diagnose(_:on:anchor:notes:)