[GR-9831] format the message for Syntax error to add more context akin to how cpython does it

cosminbasca · cosminbasca · commit d9dd0e517c2b · 2018-05-23T07:41:18.000-07:00
PullRequest: graalpython/49
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/test_parser.py b/graalpython/com.oracle.graal.python.test/src/tests/test_parser.py
@@ -65,6 +65,16 @@ def x(*, a=None, b):
     assert globs["retval"] == (None, 42)
 
 
+def test_syntax_error_simple():
+    globs = {}
+    was_exception = False
+    try:
+        exec("""c = a += 3""", globs)
+    except SyntaxError:
+        was_exception = True
+    assert was_exception
+
+
 def test_lambda_no_args_with_nested_lambdas():
     no_err = True
     try:
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonErrorStrategy.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonErrorStrategy.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates.
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or data
+ * (collectively the "Software"), free of charge and under any and all copyright
+ * rights in the Software, and any and all patent rights owned or freely
+ * licensable by each licensor hereunder covering either (i) the unmodified
+ * Software as contributed to or provided by such licensor, or (ii) the Larger
+ * Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ *     one is included with the Software (each a "Larger Work" to which the
+ *     Software is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ *
+ * The above copyright notice and either this complete permission notice or at a
+ * minimum a reference to the UPL must be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package com.oracle.graal.python.parser;
+
+import org.antlr.v4.runtime.DefaultErrorStrategy;
+import org.antlr.v4.runtime.InputMismatchException;
+import org.antlr.v4.runtime.NoViableAltException;
+import org.antlr.v4.runtime.Parser;
+import org.antlr.v4.runtime.RecognitionException;
+import org.antlr.v4.runtime.Token;
+import org.antlr.v4.runtime.TokenStream;
+import org.antlr.v4.runtime.misc.Interval;
+
+public class PythonErrorStrategy extends DefaultErrorStrategy {
+    private static final String LINE_PADDING = "  ";
+
+    @Override
+    public void recover(Parser recognizer, RecognitionException e) {
+        super.recover(recognizer, e);
+    }
+
+    static int getLine(Throwable e) {
+        if (e instanceof RecognitionException) {
+            return ((RecognitionException) e).getOffendingToken().getLine();
+        } else {
+            Throwable cause = e.getCause();
+            if (cause instanceof RecognitionException) {
+                return ((RecognitionException) cause).getOffendingToken().getLine();
+            }
+            return -1;
+        }
+    }
+
+    private static String getTokeLineText(Parser recognizer, Token token) {
+        TokenStream tokenStream = recognizer.getTokenStream();
+        int index = token.getTokenIndex();
+        // search for line start
+        int tokenIndex = index;
+        int start = -1;
+        while (tokenIndex >= 0) {
+            Token t = tokenStream.get(tokenIndex);
+            if (t.getText().equals("\n")) {
+                break;
+            }
+            start = t.getStartIndex();
+            tokenIndex--;
+        }
+
+        // search for line stop
+        tokenIndex = index;
+        int stop = -1;
+        while (tokenIndex < tokenStream.size()) {
+            Token t = tokenStream.get(tokenIndex);
+            stop = t.getStopIndex();
+            if (t.getText().equals("\n")) {
+                break;
+            }
+            tokenIndex++;
+        }
+        return token.getInputStream().getText(Interval.of(start, stop));
+    }
+
+    private static void handlePythonSyntaxError(Parser recognizer, RecognitionException e) {
+        Token offendingToken = e.getOffendingToken();
+        String lineText = getTokeLineText(recognizer, offendingToken);
+        String errorMarker = new String(new char[offendingToken.getCharPositionInLine()]).replace('\0', ' ') + "^";
+        String pythonSyntaxErrorMessage = "\n" + LINE_PADDING + lineText + "\n" + LINE_PADDING + errorMarker;
+        recognizer.notifyErrorListeners(offendingToken, pythonSyntaxErrorMessage, e);
+    }
+
+    @Override
+    protected void reportInputMismatch(Parser recognizer, InputMismatchException e) {
+        handlePythonSyntaxError(recognizer, e);
+    }
+
+    @Override
+    protected void reportNoViableAlternative(Parser recognizer, NoViableAltException e) {
+        handlePythonSyntaxError(recognizer, e);
+    }
+}
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonParserImpl.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonParserImpl.java
@@ -33,13 +33,11 @@
 
 import org.antlr.v4.runtime.CharStreams;
 import org.antlr.v4.runtime.CodePointCharStream;
-import org.antlr.v4.runtime.NoViableAltException;
 import org.antlr.v4.runtime.ParserRuleContext;
-import org.antlr.v4.runtime.RecognitionException;
-import org.antlr.v4.runtime.Token;
 
 import com.oracle.graal.python.PythonLanguage;
 import com.oracle.graal.python.nodes.PNode;
+import com.oracle.graal.python.parser.antlr.Builder;
 import com.oracle.graal.python.parser.antlr.Python3Parser;
 import com.oracle.graal.python.runtime.PythonCore;
 import com.oracle.graal.python.runtime.PythonParseResult;
@@ -56,13 +54,25 @@
 public final class PythonParserImpl implements PythonParser {
     private static final Map<String, ParserRuleContext> cachedParseTrees = new HashMap<>();
 
+    private static Python3Parser getPython3Parser(CodePointCharStream fromString) {
+        Python3Parser parser = new Builder.Parser(fromString).build();
+        parser.setErrorHandler(new PythonErrorStrategy());
+        return parser;
+    }
+
+    private static Python3Parser getPython3Parser(String string) {
+        Python3Parser parser = new Builder.Parser(string).build();
+        parser.setErrorHandler(new PythonErrorStrategy());
+        return parser;
+    }
+
     @TruffleBoundary
     private static ParserRuleContext preParseWithAntlr(PythonCore core, Source source) {
         String path = source.getURI().toString();
         String[] pathParts = path.split(Pattern.quote(PythonCore.FILE_SEPARATOR));
         String fileDirAndName = pathParts[pathParts.length - 2] + PythonCore.FILE_SEPARATOR + pathParts[pathParts.length - 1];
         CodePointCharStream fromString = CharStreams.fromString(source.getCharacters().toString(), fileDirAndName);
-        Python3Parser parser = new com.oracle.graal.python.parser.antlr.Builder.Parser(fromString).build();
+        Python3Parser parser = getPython3Parser(fromString);
         ParserRuleContext input;
         if (!core.isInitialized()) {
             input = cachedParseTrees.get(fileDirAndName);
@@ -82,23 +92,12 @@ private static ParserRuleContext preParseWithAntlr(PythonCore core, Source sourc
                     parser.reset();
                     input = parser.eval_input();
                 } catch (Throwable e2) {
-                    int line = -1;
-                    int column = -1;
                     if (source.isInteractive() && e instanceof PIncompleteSourceException) {
                         ((PIncompleteSourceException) e).setSource(source);
                         throw e;
-                    } else if (e instanceof RecognitionException) {
-                        Token token = ((RecognitionException) e).getOffendingToken();
-                        line = token.getLine();
-                        column = token.getCharPositionInLine();
-                    } else if (e.getCause() instanceof RecognitionException) {
-                        Token token = ((RecognitionException) e.getCause()).getOffendingToken();
-                        line = token.getLine();
-                        column = token.getCharPositionInLine();
-                    } else {
-                        throw core.raise(SyntaxError, e.getMessage());
                     }
-                    throw core.raise(SyntaxError, getLocation(source, line), "invalid syntax: line %d, column %d. ", line, column);
+                    Node location = getLocation(source, PythonErrorStrategy.getLine(e));
+                    throw core.raise(SyntaxError, location, e.getMessage());
                 }
             }
         }
@@ -107,7 +106,7 @@ private static ParserRuleContext preParseWithAntlr(PythonCore core, Source sourc
 
     @TruffleBoundary
     private static ParserRuleContext preParseInlineWithAntlr(PythonCore core, Source source) {
-        Python3Parser parser = new com.oracle.graal.python.parser.antlr.Builder.Parser(source.getCharacters().toString()).build();
+        Python3Parser parser = getPython3Parser(source.getCharacters().toString());
         ParserRuleContext input;
         try {
             input = parser.single_input();
@@ -116,20 +115,8 @@ private static ParserRuleContext preParseInlineWithAntlr(PythonCore core, Source
                 parser.reset();
                 input = parser.eval_input();
             } catch (Throwable e2) {
-                int line = -1;
-                int column = -1;
-                if (e instanceof RecognitionException) {
-                    Token token = ((RecognitionException) e).getOffendingToken();
-                    line = token.getLine();
-                    column = token.getCharPositionInLine();
-                } else if (e.getCause() instanceof NoViableAltException) {
-                    Token token = ((NoViableAltException) e.getCause()).getOffendingToken();
-                    line = token.getLine();
-                    column = token.getCharPositionInLine();
-                } else {
-                    throw core.raise(SyntaxError, e.getMessage());
-                }
-                throw core.raise(SyntaxError, getLocation(source, line), "invalid syntax: line %d, column %d. ", line, column);
+                Node location = getLocation(source, PythonErrorStrategy.getLine(e));
+                throw core.raise(SyntaxError, location, e.getMessage());
             }
         }
         return input;
@@ -164,7 +151,7 @@ public PNode parseInline(PythonCore core, Source source, Frame curFrame) {
     @Override
     @TruffleBoundary
     public PythonParseResult parseEval(PythonCore core, String expression, String filename) {
-        Python3Parser parser = new com.oracle.graal.python.parser.antlr.Builder.Parser(expression).build();
+        Python3Parser parser = getPython3Parser(expression);
         ParserRuleContext input;
         try {
             input = parser.eval_input();
@@ -179,7 +166,7 @@ public PythonParseResult parseEval(PythonCore core, String expression, String fi
     @Override
     @TruffleBoundary
     public PythonParseResult parseExec(PythonCore core, String expression, String filename) {
-        Python3Parser parser = new com.oracle.graal.python.parser.antlr.Builder.Parser(expression).build();
+        Python3Parser parser = getPython3Parser(expression);
         ParserRuleContext input;
         try {
             input = parser.file_input();
@@ -193,7 +180,7 @@ public PythonParseResult parseExec(PythonCore core, String expression, String fi
     @Override
     @TruffleBoundary
     public PythonParseResult parseSingle(PythonCore core, String expression, String filename) {
-        Python3Parser parser = new com.oracle.graal.python.parser.antlr.Builder.Parser(expression).build();
+        Python3Parser parser = getPython3Parser(expression);
         ParserRuleContext input;
         try {
             input = parser.single_input();
@@ -207,7 +194,7 @@ public PythonParseResult parseSingle(PythonCore core, String expression, String
     @Override
     @TruffleBoundary
     public boolean isIdentifier(PythonCore core, String snippet) {
-        Python3Parser parser = new com.oracle.graal.python.parser.antlr.Builder.Parser(snippet).build();
+        Python3Parser parser = getPython3Parser(snippet);
         Python3Parser.AtomContext input;
         try {
             input = parser.atom();
@@ -218,14 +205,7 @@ public boolean isIdentifier(PythonCore core, String snippet) {
     }
 
     private static PException handleParserError(PythonCore core, Throwable e) {
-        if (e instanceof RecognitionException) {
-            Token token = ((RecognitionException) e).getOffendingToken();
-            int line = token.getLine();
-            int column = token.getCharPositionInLine();
-            return core.raise(SyntaxError, "parser error at %d:%d\n%s", line, column, e.toString());
-        } else {
-            return core.raise(SyntaxError, e.getMessage());
-        }
+        return core.raise(SyntaxError, e.getMessage());
     }
 
     private static PythonParseResult translateParseResult(PythonCore core, String name, ParserRuleContext input, Source source) {
