Skip to content

[5.9] Add diagnostic for array type #1573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion Sources/SwiftParser/Types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,37 @@ extension Parser {
)
)
} else {
return self.parseType()
var result = self.parseType()

guard !result.hasError else {
return result
}

if self.at(.rightSquareBracket) {
let (unexpectedBeforeRSquareBracket, rightSquareBracket) = self.expect(.rightSquareBracket)
Comment on lines +1073 to +1074
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually looking at this code again, I think we should only do this recovery if the ] is not on a new line. And I also realized that we could write this as follows. If you could incorporate those changes in main, that would be great.

Suggested change
if self.at(.rightSquareBracket) {
let (unexpectedBeforeRSquareBracket, rightSquareBracket) = self.expect(.rightSquareBracket)
if let rightSquareBracket = self.consume(if: .rightSquareBracket) {

And since I just found another thing to comment on and this PR only provide value in a very niche use case, I’d prefer not to cherry-pick it to 5.9.

result = RawTypeSyntax(
RawArrayTypeSyntax(
leftSquareBracket: missingToken(.leftSquareBracket),
elementType: result,
unexpectedBeforeRSquareBracket,
rightSquareBracket: rightSquareBracket,
arena: self.arena
)
)
}

var loopProgress = LoopProgressCondition()
while loopProgress.evaluate(currentToken) {
if self.at(TokenSpec(.postfixQuestionMark, allowAtStartOfLine: false)) {
result = RawTypeSyntax(self.parseOptionalType(result))
} else if self.at(TokenSpec(.exclamationMark, allowAtStartOfLine: false)) {
result = RawTypeSyntax(self.parseImplicitlyUnwrappedOptionalType(result))
} else {
break
}
}

return result
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,23 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
return .visitChildren
}

public override func visit(_ node: ArrayTypeSyntax) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
return .skipChildren
}

if node.leftSquareBracket.presence == .missing && node.rightSquareBracket.presence == .present {
addDiagnostic(
node.rightSquareBracket,
.extraRightBracket,
fixIts: [.init(message: InsertFixIt(tokenToBeInserted: node.leftSquareBracket), changes: .makePresent(node.leftSquareBracket))],
handledNodes: [node.leftSquareBracket.id]
)
}

return .visitChildren
}

public override func visit(_ node: AttributeSyntax) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
return .skipChildren
Expand Down
11 changes: 11 additions & 0 deletions Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ extension DiagnosticMessage where Self == StaticParserError {
public static var expectedSequenceExpressionInForEachLoop: Self {
.init("expected Sequence expression for for-each loop")
}
public static var extraRightBracket: Self {
.init("unexpected ']' in type; did you mean to write an array type?")
}
public static var initializerInPattern: Self {
.init("unexpected initializer in pattern; did you mean to use '='?")
}
Expand Down Expand Up @@ -545,6 +548,14 @@ extension FixItMessage where Self == StaticParserFixIt {
}
}

public struct InsertFixIt: ParserFixIt {
public let tokenToBeInserted: TokenSyntax

public var message: String {
"insert '\(tokenToBeInserted.text)'"
}
}

