Skip to content

Commit

Permalink
Fix bug in markTypes rule from change to Declaration.name
Browse files Browse the repository at this point in the history
  • Loading branch information
calda authored and nicklockwood committed Sep 28, 2024
1 parent 32f712a commit c56db0b
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 65 deletions.
8 changes: 7 additions & 1 deletion Sources/DeclarationHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,13 @@ enum Declaration: Hashable {
return nil
}

return parser.fullyQualifiedName(startingAt: nameIndex).name
// An extension can refer to complicated types like `Foo.Bar`, `[Foo]`, `Collection<Foo>`, etc.
// Every other declaration type just uses a simple identifier.
if keyword == "extension" {
return parser.parseType(at: nameIndex)?.name
} else {
return parser.tokens[nameIndex].string
}
}

/// The original range of the tokens of this declaration in the original source file
Expand Down
66 changes: 26 additions & 40 deletions Sources/ParsingHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1341,7 +1341,7 @@ extension Formatter {
["?", "!"].contains(nextToken.string)
{
let typeRange = baseType.range.lowerBound ... nextTokenIndex
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

// Any type can be followed by a `.` which can then continue the type
Expand All @@ -1351,7 +1351,7 @@ extension Formatter {
let followingType = parseType(at: followingToken, excludeLowercaseIdentifiers: excludeLowercaseIdentifiers)
{
let typeRange = startOfTypeIndex ... followingType.range.upperBound
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

return baseType
Expand Down Expand Up @@ -1385,7 +1385,7 @@ extension Formatter {
}

let typeRange = startOfTypeIndex ... endOfScope
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

// Parse types of the form `(...)` or `(...) -> ...`
Expand All @@ -1397,12 +1397,12 @@ extension Formatter {
let returnTypeRange = parseType(at: returnTypeIndex)?.range
{
let typeRange = startOfTypeIndex ... returnTypeRange.upperBound
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

// Otherwise this is just `(...)`
let typeRange = startOfTypeIndex ... endOfScope
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

// Parse types of the form `Foo<...>`
Expand All @@ -1411,7 +1411,7 @@ extension Formatter {
let endOfScope = endOfScope(at: nextTokenIndex)
{
let typeRange = startOfTypeIndex ... endOfScope
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

// Parse types of the form `any ...`, `some ...`, `borrowing ...`, `consuming ...`
Expand All @@ -1420,7 +1420,7 @@ extension Formatter {
let followingType = parseType(at: nextToken)
{
let typeRange = startOfTypeIndex ... followingType.range.upperBound
return (name: tokens[typeRange].string, range: typeRange)
return (name: tokens[typeRange].stringExcludingNewlines, range: typeRange)
}

// Otherwise this is just a single identifier
Expand Down Expand Up @@ -1916,25 +1916,6 @@ extension Formatter {
return (argumentNames: argumentNames, inKeywordIndex: inKeywordIndex)
}

/// The fully qualified name starting at the given index
func fullyQualifiedName(startingAt index: Int) -> (name: String, endIndex: Int) {
// If the identifier is followed by a dot, it's actually the first
// part of the fully-qualified name and we should skip through
// to the last component of the name.
var name = tokens[index].string
var index = index

while token(at: index + 1)?.string == ".",
let nextIdentifier = token(at: index + 2),
nextIdentifier.is(.identifier) == true
{
name = "\(name).\(nextIdentifier.string)"
index += 2
}

return (name, index)
}

/// Get the type of the declaration starting at the index of the declaration keyword
func declarationType(at index: Int) -> String? {
guard let token = token(at: index), token.isDeclarationTypeKeyword,
Expand Down Expand Up @@ -2362,27 +2343,32 @@ extension Formatter {
func parseConformancesOfType(atKeywordIndex keywordIndex: Int) -> [(conformance: String, index: Int)] {
assert(Token.swiftTypeKeywords.contains(tokens[keywordIndex].string))

guard let startOfTypeBody = index(of: .startOfScope("{"), after: keywordIndex),
let startOfConformanceList = index(of: .delimiter(":"), in: keywordIndex ..< startOfTypeBody)
guard let startOfType = index(of: .nonSpaceOrCommentOrLinebreak, after: keywordIndex),
let typeName = parseType(at: startOfType),
let indexAfterType = index(of: .nonSpaceOrCommentOrLinebreak, after: typeName.range.upperBound),
tokens[indexAfterType] == .delimiter(":"),
let firstConformanceIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: indexAfterType)
else { return [] }

var conformances = [(conformance: String, index: Int)]()
var searchIndex = startOfConformanceList
var nextConformanceIndex = firstConformanceIndex

while let nextConformanceIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: searchIndex),
tokens[nextConformanceIndex].isIdentifier
{
conformances.append((conformance: tokens[nextConformanceIndex].string, index: nextConformanceIndex))
while let type = parseType(at: nextConformanceIndex) {
conformances.append((conformance: type.name, index: nextConformanceIndex))

if let nextTokenIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextConformanceIndex) {
searchIndex = nextTokenIndex
if let nextTokenIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: type.range.upperBound) {
nextConformanceIndex = nextTokenIndex

// Skip over any comma tokens that separate conformances
if tokens[nextTokenIndex] == .delimiter(","),
let followingConformanceIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex)
{
searchIndex = followingConformanceIndex
// Skip over any comma tokens that separate conformances.
// If we find something other than a comma, like a `where` or `{`,
// then we reached the end of the conformance list.
guard tokens[nextTokenIndex] == .delimiter(","),
let followingConformanceIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex)
else {
return conformances
}

nextConformanceIndex = followingConformanceIndex
}
}

Expand Down
22 changes: 2 additions & 20 deletions Sources/Rules/MarkTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,26 +101,8 @@ public extension FormatRule {

// If this declaration is extension, check if it has any conformances
var conformanceNames: String?
if declaration.keyword == "extension",
var conformanceSearchIndex = openingFormatter.index(of: .delimiter(":"), after: keywordIndex)
{
var conformances = [String]()

let endOfConformances = openingFormatter.index(of: .keyword("where"), after: keywordIndex)
?? openingFormatter.index(of: .startOfScope("{"), after: keywordIndex)
?? openingFormatter.tokens.count

while let token = openingFormatter.token(at: conformanceSearchIndex),
conformanceSearchIndex < endOfConformances
{
if token.isIdentifier {
let (fullyQualifiedName, next) = openingFormatter.fullyQualifiedName(startingAt: conformanceSearchIndex)
conformances.append(fullyQualifiedName)
conformanceSearchIndex = next
}

conformanceSearchIndex += 1
}
if declaration.keyword == "extension" {
var conformances = openingFormatter.parseConformancesOfType(atKeywordIndex: keywordIndex).map(\.conformance)

if !conformances.isEmpty {
conformanceNames = conformances.joined(separator: ", ")
Expand Down
7 changes: 4 additions & 3 deletions Sources/Rules/RedundantEquatable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,15 @@ public extension FormatRule {
+ import MyMacroLib
+ @Equatable
+ class Bar {
- class Bar: Equatable {
class Bar {
let baaz: Baaz
}
- extension Bar: Equatable {
- static func ==(lhs: Bar, rhs: Bar) -> Bool {
- lhs.baaz == rhs.baaz
- }
}
- }
```
"""
}
Expand Down
24 changes: 23 additions & 1 deletion Sources/Tokenizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -555,10 +555,32 @@ extension Token {
}
}

extension Collection where Element == Token {
extension Collection where Element == Token, Index == Int {
var string: String {
map(\.string).joined()
}

/// A string representation of this array of tokens,
/// excluding any newlines and following indentation.
var stringExcludingNewlines: String {
var tokens: [Token] = []

var index = indices.startIndex
while index < indices.endIndex {
// Skip over any linebreaks, and any indentation following the linebreak
if self[index].isLinebreak {
index += 1
while self[index].isSpace, index < indices.endIndex {
index += 1
}
}

tokens.append(self[index])
index += 1
}

return tokens.string
}
}

extension UnicodeScalar {
Expand Down
12 changes: 12 additions & 0 deletions Tests/ParsingHelpersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2116,6 +2116,18 @@ class ParsingHelpersTests: XCTestCase {
XCTAssertEqual(formatter.parseType(at: 7)?.name, nil)
}

func testMultilineType() {
let formatter = Formatter(tokenize("""
extension Foo.Bar
.Baaz.Quux
.InnerType1
.InnerType2
{ }
"""))

XCTAssertEqual(formatter.parseType(at: 2)?.name, "Foo.Bar.Baaz.Quux.InnerType1.InnerType2")
}

func testEndOfDeclaration() {
let formatter = Formatter(tokenize("""
public enum MyFeatureCacheStrategy {
Expand Down
84 changes: 84 additions & 0 deletions Tests/Rules/MarkTypesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -836,4 +836,88 @@ class MarkTypesTests: XCTestCase {
.init(line: 2, rule: .markTypes, filePath: nil, isMove: false),
])
}

func testComplexTypeNames() throws {
let input = """
extension [Foo]: TestProtocol {
func test() {}
}
extension Foo.Bar.Baaz: TestProtocol {
func test() {}
}
extension Collection<Foo>: TestProtocol {
func test() {}
}
extension Foo?: TestProtocol {
func test()
}
"""

let output = """
// MARK: - [Foo] + TestProtocol
extension [Foo]: TestProtocol {
func test() {}
}
// MARK: - Foo.Bar.Baaz + TestProtocol
extension Foo.Bar.Baaz: TestProtocol {
func test() {}
}
// MARK: - Collection<Foo> + TestProtocol
extension Collection<Foo>: TestProtocol {
func test() {}
}
// MARK: - Foo? + TestProtocol
extension Foo?: TestProtocol {
func test()
}
"""

testFormatting(for: input, output, rule: .markTypes)
}

func testMarkCommentOnExtensionWithWrappedType() {
let input = """
extension Foo.Bar
.Baaz.Quux: Foo
.Bar.Baaz
.QuuxProtocol
{
func test() {}
}
extension [
String: AnyHashable
]: Hashable {}
"""

let output = """
// MARK: - Foo.Bar.Baaz.Quux + Foo.Bar.Baaz.QuuxProtocol
extension Foo.Bar
.Baaz.Quux: Foo
.Bar.Baaz
.QuuxProtocol
{
func test() {}
}
// MARK: - [String: AnyHashable] + Hashable
extension [
String: AnyHashable
]: Hashable {}
"""

testFormatting(for: input, output, rule: .markTypes)
}
}

0 comments on commit c56db0b

Please sign in to comment.