Skip to content

Commit e5ebdb6

Browse files
authored
Merge pull request #947 from ahoppen/ahoppen/misplaced-specifiers
Add diagnostic for misplaced specifiers
2 parents 00c1498 + 60f0e62 commit e5ebdb6

File tree

11 files changed

+225
-108
lines changed

11 files changed

+225
-108
lines changed

Sources/SwiftParser/Declarations.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,7 +1339,7 @@ extension Parser {
13391339
switch self.at(anyIn: ParameterModifier.self) {
13401340
case (._const, let handle)?:
13411341
elements.append(RawDeclModifierSyntax(name: self.eat(handle), detail: nil, arena: self.arena))
1342-
case (.isolated, let handle)? where !self.lookahead().startsParameterName(subject.isClosure):
1342+
case (.isolated, let handle)? where self.withLookahead({ !$0.startsParameterName(isClosure: subject.isClosure, allowMisplacedSpecifierRecovery: false) }):
13431343
elements.append(RawDeclModifierSyntax(name: self.eat(handle), detail: nil, arena: self.arena))
13441344
default:
13451345
break MODIFIER_LOOP
@@ -1377,13 +1377,18 @@ extension Parser {
13771377

13781378
let modifiers = parseParameterModifiers(for: subject)
13791379

1380+
var misplacedSpecifiers: [RawTokenSyntax] = []
1381+
while let specifier = self.consume(ifAnyIn: TypeSpecifier.self) {
1382+
misplacedSpecifiers.append(specifier)
1383+
}
1384+
13801385
let firstName: RawTokenSyntax?
13811386
let secondName: RawTokenSyntax?
13821387
let unexpectedBeforeColon: RawUnexpectedNodesSyntax?
13831388
let colon: RawTokenSyntax?
13841389
let shouldParseType: Bool
13851390

1386-
if self.lookahead().startsParameterName(subject.isClosure) {
1391+
if self.withLookahead({ $0.startsParameterName(isClosure: subject.isClosure, allowMisplacedSpecifierRecovery: false) }) {
13871392
if self.currentToken.canBeArgumentLabel {
13881393
firstName = self.parseArgumentLabel()
13891394
} else {
@@ -1413,7 +1418,7 @@ extension Parser {
14131418

14141419
let type: RawTypeSyntax?
14151420
if shouldParseType {
1416-
type = self.parseType()
1421+
type = self.parseType(misplacedSpecifiers: misplacedSpecifiers)
14171422
} else {
14181423
type = nil
14191424
}
@@ -1437,6 +1442,7 @@ extension Parser {
14371442
elements.append(RawFunctionParameterSyntax(
14381443
attributes: attrs,
14391444
modifiers: modifiers,
1445+
RawUnexpectedNodesSyntax(misplacedSpecifiers, arena: self.arena),
14401446
firstName: firstName,
14411447
secondName: secondName,
14421448
unexpectedBeforeColon,

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,9 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
202202
if node.presence == .missing {
203203
// If there is an unexpected token in front of the identifier, we assume
204204
// that this unexpected token was intended to be the identifier we are missing.
205-
if case .identifier = node.tokenKind,
206-
let invalidIdentifier = node.previousToken(viewMode: .all),
207-
let previousParent = invalidIdentifier.parent?.as(UnexpectedNodesSyntax.self) {
205+
if node.tokenKind.isIdentifier,
206+
let invalidIdentifier = node.previousToken(viewMode: .all),
207+
let previousParent = invalidIdentifier.parent?.as(UnexpectedNodesSyntax.self) {
208208
let fixIts: [FixIt]
209209
if invalidIdentifier.tokenKind.isKeyword {
210210
fixIts = [FixIt(message: .wrapKeywordInBackticks, changes: [
@@ -296,9 +296,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
296296
return
297297
}
298298
let message: DiagnosticMessage?
299-
if let identifier = unexpected.onlyToken(where: {
300-
if case .identifier = $0.tokenKind { return true } else { return false }
301-
}) {
299+
if let identifier = unexpected.onlyToken(where: { $0.tokenKind.isIdentifier }) {
302300
message = IdentifierNotAllowedInOperatorName(identifier: identifier)
303301
} else if let tokens = unexpected.onlyTokens(satisfying: { _ in true }) {
304302
message = TokensNotAllowedInOperatorName(tokens: tokens)
@@ -482,6 +480,21 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
482480
return .visitChildren
483481
}
484482

483+
public override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
484+
if shouldSkip(node) {
485+
return .skipChildren
486+
}
487+
exchangeTokens(
488+
unexpected: node.unexpectedBetweenModifiersAndFirstName,
489+
unexpectedTokenCondition: { TypeSpecifier(token: $0) != nil },
490+
correctTokens: [node.type?.as(AttributedTypeSyntax.self)?.specifier],
491+
message: { SpecifierOnParameterName(misplacedSpecifiers: $0) },
492+
moveFixIt: { MoveTokensInFrontOfTypeFixIt(movedTokens: $0) },
493+
removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
494+
)
495+
return .visitChildren
496+
}
497+
485498
public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
486499
if shouldSkip(node) {
487500
return .skipChildren
@@ -560,6 +573,21 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
560573
return .visitChildren
561574
}
562575

576+
public override func visit(_ node: TupleTypeElementSyntax) -> SyntaxVisitorContinueKind {
577+
if shouldSkip(node) {
578+
return .skipChildren
579+
}
580+
exchangeTokens(
581+
unexpected: node.unexpectedBetweenInOutAndName,
582+
unexpectedTokenCondition: { TypeSpecifier(token: $0) != nil },
583+
correctTokens: [node.type.as(AttributedTypeSyntax.self)?.specifier],
584+
message: { SpecifierOnParameterName(misplacedSpecifiers: $0) },
585+
moveFixIt: { MoveTokensInFrontOfTypeFixIt(movedTokens: $0) },
586+
removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
587+
)
588+
return .visitChildren
589+
}
590+
563591
public override func visit(_ node: TypeInitializerClauseSyntax) -> SyntaxVisitorContinueKind {
564592
if shouldSkip(node) {
565593
return .skipChildren

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

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

155+
public struct SpecifierOnParameterName: ParserError {
156+
public let misplacedSpecifiers: [TokenSyntax]
157+
158+
public var message: String {
159+
return "\(nodesDescription(misplacedSpecifiers, format: false)) before a parameter name is not allowed"
160+
}
161+
}
162+
155163
public struct TokensNotAllowedInOperatorName: ParserError {
156164
public let tokens: [TokenSyntax]
157165

@@ -231,6 +239,15 @@ public struct MoveTokensInFrontOfFixIt: ParserFixIt {
231239
}
232240
}
233241

242+
public struct MoveTokensInFrontOfTypeFixIt: ParserFixIt {
243+
/// The token that should be moved
244+
public let movedTokens: [TokenSyntax]
245+
246+
public var message: String {
247+
"move \(nodesDescription(movedTokens, format: false)) in front of type"
248+
}
249+
}
250+
234251
public struct RemoveRedundantFixIt: ParserFixIt {
235252
public let removeTokens: [TokenSyntax]
236253

Sources/SwiftParser/Diagnostics/SyntaxExtensions.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,14 @@ extension SyntaxProtocol {
102102
return nil
103103
}
104104
}
105+
106+
extension TokenKind {
107+
var isIdentifier: Bool {
108+
switch self {
109+
case .identifier:
110+
return true
111+
default:
112+
return false
113+
}
114+
}
115+
}

Sources/SwiftParser/Lookahead.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ extension Parser {
5151
public func lookahead() -> Lookahead {
5252
return Lookahead(cloning: self)
5353
}
54+
55+
public func withLookahead<T>(_ body: (_: inout Lookahead) -> T) -> T {
56+
var lookahead = lookahead()
57+
return body(&lookahead)
58+
}
5459
}
5560

5661
extension Parser.Lookahead {

Sources/SwiftParser/Names.swift

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -237,15 +237,13 @@ extension Parser.Lookahead {
237237

238238
extension Lexer.Lexeme {
239239
var canBeArgumentLabel: Bool {
240-
switch self.tokenKind {
241-
case .identifier where self.tokenText == "__shared" || self.tokenText == "__owned":
240+
if TypeSpecifier(lexeme: self) != nil {
242241
return false
242+
}
243+
switch self.tokenKind {
243244
case .identifier, .wildcardKeyword:
244245
// Identifiers, escaped identifiers, and '_' can be argument labels.
245246
return true
246-
case .inoutKeyword:
247-
// inout cannot be used as an argument label.
248-
return false
249247
default:
250248
// All other keywords can be argument labels.
251249
return self.isKeyword

Sources/SwiftParser/Patterns.swift

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,13 @@ extension Parser.Lookahead {
316316

317317
/// Determine whether we are at the start of a parameter name when
318318
/// parsing a parameter.
319-
func startsParameterName(_ isClosure: Bool) -> Bool {
319+
/// If `allowMisplacedSpecifierRecovery` is `true`, then this will skip over any type
320+
/// specifiers before checking whether this lookahead starts a parameter name.
321+
mutating func startsParameterName(isClosure: Bool, allowMisplacedSpecifierRecovery: Bool) -> Bool {
322+
if allowMisplacedSpecifierRecovery {
323+
while self.consume(ifAnyIn: TypeSpecifier.self) != nil {}
324+
}
325+
320326
// To have a parameter name here, we need a name.
321327
guard self.currentToken.canBeArgumentLabel else {
322328
return false
@@ -341,12 +347,11 @@ extension Parser.Lookahead {
341347
// so look ahead one more token (two total) see if we have a ':' that would
342348
// indicate that this is an argument label.
343349
do {
344-
var backtrack = self.lookahead()
345-
if backtrack.at(.colon) {
350+
if self.at(.colon) {
346351
return true // isolated :
347352
}
348-
backtrack.consumeAnyToken()
349-
return backtrack.currentToken.canBeArgumentLabel && backtrack.peek().tokenKind == .colon
353+
self.consumeAnyToken()
354+
return self.currentToken.canBeArgumentLabel && self.peek().tokenKind == .colon
350355
}
351356
}
352357

Sources/SwiftParser/RawTokenKindSubset.swift

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,46 @@ enum PrimaryExpressionStart: RawTokenKindSubset {
851851
}
852852
}
853853

854+
enum TypeSpecifier: RawTokenKindSubset {
855+
case inoutKeyword
856+
case owned
857+
case shared
858+
859+
init?(lexeme: Lexer.Lexeme) {
860+
switch (lexeme.tokenKind, lexeme.tokenText) {
861+
case (.inoutKeyword, _): self = .inoutKeyword
862+
case (.identifier, "__owned"): self = .owned
863+
case (.identifier, "__shared"): self = .shared
864+
default: return nil
865+
}
866+
}
867+
868+
init?(token: TokenSyntax) {
869+
switch (token.tokenKind, token.text) {
870+
case (.inoutKeyword, _): self = .inoutKeyword
871+
case (.contextualKeyword, "__owned"): self = .owned
872+
case (.contextualKeyword, "__shared"): self = .shared
873+
default: return nil
874+
}
875+
}
876+
877+
var rawTokenKind: RawTokenKind {
878+
switch self {
879+
case .inoutKeyword: return .inoutKeyword
880+
case .owned: return .identifier
881+
case .shared: return .identifier
882+
}
883+
}
884+
885+
var contextualKeyword: SyntaxText? {
886+
switch self {
887+
case .inoutKeyword: return nil
888+
case .owned: return "__owned"
889+
case .shared: return "__shared"
890+
}
891+
}
892+
}
893+
854894
/// Union of the following token kind subsets:
855895
/// - `AwaitTry`
856896
/// - `ExpressionPrefixOperator`

Sources/SwiftParser/TokenConsumer.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,14 @@ extension TokenConsumer {
221221
return nil
222222
}
223223
}
224+
225+
mutating func consume<Subset: RawTokenKindSubset>(ifAnyIn subset: Subset.Type) -> Self.Token? {
226+
if let (_, handle) = self.at(anyIn: subset) {
227+
return self.eat(handle)
228+
} else {
229+
return nil
230+
}
231+
}
224232
}
225233

226234
// MARK: Expecting Tokens (`expect`)

Sources/SwiftParser/Types.swift

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ extension Parser {
3232
/// type → self-type
3333
/// type → '(' type ')'
3434
@_spi(RawSyntax)
35-
public mutating func parseType() -> RawTypeSyntax {
36-
let type = self.parseTypeScalar()
35+
public mutating func parseType(misplacedSpecifiers: [RawTokenSyntax] = []) -> RawTypeSyntax {
36+
let type = self.parseTypeScalar(misplacedSpecifiers: misplacedSpecifiers)
3737

3838
// Parse pack expansion 'T...'.
3939
if self.currentToken.isEllipsis {
@@ -47,8 +47,8 @@ extension Parser {
4747
return type
4848
}
4949

50-
mutating func parseTypeScalar() -> RawTypeSyntax {
51-
let (specifier, attrList) = self.parseTypeAttributeList()
50+
mutating func parseTypeScalar(misplacedSpecifiers: [RawTokenSyntax] = []) -> RawTypeSyntax {
51+
let (specifier, attrList) = self.parseTypeAttributeList(misplacedSpecifiers: misplacedSpecifiers)
5252
var base = RawTypeSyntax(self.parseSimpleOrCompositionType())
5353
if self.lookahead().isAtFunctionTypeArrow() {
5454
let firstEffect = self.parseEffectsSpecifier()
@@ -402,7 +402,11 @@ extension Parser {
402402
let second: RawTokenSyntax?
403403
let unexpectedBeforeColon: RawUnexpectedNodesSyntax?
404404
let colon: RawTokenSyntax?
405-
if self.lookahead().startsParameterName(false) {
405+
var misplacedSpecifiers: [RawTokenSyntax] = []
406+
if self.withLookahead({ $0.startsParameterName(isClosure: false, allowMisplacedSpecifierRecovery: true) }) {
407+
while let specifier = self.consume(ifAnyIn: TypeSpecifier.self) {
408+
misplacedSpecifiers.append(specifier)
409+
}
406410
first = self.parseArgumentLabel()
407411
if let parsedColon = self.consume(if: .colon) {
408412
second = nil
@@ -423,12 +427,13 @@ extension Parser {
423427
colon = nil
424428
}
425429
// Parse the type annotation.
426-
let type = self.parseType()
430+
let type = self.parseType(misplacedSpecifiers: misplacedSpecifiers)
427431
let ellipsis = self.currentToken.isEllipsis ? self.consumeAnyToken() : nil
428432
let trailingComma = self.consume(if: .comma)
429433
keepGoing = trailingComma != nil
430434
elements.append(RawTupleTypeElementSyntax(
431435
inOut: nil,
436+
RawUnexpectedNodesSyntax(misplacedSpecifiers, arena: self.arena),
432437
name: first,
433438
secondName: second,
434439
unexpectedBeforeColon,
@@ -593,7 +598,7 @@ extension Parser.Lookahead {
593598

594599
// If the tuple element starts with "ident :", then it is followed
595600
// by a type annotation.
596-
if self.startsParameterName(/*isClosure=*/false) {
601+
if self.startsParameterName(isClosure: false, allowMisplacedSpecifierRecovery: false) {
597602
self.consumeAnyToken()
598603
if self.currentToken.canBeArgumentLabel {
599604
self.consumeAnyToken()
@@ -783,8 +788,12 @@ extension Parser.Lookahead {
783788

784789
extension Parser {
785790
@_spi(RawSyntax)
786-
public mutating func parseTypeAttributeList() -> (RawTokenSyntax?, RawAttributeListSyntax?) {
787-
let specifier = self.consume(ifAny: [.inoutKeyword], contextualKeywords: ["__shared", "__owned"])
791+
public mutating func parseTypeAttributeList(misplacedSpecifiers: [RawTokenSyntax] = []) -> (RawTokenSyntax?, RawAttributeListSyntax?) {
792+
var specifier: RawTokenSyntax? = self.consume(ifAnyIn: TypeSpecifier.self)
793+
// We can only stick one specifier on this type. Let's pick the first one
794+
if specifier == nil, let misplacedSpecifier = misplacedSpecifiers.first {
795+
specifier = missingToken(misplacedSpecifier.tokenKind, text: misplacedSpecifier.tokenText)
796+
}
788797

789798
if self.at(any: [.atSign, .inoutKeyword]) {
790799
return (specifier, self.parseTypeAttributeListPresent())

0 commit comments

Comments
 (0)