diff --git a/Sources/DeclarationV1.swift b/Sources/DeclarationV1.swift index 56bd301e0..932d82a4f 100644 --- a/Sources/DeclarationV1.swift +++ b/Sources/DeclarationV1.swift @@ -987,9 +987,10 @@ 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) }, @@ -997,18 +998,24 @@ extension Declaration { ) 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 } } diff --git a/Sources/DeclarationV2.swift b/Sources/DeclarationV2.swift index 36d55e1f4..992dedad5 100644 --- a/Sources/DeclarationV2.swift +++ b/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 - 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 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 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)) - } -} diff --git a/Sources/Formatter.swift b/Sources/Formatter.swift index 6b6f5f340..44775031b 100644 --- a/Sources/Formatter.swift +++ b/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 } } diff --git a/Sources/ParsingHelpers.swift b/Sources/ParsingHelpers.swift index 1363a3721..6fb70ab72 100644 --- a/Sources/ParsingHelpers.swift +++ b/Sources/ParsingHelpers.swift @@ -1709,7 +1709,8 @@ extension Formatter { let rangeInsideBody: ClosedRange 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 { @@ -2357,11 +2358,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("(") @@ -2392,11 +2391,9 @@ extension Formatter { } } - let startOfModifiers = startOfModifiers(at: declarationKeywordIndex, includingAttributes: false) - insert( [.keyword(visibilityKeyword.rawValue), .space(" ")], - at: startOfModifiers + at: startOfModifiers(at: declarationKeywordIndex, includingAttributes: false) ) } diff --git a/Sources/Rules/EnvironmentEntry.swift b/Sources/Rules/EnvironmentEntry.swift index 8a2b28d9e..5e7fc2c9a 100644 --- a/Sources/Rules/EnvironmentEntry.swift +++ b/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 } diff --git a/Sources/Rules/ExtensionAccessControl.swift b/Sources/Rules/ExtensionAccessControl.swift index fb81172b4..b59116594 100644 --- a/Sources/Rules/ExtensionAccessControl.swift +++ b/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 diff --git a/Tests/ParsingHelpersTests.swift b/Tests/ParsingHelpersTests.swift index 6d5fea3da..a95ce7847 100644 --- a/Tests/ParsingHelpersTests.swift +++ b/Tests/ParsingHelpersTests.swift @@ -2439,7 +2439,7 @@ class ParsingHelpersTests: XCTestCase { XCTAssertFalse(isStoredProperty(""" var foo: String { get { "foo" } - set { print(newValue} } + set { print(newValue) } } """)) } diff --git a/Tests/Rules/BracesTests.swift b/Tests/Rules/BracesTests.swift index 1d1ecbf04..320f2a543 100644 --- a/Tests/Rules/BracesTests.swift +++ b/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: [.emptyExtension]) } 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: [.emptyExtension]) } func testEmptyAllmanIfElseBraces() { diff --git a/Tests/Rules/ExtensionAccessControlTests.swift b/Tests/Rules/ExtensionAccessControlTests.swift index 1dbbd3aac..ab1b80489 100644 --- a/Tests/Rules/ExtensionAccessControlTests.swift +++ b/Tests/Rules/ExtensionAccessControlTests.swift @@ -458,6 +458,6 @@ class ExtensionAccessControlTests: XCTestCase { } """ - testFormatting(for: input, rule: .extensionAccessControl) + testFormatting(for: input, rule: .extensionAccessControl, exclude: [.emptyExtension]) } }