Skip to content

Simplify signature of makeMissing and makePresent #961

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

Merged
merged 1 commit into from
Oct 17, 2022
Merged
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
31 changes: 7 additions & 24 deletions Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ extension FixIt {

extension FixIt.Changes {
/// Replaced a present token with a missing node
static func makeMissing(token: TokenSyntax) -> Self {
return makeMissing(tokens: [token])
static func makeMissing(_ token: TokenSyntax) -> Self {
return makeMissing([token])
}

/// Replace present tokens with missing tokens
static func makeMissing(tokens: [TokenSyntax]) -> Self {
static func makeMissing(_ tokens: [TokenSyntax]) -> Self {
assert(!tokens.isEmpty)
assert(tokens.allSatisfy({ $0.presence == .present }))
var changes = tokens.map {
Expand All @@ -52,33 +52,16 @@ extension FixIt.Changes {
return FixIt.Changes(changes: changes)
}

static func makeMissing<SyntaxType: SyntaxProtocol>(node: SyntaxType) -> Self {
static func makeMissing<SyntaxType: SyntaxProtocol>(_ node: SyntaxType) -> Self {
return FixIt.Changes(changes: [
.replace(oldNode: Syntax(node), newNode: MissingMaker().visit(Syntax(node)))
])
}

/// Remove the nodes in `unexpected`.
static func remove(unexpected: UnexpectedNodesSyntax) -> Self {
var changes: [FixIt.Change] = [
FixIt.Change.replace(
oldNode: Syntax(unexpected),
newNode: Syntax(UnexpectedNodesSyntax([]))
)
]
if let firstToken = unexpected.firstToken(viewMode: .sourceAccurate),
firstToken.leadingTrivia.isEmpty == false,
let nextToken = unexpected.lastToken(viewMode: .sourceAccurate)?.nextToken(viewMode: .sourceAccurate),
!nextToken.leadingTrivia.contains(where: { $0.isNewline }) {
changes.append(.replaceLeadingTrivia(token: nextToken, newTrivia: firstToken.leadingTrivia))
}
return FixIt.Changes(changes: changes)
}

/// Make a node present. If `leadingTrivia` or `trailingTrivia` is specified,
/// override the default leading/trailing trivia inferred from `BasicFormat`.
static func makePresent<T: SyntaxProtocol>(
node: T,
_ node: T,
leadingTrivia: Trivia? = nil,
trailingTrivia: Trivia? = nil
) -> Self {
Expand All @@ -96,7 +79,7 @@ extension FixIt.Changes {
}

/// Makes the `token` present, moving it in front of the previous token's trivia.
static func makePresentBeforeTrivia(token: TokenSyntax) -> Self {
static func makePresentBeforeTrivia(_ token: TokenSyntax) -> Self {
if let previousToken = token.previousToken(viewMode: .sourceAccurate) {
var presentToken = PresentMaker().visit(token).as(TokenSyntax.self)!
if !previousToken.trailingTrivia.isEmpty {
Expand All @@ -107,7 +90,7 @@ extension FixIt.Changes {
.replace(oldNode: Syntax(token), newNode: Syntax(presentToken)),
]
} else {
return .makePresent(node: token)
return .makePresent(token)
}
}
}
4 changes: 2 additions & 2 deletions Sources/SwiftParser/Diagnostics/MissingNodesError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@ extension ParseDiagnosticsGenerator {

let changes = missingNodes.enumerated().map { (index, missingNode) -> FixIt.Changes in
if index == 0, let token = missingNode.as(TokenSyntax.self), token.tokenKind.isPunctuation == true, token.previousToken(viewMode: .sourceAccurate)?.tokenKind.isPunctuation == false {
return .makePresentBeforeTrivia(token: token)
return .makePresentBeforeTrivia(token)
} else {
return .makePresent(node: missingNode)
return .makePresent(missingNode)
}
}
let fixIt = FixIt(
Expand Down
54 changes: 20 additions & 34 deletions Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if misplacedTokens.count == 1, let misplacedToken = misplacedTokens.first,
misplacedToken.nextToken(viewMode: .all) == correctToken || misplacedToken.previousToken(viewMode: .all) == correctToken {
changes.append(FixIt.Changes.makePresent(
node: correctToken,
correctToken,
leadingTrivia: misplacedToken.leadingTrivia,
trailingTrivia: misplacedToken.trailingTrivia
))
} else {
changes.append(FixIt.Changes.makePresent(
node: correctToken,
correctToken,
leadingTrivia: nil,
trailingTrivia: nil
))
Expand Down Expand Up @@ -161,7 +161,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
}
let fixit = FixIt(
message: RemoveNodesFixIt(unexpected),
changes: [.remove(unexpected: unexpected)]
changes: .makeMissing(unexpected)
)
addDiagnostic(
unexpected, message(misplacedToken), fixIts: [fixit],
Expand Down Expand Up @@ -205,14 +205,14 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
var fixIts: [FixIt] = [
FixIt(message: .joinIdentifiers, changes: [
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
.makeMissing(tokens: tokens)
.makeMissing(tokens)
])
]
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
let joinedUsingCamelCase = previousToken.text + tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
fixIts.append(FixIt(message: .joinIdentifiersWithCamelCase, changes: [
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
.makeMissing(tokens: tokens)
.makeMissing(tokens)
]))
}
addDiagnostic(node, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: tokens), fixIts: fixIts)
Expand Down Expand Up @@ -281,17 +281,15 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
// If the item contains errors, the root cause is most likely something different and not the missing semicolon.
let position = semicolon.previousToken(viewMode: .sourceAccurate)?.endPositionBeforeTrailingTrivia
addDiagnostic(semicolon, position: position, .consecutiveStatementsOnSameLine, fixIts: [
FixIt(message: .insertSemicolon, changes: .makePresentBeforeTrivia(token: semicolon))
FixIt(message: .insertSemicolon, changes: .makePresentBeforeTrivia(semicolon))
], handledNodes: [semicolon.id])
} else {
handledNodes.append(semicolon.id)
}
}
if let semicolon = node.semicolon, semicolon.presence == .present, node.item.isMissingAllTokens {
addDiagnostic(node, .standaloneSemicolonStatement, fixIts: [
FixIt(message: RemoveNodesFixIt(semicolon), changes: [
.makeMissing(tokens: [semicolon])
])
FixIt(message: RemoveNodesFixIt(semicolon), changes: .makeMissing(semicolon))
], handledNodes: [node.item.id])
}
if let switchCase = node.unexpectedBeforeItem?.only?.as(SwitchCaseSyntax.self) {
Expand All @@ -312,9 +310,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if let unexpected = node.unexpectedAfterOperatorPrecedenceAndTypes,
unexpected.contains(where: { $0.is(PrecedenceGroupAttributeListSyntax.self) }) == true {
addDiagnostic(unexpected, .operatorShouldBeDeclaredWithoutBody, fixIts: [
FixIt(message: .removeOperatorBody, changes: [
.remove(unexpected: unexpected)
])
FixIt(message: .removeOperatorBody, changes: .makeMissing(unexpected))
], handledNodes: [unexpected.id])
}

Expand All @@ -333,9 +329,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if let message = message {
let fixIts: [FixIt]
if node.identifier.presence == .present {
fixIts = [
FixIt(message: RemoveNodesFixIt(unexpected), changes: .remove(unexpected: unexpected))
]
fixIts = [FixIt(message: RemoveNodesFixIt(unexpected), changes: .makeMissing(unexpected))]
} else {
fixIts = []
}
Expand All @@ -356,13 +350,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if let unexpected = node.unexpectedBetweenSubscriptKeywordAndGenericParameterClause,
let nameTokens = unexpected.onlyTokens(satisfying: { !$0.tokenKind.isKeyword }) {
addDiagnostic(unexpected, .subscriptsCannotHaveNames, fixIts: [
FixIt(message: RemoveNodesFixIt(nameTokens), changes: .makeMissing(tokens: nameTokens))
FixIt(message: RemoveNodesFixIt(nameTokens), changes: .makeMissing(nameTokens))
], handledNodes: [unexpected.id])
}
if let unexpected = node.indices.unexpectedBeforeLeftParen,
let nameTokens = unexpected.onlyTokens(satisfying: { !$0.tokenKind.isKeyword }) {
addDiagnostic(unexpected, .subscriptsCannotHaveNames, fixIts: [
FixIt(message: RemoveNodesFixIt(nameTokens), changes: .makeMissing(tokens: nameTokens))
FixIt(message: RemoveNodesFixIt(nameTokens), changes: .makeMissing(nameTokens))
], handledNodes: [unexpected.id])
}
return .visitChildren
Expand All @@ -388,7 +382,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
// If the decl contains errors, the root cause is most likely something different and not the missing semicolon.
let position = semicolon.previousToken(viewMode: .sourceAccurate)?.endPositionBeforeTrailingTrivia
addDiagnostic(semicolon, position: position, .consecutiveDeclarationsOnSameLine, fixIts: [
FixIt(message: .insertSemicolon, changes: .makePresentBeforeTrivia(token: semicolon))
FixIt(message: .insertSemicolon, changes: .makePresentBeforeTrivia(semicolon))
], handledNodes: [semicolon.id])
} else {
handledNodes.append(semicolon.id)
Expand Down Expand Up @@ -427,9 +421,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
}
if let argument = node.argument, argument.isMissingAllTokens {
addDiagnostic(argument, MissingAttributeArgument(attributeName: node.attributeName), fixIts: [
FixIt(message: .insertAttributeArguments, changes: [
.makePresent(node: argument)
])
FixIt(message: .insertAttributeArguments, changes: .makePresent(argument))
], handledNodes: [argument.id])
return .visitChildren
}
Expand Down Expand Up @@ -457,13 +449,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if let unexpected = node.unexpectedBetweenDeinitKeywordAndBody,
let name = unexpected.filter({ $0.as(TokenSyntax.self)?.tokenKind.isIdentifier == true }).only?.as(TokenSyntax.self) {
addDiagnostic(name, .deinitCannotHaveName, fixIts: [
FixIt(message: RemoveNodesFixIt(name), changes: .makeMissing(token: name))
FixIt(message: RemoveNodesFixIt(name), changes: .makeMissing(name))
], handledNodes: [name.id])
}
if let unexpected = node.unexpectedBetweenDeinitKeywordAndBody,
let signature = unexpected.compactMap({ $0.as(FunctionSignatureSyntax.self) }).only {
addDiagnostic(signature, .deinitCannotHaveParameters, fixIts: [
FixIt(message: RemoveNodesFixIt(signature), changes: .makeMissing(node: signature))
FixIt(message: RemoveNodesFixIt(signature), changes: .makeMissing(signature))
], handledNodes: [signature.id])
}

Expand Down Expand Up @@ -600,9 +592,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
}
if node.unknownAttr?.isMissingAllTokens != false && node.label.isMissingAllTokens {
addDiagnostic(node.statements, .allStatmentsInSwitchMustBeCoveredByCase, fixIts: [
FixIt(message: InsertTokenFixIt(missingNodes: [node.label]), changes: [
.makePresent(node: node.label, leadingTrivia: .newline)
])
FixIt(message: InsertTokenFixIt(missingNodes: [node.label]), changes: .makePresent(node.label, leadingTrivia: .newline))
], handledNodes: [node.label.id])
}
return .visitChildren
Expand All @@ -628,9 +618,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
}
if node.expression.is(MissingExprSyntax.self) {
addDiagnostic(node.expression, .expectedExpressionAfterTry, fixIts: [
FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.expression)]), changes: [
.makePresent(node: node.expression)
])
FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.expression)]), changes: .makePresent(node.expression))
], handledNodes: [node.expression.id])
}
return .visitChildren
Expand Down Expand Up @@ -677,15 +665,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
nextSibling.is(MissingExprSyntax.self) {
addDiagnostic(node.colonMark, .missingColonAndExprInTernaryExpr, fixIts: [
FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.colonMark), Syntax(nextSibling)]), changes: [
.makePresent(node: node.colonMark),
.makePresent(node: nextSibling),
.makePresent(node.colonMark),
.makePresent(nextSibling),
])
], handledNodes: [node.colonMark.id, nextSibling.id])
} else {
addDiagnostic(node.colonMark, .missingColonInTernaryExpr, fixIts: [
FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.colonMark)]), changes: [
.makePresent(node: node.colonMark),
])
FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.colonMark)]), changes: .makePresent(node.colonMark))
], handledNodes: [node.colonMark.id])
}
}
Expand Down