raise more complete SyntaxErrors in more cases

timfel · timfel · commit b0e08fea3b87 · 2018-11-12T17:50:00.000+01:00
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Python3Core.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Python3Core.java
@@ -694,15 +694,15 @@ public PInt getFalse() {
         return pyFalse;
     }
 
-    public RuntimeException raiseInvalidSyntax(Source source, SourceSection section) {
+    public RuntimeException raiseInvalidSyntax(Source source, SourceSection section, String message, Object... arguments) {
         Node location = new Node() {
             @Override
             public SourceSection getSourceSection() {
                 return section;
             }
         };
         PBaseException instance;
-        instance = factory().createBaseException(SyntaxError, "invalid syntax", new Object[0]);
+        instance = factory().createBaseException(SyntaxError, message, arguments);
         String path = source.getPath();
         instance.setAttribute("filename", path != null ? path : source.getName() != null ? source.getName() : "<string>");
         instance.setAttribute("text", section.isAvailable() ? source.getCharacters(section.getStartLine()) : "");
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/DestructuringAssignmentNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/DestructuringAssignmentNode.java
@@ -25,11 +25,10 @@
  */
 package com.oracle.graal.python.nodes.frame;
 
-import static com.oracle.graal.python.runtime.exception.PythonErrorType.IndexError;
-import static com.oracle.graal.python.runtime.exception.PythonErrorType.SyntaxError;
-import static com.oracle.graal.python.runtime.exception.PythonErrorType.ValueError;
 import static com.oracle.graal.python.builtins.objects.PNone.NO_VALUE;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.__GETITEM__;
+import static com.oracle.graal.python.runtime.exception.PythonErrorType.IndexError;
+import static com.oracle.graal.python.runtime.exception.PythonErrorType.ValueError;
 
 import java.util.Arrays;
 
@@ -182,7 +181,7 @@ public void doWrite(VirtualFrame frame, Object rhsValue) {
         try {
             getNonExistingItem.execute(rhsValue, nonExistingItem);
             tooManyValuesProfile.enter();
-            throw raise(SyntaxError, "too many values to unpack (expected %d)", nonExistingItem);
+            throw getCore().raiseInvalidSyntax(getEncapsulatingSourceSection().getSource(), getEncapsulatingSourceSection(), "too many values to unpack (expected %d)", nonExistingItem);
         } catch (PException e) {
             if (tooManyValuesErrorProfile.profileException(e, IndexError)) {
                 // expected, fall through
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonTreeTranslator.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonTreeTranslator.java
@@ -302,7 +302,7 @@ private void visitCallArglist(Python3Parser.ArglistContext arglist, List<Express
                         }
                     } else if (isStararg(argctx)) {
                         if (kwargs != null) {
-                            throw errors.raise(SyntaxError, "iterable argument unpacking follows keyword argument unpacking");
+                            throw errors.raiseInvalidSyntax(source, deriveSourceSection(argctx), "iterable argument unpacking follows keyword argument unpacking");
                         }
                         if (starargs != null) {
                             starargs = factory.createBinaryOperation("+", starargs, arg);
@@ -311,10 +311,10 @@ private void visitCallArglist(Python3Parser.ArglistContext arglist, List<Express
                         }
                     } else {
                         if (!keywords.isEmpty()) {
-                            throw errors.raise(SyntaxError, "positional argument follows keyword argument");
+                            throw errors.raiseInvalidSyntax(source, deriveSourceSection(argctx), "positional argument follows keyword argument");
                         }
                         if (kwargs != null) {
-                            throw errors.raise(SyntaxError, "positional argument follows keyword argument unpacking");
+                            throw errors.raiseInvalidSyntax(source, deriveSourceSection(argctx), "positional argument follows keyword argument unpacking");
                         }
                         argumentNodes.add(arg);
                     }
@@ -355,7 +355,7 @@ private ExpressionNode getDefaultarg(Python3Parser.ArgumentContext ctx) {
             // In CPython, ast.c ensures this
             String argName = ctx.test(0).accept(new ExtractNameVisitor());
             if (argName == null) {
-                throw errors.raise(SyntaxError, "Keyword can't be an expression");
+                throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "Keyword can't be an expression");
             }
             return factory.createKeywordLiteral((ExpressionNode) ctx.test(1).accept(this), argName);
         } else {
@@ -428,7 +428,7 @@ public Object visitAtom(Python3Parser.AtomContext ctx) {
         } else if (ctx.NAME() != null) {
             return environment.findVariable(ctx.NAME().getText());
         } else if (ctx.getChildCount() == 1) {
-            return parseSpecialLiteral(ctx.getText());
+            return parseSpecialLiteral(ctx);
         } else if (ctx.dictorsetmaker() != null) {
             return super.visitAtom(ctx);
         } else if (ctx.getChild(0).getText().equals("{")) { // empty dict
@@ -540,23 +540,24 @@ private ExpressionNode visitNormalDictmaker(Python3Parser.DictmakerContext ctx)
 
     private PNode visitDictmakerComprehension(Python3Parser.DictmakerContext ctx) {
         if (!ctx.expr().isEmpty()) {
-            throw errors.raise(SyntaxError, "dict unpacking cannot be used in dict comprehension");
+            throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "dict unpacking cannot be used in dict comprehension");
         }
         return factory.callBuiltin(DICT,
                         createComprehensionExpression(ctx, ctx.comp_for(), c -> factory.createTupleLiteral((ExpressionNode) ctx.test(0).accept(this), (ExpressionNode) ctx.test(1).accept(this))));
     }
 
-    private Object parseSpecialLiteral(String text) {
-        if (text.equals("...")) {
+    private Object parseSpecialLiteral(Python3Parser.AtomContext ctx) {
+        String txt = ctx.getText();
+        if (txt.equals("...")) {
             return factory.createObjectLiteral(PEllipsis.INSTANCE);
-        } else if (text.equals("None")) {
+        } else if (txt.equals("None")) {
             return factory.createObjectLiteral(PNone.NONE);
-        } else if (text.equals("True")) {
+        } else if (txt.equals("True")) {
             return factory.createBooleanLiteral(true);
-        } else if (text.equals("False")) {
+        } else if (txt.equals("False")) {
             return factory.createBooleanLiteral(false);
         } else {
-            throw errors.raise(SyntaxError, "Unknown literal %s", text);
+            throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "Unknown literal %s", txt);
         }
     }
 
@@ -975,7 +976,7 @@ public Object visitImport_from(Python3Parser.Import_fromContext ctx) {
             return factory.createImportFrom(sb.toString(), fromlist.toArray(new String[0]), asNodes.toArray(new WriteNode[0]), level);
         } else {
             if (!environment.atModuleLevel()) {
-                throw errors.raise(SyntaxError, "import * only allowed at module level");
+                throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "import * only allowed at module level");
             }
             return factory.createImportStar(sb.toString(), level);
         }
@@ -1097,7 +1098,7 @@ private void delTarget(List<StatementNode> blockList, PNode target) {
                 delTarget(blockList, targetValue);
             }
         } else {
-            throw errors.raise(SyntaxError, "can't delete '%s'", target.getSourceSection().getCharacters());
+            throw errors.raiseInvalidSyntax(target.getSourceSection().getSource(), target.getSourceSection(), "can't delete '%s'", target.getSourceSection().getCharacters());
         }
     }
 
@@ -1156,7 +1157,7 @@ public Object visitReturn_stmt(Python3Parser.Return_stmtContext ctx) {
                 return factory.createFrameReturn(factory.createWriteLocal((ExpressionNode) ctx.testlist().accept(this), environment.getReturnSlot()));
             }
         }
-        throw errors.raise(SyntaxError, "'return' outside function");
+        throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "'return' outside function");
     }
 
     private static boolean lastChildIsComma(ParserRuleContext ctx) {
@@ -1369,7 +1370,7 @@ public Object visitTry_stmt(Python3Parser.Try_stmtContext ctx) {
                 WriteNode exceptName = null;
                 if (excctx.test() != null) {
                     if (gotDefaultExcept) {
-                        throw errors.raise(SyntaxError, "default except: must be last");
+                        throw errors.raiseInvalidSyntax(source, deriveSourceSection(excctx), "default except: must be last");
                     }
                     exceptType = (ExpressionNode) excctx.test().accept(this);
                     if (excctx.NAME() != null) {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonParser.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonParser.java
@@ -72,7 +72,11 @@ public enum ParserMode {
     public interface ParserErrorCallback {
         RuntimeException raise(PythonBuiltinClassType type, String message, Object... args);
 
-        RuntimeException raiseInvalidSyntax(Source source, SourceSection section);
+        RuntimeException raiseInvalidSyntax(Source source, SourceSection section, String message, Object... arguments);
+
+        default RuntimeException raiseInvalidSyntax(Source source, SourceSection section) {
+            return raiseInvalidSyntax(source, section, "invalid syntax", new Object[0]);
+        }
 
         PythonLanguage getLanguage();
     }

Original file line number	Diff line number	Diff line change
`@@ -302,7 +302,7 @@ private void visitCallArglist(Python3Parser.ArglistContext arglist, List<Express`
`302`	`302`	`}`
`303`	`303`	`} else if (isStararg(argctx)) {`
`304`	`304`	`if (kwargs != null) {`
`305`		`- throw errors.raise(SyntaxError, "iterable argument unpacking follows keyword argument unpacking");`
	`305`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(argctx), "iterable argument unpacking follows keyword argument unpacking");`
`306`	`306`	`}`
`307`	`307`	`if (starargs != null) {`
`308`	`308`	`starargs = factory.createBinaryOperation("+", starargs, arg);`
`@@ -311,10 +311,10 @@ private void visitCallArglist(Python3Parser.ArglistContext arglist, List<Express`
`311`	`311`	`}`
`312`	`312`	`} else {`
`313`	`313`	`if (!keywords.isEmpty()) {`
`314`		`- throw errors.raise(SyntaxError, "positional argument follows keyword argument");`
	`314`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(argctx), "positional argument follows keyword argument");`
`315`	`315`	`}`
`316`	`316`	`if (kwargs != null) {`
`317`		`- throw errors.raise(SyntaxError, "positional argument follows keyword argument unpacking");`
	`317`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(argctx), "positional argument follows keyword argument unpacking");`
`318`	`318`	`}`
`319`	`319`	`argumentNodes.add(arg);`
`320`	`320`	`}`
`@@ -355,7 +355,7 @@ private ExpressionNode getDefaultarg(Python3Parser.ArgumentContext ctx) {`
`355`	`355`	`// In CPython, ast.c ensures this`
`356`	`356`	`String argName = ctx.test(0).accept(new ExtractNameVisitor());`
`357`	`357`	`if (argName == null) {`
`358`		`- throw errors.raise(SyntaxError, "Keyword can't be an expression");`
	`358`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "Keyword can't be an expression");`
`359`	`359`	`}`
`360`	`360`	`return factory.createKeywordLiteral((ExpressionNode) ctx.test(1).accept(this), argName);`
`361`	`361`	`} else {`
`@@ -428,7 +428,7 @@ public Object visitAtom(Python3Parser.AtomContext ctx) {`
`428`	`428`	`} else if (ctx.NAME() != null) {`
`429`	`429`	`return environment.findVariable(ctx.NAME().getText());`
`430`	`430`	`} else if (ctx.getChildCount() == 1) {`
`431`		`- return parseSpecialLiteral(ctx.getText());`
	`431`	`+ return parseSpecialLiteral(ctx);`
`432`	`432`	`} else if (ctx.dictorsetmaker() != null) {`
`433`	`433`	`return super.visitAtom(ctx);`
`434`	`434`	`} else if (ctx.getChild(0).getText().equals("{")) { // empty dict`
`@@ -540,23 +540,24 @@ private ExpressionNode visitNormalDictmaker(Python3Parser.DictmakerContext ctx)`
`540`	`540`
`541`	`541`	`private PNode visitDictmakerComprehension(Python3Parser.DictmakerContext ctx) {`
`542`	`542`	`if (!ctx.expr().isEmpty()) {`
`543`		`- throw errors.raise(SyntaxError, "dict unpacking cannot be used in dict comprehension");`
	`543`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "dict unpacking cannot be used in dict comprehension");`
`544`	`544`	`}`
`545`	`545`	`return factory.callBuiltin(DICT,`
`546`	`546`	`createComprehensionExpression(ctx, ctx.comp_for(), c -> factory.createTupleLiteral((ExpressionNode) ctx.test(0).accept(this), (ExpressionNode) ctx.test(1).accept(this))));`
`547`	`547`	`}`
`548`	`548`
`549`		`- private Object parseSpecialLiteral(String text) {`
`550`		`- if (text.equals("...")) {`
	`549`	`+ private Object parseSpecialLiteral(Python3Parser.AtomContext ctx) {`
	`550`	`+ String txt = ctx.getText();`
	`551`	`+ if (txt.equals("...")) {`
`551`	`552`	`return factory.createObjectLiteral(PEllipsis.INSTANCE);`
`552`		`- } else if (text.equals("None")) {`
	`553`	`+ } else if (txt.equals("None")) {`
`553`	`554`	`return factory.createObjectLiteral(PNone.NONE);`
`554`		`- } else if (text.equals("True")) {`
	`555`	`+ } else if (txt.equals("True")) {`
`555`	`556`	`return factory.createBooleanLiteral(true);`
`556`		`- } else if (text.equals("False")) {`
	`557`	`+ } else if (txt.equals("False")) {`
`557`	`558`	`return factory.createBooleanLiteral(false);`
`558`	`559`	`} else {`
`559`		`- throw errors.raise(SyntaxError, "Unknown literal %s", text);`
	`560`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "Unknown literal %s", txt);`
`560`	`561`	`}`
`561`	`562`	`}`
`562`	`563`
`@@ -975,7 +976,7 @@ public Object visitImport_from(Python3Parser.Import_fromContext ctx) {`
`975`	`976`	`return factory.createImportFrom(sb.toString(), fromlist.toArray(new String[0]), asNodes.toArray(new WriteNode[0]), level);`
`976`	`977`	`} else {`
`977`	`978`	`if (!environment.atModuleLevel()) {`
`978`		`- throw errors.raise(SyntaxError, "import * only allowed at module level");`
	`979`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "import * only allowed at module level");`
`979`	`980`	`}`
`980`	`981`	`return factory.createImportStar(sb.toString(), level);`
`981`	`982`	`}`
`@@ -1097,7 +1098,7 @@ private void delTarget(List<StatementNode> blockList, PNode target) {`
`1097`	`1098`	`delTarget(blockList, targetValue);`
`1098`	`1099`	`}`
`1099`	`1100`	`} else {`
`1100`		`- throw errors.raise(SyntaxError, "can't delete '%s'", target.getSourceSection().getCharacters());`
	`1101`	`+ throw errors.raiseInvalidSyntax(target.getSourceSection().getSource(), target.getSourceSection(), "can't delete '%s'", target.getSourceSection().getCharacters());`
`1101`	`1102`	`}`
`1102`	`1103`	`}`
`1103`	`1104`
`@@ -1156,7 +1157,7 @@ public Object visitReturn_stmt(Python3Parser.Return_stmtContext ctx) {`
`1156`	`1157`	`return factory.createFrameReturn(factory.createWriteLocal((ExpressionNode) ctx.testlist().accept(this), environment.getReturnSlot()));`
`1157`	`1158`	`}`
`1158`	`1159`	`}`
`1159`		`- throw errors.raise(SyntaxError, "'return' outside function");`
	`1160`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(ctx), "'return' outside function");`
`1160`	`1161`	`}`
`1161`	`1162`
`1162`	`1163`	`private static boolean lastChildIsComma(ParserRuleContext ctx) {`
`@@ -1369,7 +1370,7 @@ public Object visitTry_stmt(Python3Parser.Try_stmtContext ctx) {`
`1369`	`1370`	`WriteNode exceptName = null;`
`1370`	`1371`	`if (excctx.test() != null) {`
`1371`	`1372`	`if (gotDefaultExcept) {`
`1372`		`- throw errors.raise(SyntaxError, "default except: must be last");`
	`1373`	`+ throw errors.raiseInvalidSyntax(source, deriveSourceSection(excctx), "default except: must be last");`
`1373`	`1374`	`}`
`1374`	`1375`	`exceptType = (ExpressionNode) excctx.test().accept(this);`
`1375`	`1376`	`if (excctx.NAME() != null) {`