Greatly improved syntax errors for poorly-closed generic type args and type params

app/grammars/pde.g

@@ -24,6 +24,16 @@ tokens {
 }
 
 {
+	// this clause copied from java15.g! ANTLR does not copy this
+	// section from the super grammar.
+	/**
+	 * Counts the number of LT seen in the typeArguments production.
+	 * It is used in semantic predicates to ensure we have seen
+	 * enough closing '>' characters; which actually may have been
+	 * either GT, SR or BSR tokens.
+	 */
+	private int ltCounter = 0;
+
 	private PdePreprocessor pp;
 	public PdeRecognizer(final PdePreprocessor pp, final TokenStream ts) {
 	    this(ts);
@@ -83,38 +93,66 @@ constant
     ; 
 
 // fix bug http://dev.processing.org/bugs/show_bug.cgi?id=1519
-// by removing a syntactic predicate whose sole purpose is to
+// by altering a syntactic predicate whose sole purpose is to
 // emit a useless error with no line numbers.
-// This is from Java15.g, with a few lines removed.
+// These are from Java15.g, with a few lines edited to make nice errors.
+
+// Type arguments to a class or interface type
 typeArguments
+{int currentLtLevel = 0;}
 	:
-		LT! 
+		{currentLtLevel = ltCounter;}
+		LT! {ltCounter++;}
 		typeArgument
 		(options{greedy=true;}: // match as many as possible
+			{if (! (inputState.guessing !=0 || ltCounter == currentLtLevel + 1)) {
+				throw new RecognitionException("Maybe too many > characters?",
+	    	                                     getFilename(), LT(1).getLine(), LT(1).getColumn());
+			}}
 			COMMA! typeArgument
 		)*
 
 		(	// turn warning off since Antlr generates the right code,
+			// plus we have our semantic predicate below
 			options{generateAmbigWarnings=false;}:
 			typeArgumentsOrParametersEnd
 		)?
 
+		// make sure we have gobbled up enough '>' characters
+		// if we are at the "top level" of nested typeArgument productions
+		{if (! ((currentLtLevel != 0) || ltCounter == currentLtLevel)) {
+			throw new RecognitionException("Maybe too many > characters?",
+    	                                     getFilename(), LT(1).getLine(), LT(1).getColumn());
+		}}
+
 		{#typeArguments = #(#[TYPE_ARGUMENTS, "TYPE_ARGUMENTS"], #typeArguments);}
 	;
 
-// more of the same
 typeParameters
+{int currentLtLevel = 0;}
 	:
-		LT! 
+		{currentLtLevel = ltCounter;}
+		LT! {ltCounter++;}
 		typeParameter (COMMA! typeParameter)*
 		(typeArgumentsOrParametersEnd)?
+
+		// make sure we have gobbled up enough '>' characters
+		// if we are at the "top level" of nested typeArgument productions
+		{if (! ((currentLtLevel != 0) || ltCounter == currentLtLevel)) {
+			throw new RecognitionException("Maybe too many > characters?",
+    	                                     getFilename(), LT(1).getLine(), LT(1).getColumn());
+		}}
+
 		{#typeParameters = #(#[TYPE_PARAMETERS, "TYPE_PARAMETERS"], #typeParameters);}
 	;
+	
 
+// this gobbles up *some* amount of '>' characters, and counts how many
+// it gobbled.
 protected typeArgumentsOrParametersEnd
-	:	GT!
-	|	SR!
-	|	BSR!
+	:	GT! {ltCounter-=1;}
+	|	SR! {ltCounter-=2;}
+	|	BSR! {ltCounter-=3;}
 	;
 
 // of the form #cc008f in PDE

app/src/processing/app/preproc/PdePreprocessor.java

@@ -49,7 +49,9 @@ import antlr.ASTFactory;
 import antlr.CommonAST;
 import antlr.CommonASTWithHiddenTokens;
 import antlr.CommonHiddenStreamToken;
+import antlr.RecognitionException;
 import antlr.TokenStreamCopyingHiddenTokenFilter;
+import antlr.TokenStreamException;
 import antlr.collections.AST;
 
 /**
@@ -266,13 +268,13 @@ public class PdePreprocessor implements PdeTokenTypes {
   }
 
   public PreprocessResult write(final Writer out, String program)
-      throws RunnerException, ANTLRException {
+      throws RunnerException, RecognitionException, TokenStreamException {
     return write(out, program, null);
   }
 
   public PreprocessResult write(final Writer out, String program,
                                 final String codeFolderPackages[])
-      throws RunnerException, ANTLRException {
+      throws RunnerException, RecognitionException, TokenStreamException {
 
     // these ones have the .* at the end, since a class name might be at the end
     // instead of .* which would make trouble other classes using this can lop
@@ -371,7 +373,7 @@ public class PdePreprocessor implements PdeTokenTypes {
    * @return the class name of the exported Java
    */
   private String write(final String program, final PrintWriter stream)
-      throws RunnerException, ANTLRException {
+      throws RunnerException, RecognitionException, TokenStreamException {
     // create a lexer with the stream reader, and tell it to handle
     // hidden tokens (eg whitespace, comments) since we want to pass these
     // through so that the line numbers when the compiler reports errors

app/test/resources/bug1516.expected

@@ -0,0 +1,50 @@
+import processing.core.*; 
+import processing.xml.*; 
+
+import java.util.ArrayList; 
+import java.util.List; 
+import java.util.Collections; 
+import java.util.Comparator; 
+
+import java.applet.*; 
+import java.awt.Dimension; 
+import java.awt.Frame; 
+import java.awt.event.MouseEvent; 
+import java.awt.event.KeyEvent; 
+import java.awt.event.FocusEvent; 
+import java.awt.Image; 
+import java.io.*; 
+import java.net.*; 
+import java.text.*; 
+import java.util.*; 
+import java.util.zip.*; 
+import java.util.regex.*; 
+
+public class bug1516 extends PApplet {
+
+
+
+
+
+
+public void setup()
+{
+List<String> list = new ArrayList<String>();
+list.add("foo");
+list.add("bar");
+list.add("baz");
+
+Comparator<String> comparator = new Comparator<String>()
+{
+public int compare(final String value0, final String value1)
+{
+return value0.compareTo(value1);
+}
+};
+
+Collections.sort(list, comparator);
+} 
+    static public void main(String args[]) {
+        PApplet.main(new String[] { "--bgcolor=null", "bug1516" });
+    }
+}

app/test/resources/bug1516.pde

@@ -0,0 +1,22 @@
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Collections;
+import java.util.Comparator;
+
+void setup()
+{
+List<String> list = new ArrayList<String>();
+list.add("foo");
+list.add("bar");
+list.add("baz");
+
+Comparator<String> comparator = new Comparator<String>()
+{
+public int compare(final String value0, final String value1)
+{
+return value0.compareTo(value1);
+}
+};
+
+Collections.sort(list, comparator);
+} 

app/test/resources/bug1517.expected

@@ -0,0 +1,50 @@
+import processing.core.*; 
+import processing.xml.*; 
+
+import java.util.ArrayList; 
+import java.util.List; 
+import java.util.Collections; 
+import java.util.Comparator; 
+
+import java.applet.*; 
+import java.awt.Dimension; 
+import java.awt.Frame; 
+import java.awt.event.MouseEvent; 
+import java.awt.event.KeyEvent; 
+import java.awt.event.FocusEvent; 
+import java.awt.Image; 
+import java.io.*; 
+import java.net.*; 
+import java.text.*; 
+import java.util.*; 
+import java.util.zip.*; 
+import java.util.regex.*; 
+
+public class bug1517 extends PApplet {
+
+
+
+
+
+
+Comparator<String> comparator = new Comparator<String>()
+{
+public int compare(final String value0, final String value1)
+{
+return value0.compareTo(value1);
+}
+};
+
+public void setup()
+{
+List<String> list = new ArrayList<String>();
+list.add("foo");
+list.add("bar");
+list.add("baz");
+
+Collections.sort(list, comparator);
+}
+    static public void main(String args[]) {
+        PApplet.main(new String[] { "--bgcolor=null", "bug1517" });
+    }
+}

app/test/resources/bug1517.pde

@@ -0,0 +1,22 @@
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Collections;
+import java.util.Comparator;
+
+Comparator<String> comparator = new Comparator<String>()
+{
+public int compare(final String value0, final String value1)
+{
+return value0.compareTo(value1);
+}
+};
+
+void setup()
+{
+List<String> list = new ArrayList<String>();
+list.add("foo");
+list.add("bar");
+list.add("baz");
+
+Collections.sort(list, comparator);
+}

app/test/resources/bug1519.pde

@@ -0,0 +1,9 @@
+import java.util.ArrayList;
+import java.util.List;
+
+void setup()
+{
+List<String> list = new ArrayList<String>();
+List<List<String>>> listOfLists = new ArrayList<List<String>>();
+listOfLists.add(list);
+}

app/test/src/test/processing/parsing/ParserTests.java

@@ -15,6 +15,7 @@ import processing.app.debug.RunnerException;
 import processing.app.preproc.PdePreprocessor;
 import processing.util.exec.ProcessResult;
 import antlr.ANTLRException;
+import antlr.RecognitionException;
 
 public class ParserTests {
 
@@ -65,8 +66,25 @@ public class ParserTests {
     return out.toString();
   }
 
-  static void expectGood(final String id) {
+  static void expectRecognitionException(final String id,
+                                         final String expectedMessage,
+                                         final int expectedLine) {
+    try {
+      preprocess(id, res(id + ".pde"));
+      fail("Expected to fail with \"" + expectedMessage + "\" on line "
+          + expectedLine);
+    } catch (RecognitionException e) {
+      assertEquals(expectedMessage, e.getMessage());
+      assertEquals(expectedLine, e.getLine());
+    } catch (Exception e) {
+      if (!e.equals(e.getCause()) && e.getCause() != null)
+        fail(e.getCause().getMessage());
+      else
+        fail(e.getMessage());
+    }
+  }
 
+  static void expectGood(final String id) {
     try {
       final String program = preprocess(id, res(id + ".pde"));
 
@@ -89,7 +107,7 @@ public class ParserTests {
       }
 
     } catch (Exception e) {
-      if (!e.equals(e.getCause()))
+      if (!e.equals(e.getCause()) && e.getCause() != null)
         fail(e.getCause().getMessage());
       else
         fail(e.getMessage());
@@ -121,6 +139,16 @@ public class ParserTests {
     expectGood("bug1514b");
   }
 
+  @Test
+  public void bug1516() {
+    expectGood("bug1516");
+  }
+
+  @Test
+  public void bug1517() {
+    expectGood("bug1517");
+  }
+
   @Test
   public void bug1518a() {
     expectGood("bug1518a");
@@ -130,4 +158,10 @@ public class ParserTests {
   public void bug1518b() {
     expectGood("bug1518b");
   }
+
+  @Test
+  public void bug1519() {
+    expectRecognitionException("bug1519", "Maybe too many > characters?", 7);
+  }
+
 }