Skip to content

Commit

Permalink
Make all tests pass
Browse files Browse the repository at this point in the history
  • Loading branch information
calda authored and nicklockwood committed Nov 26, 2024
1 parent 07661d2 commit 6e36f09
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 50 deletions.
13 changes: 10 additions & 3 deletions Sources/DeclarationV1.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
74 changes: 48 additions & 26 deletions Sources/DeclarationV2.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
Expand All @@ -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() {
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
}
10 changes: 6 additions & 4 deletions Sources/Formatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
11 changes: 4 additions & 7 deletions Sources/ParsingHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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("(")
Expand Down Expand Up @@ -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)
)
}

Expand Down
8 changes: 4 additions & 4 deletions Sources/Rules/EnvironmentEntry.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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])
Expand Down Expand Up @@ -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 }
Expand All @@ -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 }

Expand Down
4 changes: 2 additions & 2 deletions Sources/Rules/ExtensionAccessControl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Tests/ParsingHelpersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2439,7 +2439,7 @@ class ParsingHelpersTests: XCTestCase {
XCTAssertFalse(isStoredProperty("""
var foo: String {
get { "foo" }
set { print(newValue} }
set { print(newValue) }
}
"""))
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/Rules/BracesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class BracesTests: XCTestCase {
// foo
}
"""
testFormatting(for: input, output, rule: .braces)
testFormatting(for: input, output, rule: .braces, exclude: [.emptyExtensions])
}

func testBracesForOptionalInit() {
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion Tests/Rules/ExtensionAccessControlTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,6 @@ class ExtensionAccessControlTests: XCTestCase {
}
"""

testFormatting(for: input, rule: .extensionAccessControl)
testFormatting(for: input, rule: .extensionAccessControl, exclude: [.emptyExtensions])
}
}

0 comments on commit 6e36f09

Please sign in to comment.