Revision: 8202
Author: sco...@google.com
Date: Fri May 21 13:07:23 2010
Log: JsParserException better error reporting.

http://gwt-code-reviews.appspot.com/546801/show
Review by: spoon

http://code.google.com/p/google-web-toolkit/source/detail?r=8202

Added:
 /branches/2.1/dev/core/test/com/google/gwt/dev/js/JsParserTest.java
Modified:
/branches/2.1/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java
 /branches/2.1/dev/core/src/com/google/gwt/dev/cfg/ModuleDefSchema.java
 /branches/2.1/dev/core/src/com/google/gwt/dev/javac/JsniCollector.java
 /branches/2.1/dev/core/src/com/google/gwt/dev/js/JsParser.java
 /branches/2.1/dev/core/src/com/google/gwt/dev/js/JsParserException.java
 /branches/2.1/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java
 /branches/2.1/dev/core/test/com/google/gwt/dev/javac/JsniCollectorTest.java

=======================================
--- /dev/null
+++ /branches/2.1/dev/core/test/com/google/gwt/dev/js/JsParserTest.java Fri May 21 13:07:23 2010
@@ -0,0 +1,92 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.js;
+
+import com.google.gwt.dev.jjs.SourceInfo;
+import com.google.gwt.dev.js.ast.JsBlock;
+import com.google.gwt.dev.js.ast.JsProgram;
+
+import junit.framework.TestCase;
+
+import java.io.IOException;
+import java.io.StringReader;
+
+/**
+ * Tests {...@link JsParser}.
+ */
+public class JsParserTest extends TestCase {
+
+  public class Result {
+
+    private String source;
+    private JsParserException exception;
+
+    public Result(JsBlock jsBlock) {
+      this.source = jsBlock.toSource().replaceAll("\\s+", " ");
+    }
+
+    public Result(JsParserException e) {
+      this.exception = e;
+    }
+
+    public void into(String expected) throws JsParserException {
+      if (exception != null) {
+        throw exception;
+      }
+      assertEquals(expected, source);
+    }
+
+    public void error(String expectedMsg) {
+      assertNotNull("No JsParserException was thrown", exception);
+      assertEquals(expectedMsg, exception.getMessage());
+    }
+  }
+
+  public void testBasic() throws JsParserException {
+    parse("foo").into("foo; ");
+    parse(" foo ").into("foo; ");
+    parse("foo()").into("foo(); ");
+    parse("foo(); bar()").into("foo(); bar(); ");
+    parse("window.alert('3' + 3)").into("window.alert('3' + 3); ");
+    parse("{ foo() }").into("{ foo(); } ");
+  }
+
+  public void testParseErrors() {
+    parse("1a2b").error(
+        "test.js(1): missing ; before statement\n> 1a2b\n> ----^");
+    parse("foo(").error("test.js(1): syntax error\n> \n> ^");
+    parse("+").error("test.js(1): syntax error\n> \n> ^");
+    parse(")").error("test.js(1): syntax error\n> )\n> -^");
+    parse("}").error("test.js(1): syntax error\n> }\n> -^");
+    parse("foo();\n}").error("test.js(2): syntax error\n> }\n> -^");
+    parse("foo();\nbar;\n}").error("test.js(3): syntax error\n> }\n> -^");
+  }
+
+  private Result parse(String js) {
+    try {
+      JsProgram program = new JsProgram();
+      SourceInfo rootSourceInfo = program.createSourceInfo(1, "test.js");
+      JsBlock block = program.getGlobalBlock();
+      JsParser.parseInto(rootSourceInfo, program.getScope(), block,
+          new StringReader(js));
+      return new Result(block);
+    } catch (IOException e) {
+ throw new RuntimeException("Unexpected error reading in-memory stream", e);
+    } catch (JsParserException e) {
+      return new Result(e);
+    }
+  }
+}
=======================================
--- /branches/2.1/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java Wed Mar 3 09:01:30 2010 +++ /branches/2.1/dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java Fri May 21 13:07:23 2010
@@ -415,12 +415,11 @@
     funcName.setObfuscatable(false);

     try {
-      SourceInfo sourceInfo = jsProgram.createSourceInfoSynthetic(
-          StandardLinkerContext.class, "Linker-derived JS");
+      SourceInfo sourceInfo = jsProgram.createSourceInfo(1,
+          "StandardLinkerContext.optimizeJavaScript");
JsParser.parseInto(sourceInfo, topScope, jsProgram.getGlobalBlock(), r);
     } catch (IOException e) {
-      logger.log(TreeLogger.ERROR, "Unable to parse JavaScript", e);
-      throw new UnableToCompleteException();
+ throw new RuntimeException("Unexpected error reading in-memory stream", e);
     } catch (JsParserException e) {
       logger.log(TreeLogger.ERROR, "Unable to parse JavaScript", e);
       throw new UnableToCompleteException();
=======================================
--- /branches/2.1/dev/core/src/com/google/gwt/dev/cfg/ModuleDefSchema.java Thu Mar 25 12:00:47 2010 +++ /branches/2.1/dev/core/src/com/google/gwt/dev/cfg/ModuleDefSchema.java Fri May 21 13:07:23 2010
@@ -22,7 +22,6 @@
 import com.google.gwt.core.ext.linker.LinkerOrder;
 import com.google.gwt.dev.js.JsParser;
 import com.google.gwt.dev.js.JsParserException;
-import com.google.gwt.dev.js.JsParserException.SourceDetail;
 import com.google.gwt.dev.js.ast.JsExprStmt;
 import com.google.gwt.dev.js.ast.JsFunction;
 import com.google.gwt.dev.js.ast.JsProgram;
@@ -1267,20 +1266,7 @@
       logger.log(TreeLogger.ERROR, "Error reading script source", e);
       throw new UnableToCompleteException();
     } catch (JsParserException e) {
-      SourceDetail dtl = e.getSourceDetail();
-      if (dtl != null) {
-        StringBuffer sb = new StringBuffer();
-        sb.append(moduleURL.toExternalForm());
-        sb.append("(");
-        sb.append(dtl.getLine());
-        sb.append(", ");
-        sb.append(dtl.getLineOffset());
-        sb.append("): ");
-        sb.append(e.getMessage());
-        logger.log(TreeLogger.ERROR, sb.toString(), e);
-      } else {
-        logger.log(TreeLogger.ERROR, "Error parsing JavaScript source", e);
-      }
+      logger.log(TreeLogger.ERROR, "Error parsing JavaScript source", e);
       throw new UnableToCompleteException();
     }