public struct MoveTokensAfterFixIt: ParserFixIt {
/// The token that should be moved
public let movedTokens: [TokenSyntax]
Expand Down
8 changes: 8 additions & 0 deletions Sources/SwiftSyntax/Raw/RawSyntaxNodeProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ public extension RawSyntaxNodeProtocol {
var isEmpty: Bool {
return raw.byteLength == 0
}

/// Whether the tree contained by this layout has any
/// - missing nodes or
/// - unexpected nodes or
/// - tokens with a `TokenDiagnostic` of severity `error`
var hasError: Bool {
return raw.recursiveFlags.contains(.hasError)
}
}

/// `RawSyntax` itself conforms to `RawSyntaxNodeProtocol`.
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftSyntax/Syntax.swift
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ public extension SyntaxProtocol {
/// Whether the tree contained by this layout has any
/// - missing nodes or
/// - unexpected nodes or
/// - tokens with a `TokenDiagnostic` of severity `error`
/// - tokens with a ``TokenDiagnostic`` of severity ``TokenDiagnostic/Severity-swift.enum/error``.
var hasError: Bool {
return raw.recursiveFlags.contains(.hasError)
return raw.hasError
}

/// Whether the tree contained by this layout has any tokens with a
Expand Down
97 changes: 24 additions & 73 deletions Tests/SwiftParserTest/translated/RecoveryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1119,15 +1119,27 @@ final class RecoveryTests: XCTestCase {
)
}

func testRecovery98a() {
assertParse(
"""
let a1: Swift.Int1️⃣]
""",
diagnostics: [
DiagnosticSpec(message: "extraneous code ']' at top level")
]
)
func testRecovery98() {
let testCases: [UInt: (testCase: String, fixedSource: String)] = [
#line: ("let a1: Swift.Int1️⃣]", "let a1: [Swift.Int]"),
#line: ("let a3: Set<Int>1️⃣]", "let a3: [Set<Int>]"),
#line: ("let a4: Int1️⃣]?", "let a4: [Int]?"),
#line: ("let a5: Int?1️⃣]", "let a5: [Int?]"),
#line: ("let a6: [Int]1️⃣]", "let a6: [[Int]]"),
#line: ("let a7: [String: Int]1️⃣]", "let a7: [[String: Int]]"),
#line: ("func foo() -> Int1️⃣]??", "func foo() -> [Int]??"),
]

for (line, testCase) in testCases {
assertParse(
testCase.testCase,
diagnostics: [
DiagnosticSpec(message: "unexpected ']' in type; did you mean to write an array type?", fixIts: ["insert '['"], line: line)
],
fixedSource: testCase.fixedSource,
line: line
)
}
}

func testRecovery98b() {
Expand All @@ -1142,66 +1154,6 @@ final class RecoveryTests: XCTestCase {
)
}

func testRecovery98c() {
assertParse(
"""
let a3: Set<Int>1️⃣]
""",
diagnostics: [
// TODO: Old parser expected error on line 4: unexpected ']' in type; did you mean to write an array type?, Fix-It replacements: 11 - 11 = '['
DiagnosticSpec(message: "extraneous code ']' at top level")
]
)
}

func testRecovery98d() {
assertParse(
"""
let a4: Int1️⃣]?
""",
diagnostics: [
// TODO: Old parser expected error on line 5: unexpected ']' in type; did you mean to write an array type?, Fix-It replacements: 11 - 11 = '['
DiagnosticSpec(message: "extraneous code ']?' at top level")
]
)
}

func testRecovery98e() {
assertParse(
"""
let a5: Int?1️⃣]
""",
diagnostics: [
// TODO: Old parser expected error on line 6: unexpected ']' in type; did you mean to write an array type?, Fix-It replacements: 11 - 11 = '['
DiagnosticSpec(message: "extraneous code ']' at top level")
]
)
}

func testRecovery98f() {
assertParse(
"""
let a6: [Int]1️⃣]
""",
diagnostics: [
// TODO: Old parser expected error on line 7: unexpected ']' in type; did you mean to write an array type?, Fix-It replacements: 11 - 11 = '['
DiagnosticSpec(message: "extraneous code ']' at top level")
]
)
}

func testRecovery98g() {
assertParse(
"""
let a7: [String: Int]1️⃣]
""",
diagnostics: [
// TODO: Old parser expected error on line 8: unexpected ']' in type; did you mean to write an array type?, Fix-It replacements: 11 - 11 = '['
DiagnosticSpec(message: "extraneous code ']' at top level")
]
)
}

func testRecovery99() {
assertParse(
"""
Expand Down Expand Up @@ -1239,10 +1191,9 @@ final class RecoveryTests: XCTestCase {
4️⃣}
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '}' to end struct"),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ']' to end array"),
// TODO: Old parser expected error on line 5: unexpected ']' in type; did you mean to write an array type?, Fix-It replacements: 17 - 17 = '['
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code ']' in function"),
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '}' to end struct", fixIts: ["insert '}'"]),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ']' to end array", fixIts: ["insert ']'"]),
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected ']' in type; did you mean to write an array type?", fixIts: ["insert '['"]),
DiagnosticSpec(locationMarker: "4️⃣", message: "extraneous brace at top level"),
]
)
Expand Down