Skip to content

Commit a09125e

Browse files
ahoppenahoppen
authored
Merge pull request #954 from ahoppen/ahoppen/multiple-identifiers-after-decl-name
Diagnose if multiple identifiers are used for declaration names
2 parents 9f67756 + 7876384 commit a09125e

File tree

5 files changed

+120
-28
lines changed

5 files changed

+120
-28
lines changed

‎Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,29 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
189189
addDiagnostic(node, .unexpectedSemicolon, fixIts: [
190190
FixIt(message: RemoveNodesFixIt(semicolons), changes: semicolons.map(FixIt.Changes.makeMissing))
191191
])
192+
} else if node.first?.as(TokenSyntax.self)?.tokenKind.isIdentifier == true,
193+
let previousToken = node.previousToken(viewMode: .sourceAccurate),
194+
previousToken.tokenKind.isIdentifier,
195+
previousToken.parent?.is(DeclSyntax.self) == true {
196+
// If multiple identifiers are used for a declaration name, offer to join them together.
197+
let tokens = node
198+
.prefix(while: { $0.as(TokenSyntax.self)?.tokenKind.isIdentifier == true })
199+
.map({ $0.as(TokenSyntax.self)! })
200+
let joined = previousToken.text + tokens.map(\.text).joined()
201+
var fixIts: [FixIt] = [
202+
FixIt(message: .joinIdentifiers, changes: [
203+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
204+
.makeMissing(tokens: tokens)
205+
])
206+
]
207+
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
208+
let joinedUsingCamelCase = previousToken.text + tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
209+
fixIts.append(FixIt(message: .joinIdentifiersWithCamelCase, changes: [
210+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
211+
.makeMissing(tokens: tokens)
212+
]))
213+
}
214+
addDiagnostic(node, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: tokens), fixIts: fixIts)
192215
} else {
193216
addDiagnostic(node, UnexpectedNodesError(unexpectedNodes: node), highlights: [Syntax(node)])
194217
}

‎Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,21 @@ public struct MissingAttributeArgument: ParserError {
152152
}
153153
}
154154