=======================================
--- /branches/2.1/dev/core/src/com/google/gwt/dev/javac/JsniCollector.java Mon Jan 11 17:58:07 2010 +++ /branches/2.1/dev/core/src/com/google/gwt/dev/javac/JsniCollector.java Fri May 21 13:07:23 2010
@@ -243,7 +243,11 @@
           e.getSourceDetail());
       SourceInfo errorInfo = SourceOrigin.create(problemCharPos,
problemCharPos, e.getSourceDetail().getLine(), info.getFileName());
-      reportJsniError(errorInfo, method, e.getMessage());
+      // Strip the file/line header because reportJsniError will add that.
+      String msg = e.getMessage();
+      int pos = msg.indexOf(": ");
+      msg = msg.substring(pos + 2);
+      reportJsniError(errorInfo, method, msg);
       return null;
     }
   }
=======================================
--- /branches/2.1/dev/core/src/com/google/gwt/dev/js/JsParser.java Mon Mar 1 10:24:16 2010 +++ /branches/2.1/dev/core/src/com/google/gwt/dev/js/JsParser.java Fri May 21 13:07:23 2010
@@ -99,20 +99,20 @@
     this.program = program;
   }

- List<JsStatement> parseImpl(SourceInfo rootSourceInfo, JsScope scope, Reader r)
-      throws JsParserException, IOException {
+ List<JsStatement> parseImpl(final SourceInfo rootSourceInfo, JsScope scope,
+      Reader r) throws JsParserException, IOException {
// Create a custom error handler so that we can throw our own exceptions.
     Context.enter().setErrorReporter(new ErrorReporter() {
public void error(String msg, String loc, int ln, String src, int col) {
         throw new UncheckedJsParserException(new JsParserException(msg, ln,
-            src, col));
+            src, col, rootSourceInfo.getFileName()));
       }

public EvaluatorException runtimeError(String msg, String loc, int ln,
           String src, int col) {
         // Never called, but just in case.
         throw new UncheckedJsParserException(new JsParserException(msg, ln,
-            src, col));
+            src, col, rootSourceInfo.getFileName()));
       }

