better errors for invalid js decorator syntax

evanw · Mar 14, 2024 · 30bed2d · 30bed2d
1 parent 300eeb7
commit 30bed2d
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 13 deletions.
diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -6713,6 +6713,10 @@ func (p *parser) parseDecorator() js_ast.Expr {
 
 	memberExpr := js_ast.Expr{Loc: nameRange.Loc, Data: &js_ast.EIdentifier{Ref: p.storeNameInRef(name)}}
 
+	// Custom error reporting for error recovery
+	var syntaxError logger.MsgData
+	wrapRange := nameRange
+
 loop:
 	for {
 		switch p.lexer.Token {
@@ -6724,10 +6728,17 @@ loop:
 			if !p.options.ts.Parse {
 				p.lexer.Unexpected()
 			}
+			wrapRange.Len = p.lexer.Range().End() - wrapRange.Loc.Start
 			p.lexer.Next()
 
-		case js_lexer.TDot:
+		case js_lexer.TDot, js_lexer.TQuestionDot:
+			// The grammar for "DecoratorMemberExpression" currently forbids "?."
+			if p.lexer.Token == js_lexer.TQuestionDot && syntaxError.Location == nil {
+				syntaxError = p.tracker.MsgData(p.lexer.Range(), "JavaScript decorator syntax does not allow \"?.\" here")
+			}
+
 			p.lexer.Next()
+			wrapRange.Len = p.lexer.Range().End() - wrapRange.Loc.Start
 
 			if p.lexer.Token == js_lexer.TPrivateIdentifier {
 				name := p.lexer.Identifier
@@ -6746,10 +6757,6 @@ loop:
 				p.lexer.Expect(js_lexer.TIdentifier)
 			}
 
-		case js_lexer.TQuestionDot:
-			// The grammar for "DecoratorMemberExpression" currently forbids "?."
-			p.lexer.Expect(js_lexer.TDot)
-
 		case js_lexer.TOpenParen:
 			args, closeParenLoc, isMultiLine := p.parseCallArgs()
 			memberExpr.Data = &js_ast.ECall{
@@ -6759,6 +6766,15 @@ loop:
 				IsMultiLine:   isMultiLine,
 				Kind:          js_ast.TargetWasOriginallyPropertyAccess,
 			}
+			wrapRange.Len = closeParenLoc.Start + 1 - wrapRange.Loc.Start
+
+			// The grammar for "DecoratorCallExpression" currently forbids anything after it
+			if p.lexer.Token == js_lexer.TDot {
+				if syntaxError.Location == nil {
+					syntaxError = p.tracker.MsgData(p.lexer.Range(), "JavaScript decorator syntax does not allow \".\" after a call expression")
+				}
+				continue
+			}
 			break loop
 
 		default:
@@ -6770,6 +6786,21 @@ loop:
 		}
 	}
 
+	// Suggest that non-decorator expressions be wrapped in parentheses
+	if syntaxError.Location != nil {
+		var notes []logger.MsgData
+		if text := p.source.TextForRange(wrapRange); !strings.ContainsRune(text, '\n') {
+			note := p.tracker.MsgData(wrapRange, "Wrap this decorator in parentheses to allow arbitrary expressions:")
+			note.Location.Suggestion = fmt.Sprintf("(%s)", text)
+			notes = []logger.MsgData{note}
+		}
+		p.log.AddMsg(logger.Msg{
+			Kind:  logger.Error,
+			Data:  syntaxError,
+			Notes: notes,
+		})
+	}
+
 	return memberExpr
 }
 

diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
@@ -2028,15 +2028,19 @@ func TestDecorators(t *testing.T) {
 	expectPrinted(t, "class Foo { #x = @y.#x.y.#x class {} }", "class Foo {\n  #x = @y.#x.y.#x class {\n  };\n}\n")
 	expectParseError(t, "@123 class Foo {}", "<stdin>: ERROR: Expected identifier but found \"123\"\n")
 	expectParseError(t, "@x[y] class Foo {}", "<stdin>: ERROR: Expected \";\" but found \"class\"\n")
-	expectParseError(t, "@x?.() class Foo {}", "<stdin>: ERROR: Expected \".\" but found \"?.\"\n")
-	expectParseError(t, "@x?.y() class Foo {}", "<stdin>: ERROR: Expected \".\" but found \"?.\"\n")
-	expectParseError(t, "@x?.[y]() class Foo {}", "<stdin>: ERROR: Expected \".\" but found \"?.\"\n")
+	expectParseError(t, "@x?.() class Foo {}", "<stdin>: ERROR: Expected identifier but found \"(\"\n")
+	expectParseError(t, "@x?.y() class Foo {}",
+		"<stdin>: ERROR: JavaScript decorator syntax does not allow \"?.\" here\n"+
+			"<stdin>: NOTE: Wrap this decorator in parentheses to allow arbitrary expressions:\n")
+	expectParseError(t, "@x?.[y]() class Foo {}", "<stdin>: ERROR: Expected identifier but found \"[\"\n")
 	expectParseError(t, "@new Function() class Foo {}", "<stdin>: ERROR: Expected identifier but found \"new\"\n")
 	expectParseError(t, "@() => {} class Foo {}", "<stdin>: ERROR: Unexpected \")\"\n")
 	expectParseError(t, "x = @y function() {}", "<stdin>: ERROR: Expected \"class\" but found \"function\"\n")
 
 	// See: https://github.com/microsoft/TypeScript/issues/55336
-	expectParseError(t, "@x().y() class Foo {}", "<stdin>: ERROR: Unexpected \".\"\n")
+	expectParseError(t, "@x().y() class Foo {}",
+		"<stdin>: ERROR: JavaScript decorator syntax does not allow \".\" after a call expression\n"+
+			"<stdin>: NOTE: Wrap this decorator in parentheses to allow arbitrary expressions:\n")
 
 	errorText := "<stdin>: ERROR: Transforming JavaScript decorators to the configured target environment is not supported yet\n"
 	expectParseErrorWithUnsupportedFeatures(t, compat.Decorators, "@dec class Foo {}", errorText)

diff --git a/internal/js_parser/ts_parser_test.go b/internal/js_parser/ts_parser_test.go
@@ -2092,9 +2092,11 @@ func TestTSDecorators(t *testing.T) {
 	expectPrintedTS(t, "class Foo { #x = @y.#x.y.#x class {} }", "class Foo {\n  #x = @y.#x.y.#x class {\n  };\n}\n")
 	expectParseErrorTS(t, "@123 class Foo {}", "<stdin>: ERROR: Expected identifier but found \"123\"\n")
 	expectParseErrorTS(t, "@x[y] class Foo {}", "<stdin>: ERROR: Expected \";\" but found \"class\"\n")
-	expectParseErrorTS(t, "@x?.() class Foo {}", "<stdin>: ERROR: Expected \".\" but found \"?.\"\n")
-	expectParseErrorTS(t, "@x?.y() class Foo {}", "<stdin>: ERROR: Expected \".\" but found \"?.\"\n")
-	expectParseErrorTS(t, "@x?.[y]() class Foo {}", "<stdin>: ERROR: Expected \".\" but found \"?.\"\n")
+	expectParseErrorTS(t, "@x?.() class Foo {}", "<stdin>: ERROR: Expected identifier but found \"(\"\n")
+	expectParseErrorTS(t, "@x?.y() class Foo {}",
+		"<stdin>: ERROR: JavaScript decorator syntax does not allow \"?.\" here\n"+
+			"<stdin>: NOTE: Wrap this decorator in parentheses to allow arbitrary expressions:\n")
+	expectParseErrorTS(t, "@x?.[y]() class Foo {}", "<stdin>: ERROR: Expected identifier but found \"[\"\n")
 	expectParseErrorTS(t, "@new Function() class Foo {}", "<stdin>: ERROR: Expected identifier but found \"new\"\n")
 	expectParseErrorTS(t, "@() => {} class Foo {}", "<stdin>: ERROR: Unexpected \")\"\n")
 	expectParseErrorTS(t, "x = @y function() {}", "<stdin>: ERROR: Expected \"class\" but found \"function\"\n")
@@ -2107,7 +2109,9 @@ func TestTSDecorators(t *testing.T) {
 
 	// TypeScript 5.0+ allows this but Babel doesn't. I believe this is a bug
 	// with TypeScript: https://github.com/microsoft/TypeScript/issues/55336
-	expectParseErrorTS(t, "class Foo { @x<{}>().y<[], () => {}>() z: any }", "<stdin>: ERROR: Expected identifier but found \".\"\n")
+	expectParseErrorTS(t, "class Foo { @x<{}>().y<[], () => {}>() z: any }",
+		"<stdin>: ERROR: JavaScript decorator syntax does not allow \".\" after a call expression\n"+
+			"<stdin>: NOTE: Wrap this decorator in parentheses to allow arbitrary expressions:\n")
 
 	errorText := "<stdin>: ERROR: Transforming JavaScript decorators to the configured target environment is not supported yet\n"
 	expectParseErrorWithUnsupportedFeaturesTS(t, compat.Decorators, "@dec class Foo {}", errorText)