Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 150 commits into
base: java11
Choose a base branch
from

Conversation

sampottinger
Copy link
Owner

@sampottinger sampottinger commented Mar 26, 2019

Introduces ANTLR4 and Java 8 language feature support within IDE while also adding additional hooks for localization of syntax error messages, addressing processing#3054 and processing#3055.

The PR is broadly a continuation of processing#3055, bringing it up to speed with the latest Processing master plus the changes introduced at processing#5753. Requires processing#5753 as pre-requisite. This introduces a number of edits beyond processing#3055 beyond compatibility with current Processing master and processing#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)

JakubValtar and others added 6 commits February 16, 2015 23:41
based on work of Florian Jenett

- included ANTLR 4.5, building PDE and running sketches works
- error reporting is not yet implemented
- parses size() and adds sketchXxxx methods
- moved tests to java mode folder
- fixed expected whitespace in tests
- added sketchXxxx methods to expected test outputs
- error reporting tests are disabled, other tests are gree
@sampottinger sampottinger changed the title Move to ANTLR 4 with Java 8 language features. [WIP] Move to ANTLR 4 with Java 8 language features. Mar 26, 2019
@sampottinger
Copy link
Owner Author

sampottinger commented Mar 26, 2019

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.

java8_screenshot

Revert of sampottinger/processing #3 for the antlr branch.
@jeremydouglass
Copy link

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

@sampottinger
Copy link
Owner Author

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 processing#5753. It would be nice to test tweaks mode with my branch.

sampottinger and others added 4 commits August 21, 2019 08:52
Minor stylistic cleanup in PdeIssueEmitter.java, keeping identical base class method implementations in place and fixing long lines.
Minor stylistic cleanup in PdeIssueEmitter.java
We are seeing some sketches exceed the heap limits either through the JDT or ANTLR when they are very large (>50k lines). This PR increases the heap for helping the IDE keep up when the parse trees get larger. Separately, looking at benchmarks online, Java 11 is super fast but it may be a little less prudent on the heap. So, increasing the default sketch max memory to match the IDE.
sampottinger added a commit to sampottinger/ROMANESCO-Processing that referenced this pull request Aug 23, 2019
Unfortunately, mainline Processing is inconsistent on if imports within class bodies are allowed by the language definition. JavaBuild currently allows it (though does not explicitly permit it) while SourceUtil assumes that imports are "hoistable" to the top of the sketch so imply that the language does not permit it. The Sam Pottinger branch of Processing (specifically sampottinger/processing#15) makes SourceUtil's assumption explicity by dissallowing import statements within class bodies. This PR updates romanesco in response.
sampottinger and others added 9 commits August 23, 2019 11:47
Bump memory for Processing and default sketch memory.
Minor fix in error line attribution for ANTLR.
Implemented check for imports in class bodies without localization, providing a more informative error message when imports end up in unexpected places.
Moved from Java 8 to Java 11 for Compiler and JDT. Also added gramatical support for local variable type inference (var).
Opened access to Java 11 language features.
@jeremydouglass
Copy link

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 sampottinger changed the title Move to ANTLR 4 with Java 8 language features and localization of more syntax errors. Move to ANTLR 4 with Java 11 language features and localization of more syntax errors. Aug 29, 2019
@sampottinger
Copy link
Owner Author

sampottinger commented Aug 29, 2019

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
Copy link
Owner Author

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

@jeremydouglass
Copy link

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
Copy link
Owner Author

sampottinger commented Aug 29, 2019

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 Update ANTLR grammar to support Java 1.8 syntax processing/processing#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 Keyword var in for loop not working #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
Copy link

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
Copy link
Owner Author

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

@sampottinger
Copy link
Owner Author

sampottinger commented Aug 30, 2019

(Sorry ... less for you @jeremydouglass and more for googlers from the future...) Just to clarify for progeny, it was decided in processing#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 processing#5753 for further details.

@sampottinger
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants