Move to ANTLR 4 with Java 11 language features and localization of more syntax errors.

[sampottinger]

Introduces ANTLR4 and Java 8 language feature support within IDE while also adding additional hooks for localization of syntax error messages, addressing https://github.com/processing/processing/issues/3054 and https://github.com/processing/processing/issues/3055.

The PR is broadly a continuation of https://github.com/processing/processing/issues/3055, bringing it up to speed with the latest Processing master plus the changes introduced at https://github.com/processing/processing/pull/5753. Requires https://github.com/processing/processing/pull/5753 as pre-requisite. This introduces a number of edits beyond https://github.com/processing/processing/issues/3055 beyond compatibility with current Processing master and https://github.com/processing/processing/pull/5753 including:

• Update to the grammar itself
• Change ANTLR listeners to emit TextTransform.Edit to unify JDT-based PreprocessingService and JavaBuild, removing code with duplicate purpose.
• Introduction of syntax error rewriting with support for localization.
• Addition of complete localized strings set for English and Spanish.
• Addition of partial localized strings set for other languages.
• Refactor of ANTLR-related code for testability and readability
• Expansion of tests including full parse tests for new Java features (type inference, lambdas)

[sampottinger]

Ohhh man. 🎉 I feel alive. This is working though there is some error handling still remaining. For a working example with Java 8 features, see: https://gist.github.com/sampottinger/45f63c1360a6622e9cb59bea978d18eb.

[sampottinger]

Dropping the [WIP] part of this PR title. This is ready for review.

[sampottinger]

Hey there! One more thing... I have included English and Spanish localizations (espro que mi traducción para los mensajes de errors es suficiente) of some error messages along with limited support for other languages using existing strings or (properly attributed) wikipedia translation of short phrases as comfortable. I have also updated #18 to include wikipedia attribution for other languages.

[monkstone]

Hi I just wanted to share with you following ParserTests failures on my linux build Apache Ant(TM) version 1.10.5 compiled on September 9 2018 openjdk version "11.0.3" 2019-04-16 OpenJDK Runtime Environment (build 11.0.3+4) OpenJDK 64-Bit Server VM (build 11.0.3+4, mixed mode):-

test:
    [junit] Testsuite: processing.mode.java.AutoFormatTests
    [junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.067 sec
    [junit] 
    [junit] Testsuite: processing.mode.java.ParserTests
    [junit] Tests run: 39, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 53.808 sec
    [junit] 
    [junit] Testcase: bug820(processing.mode.java.ParserTests): FAILED
    [junit] java.lang.IllegalStateException: No match found
    [junit] junit.framework.AssertionFailedError: java.lang.IllegalStateException: No match found
    [junit]     at processing.mode.java.ParserTests.expectCompilerException(ParserTests.java:81)
    [junit]     at processing.mode.java.ParserTests.bug820(ParserTests.java:203)
    [junit]     at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [junit]     at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit] 
    [junit] 
    [junit] Testcase: bug1145(processing.mode.java.ParserTests):    FAILED
    [junit] java.lang.IllegalStateException: No match found
    [junit] junit.framework.AssertionFailedError: java.lang.IllegalStateException: No match found
    [junit]     at processing.mode.java.ParserTests.expectCompilerException(ParserTests.java:81)
    [junit]     at processing.mode.java.ParserTests.bug1145(ParserTests.java:213)
    [junit]     at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [junit]     at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit]     at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
    [junit]     at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [junit] 
    [junit] 

BUILD FAILED
/home/sid/processing/build/build.xml:449: The following error occurred while executing this line:
/home/sid/processing/build/build.xml:537: The following error occurred while executing this line:
/home/sid/processing/java/build.xml:128: Test processing.mode.java.ParserTests failed

[sampottinger]

Thanks @monkstone! I’ll take a look today.

[sampottinger]

Hey @monkstone! I wasn't able to reproduce in Linux (Ubuntu 18.04.1) or Windows (10 via Paperspace). That said, I see that you are running 11.0.3 while AdoptOpenJDK is only on 11.0.2+9 and the OpenJDK PPA on 11.0.1+13.

That in mind, I am wondering if the output format of javac may have changed? That's because both of your failed tests are on "expected compiler exceptions" and those tests are running a regex against stderr output. That seems mildly error prone ha ha! I'm also not sure that checking for the offending line here is useful considering that JavaBuild and PreprocessingService use the JDT. My guess is that those checks are just left over from pre-JDT times.

So, I updated the tests to ensure that things fail to compile for those tests as expected. However, I removed the line check since I don't think it offers much value given how Processing later compiles files and interprets errors.

Can you give it another shot and let me know how it goes? Thanks!

[sampottinger]

That fixed it for me but for some reason I had to explicitly checkout the commit 99b4569 (weird), and also needed to install rsync on Archlinux. I look forward to doing some more experimenting.

That is very strange :( I'll go ahead create a fresh clone on a fresh linux vm and make sure there isn't anything else that crops up. Thanks so much @monkstone! (and sorry for the earlier inconvenience.)

[sampottinger]

Note: Important merge from #30 just completed - fixes use of contributed libraries.

[sampottinger]

Hey there! Sorry for a little noise. Just made a few (mostly minor) fixes after some testing today. I think noise should reduce now.

[monkstone]

I've been doing a bit of testing with the latest build, and for some reason calling size() on ArrayList/List gives syntax error in processing ide. Further when void mousePressed() is included in code the ide complains about reducing visibility of inherited PApplet method. I hacked Shiffman box2d as follows to get it to work (replaced for loop, and made mousePressed explicitly public).

 // The Nature of Code
// <http://www.shiffman.net/teaching/nature>
// Spring 2011
// Box2DProcessing example

// Basic example of falling rectangles

import shiffman.box2d.*;
import org.jbox2d.collision.shapes.*;
import org.jbox2d.common.*;
import org.jbox2d.dynamics.*;
import java.util.List;
import java.util.Iterator;
// A reference to our box2d world
Box2DProcessing box2d;

// A list we'll use to track fixed objects
List<Boundary> boundaries;
// A list for all of our rectangles
List<CustomShape> polygons;

void settings() {
  size(640,360);
  smooth();
}
void setup() {

  // Initialize box2d physics and create the world
  box2d = new Box2DProcessing(this);
  box2d.createWorld();
  // We are setting a custom gravity
  box2d.setGravity(0, -20);

  // Create ArrayLists  
  polygons = new ArrayList<>();
  boundaries = new ArrayList<>();

  // Add a bunch of fixed boundaries
  boundaries.add(new Boundary(width/4,height-5,width/2-50,10,0));
  boundaries.add(new Boundary(3*width/4,height-50,width/2-50,10,0));
  boundaries.add(new Boundary(width-5,height/2,10,height,0));
  boundaries.add(new Boundary(5,height/2,10,height,0));
}

void draw() {
  background(255);

  // We must always step through time!
  box2d.step();

  // Display all the boundaries
  for (Boundary wall: boundaries) {
    wall.display();
  }

  // Display all the people
  for (CustomShape cs: polygons) {
    cs.display();
  }

  // people that leave the screen, we delete them
  // (note they have to be deleted from both the box2d world and our list
  Iterator<CustomShape> shapeIterator = polygons.iterator();
  while (shapeIterator.hasNext()) {
    CustomShape cs = shapeIterator.next();
    if (cs.done()) {
      shapeIterator.remove();
    }
  }
}

void mousePressed() {
  CustomShape cs = new CustomShape(mouseX,mouseY);
  polygons.add(cs);
}

[sampottinger]

Hey @monkstone! I’ll take a look today thanks for reporting!

[sampottinger]

Hey @monkstone! Thank you so much for sticking with me here and apologies for that slightly embarrassing oversight.

There was an issue with the way that "special" methods were processed and, in part, this was caused by incompatibility with the new-ish settings behavior on mainline processing. I have resolved these issues, added tests to ensure they are caught in case of regression in the future, and can confirm that I can run the unmodified example code from https://github.com/shiffman/Box2D-for-Processing/tree/master/Box2D-for-Processing/dist/box2d_processing/examples/Boxes with this updated branch.

[sampottinger]

Humm... #40 might have had adverse effect. Seeing some public public action. Taking a look...

[sampottinger]

Hey there! Note that some important stuff merged in recently. Please re-pull before reporting issues here. Thanks!

[sampottinger]

Hey @dzaima! Really excited to share some changes with you. See below:

Also the live editor still shows errors on classes from code/ jar files though it runs fine.

I believe this is resolved with #46.

Another live error fail: char c = '\n';

I believe this is resolved with #49.

and it takes ~20 seconds

Wow this was more than just a little bit of a bummer. I ended up needing to change the Java grammar definitions :(. It's a long story but involves this: https://stackoverflow.com/questions/32910013/antlr-v4-java8-grammar-outofmemoryexception. Anyway, the bad news is that it is possible there may be a regression or two not captured yet by unit tests. The good news is that is MUCH more performant (unfortunately still not quite to the point of mainline processing but...).

I'll continue testing but this is all going to get merged in with #50 / #51.

Thanks so much for your help @dzaima! Also, have things been going well @monkstone?

[dzaima]

Now, with a jar in the code folder the live editor works fine, but attempting to run results in

Libraries must be installed in a folder named 'libraries' inside the sketchbook folder (see the Preferences window).
The package “APL” does not exist. You might be missing a library.

:P
Note that this also happens if there's no usage of the library in the code. Besides that, now this seems to be just as fast as official master :D

Also the console seems to be constantly spammed on any modification when there are antlr errors.

[sampottinger]

Thanks @dzaima! I’ll look into that today. Reallllyy happy it is faster though thanks for confirming that.

[sampottinger]

Thanks again for all of your help @dzaima! Code folder imports should be fixed with #52. I'll look for ANTLR error spamming.

[sampottinger]

Error spamming should be fixed! Thanks @dzaima.

[dzaima]

Yep, with the latest fixes the editor seems pretty stable! Will continue to report problems here as I encounter them working on stuff.

[dzaima]

@Override without public is broken again: @Override void setup() { }

Editor import suggestions seem to prepend classes. to the wanted path - e.g. using the import functionality from the error tab on Map<Long, Long> map;

[sampottinger]

Hey @dzaima!

@Override without public is broken again

Humm there's a unit test for that now but maybe there's an issue with the JDT edit. I'll take a look.

using the import functionality from the error tab on Map<Long, Long> map;

I'll try it out

[sampottinger]

Hey @dzaima! Thank you for that bug report. I have the fixes ready for you but am waiting for #56 to clear CI.

@Override without public is broken again

This regression was caused by an issue with the JDT edit emitted. Should be resolved now.

using the import functionality from the error tab on Map<Long, Long> map;

I think this was caused by modules in Java 11? I have resolved this issue on the java11 and antlr branches. It will be synced to master with #56.

Thanks!

[sampottinger]

56 is merged!

[anonymix007]

If I try to print some unicode letters, π for example, then it just prints ? else, println('π'+0) gives 63. In original Processing println('π'+0) gives 960 in console. println('π') gives ?.

JDK11:

Original:

[dzaima]

@anonymix007 For me, println('π') displays correctly in the console, println('π'+0) prints 960, and π shown with text() draws correctly on Linux.

From the 63 it seems that π in the source is getting literally replaced with ? - no idea why that'd be different between OSes - possibly wrong encoding usage? If you save a sketch with π in the source, does closing & reopening the sketch keep it? If you export the sketch, does π appear in sources/projectName.java, or is it already a ??

[anonymix007]

@dzaima Closing & reopening keeps π on place. After exporting sketch π replaced with ?, but in *.pde π symbol isn't replaced.

Original does well except for console: And JDK11 one:

[dzaima]

https://github.com/sampottinger/processing/blob/e3c965bcb534d1160d6cff44780fdeef007ba9d8/java/src/processing/mode/java/JavaBuild.java#L232

This line should be changed to

final PrintWriter stream = new PrintWriter(new FileWriter(java, StandardCharsets.UTF_8));

to fix the above unicode issue for Windows.

[sampottinger]

Hello! Sorry for going incognito there. I’ve been trying to finish up a paper submission before the end of my sabbatical (I might still be a bit slow to respond). Thank you very much @dzaima and @anonymix007! I will make this change.

[sampottinger]

Thank you very much @dzaima and @anonymix007! This is resolved.

[dzaima]

What's with the addEmptyLines in this commit? (let me guess - didn't uncheck replacing everywhere. such an annoying feature sometimes)

More specifically, it's one of the things (besides what's already wrong in official master) that prevents tweak mode from working (i did manage to get it to work here after fixing things):

