From 0698f0ed437e436d5855454c799bd7d6607eceb2 Mon Sep 17 00:00:00 2001 From: Cal Stephens Date: Sun, 3 Nov 2024 10:02:26 -0800 Subject: [PATCH] Migrate redundantEquatable rule to DeclarationV2 --- Sources/DeclarationV2.swift | 40 ++++++++ Sources/Formatter.swift | 4 + Sources/Rules/RedundantEquatable.swift | 110 +++++++++------------- Tests/Rules/RedundantEquatableTests.swift | 2 +- 4 files changed, 88 insertions(+), 68 deletions(-) diff --git a/Sources/DeclarationV2.swift b/Sources/DeclarationV2.swift index c452e846d..d8d11f332 100644 --- a/Sources/DeclarationV2.swift +++ b/Sources/DeclarationV2.swift @@ -30,6 +30,9 @@ protocol DeclarationV2: AnyObject { /// The concrete kind of declaration represented by this value. var kind: DeclarationKind { get } + + /// This declaration's parent declaration, if this isn't a top-level declaration. + var parent: DeclarationV2? { get set } } enum DeclarationKind { @@ -71,6 +74,13 @@ extension DeclarationV2 { formatter.declarationName(keywordIndex: keywordIndex) } + /// The fully qualified name of this declaration, including the name of each parent declaration. + var fullyQualifiedName: String? { + guard let name = name else { return nil } + let typeNames = parentDeclarations.compactMap(\.name) + [name] + return typeNames.joined(separator: ".") + } + /// A `Hashable` reference to this declaration. var identity: AnyHashable { ObjectIdentifier(self) @@ -89,6 +99,19 @@ extension DeclarationV2 { } } + /// Whether or not this declaration defines a type (a class, enum, etc, but not an extension) + var definesType: Bool { + var typeKeywords = Token.swiftTypeKeywords + typeKeywords.remove("extension") + return typeKeywords.contains(keyword) + } + + /// The start index of this declaration's modifiers, + /// which represents the first non-space / non-comment token in the declaration. + var startOfModifiersIndex: Int { + formatter.startOfModifiers(at: keywordIndex, includingAttributes: true) + } + /// The modifiers before this declaration's keyword, including any attributes. var modifiers: [String] { var allModifiers = [String]() @@ -121,6 +144,12 @@ extension DeclarationV2 { return formatter.parsePropertyDeclaration(atIntroducerIndex: keywordIndex) } + /// A list of all declarations that are a parent of this declaration + var parentDeclarations: [DeclarationV2] { + guard let parent = parent else { return [] } + return parent.parentDeclarations + [parent] + } + /// Removes this declaration from the source file. /// After this point, this declaration reference is no longer valid. func remove() { @@ -141,6 +170,7 @@ final class SimpleDeclaration: DeclarationV2 { var keyword: String var range: ClosedRange + weak var parent: DeclarationV2? let formatter: Formatter var kind: DeclarationKind { @@ -156,12 +186,17 @@ final class TypeDeclaration: DeclarationV2 { self.range = range self.body = body self.formatter = formatter + formatter.registerDeclaration(self) + for child in body { + child.parent = self + } } var keyword: String var range: ClosedRange var body: [DeclarationV2] + weak var parent: DeclarationV2? let formatter: Formatter var kind: DeclarationKind { @@ -191,12 +226,17 @@ final class ConditionalCompilationDeclaration: DeclarationV2 { self.range = range self.body = body self.formatter = formatter + formatter.registerDeclaration(self) + for child in body { + child.parent = self + } } let keyword = "#if" var range: ClosedRange var body: [DeclarationV2] + weak var parent: DeclarationV2? let formatter: Formatter var kind: DeclarationKind { diff --git a/Sources/Formatter.swift b/Sources/Formatter.swift index e6ad6bc15..6b6f5f340 100644 --- a/Sources/Formatter.swift +++ b/Sources/Formatter.swift @@ -830,6 +830,10 @@ 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. declaration.range = startIndex ... endIndex } } diff --git a/Sources/Rules/RedundantEquatable.swift b/Sources/Rules/RedundantEquatable.swift index f41001b9e..804373f02 100644 --- a/Sources/Rules/RedundantEquatable.swift +++ b/Sources/Rules/RedundantEquatable.swift @@ -8,14 +8,9 @@ public extension FormatRule { options: ["equatablemacro"] ) { formatter in // Find all of the types with an `Equatable` conformance and a manually-implemented `static func ==` implementation. - let declarations = formatter.parseDeclarations() + let declarations = formatter.parseDeclarationsV2() let typesManuallyImplementingEquatableConformance = formatter.manuallyImplementedEquatableTypes(in: declarations) - // To avoid invalidating indices within the `typesManuallyImplementingEquatableConformance`, - // compute all of the modifications we need to make and then apply them in reverse order at the end. - var modificationsByIndex = [Int: () -> Void]() - var importsToAddIfNeeded = Set() - for equatableType in typesManuallyImplementingEquatableConformance { let isEligibleForAutoEquatableConformance: Bool switch equatableType.typeDeclaration.keyword { @@ -31,8 +26,7 @@ public extension FormatRule { } guard isEligibleForAutoEquatableConformance, - let typeBody = equatableType.typeDeclaration.body, - let typeKeywordIndex = equatableType.typeDeclaration.originalKeywordIndex(in: formatter) + let typeBody = equatableType.typeDeclaration.body else { continue } // Find all of the stored instance properties in this type. @@ -50,60 +44,44 @@ public extension FormatRule { // The compiler automatically synthesizes Equatable implementations for structs if equatableType.typeDeclaration.keyword == "struct" { - let rangeToRemove = equatableType.equatableFunction.originalRange - modificationsByIndex[rangeToRemove.lowerBound] = { - formatter.removeTokens(in: rangeToRemove) - } + equatableType.equatableFunction.remove() } // In projects using an `@Equatable` macro, the Equatable implementation // can be generated by that macro instead of written manually. else if let equatableMacroInfo = formatter.options.equatableMacroInfo { - let conformanceIndex = equatableType.equatableConformanceIndex + let declarationWithEquatableConformance = equatableType.declarationWithEquatableConformance + + guard let equatableConformance = formatter.parseConformancesOfType(atKeywordIndex: declarationWithEquatableConformance.keywordIndex).first(where: { $0.conformance == "Equatable" || $0.conformance == "Hashable" }) + else { continue } // Exclude cases where the Equatable conformance is defined in an extension with a where clause, // since this wouldn't usually be captured in the generated conformance. - if let startOfExtensionTypeBody = formatter.index(of: .startOfScope("{"), after: conformanceIndex), - formatter.index(of: .keyword("where"), in: conformanceIndex ..< startOfExtensionTypeBody) != nil + if let startOfExtensionTypeBody = formatter.index(of: .startOfScope("{"), after: equatableConformance.index), + formatter.index(of: .keyword("where"), in: equatableConformance.index ..< startOfExtensionTypeBody) != nil { continue } // Remove the `==` implementation - let rangeToRemove = equatableType.equatableFunction.originalRange - modificationsByIndex[rangeToRemove.lowerBound] = { - formatter.removeTokens(in: rangeToRemove) - } + equatableType.equatableFunction.remove() // Remove the `: Equatable` conformance. // - If this type uses as `: Hashable` conformance, we have to preserve that. - if formatter.tokens[conformanceIndex] == .identifier("Equatable") { - modificationsByIndex[conformanceIndex] = { - formatter.removeConformance(at: conformanceIndex) - } + if equatableConformance.conformance == "Equatable" { + formatter.removeConformance(at: equatableConformance.index) } // Add the `@Equatable` macro - modificationsByIndex[typeKeywordIndex] = { - let startOfModifiers = formatter.startOfModifiers(at: typeKeywordIndex, includingAttributes: true) - - formatter.insert( - [.keyword(equatableMacroInfo.macro), .space(" ")], - at: startOfModifiers - ) - } + formatter.insert( + [.keyword(equatableMacroInfo.macro), .space(" ")], + at: equatableType.typeDeclaration.startOfModifiersIndex + ) // Import the module that defines the `@Equatable` macro if needed - importsToAddIfNeeded.insert(equatableMacroInfo.moduleName) + formatter.addImports([equatableMacroInfo.moduleName]) } } - - // Apply the modifications in backwards order to avoid invalidating existing indices - for (_, applyModification) in modificationsByIndex.sorted(by: { $0.key < $1.key }).reversed() { - applyModification() - } - - formatter.addImports(importsToAddIfNeeded) } examples: { """ ```diff @@ -153,52 +131,49 @@ public extension FormatRule { extension Formatter { struct EquatableType { /// The main type declaration of the type that has an Equatable conformance - let typeDeclaration: Declaration + let typeDeclaration: DeclarationV2 /// The Equatable `static func ==` implementation, which could be defined in an extension. - let equatableFunction: Declaration - /// The index of the `: Equatable` conformance, which could be defined in an extension. - let equatableConformanceIndex: Int + let equatableFunction: DeclarationV2 + /// The declaration that contains the `: Equatable` conformance, which may be an extension. + let declarationWithEquatableConformance: DeclarationV2 } /// Finds all of the types in the current file with an Equatable conformance, /// which also have a manually-implemented `static func ==` method. - func manuallyImplementedEquatableTypes(in declarations: [Declaration]) -> [EquatableType] { - var typeDeclarationsByFullyQualifiedName: [String: Declaration] = [:] - var typesWithEquatableConformances: [(fullyQualifiedTypeName: String, equatableConformanceIndex: Int)] = [] - var equatableImplementationsByFullyQualifiedName: [String: Declaration] = [:] + func manuallyImplementedEquatableTypes(in declarations: [DeclarationV2]) -> [EquatableType] { + var typeDeclarationsByFullyQualifiedName: [String: DeclarationV2] = [:] + var typesWithEquatableConformances: [(fullyQualifiedTypeName: String, declarationWithEquatableConformance: DeclarationV2)] = [] + var equatableImplementationsByFullyQualifiedName: [String: DeclarationV2] = [:] - declarations.forEachRecursiveDeclaration { declaration, parentDeclarations in + declarations.forEachRecursiveDeclaration { declaration in guard let declarationName = declaration.name else { return } - let fullyQualifiedName = declaration.fullyQualifiedName(parentDeclarations: parentDeclarations) - if declaration.definesType, let fullyQualifiedName = fullyQualifiedName { + if declaration.definesType, let fullyQualifiedName = declaration.fullyQualifiedName { typeDeclarationsByFullyQualifiedName[fullyQualifiedName] = declaration } // Support the Equatable conformance being declared in an extension // separately from the Equatable - if declaration.definesType || declaration.keyword == "extension", - let keywordIndex = declaration.originalKeywordIndex(in: self), - let fullyQualifiedName = fullyQualifiedName + if declaration is TypeDeclaration, + let fullyQualifiedName = declaration.fullyQualifiedName { - let conformances = parseConformancesOfType(atKeywordIndex: keywordIndex) + let conformances = parseConformancesOfType(atKeywordIndex: declaration.keywordIndex) // Both an Equatable and Hashable conformance will cause the Equatable conformance to be synthesized - if let equatableConformance = conformances.first(where: { + if conformances.contains(where: { $0.conformance == "Equatable" || $0.conformance == "Hashable" }) { typesWithEquatableConformances.append(( fullyQualifiedTypeName: fullyQualifiedName, - equatableConformanceIndex: equatableConformance.index + declarationWithEquatableConformance: declaration )) } } if declaration.keyword == "func", declarationName == "==", - let funcKeywordIndex = declaration.originalKeywordIndex(in: self), - modifiersForDeclaration(at: funcKeywordIndex, contains: "static"), - let startOfArguments = index(of: .startOfScope("("), after: funcKeywordIndex) + modifiersForDeclaration(at: declaration.keywordIndex, contains: "static"), + let startOfArguments = index(of: .startOfScope("("), after: declaration.keywordIndex) { let functionArguments = parseFunctionDeclarationArguments(startOfScope: startOfArguments) @@ -210,10 +185,10 @@ extension Formatter { { var comparedTypeName = functionArguments[0].type - if let parentDeclaration = parentDeclarations.last { + if let parentDeclaration = declaration.parent { // If the function uses `Self`, resolve that to the name of the parent type if comparedTypeName == "Self", - let parentDeclarationName = parentDeclaration.fullyQualifiedName(parentDeclarations: parentDeclarations.dropLast()) + let parentDeclarationName = parentDeclaration.fullyQualifiedName { comparedTypeName = parentDeclarationName } @@ -232,7 +207,7 @@ extension Formatter { // the name of the compared type to be the fully-qualified parent type. // - For example, `Bar` could be defined in a parent `Foo` type. if comparedTypeName == parentDeclaration.name, - let parentDeclarationName = parentDeclaration.fullyQualifiedName(parentDeclarations: parentDeclarations.dropLast()) + let parentDeclarationName = parentDeclaration.fullyQualifiedName { comparedTypeName = parentDeclarationName } @@ -243,7 +218,7 @@ extension Formatter { } } - return typesWithEquatableConformances.compactMap { typeName, equatableConformanceIndex in + return typesWithEquatableConformances.compactMap { typeName, declarationWithEquatableConformance in guard let typeDeclaration = typeDeclarationsByFullyQualifiedName[typeName], let equatableImplementation = equatableImplementationsByFullyQualifiedName[typeName] else { return nil } @@ -251,7 +226,7 @@ extension Formatter { return EquatableType( typeDeclaration: typeDeclaration, equatableFunction: equatableImplementation, - equatableConformanceIndex: equatableConformanceIndex + declarationWithEquatableConformance: declarationWithEquatableConformance ) } } @@ -259,9 +234,10 @@ extension Formatter { /// Finds the set of properties that are compared in the given Equatable `func`, /// following the pattern `lhs.{property} == rhs.{property}`. /// - Returns `nil` if there are any comparisons that don't match this pattern. - func parseComparedProperties(inEquatableImplementation equatableImplementation: Declaration) -> Set? { - guard let funcIndex = equatableImplementation.originalKeywordIndex(in: self), - let startOfBody = index(of: .startOfScope("{"), after: funcIndex), + func parseComparedProperties(inEquatableImplementation equatableImplementation: DeclarationV2) -> Set? { + let funcIndex = equatableImplementation.keywordIndex + + guard let startOfBody = index(of: .startOfScope("{"), after: funcIndex), let firstIndexInBody = index(of: .nonSpaceOrCommentOrLinebreak, after: startOfBody), let endOfBody = endOfScope(at: startOfBody) else { return nil } diff --git a/Tests/Rules/RedundantEquatableTests.swift b/Tests/Rules/RedundantEquatableTests.swift index 118e0720a..074ebc1e1 100644 --- a/Tests/Rules/RedundantEquatableTests.swift +++ b/Tests/Rules/RedundantEquatableTests.swift @@ -166,7 +166,7 @@ final class RedundantEquatableTests: XCTestCase { } } - class Quux: Equatable { + class Quux { let bar: Bar let baaz: Baaz }