Simpler SyntxError handling:

Since changes to fix bug 254778 made the parser the sole source of syntax error reports, it removed the need to check for errors after tree transformation.

The patch removes those checks and moves all reporting about syntax errors into omj/Parser.java.

js/rhino/src/org/mozilla/javascript/CompilerEnvirons.java

@@ -45,8 +45,6 @@ public class CompilerEnvirons
     public CompilerEnvirons()
     {
         this.errorReporter = DefaultErrorReporter.instance;
-        this.syntaxErrorCount = 0;
-        this.fromEval = false;
         this.languageVersion = Context.VERSION_DEFAULT;
         this.generateDebugInfo = true;
         this.useDynamicScope = false;
@@ -74,11 +72,6 @@ public class CompilerEnvirons
         this.activationNames = cx.activationNames;
     }
 
-    public final int getSyntaxErrorCount()
-    {
-        return syntaxErrorCount;
-    }
-
     public final ErrorReporter getErrorReporter()
     {
         return errorReporter;
@@ -90,40 +83,6 @@ public class CompilerEnvirons
         this.errorReporter = errorReporter;
     }
 
-    final void reportSyntaxError(String message,
-                                 String sourceName, int lineno,
-                                 String lineText, int lineOffset)
-    {
-        ++syntaxErrorCount;
-        if (fromEval) {
-            // We're probably in an eval. Need to throw an exception.
-            throw ScriptRuntime.constructError(
-                "SyntaxError", message, sourceName,
-                lineno, lineText, lineOffset);
-        } else {
-            getErrorReporter().error(message, sourceName, lineno,
-                                     lineText, lineOffset);
-        }
-    }
-
-    final void reportSyntaxWarning(String message,
-                                   String sourceName, int lineno,
-                                   String lineText, int lineOffset)
-    {
-        getErrorReporter().warning(message, sourceName,
-                                   lineno, lineText, lineOffset);
-    }
-
-    final boolean isFromEval()
-    {
-        return fromEval;
-    }
-
-    final void setFromEval(boolean fromEval)
-    {
-        this.fromEval = fromEval;
-    }
-
     public final int getLanguageVersion()
     {
         return languageVersion;
@@ -191,9 +150,6 @@ public class CompilerEnvirons
     }
 
     private ErrorReporter errorReporter;
-    private int syntaxErrorCount;
-
-    private boolean fromEval;
 
     int languageVersion = Context.VERSION_DEFAULT;
     boolean generateDebugInfo = true;

js/rhino/src/org/mozilla/javascript/Context.java

@@ -1197,11 +1197,10 @@ public class Context
         boolean errorseen = false;
         CompilerEnvirons compilerEnv = new CompilerEnvirons();
         compilerEnv.initFromContext(this);
-        compilerEnv.setErrorReporter(DefaultErrorReporter.instance);
         // no source name or source text manager, because we're just
         // going to throw away the result.
         compilerEnv.setGeneratingSource(false);
