[GR-23226] More syntax checks

PullRequest: graalpython/1029

Author: msimacek

graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/BasicTests.java

@@ -554,7 +554,7 @@ public void not01() throws Exception {
 
     @Test
     public void nonlocal02() throws Exception {
-        checkTreeResult("nonlocal x");
+        checkSyntaxError("nonlocal x");
     }
 
     @Test

graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/SSTSerializationTests.java

@@ -768,11 +768,6 @@ public void moduleDocTest() throws Exception {
                                         "\"\"\"MODULE A DOC\"\"\"\n" + "print(\"module A\")");
     }
 
-    @Test
-    public void nonlocalTest() throws Exception {
-        checkSerialization("nonlocal x");
-    }
-
     @Test
     public void numberBinTest() throws Exception {
         checkSerialization("0b101");
@@ -1084,7 +1079,7 @@ public void withTest() throws Exception {
     }
 
     @Test
-    public void yeildTest() throws Exception {
+    public void yieldTest() throws Exception {
         checkSerialization("def f(): yield 1");
         checkSerialization("def f(): yield");
         checkSerialization("def f(): x += yield");

graalpython/com.oracle.graal.python.test/testData/goldenFiles/BasicTests/nonlocal02.tast

@@ -40,6 +40,8 @@
 import java.util.Map.Entry;
 import java.util.ServiceLoader;
 import java.util.logging.Level;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.graalvm.nativeimage.ImageInfo;
 
@@ -209,6 +211,8 @@ public final class Python3Core implements PythonCore {
     private static final TruffleLogger LOGGER = PythonLanguage.getLogger(Python3Core.class);
     private final String[] coreFiles;
 
+    public static final Pattern MISSING_PARENTHESES_PATTERN = Pattern.compile("^(print|exec) +([^(][^;]*).*");
+
     private static final String[] initializeCoreFiles() {
         // Order matters!
         List<String> coreFiles = new ArrayList<>(Arrays.asList(
@@ -774,6 +778,22 @@ public RuntimeException raiseInvalidSyntax(PythonParser.ErrorType type, Node loc
         } else {
             msg = "invalid syntax";
         }
+        if (section.isAvailable() && type == PythonParser.ErrorType.Generic) {
+            Matcher matcher = MISSING_PARENTHESES_PATTERN.matcher(source.getCharacters(section.getStartLine()));
+            if (matcher.matches()) {
+                String fn = matcher.group(1);
+                if (fn.equals("print")) {
+                    String arg = matcher.group(2).trim();
+                    String maybeEnd = "";
+                    if (arg.endsWith(",")) {
+                        maybeEnd = " end=\" \"";
+                    }
+                    msg = (new ErrorMessageFormatter()).format("Missing parentheses in call to 'print'. Did you mean print(%s%s)?", arg, maybeEnd);
+                } else {
+                    msg = "Missing parentheses in call to 'exec'";
+                }
+            }
+        }
         instance.setAttribute("msg", msg);
         throw PException.fromObject(instance, location);
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Python3Core.java

@@ -166,6 +166,7 @@ public abstract class ErrorMessages {
     public static final String COUNT_FUNC_MATH = "count function in Math";
     public static final String COVERAGE_TRACKER_NOT_RUNNING = "coverage tracker not running";
     public static final String CREATING_SOCKETS_NOT_ALLOWED = "creating sockets not allowed";
+    public static final String DEFAULT_EXCEPT_MUST_BE_LAST = "default 'except:' must be last";
     public static final String DESC_S_FOR_S_DOESNT_APPLY_TO_S = "descriptor '%s' for '%s' objects doesn't apply to '%s' object";
     public static final String DESCRIPTOR_REQUIRES_OBJ = "descriptor '%s' requires a '%s' object but received a '%p'";
     public static final String DICT_CHANGED_DURING_COMPARISON = "dictionary changed during comparison operation";
@@ -215,6 +216,7 @@ public abstract class ErrorMessages {
     public static final String FUNC_CONSTRUCTION_NOT_SUPPORTED = "function construction not supported for (%p, %p, %p, %p, %p, %p)";
     public static final String FUNC_TAKES_AT_LEAST_D_ARGS = "function takes at least %d arguments (%d given)";
     public static final String FUNC_TAKES_EXACTLY_D_ARGS = "function takes exaclty %d arguments (%d given)";
+    public static final String GENERATOR_EXPR_MUST_BE_PARENTHESIZED = "Generator expression must be parenthesized";
     public static final String GENERATOR_IGNORED_EXIT = "generator ignored GeneratorExit";
     public static final String GENERATOR_RAISED_STOPITER = "generator raised StopIteration";
     public static final String GENERATOR_ALREADY_EXECUTING = "generator already executing";
@@ -329,6 +331,7 @@ public abstract class ErrorMessages {
     public static final String MUST_BE_STRINGS_NOT_P = "%s must be strings, not %p";
     public static final String MUST_SPECIFY_FILTERS = "Must specify filters for FORMAT_RAW";
     public static final String MUTATED_DURING_UPDATE = "%s mutated during update";
+    public static final String NAME_IS_ASSIGNED_BEFORE_GLOBAL = "name '%s' is assigned to before global declaration";
     public static final String NAME_IS_ASSIGNED_BEFORE_NONLOCAL = "name '%s' is assigned to before nonlocal declaration";
     public static final String NAME_NOT_DEFINED = "name '%s' is not defined";
     public static final String NEED_BYTELIKE_OBJ = "decoding to str: need a bytes-like object, %p found";
@@ -343,6 +346,8 @@ public abstract class ErrorMessages {
     public static final String NO_CURRENT_FRAME = "%s: no current frame";
     public static final String NO_FUNCTION_FOUND = "no function %s%s found in %s";
     public static final String NO_SUCH_FILE_OR_DIR = "No such file or directory: '%s:/%s'";
+    public static final String NONLOCAL_AND_GLOBAL = "name '%s' is nonlocal and global";
+    public static final String NONLOCAL_AT_MODULE_LEVEL = "nonlocal declaration not allowed at module level";
     public static final String NON_HEX_DIGIT_FOUND = "Non-hexadecimal digit found";
     public static final String NON_STRING_IN_CODE_SLOT = "non-string found in code slot";
     public static final String NOT_A_ZIP_FILE = "not a Zip file: '%s'";

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/ErrorMessages.java

@@ -129,6 +129,14 @@ public SSTNode registerGlobal(String[] names, int startOffset, int endOffset) {
         ScopeInfo scopeInfo = scopeEnvironment.getCurrentScope();
         ScopeInfo globalScope = scopeEnvironment.getGlobalScope();
         for (String name : names) {
+            if (scopeInfo.isExplicitNonlocalVariable(name)) {
+                throw errors.raiseInvalidSyntax(source, createSourceSection(startOffset, endOffset), ErrorMessages.NONLOCAL_AND_GLOBAL, name);
+            }
+            if (scopeInfo.findFrameSlot(name) != null) {
+                // The expectation is that in the local context the variable can not have slot yet.
+                // The slot is created by assignment or declaration
+                throw errors.raiseInvalidSyntax(source, createSourceSection(startOffset, endOffset), ErrorMessages.NAME_IS_ASSIGNED_BEFORE_GLOBAL, name);
+            }
             scopeInfo.addExplicitGlobalVariable(name);
             globalScope.createSlotIfNotPresent(name);
         }
@@ -137,7 +145,13 @@ public SSTNode registerGlobal(String[] names, int startOffset, int endOffset) {
 
     public SSTNode registerNonLocal(String[] names, int startOffset, int endOffset) {
         ScopeInfo scopeInfo = scopeEnvironment.getCurrentScope();
+        if (scopeInfo.getScopeKind() == ScopeKind.Module) {
+            throw errors.raiseInvalidSyntax(source, createSourceSection(startOffset, endOffset), ErrorMessages.NONLOCAL_AT_MODULE_LEVEL);
+        }
         for (String name : names) {
+            if (scopeInfo.isExplicitGlobalVariable(name)) {
+                throw errors.raiseInvalidSyntax(source, createSourceSection(startOffset, endOffset), ErrorMessages.NONLOCAL_AND_GLOBAL, name);
+            }
             if (scopeInfo.findFrameSlot(name) != null) {
                 // the expectation is that in the local context the variable can not have slot yet.
                 // The slot is created by assignment or declaration
@@ -173,6 +187,9 @@ public SSTNode createAssignment(SSTNode[] lhs, SSTNode rhs, int start, int stop)
     }
 
     public SSTNode createAnnAssignment(SSTNode lhs, SSTNode type, SSTNode rhs, int start, int end) {
+        if (lhs instanceof CollectionSSTNode) {
+            throw errors.raiseInvalidSyntax(source, createSourceSection(lhs.getStartOffset(), lhs.getEndOffset()), "only single target (not tuple) can be annotated");
+        }
         declareVar(lhs);
         if (!scopeEnvironment.getCurrentScope().hasAnnotations()) {
             scopeEnvironment.getCurrentScope().setHasAnnotations(true);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonSSTNodeFactory.java

@@ -65,7 +65,10 @@ public class DescriptiveBailErrorListener extends BaseErrorListener {
     public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol,
                     int line, int charPositionInLine,
                     String msg, RecognitionException e) {
-
+        // If we already constructed the error, keep it, it should be the most specific one
+        if (e instanceof EmptyRecognitionException) {
+            throw e;
+        }
         String entireMessage = e == null || e.getMessage() == null ? "invalid syntax" : e.getMessage();
 
         IntervalSet expectedTokens = null;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/antlr/DescriptiveBailErrorListener.java

@@ -544,6 +544,11 @@ typedargslist [ArgDefListBuilder args]
             ( ',' ( kwargsparameter[args] ','? )? )?
 	| kwargsparameter[args] ','?
     )
+    {
+        if (!args.validateArgumentsAfterSplat()) {
+            throw new PythonRecognitionException("named arguments must follow bare *", this, _input, $ctx, getCurrentToken());
+        }
+    }
 ;
 
 defparameter [ArgDefListBuilder args]
@@ -609,6 +614,11 @@ varargslist returns [ArgDefListBuilder result]
                 ( ',' (vkwargsparameter[args] ','? )? )?
             | vkwargsparameter[args] ','?
 	)
+    {
+        if (!args.validateArgumentsAfterSplat()) {
+            throw new PythonRecognitionException("named arguments must follow bare *", this, _input, $ctx, getCurrentToken());
+        }
+    }
 	{ $result = args; }
 ;
 
@@ -768,15 +778,15 @@ flow_stmt
 	b='break' 
 	{
             if (loopState == null) {
-                throw new PythonRecognitionException("'break' outside loop", this, _input, _localctx, getCurrentToken());
+                throw new PythonRecognitionException("'break' outside loop", this, _input, _localctx, $b);
             }
             push(new SimpleSSTNode(SimpleSSTNode.Type.BREAK, getStartIndex($b), getStopIndex($b)));
             loopState.containsBreak = true;
         }
 	| c='continue'
 	{
 	        if (loopState == null) {
-	            throw new PythonRecognitionException("'continue' not properly in loop", this, _input, _localctx, getCurrentToken());
+	            throw new PythonRecognitionException("'continue' not properly in loop", this, _input, _localctx, $c);
 	        }
             push(new SimpleSSTNode(SimpleSSTNode.Type.CONTINUE, getStartIndex($c), getStopIndex($c)));
             loopState.containsContinue = true;
@@ -1612,7 +1622,7 @@ argument [ArgListBuilder args] returns [SSTNode result]
                 }
 		test comp_for[$test.result, null, PythonBuiltinClassType.PGenerator, 0]
                 {
-                    args.addArg($comp_for.result);
+                    args.addNakedForComp($comp_for.result);
                    scopeEnvironment.popScope();
                 }
 	|