155+
public struct SpaceSeparatedIdentifiersError: ParserError {
156+
public let firstToken: TokenSyntax
157+
public let additionalTokens: [TokenSyntax]
158+
159+
public var message: String {
160+
if let anchorParent = firstToken.parent?.ancestorOrSelf(where: {
161+
$0.nodeTypeNameForDiagnostics(allowBlockNames: false) != nil
162+
}) {
163+
return "found an unexpected second identifier in \(anchorParent.nodeTypeNameForDiagnostics(allowBlockNames: false)!)"
164+
} else {
165+
return "found an unexpected second identifier"
166+
}
167+
}
168+
}
169+
155170
public struct SpecifierOnParameterName: ParserError {
156171
public let misplacedSpecifiers: [TokenSyntax]
157172

@@ -205,6 +220,8 @@ public struct UnknownDirectiveError: ParserError {
205220
public enum StaticParserFixIt: String, FixItMessage {
206221
case insertSemicolon = "insert ';'"
207222
case insertAttributeArguments = "insert attribute argument"
223+
case joinIdentifiers = "join the identifiers together"
224+
case joinIdentifiersWithCamelCase = "join the identifiers together with camel-case"
208225
case removeOperatorBody = "remove operator body"
209226
case wrapKeywordInBackticks = "if this name is unavoidable, use backticks to escape it"
210227

‎Sources/SwiftParser/Diagnostics/Utils.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ extension String {
2222
}
2323
return String(result)
2424
}
25+
26+
func withFirstLetterUppercased() -> String {
27+
if let firstLetter = self.first {
28+
return firstLetter.uppercased() + self.dropFirst()
29+
} else {
30+
return self
31+
}
32+
}
2533
}
2634

2735
extension Collection {

‎Tests/SwiftParserTest/Assertions.swift

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,17 @@ struct DiagnosticSpec {
119119
class FixItApplier: SyntaxRewriter {
120120
var changes: [FixIt.Change]
121121

122-
init(diagnostics: [Diagnostic]) {
123-
self.changes = diagnostics.flatMap({ $0.fixIts }).flatMap({ $0.changes.changes })
122+
init(diagnostics: [Diagnostic], withMessages messages: [String]?) {
123+
self.changes = diagnostics
124+
.flatMap { $0.fixIts }
125+
.filter {
126+
if let messages = messages {
127+
return messages.contains($0.message.message)
128+
} else {
129+
return true
130+
}
131+
}
132+
.flatMap { $0.changes.changes }
124133
}
125134

126135
public override func visitAny(_ node: Syntax) -> Syntax? {
@@ -150,9 +159,10 @@ class FixItApplier: SyntaxRewriter {
150159
return Syntax(modifiedNode)
151160
}
152161

153-
/// Applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
154-
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], to tree: T) -> Syntax {
155-
let applier = FixItApplier(diagnostics: diagnostics)
162+
/// If `messages` is `nil`, applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
163+
/// If `messages` is not `nil`, applies only Fix-Its whose message is in `messages`.
164+
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], withMessages messages: [String]?, to tree: T) -> Syntax {
165+
let applier = FixItApplier(diagnostics: diagnostics, withMessages: messages)
156166
return applier.visit(Syntax(tree))
157167
}
158168
}
@@ -282,6 +292,7 @@ func AssertParse(
282292
substructure expectedSubstructure: Syntax? = nil,
283293
substructureAfterMarker: String = "START",
284294
diagnostics expectedDiagnostics: [DiagnosticSpec] = [],
295+
applyFixIts: [String]? = nil,
285296
fixedSource expectedFixedSource: String? = nil,
286297
file: StaticString = #file,
287298
line: UInt = #line
@@ -291,6 +302,7 @@ func AssertParse(
291302
substructure: expectedSubstructure,
292303
substructureAfterMarker: substructureAfterMarker,
293304
diagnostics: expectedDiagnostics,
305+
applyFixIts: applyFixIts,
294306
fixedSource: expectedFixedSource,
295307
file: file,
296308
line: line)
@@ -299,7 +311,10 @@ func AssertParse(
299311
/// Parse `markedSource` and perform the following assertions:
300312
/// - The parsed syntax tree should be printable back to the original source code (round-tripping)
301313
/// - Parsing produced the given `diagnostics` (`diagnostics = []` asserts that the parse was successful)
302-
/// - If `fixedSource` is not `nil`, assert that applying all fixes from the diagnostics produces `fixedSource`
314+
/// - If `fixedSource` is not `nil`, assert that applying the Fix-Its with the
315+
/// messages in `applyFixIts` (or all if `applyFixIts` is `nil`) all fixes
316+
/// from the diagnostics produces `fixedSource`
317+
///
303318
/// The source file can be marked with markers of the form `1️⃣` to mark source locations that can be referred to by `diagnostics`.
304319
/// These markers are removed before parsing the source file.
305320
/// By default, `DiagnosticSpec` asserts that the diagnostics is produced at a location marked by `1️⃣`.
@@ -311,6 +326,7 @@ func AssertParse<Node: RawSyntaxNodeProtocol>(
311326
substructure expectedSubstructure: Syntax? = nil,
312327
substructureAfterMarker: String = "START",
313328
diagnostics expectedDiagnostics: [DiagnosticSpec] = [],
329+
applyFixIts: [String]? = nil,
314330
fixedSource expectedFixedSource: String? = nil,
315331
file: StaticString = #file,
316332
line: UInt = #line
@@ -357,7 +373,7 @@ func AssertParse<Node: RawSyntaxNodeProtocol>(
357373

358374
// Applying Fix-Its
359375
if let expectedFixedSource = expectedFixedSource {
360-
let fixedTree = FixItApplier.applyFixes(in: diags, to: tree)
376+
let fixedTree = FixItApplier.applyFixes(in: diags, withMessages: applyFixIts, to: tree)
361377
AssertStringsEqualWithDiff(
362378
fixedTree.description.trimmingTrailingWhitespace(),
363379
expectedFixedSource.trimmingTrailingWhitespace(),

‎Tests/SwiftParserTest/translated/InvalidTests.swift

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -376,17 +376,51 @@ final class InvalidTests: XCTestCase {
376376
)
377377
}
378378

379-
func testInvalid23() {
379+
func testInvalid23a() {
380380
AssertParse(
381381
"""
382382
func dog 1️⃣cow() {}
383383
""",
384384
diagnostics: [
385-
// TODO: Old parser expected error on line 1: found an unexpected second identifier in function declaration; is there an accidental break?
386-
// TODO: Old parser expected note on line 1: join the identifiers together, Fix-It replacements: 6 - 13 = 'dogcow'
387-
// TODO: Old parser expected note on line 1: join the identifiers together with camel-case, Fix-It replacements: 6 - 13 = 'dogCow'
388-
DiagnosticSpec(message: "unexpected code 'cow' before parameter clause"),
389-
]
385+
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: [
386+
"join the identifiers together",
387+
"join the identifiers together with camel-case"
388+
]),
389+
],
390+
applyFixIts: ["join the identifiers together"],
391+
fixedSource: "func dogcow() {}"
392+
)
393+
}
394+
395+
func testInvalid23b() {
396+
AssertParse(
397+
"""
398+
func dog 1️⃣cow() {}
399+
""",
400+
diagnostics: [
401+
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: [
402+
"join the identifiers together",
403+
"join the identifiers together with camel-case"
404+
]),
405+
],
406+
applyFixIts: ["join the identifiers together with camel-case"],
407+
fixedSource: "func dogCow() {}"
408+
)
409+
}
410+
411+
func testThreeIdentifersForFunctionName() {
412+
AssertParse(
413+
"""
414+
func dog 1️⃣cow sheep() {}
415+
""",
416+
diagnostics: [
417+
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: [
418+
"join the identifiers together",
419+
"join the identifiers together with camel-case"
420+
]),
421+
],
422+
applyFixIts: ["join the identifiers together with camel-case"],
423+
fixedSource: "func dogCowSheep() {}"
390424
)
391425
}
392426

@@ -396,10 +430,8 @@ final class InvalidTests: XCTestCase {
396430
func cat 1️⃣Mouse() {}
397431
""",
398432
diagnostics: [
399-
// TODO: Old parser expected error on line 1: found an unexpected second identifier in function declaration; is there an accidental break?
400-
// TODO: Old parser expected note on line 1: join the identifiers together, Fix-It replacements: 6 - 15 = 'catMouse'
401-
DiagnosticSpec(message: "unexpected code 'Mouse' before parameter clause"),
402-
]
433+
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: ["join the identifiers together"]),
434+
], fixedSource: "func catMouse() {}"
403435
)
404436
}
405437

@@ -409,11 +441,13 @@ final class InvalidTests: XCTestCase {
409441
func friend 1️⃣ship<T>(x: T) {}
410442
""",
411443
diagnostics: [
412-
// TODO: Old parser expected error on line 1: found an unexpected second identifier in function declaration; is there an accidental break?
413-
// TODO: Old parser expected note on line 1: join the identifiers together, Fix-It replacements: 6 - 17 = 'friendship'
414-
// TODO: Old parser expected note on line 1: join the identifiers together with camel-case, Fix-It replacements: 6 - 17 = 'friendShip'
415-
DiagnosticSpec(message: "unexpected code 'ship<T>' before parameter clause"),
416-
]
444+
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: [
445+
"join the identifiers together",
446+
"join the identifiers together with camel-case"
447+
]),
448+
],
449+
applyFixIts: ["join the identifiers together with camel-case"],
450+
fixedSource: "func friendShip<T>(x: T) {}"
417451
)
418452
}
419453

@@ -425,9 +459,6 @@ final class InvalidTests: XCTestCase {
425459
""",
426460
diagnostics: [
427461
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '(' to start parameter clause"),
428-
// TODO: Old parser expected error on line 2: found an unexpected second identifier in function declaration; is there an accidental break?
429-
// TODO: Old parser expected note on line 2: join the identifiers together, Fix-It replacements: 6 - 5 = 'werewolf'
430-
// TODO: Old parser expected note on line 2: join the identifiers together with camel-case, Fix-It replacements: 6 - 5 = 'wereWolf'
431462
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end parameter clause"),
432463
]
433464
)
@@ -441,9 +472,6 @@ final class InvalidTests: XCTestCase {
441472
""",
442473
diagnostics: [
443474
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '(' to start parameter clause"),
444-
// TODO: Old parser expected error on line 2: found an unexpected second identifier in function declaration; is there an accidental break?
445-
// TODO: Old parser expected note on line 2: join the identifiers together, Fix-It replacements: 6 - 9 = 'hammerleavings'
446-
// TODO: Old parser expected note on line 2: join the identifiers together with camel-case, Fix-It replacements: 6 - 9 = 'hammerLeavings'
447475
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end parameter clause"),
448476
]
449477
)

0 commit comments

Comments
 (0)