-        Parser p = new Parser(compilerEnv);
+        Parser p = new Parser(compilerEnv, DefaultErrorReporter.instance);
         try {
             p.parse(source, null, 1);
         } catch (EvaluatorException ee) {
@@ -1255,7 +1254,7 @@ public class Context
                 "Line number can not be negative:"+lineno);
         }
         return (Script) compile(null, in, null, sourceName, lineno,
-                                securityDomain, false, false);
+                                securityDomain, false, null);
     }
 
     /**
@@ -1281,16 +1280,18 @@ public class Context
             throw new IllegalArgumentException(
                 "Line number can not be negative:"+lineno);
         }
-        return compileString(source, false, sourceName, lineno, securityDomain);
+        return compileString(source, null, sourceName, lineno, securityDomain);
     }
 
-    final Script compileString(String source, boolean fromEval,
+    final Script compileString(String source,
+                               ErrorReporter compilationErrorReporter,
                                String sourceName, int lineno,
                                Object securityDomain)
     {
         try {
             return (Script) compile(null, null, source, sourceName, lineno,
-                                    securityDomain, false, fromEval);
+                                    securityDomain, false,
+                                    compilationErrorReporter);
         } catch (IOException ex) {
             // Should not happen when dealing with source as string
             throw new RuntimeException();
@@ -1320,7 +1321,7 @@ public class Context
     {
         try {
             return (Function) compile(scope, null, source, sourceName, lineno,
-                                      securityDomain, true, false);
+                                      securityDomain, true, null);
         }
         catch (IOException ioe) {
             // Should never happen because we just made the reader
@@ -2290,7 +2291,7 @@ public class Context
                            Reader sourceReader, String sourceString,
                            String sourceName, int lineno,
                            Object securityDomain, boolean returnFunction,
-                           boolean fromEval)
+                           ErrorReporter compilationErrorReporter)
         throws IOException
     {
         if (securityDomain != null && securityController == null) {
@@ -2305,7 +2306,9 @@ public class Context
 
         CompilerEnvirons compilerEnv = new CompilerEnvirons();
         compilerEnv.initFromContext(this);
-        compilerEnv.setFromEval(fromEval);
+        if (compilationErrorReporter == null) {
+            compilationErrorReporter = compilerEnv.getErrorReporter();
+        }
 
         if (debugger != null) {
             if (sourceReader != null) {
@@ -2314,50 +2317,40 @@ public class Context
             }
         }
 
-        Parser p = new Parser(compilerEnv);
+        Parser p = new Parser(compilerEnv, compilationErrorReporter);
         ScriptOrFnNode tree;
         if (sourceString != null) {
             tree = p.parse(sourceString, sourceName, lineno);
         } else {
             tree = p.parse(sourceReader, sourceName, lineno);
         }
-        int syntaxErrorCount = compilerEnv.getSyntaxErrorCount();
+        if (returnFunction) {
-        if (syntaxErrorCount == 0) {
+            if (!(tree.getFunctionCount() == 1
-            if (returnFunction) {
+                  && tree.getFirstChild() != null
-                if (!(tree.getFunctionCount() == 1
+                  && tree.getFirstChild().getType() == Token.FUNCTION))
-                      && tree.getFirstChild() != null
+            {
-                      && tree.getFirstChild().getType() == Token.FUNCTION))
+                // XXX: the check just look for the first child
-                {
+                // and allows for more nodes after it for compatibility
-                    // XXX: the check just look for the first child
+                // with sources like function() {};;;
-                    // and allows for more nodes after it for compatibility
+                throw new IllegalArgumentException(
-                    // with sources like function() {};;;
+                    "compileFunction only accepts source with single JS function: "+sourceString);
-                    throw new IllegalArgumentException(
-                        "compileFunction only accepts source with single JS function: "+sourceString);
-                }
-            }
-
-            Interpreter compiler = createCompiler();
-
-            String encodedSource = p.getEncodedSource();
-
-            Object result = compiler.compile(scope, compilerEnv,
-                                             tree, encodedSource,
-                                             returnFunction,
-                                             securityDomain);
-            syntaxErrorCount = compilerEnv.getSyntaxErrorCount();
-            if (syntaxErrorCount == 0) {
-                if (debugger != null) {
-                    if (sourceString == null) Kit.codeBug();
-                    compiler.notifyDebuggerCompilationDone(this, result,
-                                                           sourceString);
-                }
-                return result;
             }
         }
-        String msg = Context.getMessage1("msg.got.syntax.errors",
+
-                                         String.valueOf(syntaxErrorCount));
+        Interpreter compiler = createCompiler();
-        throw compilerEnv.getErrorReporter().
+
-            runtimeError(msg, sourceName, lineno, null, 0);
+        String encodedSource = p.getEncodedSource();
+
+        Object result = compiler.compile(scope, compilerEnv,
+                                         tree, encodedSource,
+                                         returnFunction,
+                                         securityDomain);
+        if (debugger != null) {
+            if (sourceString == null) Kit.codeBug();
+            compiler.notifyDebuggerCompilationDone(this, result,
+                                                   sourceString);
+        }
+        return result;
     }
 
     private static Class codegenClass = Kit.classOrNull(

js/rhino/src/org/mozilla/javascript/DefaultErrorReporter.java

@@ -44,24 +44,56 @@ class DefaultErrorReporter implements ErrorReporter
 {
     static final DefaultErrorReporter instance = new DefaultErrorReporter();
 
-    public void warning(String message, String sourceName, int line,
+    private boolean forEval;
-                        String lineSource, int lineOffset)
+    private ErrorReporter chainedReporter;
+
+    private DefaultErrorReporter() { }
+
+    static ErrorReporter forEval(ErrorReporter reporter)
     {
-        // do nothing
+        DefaultErrorReporter r = new DefaultErrorReporter();
+        r.forEval = true;
+        r.chainedReporter = reporter;
+        return r;
     }
 
-    public void error(String message, String sourceName, int line,
+    public void warning(String message, String sourceURI, int line,
-                      String lineSource, int lineOffset)
+                        String lineText, int lineOffset)
     {
-        throw new EvaluatorException(message, sourceName, line,
+        if (chainedReporter != null) {
-                                     lineSource, lineOffset);
+            chainedReporter.warning(
+                message, sourceURI, line, lineText, lineOffset);
+        } else {
+            // Do nothing
+        }
     }
 
-    public EvaluatorException runtimeError(String message, String sourceName,
+    public void error(String message, String sourceURI, int line,
-                                           int line, String lineSource,
+                      String lineText, int lineOffset)
+    {
+        if (forEval) {
+            throw ScriptRuntime.constructError(
+                "SyntaxError", message, sourceURI, line, lineText, lineOffset);
+        }
+        if (chainedReporter != null) {
+            chainedReporter.error(
+                message, sourceURI, line, lineText, lineOffset);
+        } else {
+            throw runtimeError(
+                message, sourceURI, line, lineText, lineOffset);
+        }
+    }
+
+    public EvaluatorException runtimeError(String message, String sourceURI,
+                                           int line, String lineText,
                                            int lineOffset)
     {
-        throw new EvaluatorException(message, sourceName, line,
+        if (chainedReporter != null) {
-                                     lineSource, lineOffset);
+            return chainedReporter.runtimeError(
+                message, sourceURI, line, lineText, lineOffset);
+        } else {
+            return new EvaluatorException(
+                message, sourceURI, line, lineText, lineOffset);
+        }
     }
 }

js/rhino/src/org/mozilla/javascript/Interpreter.java

@@ -298,7 +298,6 @@ public class Interpreter
             generateFunctionICode();
             return createFunction(cx, scope, itsData, false);
         } else {
-            itsData.itsFromEvalCode = compilerEnv.isFromEval();
             generateICodeFromTree(scriptOrFn);
             return new InterpretedScript(itsData);
         }

js/rhino/src/org/mozilla/javascript/NativeScript.java

@@ -196,7 +196,9 @@ class NativeScript extends NativeFunction implements Script
             filename = "<Script object>";
             linep[0] = 1;
         }
-        return cx.compileString(source, filename, linep[0], null);
+        ErrorReporter reporter;
+        reporter = DefaultErrorReporter.forEval(cx.getErrorReporter());
+        return cx.compileString(source, reporter, filename, linep[0], null);
     }
 
 // #string_id_map#

js/rhino/src/org/mozilla/javascript/Parser.java

@@ -58,9 +58,10 @@ import java.util.Hashtable;
 
 public class Parser
 {
-    public Parser(CompilerEnvirons compilerEnv)
+    public Parser(CompilerEnvirons compilerEnv, ErrorReporter errorReporter)
     {
         this.compilerEnv = compilerEnv;
+        this.errorReporter = errorReporter;
     }
 
     protected Decompiler createDecompiler(CompilerEnvirons compilerEnv)
@@ -86,7 +87,6 @@ public class Parser
 
     void reportError(String messageId)
     {
-        this.ok = false;
         ts.reportCurrentLineError(Context.getMessage0(messageId));
 
         // Throw a ParserException exception to unwind the recursive descent
@@ -94,6 +94,18 @@ public class Parser
         throw new ParserException();
     }
 
+    void reportError(String message, int lineno,
+                     String lineText, int lineOffset)
+    {
+        ++syntaxErrorCount;
+        errorReporter.error(message, sourceURI, lineno, lineText, lineOffset);
+    }
+
+    void reportWarning(String message, int lineno,
+                       String lineText, int lineOffset)
+    {
+        errorReporter.warning(message, sourceURI, lineno, lineText, lineOffset);
+    }
 
     public String getEncodedSource()
     {
@@ -154,10 +166,10 @@ public class Parser
      * CompilerEnvirons.)
      */
     public ScriptOrFnNode parse(String sourceString,
-                                String sourceLocation, int lineno)
+                                String sourceURI, int lineno)
     {
-        this.ts = new TokenStream(compilerEnv, null, sourceString,
+        this.sourceURI = sourceURI;
-                                  sourceLocation, lineno);
+        this.ts = new TokenStream(this, null, sourceString, lineno);
         try {
             return parse();
         } catch (IOException ex) {
@@ -175,11 +187,11 @@ public class Parser
      * CompilerEnvirons.)
      */
     public ScriptOrFnNode parse(Reader sourceReader,
-                                String sourceLocation, int lineno)
+                                String sourceURI, int lineno)
         throws IOException
     {
-        this.ts = new TokenStream(compilerEnv, sourceReader, null,
+        this.sourceURI = sourceURI;
-                                  sourceLocation, lineno);
+        this.ts = new TokenStream(this, sourceReader, null, lineno);
         return parse();
     }
 
@@ -193,7 +205,7 @@ public class Parser
         this.encodedSource = null;
         decompiler.addToken(Token.SCRIPT);
 
-        this.ok = true;
+        this.syntaxErrorCount = 0;
 
         int baseLineno = ts.getLineno();  // line number where source starts
 
@@ -216,7 +228,6 @@ public class Parser
                     try {
                         n = function(FunctionNode.FUNCTION_STATEMENT);
                     } catch (ParserException e) {
-                        this.ok = false;
                         break;
                     }
                 } else {
@@ -227,16 +238,18 @@ public class Parser
             }
         } catch (StackOverflowError ex) {
             String msg = Context.getMessage0("mag.too.deep.parser.recursion");
-            throw Context.reportRuntimeError(msg, ts.getSourceName(),
+            throw Context.reportRuntimeError(msg, sourceURI,
                                              ts.getLineno(), null, 0);
         }
 
-        if (!this.ok) {
+        if (this.syntaxErrorCount != 0) {
-            // XXX ts.clearPushback() call here?
+            String msg = String.valueOf(this.syntaxErrorCount);
-            return null;
+            msg = Context.getMessage1("msg.got.syntax.errors", msg);
+            throw errorReporter.runtimeError(msg, sourceURI, baseLineno,
+                                             null, 0);
         }
 
-        currentScriptOrFn.setSourceName(ts.getSourceName());
+        currentScriptOrFn.setSourceName(sourceURI);
         currentScriptOrFn.setBaseLineno(baseLineno);
         currentScriptOrFn.setEndLineno(ts.getLineno());
 
@@ -278,7 +291,7 @@ public class Parser
                 nf.addChildToBack(pn, n);
             }
         } catch (ParserException e) {
-            this.ok = false;
+            // Ignore it
         } finally {
             --nestingOfFunction;
         }