public void warning(String msg, String loc, int ln, String src, int col) {
@@ -140,9 +140,8 @@
   }

private JsParserException createParserException(String msg, Node offender) {
-    // TODO: get source info
-    offender.getLineno();
-    return new JsParserException(msg);
+    return new JsParserException(msg, offender.getLineno(), null, 0,
+        sourceInfoStack.peek().getFileName());
   }

   private JsScope getScope() {
@@ -331,8 +330,8 @@

       default:
         int tokenType = node.getType();
-        throw new JsParserException("Unexpected top-level token type: "
-            + tokenType);
+        throw createParserException("Unexpected top-level token type: "
+            + tokenType, node);
     }
   }

@@ -406,8 +405,8 @@
         return mapBinaryOperation(JsBinaryOperator.ASG_SHRU, asgNode);

       default:
-        throw new JsParserException("Unknown assignment operator variant: "
-            + asgNode.getIntDatum());
+        throw createParserException("Unknown assignment operator variant: "
+            + asgNode.getIntDatum(), asgNode);
     }
   }

@@ -572,8 +571,8 @@
         return mapBinaryOperation(JsBinaryOperator.GTE, eqNode);

       default:
-        throw new JsParserException("Unknown equality operator variant: "
-            + eqNode.getIntDatum());
+        throw createParserException("Unknown equality operator variant: "
+            + eqNode.getIntDatum(), eqNode);
     }
   }

@@ -783,8 +782,8 @@
       case TokenStream.POST:
         return mapPostfixOperation(op, node);
       default:
-        throw new JsParserException("Unknown prefix/postfix variant: "
-            + node.getIntDatum());
+        throw createParserException("Unknown prefix/postfix variant: "
+            + node.getIntDatum(), node);
     }
   }

@@ -907,7 +906,8 @@
         return program.getUndefinedLiteral();

       default:
- throw new JsParserException("Unknown primary: " + node.getIntDatum()); + throw createParserException("Unknown primary: " + node.getIntDatum(),
+            node);
     }
   }

@@ -947,8 +947,8 @@
         return mapBinaryOperation(JsBinaryOperator.INOP, relNode);

       default:
-        throw new JsParserException("Unknown relational operator variant: "
-            + relNode.getIntDatum());
+        throw createParserException("Unknown relational operator variant: "
+            + relNode.getIntDatum(), relNode);
     }
   }

@@ -1006,8 +1006,8 @@
         return mapBinaryOperation(JsBinaryOperator.SHRU, shiftNode);

       default:
-        throw new JsParserException("Unknown equality operator variant: "
-            + shiftNode.getIntDatum());
+        throw createParserException("Unknown equality operator variant: "
+            + shiftNode.getIntDatum(), shiftNode);
     }
   }

@@ -1201,8 +1201,8 @@
         return mapPrefixOperation(JsUnaryOperator.VOID, unOp);

       default:
-        throw new JsParserException("Unknown unary operator variant: "
-            + unOp.getIntDatum());
+        throw createParserException("Unknown unary operator variant: "
+            + unOp.getIntDatum(), unOp);
     }
   }