https://github.com/sampottinger/processing/blob/675af86da59987ccbdb3eef33f1ff0f6c519d7a9/java/src/processing/mode/java/tweak/TweakClient.java#L142

[sampottinger]

Hey @dzaima! So sorry for the delay. Releasing fix presently...

[sampottinger]

Oh my... it looks like the tweaks issues are present on mainline as well. I think I probably should tackle that separately from this PR / https://github.com/processing/processing/pull/5753. If I get time, I'll take a look on mainline.

[dzaima]

@sampottinger tweak mode not working is already reported on mainline, I was just mentioning a separate, independent problem with the version here. Of course the mainline should also be fixed, but this should too.

[jeremydouglass]

A new pull proposes a fix for tweak mode on master: #5909

[sampottinger]

Hey @jeremydouglass thanks for the heads up! Also thank you @galsasson. :D I'm still deciding how best to tackle PRs unrelated to Java 11 / new ANTLR but, after I give it just a little thought, I might merge into my master without having it go into https://github.com/processing/processing/pull/5753. It would be nice to test tweaks mode with my branch.

[jeremydouglass]

re: 8e0bf4e in this PR, is the idea for this PR that PDE would initially migrate to Java 8 language features, or that it would skip straight to Java 11 language features?

[sampottinger]

Originally we were anticipating going to Java 8 and then reaching for Java 11 later. Based on some community feedback, broader documentation within the Java community has started reflecting language features in Java 11 so it may be confusing for folks to say try using lambdas or for-colon loops but need to understand that var was introduced in Java 9. I think saving folks from that complexity is a goal in line with #15. The only downside that has emerged so far: this widens the gap for android processing which, as of writing, doesn't support Java 11 grammar. At the same time, Java 8 features aren't totally safe in the Android community right now anyway.