@@ -405,7 +418,7 @@ public class Parser
         }
 
         fnNode.setEncodedSourceBounds(functionSourceStart, functionSourceEnd);
-        fnNode.setSourceName(ts.getSourceName());
+        fnNode.setSourceName(sourceURI);
         fnNode.setBaseLineno(baseLineno);
         fnNode.setEndLineno(ts.getLineno());
 
@@ -1888,12 +1901,13 @@ public class Parser
     }
 
     CompilerEnvirons compilerEnv;
+    private ErrorReporter errorReporter;
+    private int syntaxErrorCount;
+    private String sourceURI;
     private TokenStream ts;
 
     private IRFactory nf;
 
-    private boolean ok; // Did the parse encounter an error?
-
     private int nestingOfFunction;
 
     private Decompiler decompiler;

js/rhino/src/org/mozilla/javascript/ScriptRuntime.java

@@ -2200,16 +2200,20 @@ public class ScriptRuntime {
         String sourceName = ScriptRuntime.
             makeUrlForGeneratedScript(true, filename, lineNumber);
 
+        ErrorReporter reporter;
+        reporter = DefaultErrorReporter.forEval(cx.getErrorReporter());
+
         // Compile the reader with opt level of -1 to force interpreter
         // mode.
         int savedLevel = cx.optimizationLevel;
         cx.optimizationLevel = -1;
         Script script;
         try {
-            script = cx.compileString((String)x, true, sourceName, 1, null);
+            script = cx.compileString((String)x, reporter, sourceName, 1, null);
         } finally {
             cx.optimizationLevel = savedLevel;
         }
+        ((InterpretedScript)script).itsData.itsFromEvalCode = true;
 
         // if the compile fails, an error has been reported by the
         // compiler, but we need to stop execution to avoid

js/rhino/src/org/mozilla/javascript/TokenStream.java

@@ -64,13 +64,11 @@ public class TokenStream
     private final static int
         EOF_CHAR = -1;
 
-    public TokenStream(CompilerEnvirons compilerEnv,
+    public TokenStream(Parser parser, Reader sourceReader, String sourceString,
-                       Reader sourceReader, String sourceString,
+                       int lineno)
-                       String sourceName, int lineno)
     {
-        this.compilerEnv = compilerEnv;
+        this.parser = parser;
         this.pushbackToken = Token.EOF;
-        this.sourceName = sourceName;
         this.lineno = lineno;
         if (sourceReader != null) {
             if (sourceString != null) Kit.codeBug();
@@ -287,18 +285,14 @@ public class TokenStream
 
     public final void reportCurrentLineError(String message)
     {
-        compilerEnv.reportSyntaxError(message, getSourceName(), getLineno(),
+        parser.reportError(message, getLineno(), getLine(), getOffset());
-                                      getLine(), getOffset());
     }
 
     public final void reportCurrentLineWarning(String message)
     {
-        compilerEnv.reportSyntaxWarning(message, getSourceName(), getLineno(),
+        parser.reportWarning(message, getLineno(), getLine(), getOffset());
-                                        getLine(), getOffset());
     }
 
-    public final String getSourceName() { return sourceName; }
-
     public final int getLineno() { return lineno; }
 
     public final int getOp() { return op; }
@@ -466,7 +460,8 @@ public class TokenStream
                     if (result != Token.EOF) {
                         if (result != Token.RESERVED) {
                             return result;
-                        } else if (!compilerEnv.reservedKeywordAsIdentifier)
+                        } else if (!parser.compilerEnv.
+                                    reservedKeywordAsIdentifier)
                         {
                             return result;
                         } else {
@@ -1448,7 +1443,6 @@ public class TokenStream
     boolean allowRegExp;
     String regExpFlags;
 
-    private String sourceName;
     private String line;
     private boolean fromEval;
     private int pushbackToken;
@@ -1492,5 +1486,5 @@ public class TokenStream
     private boolean xmlIsTagContent;
     private int xmlOpenTagsCount;
 
-    CompilerEnvirons compilerEnv;
+    private Parser parser;
 }

js/rhino/src/org/mozilla/javascript/optimizer/ClassCompiler.java

@@ -134,58 +134,48 @@ public class ClassCompiler
                                         int lineno,
                                         String mainClassName)
     {
-        Parser p = new Parser(compilerEnv);
+        Parser p = new Parser(compilerEnv, compilerEnv.getErrorReporter());
         ScriptOrFnNode tree = p.parse(source, sourceLocation, lineno);
-        int syntaxErrorCount = compilerEnv.getSyntaxErrorCount();
+        String encodedSource = p.getEncodedSource();
-        if (syntaxErrorCount == 0) {
-            String encodedSource = p.getEncodedSource();
 
-            Class superClass = getTargetExtends();
+        Class superClass = getTargetExtends();
-            Class[] interfaces = getTargetImplements();
+        Class[] interfaces = getTargetImplements();
-            String scriptClassName;
+        String scriptClassName;
-            boolean isPrimary = (interfaces == null && superClass == null);
+        boolean isPrimary = (interfaces == null && superClass == null);
-            if (isPrimary) {
+        if (isPrimary) {
-                scriptClassName = mainClassName;
+            scriptClassName = mainClassName;
-            } else {
+        } else {
-                scriptClassName = makeAuxiliaryClassName(mainClassName, "1");
+            scriptClassName = makeAuxiliaryClassName(mainClassName, "1");
-            }
+        }
 
-            Codegen codegen = new Codegen();
+        Codegen codegen = new Codegen();
-            byte[] scriptClassBytes
+        byte[] scriptClassBytes
-                = codegen.compileToClassFile(compilerEnv, scriptClassName,
+            = codegen.compileToClassFile(compilerEnv, scriptClassName,
-                                             tree, encodedSource,
+                                         tree, encodedSource,
-                                             false);
+                                         false);
 
-            syntaxErrorCount = compilerEnv.getSyntaxErrorCount();
+        if (isPrimary) {
-            if (syntaxErrorCount == 0) {
+            return new Object[] { scriptClassName, scriptClassBytes };
-                if (isPrimary) {
+        }
-                    return new Object[] { scriptClassName, scriptClassBytes };
+        int functionCount = tree.getFunctionCount();
-                }
+        ObjToIntMap functionNames = new ObjToIntMap(functionCount);
-                int functionCount = tree.getFunctionCount();
+        for (int i = 0; i != functionCount; ++i) {
-                ObjToIntMap functionNames = new ObjToIntMap(functionCount);
+            FunctionNode ofn = tree.getFunctionNode(i);
-                for (int i = 0; i != functionCount; ++i) {
+            String name = ofn.getFunctionName();
-                    FunctionNode ofn = tree.getFunctionNode(i);
+            if (name != null && name.length() != 0) {
-                    String name = ofn.getFunctionName();
+                functionNames.put(name, ofn.getParamCount());
-                    if (name != null && name.length() != 0) {
-                        functionNames.put(name, ofn.getParamCount());
-                    }
-                }
-                if (superClass == null) {
-                    superClass = ScriptRuntime.ObjectClass;
-                }
-                byte[] mainClassBytes
-                    = JavaAdapter.createAdapterCode(
-                        functionNames, mainClassName,
-                        superClass, interfaces, scriptClassName);
-
-                return new Object[] { mainClassName, mainClassBytes,
-                                      scriptClassName, scriptClassBytes };
             }
         }
-        String msg = ScriptRuntime.getMessage1(
+        if (superClass == null) {
-            "msg.got.syntax.errors", String.valueOf(syntaxErrorCount));
+            superClass = ScriptRuntime.ObjectClass;
-        throw compilerEnv.getErrorReporter().
+        }
-            runtimeError(msg, sourceLocation, lineno, null, 0);
+        byte[] mainClassBytes
+            = JavaAdapter.createAdapterCode(
+                functionNames, mainClassName,
+                superClass, interfaces, scriptClassName);
+
+        return new Object[] { mainClassName, mainClassBytes,
+                              scriptClassName, scriptClassBytes };
     }
 
     private CompilerEnvirons compilerEnv;