=======================================
--- /branches/2.1/dev/core/src/com/google/gwt/dev/js/JsParserException.java Thu Apr 5 13:07:16 2007 +++ /branches/2.1/dev/core/src/com/google/gwt/dev/js/JsParserException.java Fri May 21 13:07:23 2010
@@ -24,16 +24,21 @@
    * Represents the location of a parser exception.
    */
   public static class SourceDetail {
+    private final String fileName;
     private final int line;
-
     private final int lineOffset;
-
     private final String lineSource;

-    public SourceDetail(int line, String lineSource, int lineOffset) {
+    public SourceDetail(int line, String lineSource, int lineOffset,
+        String fileName) {
       this.line = line;
       this.lineSource = lineSource;
       this.lineOffset = lineOffset;
+      this.fileName = fileName;
+    }
+
+    public String getFileName() {
+      return fileName;
     }

     public int getLine() {
@@ -49,51 +54,45 @@
     }
   }

-  private final SourceDetail sourceDetail;
-
-  public JsParserException(String msg) {
-    super(msg);
-    sourceDetail = null;
-  }
-
-  public JsParserException(String msg, int line, String lineSource,
-      int lineOffset) {
-    this(msg, line, lineSource, lineOffset, null);
-  }
-
-  public JsParserException(String msg, int line, String lineSource,
-      int lineOffset, Throwable cause) {
-    super(msg, cause);
-    sourceDetail = new SourceDetail(line, lineSource, lineOffset);
-  }
-
-  public JsParserException(String msg, Throwable cause) {
-    super(msg, cause);
-    sourceDetail = null;
-  }
-
-  public String getDescription() {
+  private static String createMessageWithDetail(String msg,
+      SourceDetail sourceDetail) {
+    if (sourceDetail == null) {
+      return msg;
+    }
     StringBuffer sb = new StringBuffer();
-
-    if (sourceDetail != null) {
-      sb.append("Line ");
-      sb.append(sourceDetail.getLine());
-      sb.append(": ");
-      sb.append(getMessage());
-      sb.append("\n");
-      sb.append("> ");
+    sb.append(sourceDetail.getFileName());
+    sb.append('(');
+    sb.append(sourceDetail.getLine());
+    sb.append(')');
+    sb.append(": ");
+    sb.append(msg);
+    if (sourceDetail.getLineSource() != null) {
+      sb.append("\n> ");
       sb.append(sourceDetail.getLineSource());
       sb.append("\n> ");
       for (int i = 0, n = sourceDetail.getLineOffset(); i < n; ++i) {
         sb.append('-');
       }
       sb.append('^');
-    } else {
-      sb.append(getMessage());
-    }
-
+    }
     return sb.toString();
   }
+
+  private final SourceDetail sourceDetail;
+
+  public JsParserException(String msg) {
+    this(msg, null);
+  }
+
+  public JsParserException(String msg, int line, String lineSource,
+      int lineOffset, String fileName) {
+    this(msg, new SourceDetail(line, lineSource, lineOffset, fileName));
+  }
+
+  public JsParserException(String msg, SourceDetail sourceDetail) {
+    super(createMessageWithDetail(msg, sourceDetail));
+    this.sourceDetail = sourceDetail;
+  }

   /**
    * Provides additional source detail in some cases.
=======================================
--- /branches/2.1/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java Mon Jan 11 09:04:38 2010 +++ /branches/2.1/dev/core/test/com/google/gwt/dev/javac/JsniCheckerTest.java Fri May 21 13:07:23 2010
@@ -341,7 +341,8 @@
     code.append("    @Buggy;\n");
     code.append("  }-*/;\n");
     code.append("}\n");
-    shouldGenerateError(code, 3, "Expected \":\" in JSNI reference");
+    shouldGenerateError(code, 3,
+ "Expected \":\" in JSNI reference\n> @Buggy;\n" + "> ----------^");
   }

   public void testMethodArgument() {
=======================================
--- /branches/2.1/dev/core/test/com/google/gwt/dev/javac/JsniCollectorTest.java Thu Nov 19 13:51:02 2009 +++ /branches/2.1/dev/core/test/com/google/gwt/dev/javac/JsniCollectorTest.java Fri May 21 13:07:23 2010
@@ -74,7 +74,8 @@
assertEquals(source.indexOf('@') + "Bar".length(), problem.getSourceStart());
     assertEquals(3, problem.getSourceLineNumber());
     assertTrue(problem.isError());
-    assertEquals("Expected \":\" in JSNI reference", problem.getMessage());
+ assertEquals("Expected \":\" in JSNI reference\n> @Bar;\n> --------^",
+        problem.getMessage());
   }

   public void testMalformedJsniRefPositionWithExtraLines() {
@@ -91,7 +92,8 @@
assertEquals(source.indexOf('@') + "Bar".length(), problem.getSourceStart());
     assertEquals(9, problem.getSourceLineNumber());
     assertTrue(problem.isError());
-    assertEquals("Expected \":\" in JSNI reference", problem.getMessage());
+ assertEquals("Expected \":\" in JSNI reference\n> @Bar;\n> --------^",
+        problem.getMessage());
   }

   public void testSourcePosition() {

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to