Cleanup error paths (#39)

rauhul · web-flow · commit 290b28d10e1a · 2023-11-17T20:41:42.000-05:00
Updates `MacroContext.error(at:message:highlights:notes:fixIts:)` to
return an `ExpansionError` which can be immediately thrown by the caller
to simplify the early exit path. After this change `ExpansionError`
should no longer be directly created.
diff --git a/Sources/MMIOMacros/Macros/Arguments/BitFieldTypeProjection.swift b/Sources/MMIOMacros/Macros/Arguments/BitFieldTypeProjection.swift
@@ -30,11 +30,10 @@ extension BitFieldTypeProjection: ExpressibleByExprSyntax {
       let base = memberAccess.base,
       memberAccess.declName.baseName.tokenKind == .keyword(.`self`)
     else {
-      context.error(
+      throw context.error(
         at: expression,
         message: .expectedTypeReferenceLiteral(),
         fixIts: .replaceExpressionWithTypeReference(node: expression))
-      throw ExpansionError()
     }
     self.expression = base
   }
diff --git a/Sources/MMIOMacros/Macros/Arguments/BitWidth.swift b/Sources/MMIOMacros/Macros/Arguments/BitWidth.swift
@@ -26,10 +26,9 @@ extension BitWidth: ExpressibleByExprSyntax {
     let value = try Int(expression: expression, in: context)
     let validBitWidths = [8, 16, 32, 64]
     guard validBitWidths.contains(value) else {
-      context.error(
+      throw context.error(
         at: expression,
         message: .expectedLiteralValue(in: validBitWidths))
-      throw ExpansionError()
     }
     self.value = value
   }
diff --git a/Sources/MMIOMacros/Macros/Arguments/Int+ExpressibleByExprSyntax.swift b/Sources/MMIOMacros/Macros/Arguments/Int+ExpressibleByExprSyntax.swift
@@ -21,10 +21,9 @@ extension Int: ExpressibleByExprSyntax {
       let intLiteral = expression.as(IntegerLiteralExprSyntax.self),
       let int = intLiteral.value
     else {
-      context.error(
+      throw context.error(
         at: expression,
         message: .expectedIntegerLiteral())
-      throw ExpansionError()
     }
     self = int
   }
diff --git a/Sources/MMIOMacros/Macros/Arguments/Range+ExpressibleByExprSyntax.swift b/Sources/MMIOMacros/Macros/Arguments/Range+ExpressibleByExprSyntax.swift
@@ -27,10 +27,9 @@ extension Range: ExpressibleByExprSyntax where Bound: ExpressibleByExprSyntax {
     } else if let sequence = expression.as(SequenceExprSyntax.self) {
       let elements = sequence.elements
       guard elements.count == 3 else {
-        context.error(
+        throw context.error(
           at: sequence,
           message: .expectedRangeLiteral())
-        throw ExpansionError()
       }
 
       let index0 = elements.startIndex
@@ -44,10 +43,9 @@ extension Range: ExpressibleByExprSyntax where Bound: ExpressibleByExprSyntax {
         right: elements[index2],
         in: context)
     } else {
-      context.error(
+      throw context.error(
         at: expression,
         message: .expectedRangeLiteral())
-      throw ExpansionError()
     }
   }
 
@@ -62,19 +60,17 @@ extension Range: ExpressibleByExprSyntax where Bound: ExpressibleByExprSyntax {
       let op = op.as(BinaryOperatorExprSyntax.self),
       op.operator.text == "..<"
     else {
-      context.error(
+      throw context.error(
         at: overall,
         message: .expectedRangeLiteral())
-      throw ExpansionError()
     }
 
     let left = try Bound(expression: left, in: context)
     let right = try Bound(expression: right, in: context)
     guard left < right else {
-      context.error(
+      throw context.error(
         at: overall,
         message: .expectedRangeLiteral())
-      throw ExpansionError()
     }
     return Self(uncheckedBounds: (left, right))
   }
diff --git a/Sources/MMIOMacros/Macros/BitFieldMacro.swift b/Sources/MMIOMacros/Macros/BitFieldMacro.swift
@@ -35,10 +35,9 @@ extension BitFieldMacro {
   ) throws -> [AccessorDeclSyntax] {
     // Can only applied to variables.
     guard let variableDecl = declaration.as(VariableDeclSyntax.self) else {
-      context.error(
+      throw context.error(
         at: declaration,
         message: .expectedVarDecl())
-      return []
     }
 
     // Must be `var` binding.
@@ -51,32 +50,28 @@ extension BitFieldMacro {
     guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
       if binding.pattern.is(TuplePatternSyntax.self) {
         // Binding identifier must not be a tuple.
-        context.error(
+        throw context.error(
           at: binding.pattern,
           message: .unexpectedTupleBindingIdentifier())
-        return []
       } else if binding.pattern.is(WildcardPatternSyntax.self) {
-        context.error(
+        throw context.error(
           at: binding.pattern,
           message: .expectedBindingIdentifier(),
           fixIts: .insertBindingIdentifier(node: binding.pattern))
-        return []
       } else {
-        context.error(
+        throw context.error(
           at: binding.pattern,
           message: .internalError())
-        return []
       }
     }
 
     // Binding identifier must not be "_" (implicitly named).
     guard identifierPattern.identifier.tokenKind != .wildcard else {
       // FIXME: never reached
-      context.error(
+      throw context.error(
         at: binding.pattern,
         message: .expectedBindingIdentifier(),
         fixIts: .insertBindingIdentifier(node: binding.pattern))
-      return []
     }
 
     // Binding must have a type annotation.
@@ -87,19 +82,17 @@ extension BitFieldMacro {
     if let typeIdentifier = type.as(IdentifierTypeSyntax.self) {
       // Binding type must not be "_" (implicitly typed).
       guard typeIdentifier.name.tokenKind != .wildcard else {
-        context.error(
+        throw context.error(
           at: type,
           message: .unexpectedInferredType(),
           fixIts: .insertBindingType(node: binding))
-        return []
       }
     } else if type.is(MemberTypeSyntax.self) {
       // Ok
     } else {
-      context.error(
+      throw context.error(
         at: type,
         message: .unexpectedBindingType())
-      return []
     }
 
     // Binding must not have any accessors.
diff --git a/Sources/MMIOMacros/Macros/RegisterBankOffsetMacro.swift b/Sources/MMIOMacros/Macros/RegisterBankOffsetMacro.swift
@@ -49,10 +49,7 @@ extension RegisterBankOffsetMacro: MMIOAccessorMacro {
   ) throws -> [AccessorDeclSyntax] {
     // Can only applied to variables.
     guard let variableDecl = declaration.as(VariableDeclSyntax.self) else {
-      context.error(
-        at: declaration,
-        message: .expectedVarDecl())
-      return []
+      throw context.error(at: declaration, message: .expectedVarDecl())
     }
 
     // Must be `var` binding.
@@ -65,32 +62,28 @@ extension RegisterBankOffsetMacro: MMIOAccessorMacro {
     guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
       if binding.pattern.is(TuplePatternSyntax.self) {
         // Binding identifier must not be a tuple.
-        context.error(
+        throw context.error(
           at: binding.pattern,
           message: .unexpectedTupleBindingIdentifier())
-        return []
       } else if binding.pattern.is(WildcardPatternSyntax.self) {
-        context.error(
+        throw context.error(
           at: binding.pattern,
           message: .expectedBindingIdentifier(),
           fixIts: .insertBindingIdentifier(node: binding.pattern))
-        return []
       } else {
-        context.error(
+        throw context.error(
           at: binding.pattern,
           message: .internalError())
-        return []
       }
     }
 
     // Binding identifier must not be "_" (implicitly named).
     guard identifierPattern.identifier.tokenKind != .wildcard else {
       // FIXME: never reached
-      context.error(
+      throw context.error(
         at: binding.pattern,
         message: .expectedBindingIdentifier(),
         fixIts: .insertBindingIdentifier(node: binding.pattern))
-      return []
     }
 
     // Binding must have a type annotation.
@@ -101,19 +94,17 @@ extension RegisterBankOffsetMacro: MMIOAccessorMacro {
     if let typeIdentifier = type.as(IdentifierTypeSyntax.self) {
       // Binding type must not be "_" (implicitly typed).
       guard typeIdentifier.name.tokenKind != .wildcard else {
-        context.error(
+        throw context.error(
           at: type,
           message: .unexpectedInferredType(),
           fixIts: .insertBindingType(node: binding))
-        return []
       }
     } else if type.is(MemberTypeSyntax.self) {
       // Ok
     } else {
-      context.error(
+      throw context.error(
         at: type,
         message: .unexpectedBindingType())
-      return []
     }
 
     // Binding must not have any accessors.
diff --git a/Sources/MMIOMacros/Macros/RegisterMacro.swift b/Sources/MMIOMacros/Macros/RegisterMacro.swift
@@ -57,7 +57,7 @@ extension RegisterMacro: MMIOMemberMacro {
     for member in structDecl.memberBlock.members {
       // Each member must be a variable declaration.
       guard let variableDecl = member.decl.as(VariableDeclSyntax.self) else {
-        context.error(
+        _ = context.error(
           at: member.decl,
           message: .onlyMemberVarDecls())
         error = true
@@ -151,10 +151,7 @@ extension RegisterMacro: MMIOExtensionMacro {
     let `extension`: DeclSyntax = "extension \(type.trimmed): RegisterValue {}"
 
     guard let extensionDecl = `extension`.as(ExtensionDeclSyntax.self) else {
-      context.error(
-        at: `extension`,
-        message: .internalError())
-      return []
+      throw context.error(at: `extension`, message: .internalError())
     }
 
     return [extensionDecl]
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/ArgumentParsing/ArgumentContainer.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/ArgumentParsing/ArgumentContainer.swift
@@ -49,10 +49,9 @@ extension ExactlyOne: ArgumentContainer {
     in context: MacroContext<some ParsableMacro, some MacroExpansionContext>
   ) throws {
     guard initial == nil else {
-      context.error(
+      throw context.error(
         at: expression,
         message: .expectedExactlyOneArgumentValue(label: label))
-      throw ExpansionError()
     }
     let value = try Value(expression: expression, in: context)
     self.init(parsed: .init(value: value, expression: expression))
@@ -76,10 +75,9 @@ extension ZeroOrOne: ArgumentContainer {
     in context: MacroContext<some ParsableMacro, some MacroExpansionContext>
   ) throws {
     if let initial = initial, initial.parsed != nil {
-      context.error(
+      throw context.error(
         at: expression,
         message: .expectedZeroOrOneArgumentValues(label: label))
-      throw ExpansionError()
     }
     let value = try Value(expression: expression, in: context)
     self.init(parsed: .init(value: value, expression: expression))
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/ArgumentParsing/ParsableMacro.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/ArgumentParsing/ParsableMacro.swift
@@ -154,12 +154,11 @@ extension ParsableMacro {
         previousLabelMatched = false
         children.formIndex(after: &childIndex)
       } else {
-        context.error(
+        throw context.error(
           at: expression,
           message: .unexpectedArgumentLabel(
             expected: child.label,
             actual: expression.label?.text ?? "_"))
-        throw ExpansionError()
       }
     }
 
@@ -171,10 +170,9 @@ extension ParsableMacro {
     // Check that all expressions have been consumed.
     if expressionIndex != expressions.endIndex {
       let expression = expressions[expressionIndex]
-      context.error(
+      throw context.error(
         at: expression,
         message: .unexpectedExtraArgument(label: expression.label?.text ?? "_"))
-      throw ExpansionError()
     }
 
     // Check that all children have been parsed.
@@ -187,10 +185,9 @@ extension ParsableMacro {
         children.formIndex(after: &childIndex)
         continue
       }
-      context.error(
+      throw context.error(
         at: expressions.isEmpty ? Syntax(node) : Syntax(expressions),
         message: .unexpectedMissingArgument(label: child.label))
-      throw ExpansionError()
     }
   }
 }
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/DeclGroupSyntax.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/DeclGroupSyntax.swift
@@ -52,10 +52,8 @@ extension DeclGroupSyntax {
     let node: any SyntaxProtocol =
       (self as? DiagnosableDeclGroupSyntax)?.introducerKeyword ?? self
 
-    context.error(
+    throw context.error(
       at: node,
       message: .expectedDecl(Other.self))
-
-    throw ExpansionError()
   }
 }
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/Diagnostics/ExpansionError.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/Diagnostics/ExpansionError.swift
@@ -10,4 +10,8 @@
 //===----------------------------------------------------------------------===//
 
 /// A marker error used as an early exit for a failed macro expansion.
+///
+/// Expansion errors should not be directly created, instead calls to
+/// ``MacroContext.error`` will return an expansion error which can be thrown
+/// to early exit.
 struct ExpansionError: Error {}
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/Diagnostics/MacroContext.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/Diagnostics/MacroContext.swift
@@ -27,7 +27,7 @@ where Macro: ParsableMacro, Context: MacroExpansionContext {
     highlights: [Syntax]? = nil,
     notes: [Note] = [],
     fixIts: FixIt...
-  ) {
+  ) -> ExpansionError {
     self.context.diagnose(
       .init(
         node: node,
@@ -36,6 +36,7 @@ where Macro: ParsableMacro, Context: MacroExpansionContext {
         highlights: highlights,
         notes: notes,
         fixIts: fixIts))
+    return ExpansionError()
   }
 }
 
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/PatternBindingSyntax.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/PatternBindingSyntax.swift
@@ -17,11 +17,10 @@ extension PatternBindingSyntax {
     _ context: MacroContext<some ParsableMacro, some MacroExpansionContext>
   ) throws -> TypeSyntax {
     guard let type = self.typeAnnotation?.type else {
-      context.error(
+      throw context.error(
         at: self,
         message: .expectedTypeAnnotation(),
         fixIts: .insertBindingType(node: self))
-      throw ExpansionError()
     }
     return type
   }
@@ -30,11 +29,10 @@ extension PatternBindingSyntax {
     _ context: MacroContext<some ParsableMacro, some MacroExpansionContext>
   ) throws {
     if let accessorBlock = self.accessorBlock {
-      context.error(
+      throw context.error(
         at: accessorBlock,
         message: .expectedStoredProperty(),
         fixIts: .removeAccessorBlock(node: self))
-      throw ExpansionError()
     }
   }
 }
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/VariableDeclSyntax.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/VariableDeclSyntax.swift
@@ -31,12 +31,10 @@ extension VariableDeclSyntax {
     _ context: MacroContext<some ParsableMacro, some MacroExpansionContext>
   ) throws {
     guard self.bindingKind == bindingKind else {
-      context.error(
+      throw context.error(
         at: self.bindingSpecifier,
         message: .expectedBindingKind(bindingKind),
-        fixIts: .replaceWithVar(node: self.bindingSpecifier)
-      )
-      throw ExpansionError()
+        fixIts: .replaceWithVar(node: self.bindingSpecifier))
     }
   }
 }
@@ -51,10 +49,9 @@ extension VariableDeclSyntax {
     _ context: MacroContext<some ParsableMacro, some MacroExpansionContext>
   ) throws -> PatternBindingSyntax {
     guard let binding = self.singleBinding else {
-      context.error(
+      throw context.error(
         at: self.bindings,
         message: .expectedSingleBinding())
-      throw ExpansionError()
     }
     return binding
   }
diff --git a/Sources/MMIOMacros/SwiftSyntaxExtensions/WithAttributesSyntax.swift b/Sources/MMIOMacros/SwiftSyntaxExtensions/WithAttributesSyntax.swift

Original file line number	Diff line number	Diff line change
`@@ -30,11 +30,10 @@ extension BitFieldTypeProjection: ExpressibleByExprSyntax {`
`30`	`30`	`let base = memberAccess.base,`
`31`	`31`	memberAccess.declName.baseName.tokenKind == .keyword(.`self`)
`32`	`32`	`else {`
`33`		`- context.error(`
	`33`	`+ throw context.error(`
`34`	`34`	`at: expression,`
`35`	`35`	`message: .expectedTypeReferenceLiteral(),`
`36`	`36`	`fixIts: .replaceExpressionWithTypeReference(node: expression))`
`37`		`- throw ExpansionError()`
`38`	`37`	`}`
`39`	`38`	`self.expression = base`
`40`	`39`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,10 +26,9 @@ extension BitWidth: ExpressibleByExprSyntax {`
`26`	`26`	`let value = try Int(expression: expression, in: context)`
`27`	`27`	`let validBitWidths = [8, 16, 32, 64]`
`28`	`28`	`guard validBitWidths.contains(value) else {`
`29`		`- context.error(`
	`29`	`+ throw context.error(`
`30`	`30`	`at: expression,`
`31`	`31`	`message: .expectedLiteralValue(in: validBitWidths))`
`32`		`- throw ExpansionError()`
`33`	`32`	`}`
`34`	`33`	`self.value = value`
`35`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,10 +21,9 @@ extension Int: ExpressibleByExprSyntax {`
`21`	`21`	`let intLiteral = expression.as(IntegerLiteralExprSyntax.self),`
`22`	`22`	`let int = intLiteral.value`
`23`	`23`	`else {`
`24`		`- context.error(`
	`24`	`+ throw context.error(`
`25`	`25`	`at: expression,`
`26`	`26`	`message: .expectedIntegerLiteral())`
`27`		`- throw ExpansionError()`
`28`	`27`	`}`
`29`	`28`	`self = int`
`30`	`29`	`}`
Original file line number	Diff line number	Diff line change
`@@ -154,12 +154,11 @@ extension ParsableMacro {`
`154`	`154`	`previousLabelMatched = false`
`155`	`155`	`children.formIndex(after: &childIndex)`
`156`	`156`	`} else {`
`157`		`- context.error(`
	`157`	`+ throw context.error(`
`158`	`158`	`at: expression,`
`159`	`159`	`message: .unexpectedArgumentLabel(`
`160`	`160`	`expected: child.label,`
`161`	`161`	`actual: expression.label?.text ?? "_"))`
`162`		`- throw ExpansionError()`
`163`	`162`	`}`
`164`	`163`	`}`
`165`	`164`
`@@ -171,10 +170,9 @@ extension ParsableMacro {`
`171`	`170`	`// Check that all expressions have been consumed.`
`172`	`171`	`if expressionIndex != expressions.endIndex {`
`173`	`172`	`let expression = expressions[expressionIndex]`
`174`		`- context.error(`
	`173`	`+ throw context.error(`
`175`	`174`	`at: expression,`
`176`	`175`	`message: .unexpectedExtraArgument(label: expression.label?.text ?? "_"))`
`177`		`- throw ExpansionError()`
`178`	`176`	`}`
`179`	`177`
`180`	`178`	`// Check that all children have been parsed.`
`@@ -187,10 +185,9 @@ extension ParsableMacro {`
`187`	`185`	`children.formIndex(after: &childIndex)`
`188`	`186`	`continue`
`189`	`187`	`}`
`190`		`- context.error(`
	`188`	`+ throw context.error(`
`191`	`189`	`at: expressions.isEmpty ? Syntax(node) : Syntax(expressions),`
`192`	`190`	`message: .unexpectedMissingArgument(label: child.label))`
`193`		`- throw ExpansionError()`
`194`	`191`	`}`
`195`	`192`	`}`
`196`	`193`	`}`
Original file line number	Diff line number	Diff line change
`@@ -52,10 +52,8 @@ extension DeclGroupSyntax {`
`52`	`52`	`let node: any SyntaxProtocol =`
`53`	`53`	`(self as? DiagnosableDeclGroupSyntax)?.introducerKeyword ?? self`
`54`	`54`
`55`		`- context.error(`
	`55`	`+ throw context.error(`
`56`	`56`	`at: node,`
`57`	`57`	`message: .expectedDecl(Other.self))`
`58`		`-`
`59`		`- throw ExpansionError()`
`60`	`58`	`}`
`61`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,11 +17,10 @@ extension PatternBindingSyntax {`
`17`	`17`	`_ context: MacroContext<some ParsableMacro, some MacroExpansionContext>`
`18`	`18`	`) throws -> TypeSyntax {`
`19`	`19`	`guard let type = self.typeAnnotation?.type else {`
`20`		`- context.error(`
	`20`	`+ throw context.error(`
`21`	`21`	`at: self,`
`22`	`22`	`message: .expectedTypeAnnotation(),`
`23`	`23`	`fixIts: .insertBindingType(node: self))`
`24`		`- throw ExpansionError()`
`25`	`24`	`}`
`26`	`25`	`return type`
`27`	`26`	`}`
`@@ -30,11 +29,10 @@ extension PatternBindingSyntax {`
`30`	`29`	`_ context: MacroContext<some ParsableMacro, some MacroExpansionContext>`
`31`	`30`	`) throws {`
`32`	`31`	`if let accessorBlock = self.accessorBlock {`
`33`		`- context.error(`
	`32`	`+ throw context.error(`
`34`	`33`	`at: accessorBlock,`
`35`	`34`	`message: .expectedStoredProperty(),`
`36`	`35`	`fixIts: .removeAccessorBlock(node: self))`
`37`		`- throw ExpansionError()`
`38`	`36`	`}`
`39`	`37`	`}`
`40`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,12 +31,10 @@ extension VariableDeclSyntax {`
`31`	`31`	`_ context: MacroContext<some ParsableMacro, some MacroExpansionContext>`
`32`	`32`	`) throws {`
`33`	`33`	`guard self.bindingKind == bindingKind else {`
`34`		`- context.error(`
	`34`	`+ throw context.error(`
`35`	`35`	`at: self.bindingSpecifier,`
`36`	`36`	`message: .expectedBindingKind(bindingKind),`
`37`		`- fixIts: .replaceWithVar(node: self.bindingSpecifier)`
`38`		`- )`
`39`		`- throw ExpansionError()`
	`37`	`+ fixIts: .replaceWithVar(node: self.bindingSpecifier))`
`40`	`38`	`}`
`41`	`39`	`}`
`42`	`40`	`}`
`@@ -51,10 +49,9 @@ extension VariableDeclSyntax {`
`51`	`49`	`_ context: MacroContext<some ParsableMacro, some MacroExpansionContext>`
`52`	`50`	`) throws -> PatternBindingSyntax {`
`53`	`51`	`guard let binding = self.singleBinding else {`
`54`		`- context.error(`
	`52`	`+ throw context.error(`
`55`	`53`	`at: self.bindings,`
`56`	`54`	`message: .expectedSingleBinding())`
`57`		`- throw ExpansionError()`
`58`	`55`	`}`
`59`	`56`	`return binding`
`60`	`57`	`}`