Make all tests pass

Author: calda

Sources/DeclarationV1.swift

@@ -987,28 +987,35 @@ extension Declaration {
         // DeclarationV1 handles disabling rules by setting the keyword to an empty string.
         guard !keyword.isEmpty else { return nil }
 
+        let declaration: DeclarationV2
         switch self {
         case let .type(kind, _, body, _, originalRange):
-            return TypeDeclaration(
+            declaration = TypeDeclaration(
                 keyword: kind,
                 range: originalRange,
                 body: body.compactMap { $0.makeDeclarationV2(formatter: formatter) },
                 formatter: formatter
             )
 
         case let .declaration(kind, _, originalRange):
-            return SimpleDeclaration(
+            declaration = SimpleDeclaration(
                 keyword: kind,
                 range: originalRange,
                 formatter: formatter
             )
 
         case let .conditionalCompilation(_, body, _, originalRange):
-            return ConditionalCompilationDeclaration(
+            declaration = ConditionalCompilationDeclaration(
                 range: originalRange,
                 body: body.compactMap { $0.makeDeclarationV2(formatter: formatter) },
                 formatter: formatter
             )
         }
+
+        guard declaration.isValid else {
+            return nil
+        }
+
+        return declaration
     }
 }

Sources/DeclarationV2.swift

@@ -16,7 +16,7 @@
 /// automatically kept up-to-date as tokens are added, removed, or modified
 /// in the associated formatter.
 ///
-protocol DeclarationV2: AnyObject {
+protocol DeclarationV2: AnyObject, CustomDebugStringConvertible {
     /// The keyword of this declaration (`class`, `struct`, `func`, `let`, `var`, etc.)
     var keyword: String { get }
 
@@ -50,9 +50,24 @@ extension DeclarationV2 {
         formatter.tokens[range]
     }
 
+    /// Whether or not this declaration reference is still valid
+    var isValid: Bool {
+        _keywordIndex != nil
+    }
+
     /// The index of this declaration's keyword in the associated formatter.
     /// Assumes that the declaration has not been invalidated, and still contains its `keyword`.
     var keywordIndex: Int {
+        guard let keywordIndex = _keywordIndex else {
+            assertionFailure("Declaration \(self) is no longer valid.")
+            return range.lowerBound
+        }
+
+        return keywordIndex
+    }
+
+    /// The index of this declaration's keyword token, if the declaration is still valid.
+    var _keywordIndex: Int? {
         let expectedKeywordToken: Token
         switch kind {
         case .declaration, .type:
@@ -61,12 +76,7 @@ extension DeclarationV2 {
             expectedKeywordToken = .startOfScope("#if")
         }
 
-        guard let keywordIndex = formatter.index(of: expectedKeywordToken, after: range.lowerBound - 1) else {
-            assertionFailure("Declaration \(self) is no longer valid.")
-            return range.lowerBound
-        }
-
-        return keywordIndex
+        return formatter.index(of: expectedKeywordToken, after: range.lowerBound - 1)
     }
 
     /// The name of this declaration, which is always the identifier or type following the primary keyword.
@@ -139,7 +149,7 @@ extension DeclarationV2 {
     }
 
     /// Full information about this `let` or `var` property declaration.
-    var asPropertyDeclaration: Formatter.PropertyDeclaration? {
+    func parsePropertyDeclaration() -> Formatter.PropertyDeclaration? {
         guard keyword == "let" || keyword == "var" else { return nil }
         return formatter.parsePropertyDeclaration(atIntroducerIndex: keywordIndex)
     }
@@ -155,6 +165,19 @@ extension DeclarationV2 {
         return parent.parentDeclarations + [parent]
     }
 
+    /// The `CustomDebugStringConvertible` representation of this declaration
+    var debugDescription: String {
+        guard isValid else {
+            return "Invalid \(keyword) declaration reference at \(range)"
+        }
+
+        let indentation = formatter.currentIndentForLine(at: range.lowerBound)
+        return """
+        \(indentation)/* \(keyword) declaration at \(range) */
+        \(tokens.string)
+        """
+    }
+
     /// Removes this declaration from the source file.
     /// After this point, this declaration reference is no longer valid.
     func remove() {
@@ -173,10 +196,14 @@ final class SimpleDeclaration: DeclarationV2 {
         formatter.registerDeclaration(self)
     }
 
+    deinit {
+        formatter.unregisterDeclaration(self)
+    }
+
     var keyword: String
     var range: ClosedRange<Int>
-    weak var parent: DeclarationV2?
     let formatter: Formatter
+    weak var parent: DeclarationV2?
 
     var kind: DeclarationKind {
         .declaration(self)
@@ -198,16 +225,22 @@ final class TypeDeclaration: DeclarationV2 {
         }
     }
 
+    deinit {
+        formatter.unregisterDeclaration(self)
+    }
+
     var keyword: String
     var range: ClosedRange<Int>
     var body: [DeclarationV2]
-    weak var parent: DeclarationV2?
     let formatter: Formatter
+    weak var parent: DeclarationV2?
 
     var kind: DeclarationKind {
         .type(self)
     }
+}
 
+extension TypeDeclaration {
     /// The index of the open brace (`{`) before the type's body.
     /// Assumes that the declaration has not been invalidated.
     var openBraceIndex: Int {
@@ -238,11 +271,15 @@ final class ConditionalCompilationDeclaration: DeclarationV2 {
         }
     }
 
+    deinit {
+        formatter.unregisterDeclaration(self)
+    }
+
     let keyword = "#if"
     var range: ClosedRange<Int>
     var body: [DeclarationV2]
-    weak var parent: DeclarationV2?
     let formatter: Formatter
+    weak var parent: DeclarationV2?
 
     var kind: DeclarationKind {
         .conditionalCompilation(self)
@@ -286,18 +323,3 @@ extension DeclarationV2 {
         formatter.removeDeclarationVisibility(visibilityKeyword, declarationKeywordIndex: keywordIndex)
     }
 }
-
-/// We want to avoid including a Hashable requirement on DeclarationV2,
-/// so instead you can use this container type if you need a Hashable declaration.
-/// Uses reference identity of the `DeclarationV2` class value.
-struct HashableDeclaration: Hashable {
-    let declaration: DeclarationV2
-
-    static func == (lhs: HashableDeclaration, rhs: HashableDeclaration) -> Bool {
-        lhs.declaration === rhs.declaration
-    }
-
-    func hash(into hasher: inout Hasher) {
-        hasher.combine(ObjectIdentifier(declaration))
-    }
-}

Sources/Formatter.swift

@@ -830,10 +830,12 @@ extension Array where Element == WeakDeclarationReference {
                 // so doesn't invalidate the indices.
             }
 
-            // If you get a crash here where `endIndex` is less than `startIndex`,
-            // it means that the declaration is no longer valid, usually because it's
-            // been removed, but the declaration is unexpectedly still subscribed
-            // to updates from this formatter.
+            // Defend against a potential crash here if `endIndex` is less than `startIndex`.
+            guard startIndex <= endIndex else {
+                declaration.range = startIndex ... startIndex
+                continue
+            }
+
             declaration.range = startIndex ... endIndex
         }
     }

Sources/ParsingHelpers.swift

@@ -1705,7 +1705,8 @@ extension Formatter {
             let rangeInsideBody: ClosedRange<Int>
             if startOfBodyIndex + 1 != endOfScope {
                 if let firstTokenInBody = index(of: .nonSpaceOrCommentOrLinebreak, after: startOfBodyIndex + 1),
-                   let lastTokenInBody = index(of: .nonSpaceOrCommentOrLinebreak, before: endOfScope)
+                   let lastTokenInBody = index(of: .nonSpaceOrCommentOrLinebreak, before: endOfScope),
+                   firstTokenInBody <= lastTokenInBody
                 {
                     rangeInsideBody = firstTokenInBody ... lastTokenInBody
                 } else {
@@ -2365,11 +2366,9 @@ extension Formatter {
 
     /// The explicit `Visibility` of the `Declaration` with its keyword at the given index
     func declarationVisibility(keywordIndex: Int) -> Visibility? {
-        let startOfModifiers = startOfModifiers(at: keywordIndex, includingAttributes: false)
-
         // Search for a visibility keyword in the tokens before the primary keyword,
         // making sure we exclude groups like private(set).
-        var searchIndex = startOfModifiers
+        var searchIndex = startOfModifiers(at: keywordIndex, includingAttributes: false)
         while searchIndex < keywordIndex {
             if let visibility = Visibility(rawValue: tokens[searchIndex].string),
                next(.nonSpaceOrComment, after: searchIndex) != .startOfScope("(")
@@ -2400,11 +2399,9 @@ extension Formatter {
             }
         }
 
-        let startOfModifiers = startOfModifiers(at: declarationKeywordIndex, includingAttributes: false)
-
         insert(
             [.keyword(visibilityKeyword.rawValue), .space(" ")],
-            at: startOfModifiers
+            at: startOfModifiers(at: declarationKeywordIndex, includingAttributes: false)
         )
     }
 

Sources/Rules/EnvironmentEntry.swift

@@ -68,7 +68,7 @@ extension Formatter {
         var environmentKeys = [String: EnvironmentKey]()
 
         for declaration in declarations {
-            guard let typeDeclaration = declaration as? TypeDeclaration,
+            guard let typeDeclaration = declaration.asTypeDeclaration,
                   typeDeclaration.keyword == "struct" || typeDeclaration.keyword == "enum",
                   typeDeclaration.conformances.contains(where: { $0.conformance == "EnvironmentKey" }),
                   let keyName = typeDeclaration.name,
@@ -89,7 +89,7 @@ extension Formatter {
     }
 
     func findEnvironmentKeyDefaultValue(_ defaultValueDeclaration: DeclarationV2) -> [Token]? {
-        guard let property = defaultValueDeclaration.asPropertyDeclaration else { return nil }
+        guard let property = defaultValueDeclaration.parsePropertyDeclaration() else { return nil }
 
         if let valueRange = property.value?.expressionRange {
             return Array(tokens[valueRange])
@@ -125,7 +125,7 @@ extension Formatter {
 
                     // Ensure the property has a setter and a getter, this can avoid edge cases where
                     // a property references a `EnvironmentKey` and consumes it to perform some computation.
-                    guard let bodyRange = propertyDeclaration.asPropertyDeclaration?.body?.range,
+                    guard let bodyRange = propertyDeclaration.parsePropertyDeclaration()?.body?.range,
                           let indexOfSetter = index(of: .identifier("set"), in: Range(bodyRange)),
                           isAccessorKeyword(at: indexOfSetter)
                     else { return nil }
@@ -141,7 +141,7 @@ extension Formatter {
 
     func modifyEnvironmentValuesProperties(_ environmentValuesPropertiesDeclarations: [EnvironmentValueProperty]) {
         for envProperty in environmentValuesPropertiesDeclarations {
-            guard let propertyDeclaration = envProperty.declaration.asPropertyDeclaration,
+            guard let propertyDeclaration = envProperty.declaration.parsePropertyDeclaration(),
                   let bodyScopeRange = propertyDeclaration.body?.scopeRange
             else { continue }
 

Sources/Rules/ExtensionAccessControl.swift

@@ -17,7 +17,7 @@ public extension FormatRule {
 
         let declarations = formatter.parseDeclarationsV2()
         declarations.forEachRecursiveDeclaration { declaration in
-            guard let extensionDeclaration = declaration as? TypeDeclaration,
+            guard let extensionDeclaration = declaration.asTypeDeclaration,
                   extensionDeclaration.keyword == "extension"
             else { return }
 
@@ -58,7 +58,7 @@ public extension FormatRule {
                 if memberVisibility > extensionVisibility ?? .internal {
                     // Check type being extended does not have lower visibility
                     for extendedType in declarations where extendedType.name == extensionDeclaration.name {
-                        guard let type = extendedType as? TypeDeclaration else { continue }
+                        guard let type = extendedType.asTypeDeclaration else { continue }
 
                         if extendedType.keyword != "extension",
                            extendedType.visibility() ?? .internal < memberVisibility

Tests/ParsingHelpersTests.swift

@@ -2439,7 +2439,7 @@ class ParsingHelpersTests: XCTestCase {
         XCTAssertFalse(isStoredProperty("""
         var foo: String {
             get { "foo" }
-            set { print(newValue} }
+            set { print(newValue) }
         }
         """))
     }

Tests/Rules/BracesTests.swift

@@ -256,7 +256,7 @@ class BracesTests: XCTestCase {
             // foo
         }
         """
-        testFormatting(for: input, output, rule: .braces)
+        testFormatting(for: input, output, rule: .braces, exclude: [.emptyExtensions])
     }
 
     func testBracesForOptionalInit() {
@@ -505,7 +505,7 @@ class BracesTests: XCTestCase {
         }
         """
         let options = FormatOptions(allmanBraces: true)
-        testFormatting(for: input, output, rule: .braces, options: options)
+        testFormatting(for: input, output, rule: .braces, options: options, exclude: [.emptyExtensions])
     }
 
     func testEmptyAllmanIfElseBraces() {

Tests/Rules/ExtensionAccessControlTests.swift

@@ -458,6 +458,6 @@ class ExtensionAccessControlTests: XCTestCase {
         }
         """
 
-        testFormatting(for: input, rule: .extensionAccessControl)
+        testFormatting(for: input, rule: .extensionAccessControl, exclude: [.emptyExtensions])
     }
 }