Improve error messages

javaparser · Mar 3, 2017 · d4afa18 · d4afa18
1 parent b025d3b
commit d4afa18
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 3,806 deletions.
diff --git a/javaparser-core/src/main/java/com/github/javaparser/JavaParser.java b/javaparser-core/src/main/java/com/github/javaparser/JavaParser.java
@@ -113,6 +113,7 @@ public <N extends Node> ParseResult<N> parse(ParseStart<N> start, Provider provi
             parser.problems.add(new Problem("Parse error", range, p));
             return new ParseResult<>(null, parser.problems, parser.getTokens(), parser.getCommentsCollection());
         } catch (Exception e) {
+            parser.problems.add(new Problem(e.getMessage(), null, e));
             return new ParseResult<>(null, parser.problems, parser.getTokens(), parser.getCommentsCollection());
         } finally {
             try {

diff --git a/javaparser-core/src/main/java/com/github/javaparser/Problem.java b/javaparser-core/src/main/java/com/github/javaparser/Problem.java
@@ -23,51 +23,72 @@
 
 import java.util.Optional;
 
-import com.github.javaparser.utils.Utils;
-
+import static com.github.javaparser.utils.Utils.EOL;
 import static com.github.javaparser.utils.Utils.assertNotNull;
 
 /**
  * A problem that was encountered during parsing.
  */
 public class Problem {
     private final String message;
-    private final Range range;
+    private final Range location;
     private final Throwable cause;
 
-    Problem(String message, Range range, Throwable cause) {
+    public Problem(String message, Range location, Throwable cause) {
         assertNotNull(message);
 
         this.message = message;
-        this.range = range;
+        this.location = location;
         this.cause = cause;
     }
 
     @Override
     public String toString() {
-        StringBuilder str = new StringBuilder(message);
-        if (range != null)
-            str.append(" at ").append(range.begin);
+        final StringBuilder str = new StringBuilder(getVerboseMessage());
         if (cause != null) {
-            str.append(Utils.EOL).append("Problem stacktrace : ").append(Utils.EOL);
+            str.append(EOL).append("Problem stacktrace : ").append(EOL);
             for (int i = 0; i < cause.getStackTrace().length; i++) {
                 StackTraceElement ste = cause.getStackTrace()[i];
                 str.append("  ").append(ste.toString());
                 if (i + 1 != cause.getStackTrace().length)
-                    str.append(Utils.EOL);
+                    str.append(EOL);
             }
         }
         return str.toString();
     }
 
+    /**
+     * @return the message that was passed into the constructor.
+     */
     public String getMessage() {
         return message;
     }
 
+    /**
+     * @return the message plus location information.
+     */
+    public String getVerboseMessage() {
+        return getLocation().map(l -> message + " at " + l.begin).orElse(message);
+    }
+
+    /**
+     * @return the location that was passed into the constructor.
+     */
+    public Optional<Range> getLocation() {
+        return Optional.ofNullable(location);
+    }
+
+    /**
+     * @deprecated use getLocation()
+     */
+    @Deprecated
     public Optional<Range> getRange() {
-        return Optional.ofNullable(range);
+        return getLocation();
     }
 
+    /**
+     * @return the cause that was passed into the constructor.
+     */
     public Optional<Throwable> getCause() {
         return Optional.ofNullable(cause);
     }

diff --git a/javaparser-testing/src/test/java/com/github/javaparser/JavaParserTest.java b/javaparser-testing/src/test/java/com/github/javaparser/JavaParserTest.java
@@ -87,7 +87,7 @@ public void parseErrorContainsLocation() {
         ParseResult<CompilationUnit> result = new JavaParser().parse(ParseStart.COMPILATION_UNIT, Providers.provider("class X { // blah"));
 
         Problem problem = result.getProblem(0);
-        assertEquals(range(1, 9, 1, 9), problem.getRange().get());
+        assertEquals(range(1, 9, 1, 9), problem.getLocation().get());
         assertEquals("Parse error", problem.getMessage());
         assertInstanceOf(ParseException.class, problem.getCause().get());
     }

diff --git a/javaparser-testing/src/test/java/com/github/javaparser/ProblemTest.java b/javaparser-testing/src/test/java/com/github/javaparser/ProblemTest.java
@@ -0,0 +1,31 @@
+package com.github.javaparser;
+
+import org.junit.Test;
+
+import static com.github.javaparser.Range.range;
+import static com.github.javaparser.utils.TestUtils.assertInstanceOf;
+import static org.junit.Assert.assertEquals;
+
+public class ProblemTest {
+    @Test
+    public void testSimpleGetters() {
+        Problem problem = new Problem("Parse error", range(10, 10, 20, 20), new Exception());
+
+        assertEquals(range(10, 10, 20, 20), problem.getLocation().get());
+        assertEquals("Parse error", problem.getMessage());
+        assertInstanceOf(Exception.class, problem.getCause().get());
+    }
+
+    @Test
+    public void testVerboseMessage() {
+        Problem problem = new Problem("Parse error", range(10, 10, 20, 20), null);
+
+        assertEquals("Parse error at (line 10,col 10)", problem.getVerboseMessage());
+    }
+    @Test
+    public void testVerboseMessageWithoutLocation() {
+        Problem problem = new Problem("Parse error", null, null);
+
+        assertEquals("Parse error", problem.getVerboseMessage());
+    }
+}
diff --git a/javaparser-testing/src/test/java/com/github/javaparser/manual/BulkParseTest.java b/javaparser-testing/src/test/java/com/github/javaparser/manual/BulkParseTest.java
@@ -16,6 +16,7 @@
 
 import static com.github.javaparser.utils.SourceRoot.Callback.Result.DONT_SAVE;
 import static com.github.javaparser.utils.TestUtils.download;
+import static com.github.javaparser.utils.TestUtils.temporaryDirectory;
 import static com.github.javaparser.utils.TestUtils.unzip;
 
 public class BulkParseTest {
@@ -28,7 +29,7 @@ public static void main(String[] args) throws IOException {
     }
 
     private void parseOpenJdk() throws IOException {
-        Path workdir = CodeGenerationUtils.mavenModuleRoot().resolve(Paths.get("target", "tmp"));
+        Path workdir = CodeGenerationUtils.mavenModuleRoot().resolve(Paths.get(temporaryDirectory(), "javaparser_openjdk_download"));
         workdir.toFile().mkdirs();
         Path openJdkZipPath = workdir.resolve("openjdk.zip");
         if (Files.notExists(openJdkZipPath)) {
@@ -58,7 +59,7 @@ public void bulkTest(SourceRoot sourceRoot, String testResultsFileName) throws I
                     writer.write(localPath.toString());
                     writer.newLine();
                     for (Problem problem : result.getProblems()) {
-                        writer.write(problem.toString());
+                        writer.write(problem.getVerboseMessage());
                         writer.newLine();
                     }
                 }

diff --git a/javaparser-testing/src/test/java/com/github/javaparser/utils/TestUtils.java b/javaparser-testing/src/test/java/com/github/javaparser/utils/TestUtils.java
@@ -82,4 +82,7 @@ public static void download(URL url, Path destination) throws IOException {
         Files.write(destination, response.body().bytes());
     }
 
+    public static String temporaryDirectory() {
+        return System.getProperty("java.io.tmpdir");
+    }
 }
diff --git a/...ng/src/test/resources/com/github/javaparser/bulk_test_results/javaparser_test_results.txt b/...ng/src/test/resources/com/github/javaparser/bulk_test_results/javaparser_test_results.txt
@@ -1,2 +1,4 @@
 javaparser-testing/target/test-classes/com/github/javaparser/TestFileIso88591.java
+Lexical error at line 5, column 22.  Encountered: "\ufffd" (65533), after : ""
 javaparser-testing/src/test/resources/com/github/javaparser/TestFileIso88591.java
+Lexical error at line 5, column 22.  Encountered: "\ufffd" (65533), after : ""