That in mind, we can revert #105 if there is a compelling reason. Let me know what you think!

[sampottinger]

^ Ah dang sorry forgot to at-mention you @jeremydouglass in the above.

[jeremydouglass]

Very excited about Java 11, but I don't actually have a strong opinion -- if the goal is to eventually merge into upstream rather than stay forked, I'm guessing that is something to strategize with e.g. benfry, as either decision will have lots of implications for documentation and support etc. Given the installed user base, I could see arguments for either a gradual approach or an all-at-once transition. Best decision is probably whichever one sounds most survivable to the core devs....

[sampottinger]

Thanks @jeremydouglass! I completely agree and thank you again for your continued support through this. I've really been struggling with what kind of decisions this fork should feel are in purview (something similar also surfaced for #103 which relates to https://github.com/processing/processing/issues/3199 where I took the more conservative approach)... So, I am open to suggestions on this. My current thinking:

• On one hand https://github.com/processing/processing/issues/3054 is about 4 years old and predates Java 11 so one feasible interpretation of its intent is to deliver an up to date Java grammar for Processing and that it would have said Java 11 had it been released at the time. I also empathize with the sentiment from #104. Online documentation and expectations for what it means to develop with Java have also shifted so the goal posts for delivering a positive experience for future Java developers is also changing.
• On the other side, I would strongly prefer to be making these decisions with deeper connection to Processing's leadership.

After giving this some more thought, I probably wont revert #105 for now but will certainly be open to reversing if there's strong opinion in the opposite direction from the core devs / Ben. Ultimately, I don't see it as introducing too much risk for now (especially since we can't run on Java 8 anymore anyway so we'd only be limiting the API availability but couldn't buy any flexibility in targets).

[jeremydouglass]

Absolutely! Sounds good, and no question you have a strong strategy and the logic makes sense to me. Just suggesting that the 8 v 11 decision might be a key point -- if not the key point -- for coordination and communication if you have merging in mind.

[sampottinger]

Totally! When talking with @benfry, will definitely walk through that.

[sampottinger]

(Sorry ... less for you @jeremydouglass and more for googlers from the future...) Just to clarify for progeny, it was decided in https://github.com/processing/processing/pull/5753 that we would be moving to Java 11 in terms of the VM. The only question is if the grammar should support Java 11 or Java 8 features within the IDE. The Processing code base within this fork will not compile under the Java 8 JDK due to backwards incompatible changes made within the Java APIs, internal JDK structure, and how Java interacts with the OS. It is only intended to work with OpenJFX and OpenJDK 11+... See https://github.com/processing/processing/pull/5753 for further details.

[sampottinger]

Hey there! So... I spoke with Ben! We are still working through some pieces for getting this into mainline but, on this question specifically, we are going to hold on to the Java 11 language compatability changes for now.