Skip to content

Add diagnostics for missing modifier in operator declaration #1448

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
May 31, 2023
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
94 changes: 51 additions & 43 deletions CodeGeneration/Sources/SyntaxSupport/AttributeKinds.swift
Original file line number Diff line number Diff line change
Expand Up @@ -676,10 +676,57 @@ public let DECL_ATTR_KINDS: [Attribute] = [
///
/// If you're adding a new kind of attribute that is spelled with a leading
/// '@' symbol, add an entry to the `DECL_ATTR_KINDS` array instead.
public let DECL_MODIFIER_KINDS: [Attribute] = [
// These are not really attributes or modifiers in the C++ AST and they are
// serialized directly into the ASTs they are attached to rather than using
// the generic attribute serialization infrastructure.

// These are not really attributes or modifiers in the C++ AST and they are
// serialized directly into the ASTs they are attached to rather than using
// the generic attribute serialization infrastructure.
public let DECL_MODIFIER_KINDS: [Attribute] = CONTEXTUAL_DECL_MODIFIER_KINDS + NONCONTEXTUAL_DECL_MODIFIER_KINDS

// Modifiers that not exclusively used for declarations
public let CONTEXTUAL_DECL_MODIFIER_KINDS: [Attribute] = [
ContextualDeclAttribute(
name: "weak",
className: "ReferenceOwnership"
),
ContextualDeclAttributeAlias(
name: "unowned",
className: "ReferenceOwnership",
swiftName: "unowned"
),
SimpleDeclAttribute(
name: "rethrows",
className: "Rethrows",
swiftName: "`rethrows`"
),
ContextualSimpleDeclAttribute(
name: "isolated",
className: "Isolated"
),
ContextualSimpleDeclAttribute(
name: "async",
className: "Async"
),
SimpleDeclAttribute(
name: "reasync",
className: "Reasync",
swiftName: "reasync"
),
ContextualSimpleDeclAttribute(
name: "consuming",
className: "Consuming"
),
ContextualSimpleDeclAttribute(
name: "borrowing",
className: "Borrowing"
),
ContextualSimpleDeclAttribute(
name: "_const",
className: "CompileTimeConst"
),
]

// Modifiers that exclusively used for declarations
public let NONCONTEXTUAL_DECL_MODIFIER_KINDS: [Attribute] = [
BuiltinDeclModifier(
name: "static",
swiftName: "`static`"
Expand Down Expand Up @@ -775,37 +822,10 @@ public let DECL_MODIFIER_KINDS: [Attribute] = [
className: "SetterAccess",
swiftName: "__setter_access"
),
ContextualDeclAttribute(
name: "weak",
className: "ReferenceOwnership"
),
ContextualDeclAttributeAlias(
name: "unowned",
className: "ReferenceOwnership",
swiftName: "unowned"
),
SimpleDeclAttribute(
name: "rethrows",
className: "Rethrows",
swiftName: "`rethrows`"
),
ContextualSimpleDeclAttribute(
name: "indirect",
className: "Indirect"
),
ContextualSimpleDeclAttribute(
name: "isolated",
className: "Isolated"
),
ContextualSimpleDeclAttribute(
name: "async",
className: "Async"
),
SimpleDeclAttribute(
name: "reasync",
className: "Reasync",
swiftName: "reasync"
),
ContextualSimpleDeclAttribute(
name: "nonisolated",
className: "Nonisolated"
Expand All @@ -814,22 +834,10 @@ public let DECL_MODIFIER_KINDS: [Attribute] = [
name: "distributed",
className: "DistributedActor"
),
ContextualSimpleDeclAttribute(
name: "_const",
className: "CompileTimeConst"
),
ContextualSimpleDeclAttribute(
name: "_local",
className: "KnownToBeLocal"
),
ContextualSimpleDeclAttribute(
name: "consuming",
className: "Consuming"
),
ContextualSimpleDeclAttribute(
name: "borrowing",
className: "Borrowing"
),
]

public let DEPRECATED_MODIFIER_KINDS: [Attribute] = [
Expand Down
21 changes: 5 additions & 16 deletions CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1422,25 +1422,14 @@ public let DECL_NODES: [Node] = [
nameForDiagnostics: "operator declaration",
documentation: "A Swift `operator` declaration.",
traits: [
"IdentifiedDecl",
"WithAttributes",
"WithModifiers",
"IdentifiedDecl"
],
children: [
Child(
name: "Attributes",
kind: .collection(kind: .attributeList, collectionElementName: "Attribute"),
nameForDiagnostics: "attributes",
documentation: "The attributes applied to the 'operator' declaration.",
isOptional: true
),
Child(
name: "Modifiers",
kind: .collection(kind: .modifierList, collectionElementName: "Modifier"),
nameForDiagnostics: "modifiers",
documentation: "The declaration modifiers applied to the 'operator' declaration.",
isOptional: true,
classification: "Attribute"
name: "Fixity",
kind: .token(choices: [.keyword(text: "prefix"), .keyword(text: "postfix"), .keyword(text: "infix")]),
nameForDiagnostics: "fixity",
documentation: "The fixity applied to the 'operator' declaration."
),
Child(
name: "OperatorKeyword",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ let declarationModifierFile = SourceFileSyntax(leadingTrivia: copyrightHeader) {

try VariableDeclSyntax("var spec: TokenSpec") {
try SwitchExprSyntax("switch self") {
for attribute in DECL_MODIFIER_KINDS {
for attribute in NONCONTEXTUAL_DECL_MODIFIER_KINDS {
SwitchCaseSyntax("case .\(raw: attribute.swiftName):") {
StmtSyntax("return .keyword(.\(raw: attribute.swiftName))")
}
}
for attribute in CONTEXTUAL_DECL_MODIFIER_KINDS {
SwitchCaseSyntax("case .\(raw: attribute.swiftName):") {
if attribute.swiftName.hasSuffix("Keyword") {
StmtSyntax("return .\(raw: attribute.swiftName)")
} else {
StmtSyntax("return .keyword(.\(raw: attribute.swiftName))")
}
StmtSyntax("return TokenSpec(.\(raw: attribute.swiftName), recoveryPrecedence: .declKeyword)")
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions Sources/SwiftIDEUtils/generated/SyntaxClassification.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ extension SyntaxClassification {
return (.buildConfigId, false)
case \MemberTypeIdentifierSyntax.name:
return (.typeIdentifier, false)
case \OperatorDeclSyntax.modifiers:
return (.attribute, false)
case \OperatorDeclSyntax.identifier:
return (.operatorIdentifier, false)
case \PrecedenceGroupAssociativitySyntax.associativityKeyword:
Expand Down
6 changes: 1 addition & 5 deletions Sources/SwiftOperators/OperatorTable+Semantics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ extension Operator {
/// TODO: This ignores all semantic errors.
init(from syntax: OperatorDeclSyntax) {
self.syntax = syntax

kind =
syntax.modifiers?.compactMap {
OperatorKind(rawValue: $0.name.text)
}.first ?? .infix
kind = OperatorKind(rawValue: syntax.fixity.text) ?? .infix

name = syntax.identifier.text

Expand Down
6 changes: 2 additions & 4 deletions Sources/SwiftOperators/SyntaxSynthesis.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ extension Operator {
/// Synthesize a syntactic representation of this operator based on its
/// semantic definition.
public func synthesizedSyntax() -> OperatorDeclSyntax {
let modifiers = ModifierListSyntax(
[DeclModifierSyntax(name: .keyword(kind.keyword))]
)
let fixity = TokenSyntax.keyword(kind.keyword)
let operatorKeyword = TokenSyntax.keyword(.operator, leadingTrivia: .space)
let identifierSyntax =
TokenSyntax.binaryOperator(name, leadingTrivia: .space)
Expand All @@ -41,7 +39,7 @@ extension Operator {
}

return OperatorDeclSyntax(
modifiers: modifiers,
fixity: fixity,
operatorKeyword: operatorKeyword,
identifier: identifierSyntax,
operatorPrecedenceAndTypes: precedenceGroupSyntax
Expand Down
76 changes: 67 additions & 9 deletions Sources/SwiftParser/Declarations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ extension TokenConsumer {
return attrLookahead.consumeIfConfigOfAttributes()
}

let declStartKeyword: DeclarationStart?
let declStartKeyword: DeclarationKeyword?
if allowRecovery {
declStartKeyword =
subparser.canRecoverTo(
anyIn: DeclarationStart.self,
anyIn: DeclarationKeyword.self,
overrideRecoveryPrecedence: isAtTopLevel ? nil : .closingBrace
)?.0
} else {
declStartKeyword = subparser.at(anyIn: DeclarationStart.self)?.0
declStartKeyword = subparser.at(anyIn: DeclarationKeyword.self)?.0
}
switch declStartKeyword {
case .actor:
Expand Down Expand Up @@ -233,7 +233,7 @@ extension Parser {
// while recoverying to the declaration start.
let recoveryPrecedence = inMemberDeclList ? TokenPrecedence.closingBrace : nil

switch self.canRecoverTo(anyIn: DeclarationStart.self, overrideRecoveryPrecedence: recoveryPrecedence) {
switch self.canRecoverTo(anyIn: DeclarationKeyword.self, overrideRecoveryPrecedence: recoveryPrecedence) {
case (.import, let handle)?:
return RawDeclSyntax(self.parseImportDeclaration(attrs, handle))
case (.class, let handle)?:
Expand Down Expand Up @@ -1694,11 +1694,69 @@ extension Parser {
/// postfix-operator-declaration → 'postfix' 'operator' operator
/// infix-operator-declaration → 'infix' 'operator' operator infix-operator-group?
/// infix-operator-group → ':' precedence-group-name

struct OperatorDeclIntroducer {
var unexpectedBeforeFixity: RawUnexpectedNodesSyntax?
var fixity: RawTokenSyntax
var unexpectedBeforeOperatorKeyword: RawUnexpectedNodesSyntax?
var operatorKeyword: RawTokenSyntax
}

mutating func parseOperatorDeclIntroducer(_ attrs: DeclAttributes, _ handle: RecoveryConsumptionHandle) -> OperatorDeclIntroducer {
func isFixity(_ modifier: RawDeclModifierSyntax) -> Bool {
switch modifier.name {
case .keyword(.prefix),
.keyword(.infix),
.keyword(.postfix):
return true
default:
return false
}
}

var unexpectedBeforeFixity = RawUnexpectedNodesSyntax(attrs.attributes?.elements ?? [], arena: self.arena)

var fixity: RawTokenSyntax?
var unexpectedAfterFixity: RawUnexpectedNodesSyntax?

if let modifiers = attrs.modifiers?.elements {
if let firstFixityIndex = modifiers.firstIndex(where: { isFixity($0) }) {
let fixityModifier = modifiers[firstFixityIndex]
fixity = fixityModifier.name

unexpectedBeforeFixity = RawUnexpectedNodesSyntax(combining: unexpectedBeforeFixity, RawUnexpectedNodesSyntax(Array(modifiers[0..<firstFixityIndex]), arena: self.arena), fixityModifier.unexpectedBeforeName, arena: self.arena)

unexpectedAfterFixity = RawUnexpectedNodesSyntax(
combining: fixityModifier.unexpectedBetweenNameAndDetail,
RawUnexpectedNodesSyntax([fixityModifier.detail], arena: self.arena),
fixityModifier.unexpectedAfterDetail,
RawUnexpectedNodesSyntax(Array(modifiers[modifiers.index(after: firstFixityIndex)...]), arena: self.arena),
arena: self.arena
)

} else {
unexpectedBeforeFixity = RawUnexpectedNodesSyntax(combining: unexpectedBeforeFixity, RawUnexpectedNodesSyntax(modifiers, arena: self.arena), arena: self.arena)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

If modifiers.firstIndex(where: { isFixity($0) }) returns nil, we drop the modifiers. To avoid that, you need to add an else branch to the if statement that adds all modifiers to unexpectedBeforeFixity or unexpectedBeforeOperatorKeyword`. Which one you choose doesn’t really matter here because there are no present tokens between these two unexpected sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I also added a test case for this.


var (unexpectedBeforeOperatorKeyword, operatorKeyword) = self.expect(.keyword(.operator))

unexpectedBeforeOperatorKeyword = RawUnexpectedNodesSyntax(combining: unexpectedAfterFixity, unexpectedBeforeOperatorKeyword, arena: self.arena)

return OperatorDeclIntroducer(
unexpectedBeforeFixity: unexpectedBeforeFixity,
fixity: fixity ?? self.missingToken(.prefix),
unexpectedBeforeOperatorKeyword: unexpectedBeforeOperatorKeyword,
operatorKeyword: operatorKeyword
)
}

mutating func parseOperatorDeclaration(
_ attrs: DeclAttributes,
_ handle: RecoveryConsumptionHandle
) -> RawOperatorDeclSyntax {
let (unexpectedBeforeOperatorKeyword, operatorKeyword) = self.eat(handle)
let introducer = parseOperatorDeclIntroducer(attrs, handle)

let unexpectedBeforeName: RawUnexpectedNodesSyntax?
let name: RawTokenSyntax
switch self.canRecoverTo(anyIn: OperatorLike.self) {
Expand Down Expand Up @@ -1775,10 +1833,10 @@ extension Parser {
unexpectedAtEnd = nil
}
return RawOperatorDeclSyntax(
attributes: attrs.attributes,
modifiers: attrs.modifiers,
unexpectedBeforeOperatorKeyword,
operatorKeyword: operatorKeyword,
introducer.unexpectedBeforeFixity,
fixity: introducer.fixity,
introducer.unexpectedBeforeOperatorKeyword,
operatorKeyword: introducer.operatorKeyword,
unexpectedBeforeName,
identifier: name,
RawUnexpectedNodesSyntax(identifiersAfterOperatorName, arena: self.arena),
Expand Down
Loading