[core] Merge JavaCC build scripts#2211
Merged
Merged
Conversation
Generated by 🚫 Danger |
Line separators in regex are replaced with platform independent \R. Good thing we have that automatic windows build
adangel
reviewed
Jan 17, 2020
adangel
left a comment
Member
There was a problem hiding this comment.
I've seen a couple of TODOs, that need to be done, once all languages are migrated. Do you keep track of those or shall we create a issue?
I'd like to migrate at least one language (but not today though) to get to know the new way 😄
22 tasks
Member
Author
I've opened #2239 to track the process |
TODO revert when all languages are ported
86d5dc3 to
d726990
Compare
oowekyala
added a commit
to oowekyala/pmd
that referenced
this pull request
Jan 28, 2020
Introduce a ModifierList node, that ranges over the modifiers of a declaration (including annotations). This is a combination of a few old commits: Figure out modifiers Fix tests Remove AccessTypeNode Document Remove specific methods Fix symboltable test Fix tests Rename to JModifier Fix copypaste default/abstract Improve doc Test anon classes Remove useless impl Static modifier should not be present on toplevel classes Simplify impl Add visibility enum Port some tests Fix test ranges Fix modifier ordering Cleanup Fix unnecessary modifier rule Rename to use plural Improve visibility doc Simplify some things Checkstyle Remove some usages of typekind Fix missing import Remove irrelevant method REmove some duplication Replace AccessNode with ModifierOwner Rename to AccessNode to reduce diff Remove changes to rules Add convenience methods Make VariableDeclaratorId a ModifierOwner Fix variable name decl Make enum constant an implicit AccessNode Fix compil Checkstyle Cleanup Deprecate TypeKind Cleanup Remove TypeKind Revert "Remove TypeKind" This reverts commit 222c169c3401a01507726f339ae9f9b2b20dc69a. Fix doc Fix UnnecessaryModifierRule Use special node instead of ModifierSet Remove useless tests Fix tests WIP Work should be resumed when pmd#2211 is merged into java-grammar Fix some tests Doc
This was referenced Feb 14, 2020
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It would be good for all JavaCC languages to share most of their implementation, so that changes like #2166, #1446 or #2195 are propagated to all modules. To be able to change things across all languages with minimal trouble, merging their Ant build files is essential.
This PR creates a new ant build file and makes pmd-java & pmd-modelica use it as a PoC. Other modules will be ported one by one.
Changes that using the new build does to a module:
JJT${langname}ParserStateis generated. The one in pmd-core, named JjtreeBuilder, is used instead. This is an improved version that features the changes needed by java-grammar to support annotations, and the changes of [modelica] Normalize invalid node ranges #2195 about implicit tokens.JavaParserConstantsbecomeJavaTokenKinds. Lexical state IDs are not made public.Conventions that the build file relies on:
${lang-name}ParserImpl. This is something that has bugged me for a while: many modules have several public parser classes with the same name, eg JavaParser or PLSQLParser. Finding the file in an IDE is a pain and we must use FQCNs. I think the only published API should be the implementation of our Parser interface. So JavaLanguageParser is renamed JavaParser, moved into the ast package, and the generated parser is named JavaParserImpl and is package-private.Nodes don't use the constructors taking the parser anymore so I removed them. Since parsers change names, it would have made a big diff to update the constructors for no benefit. The parser was previously only used to fetch the current token in jjtClose. This is a "bug" of JJTree: it sets the last token after jjtClose. The new JjtreeBuilder sets it correctly before, so in
jjtClosethe node has all the information available.All of this is only possible if we somewhat stop relying on the JavaCC generated helper classes (eg char streams), and so some are moved to the main source tree of pmd-core. We share the implementation, and can modify them and document them independently of JavaCC.
This will be a series of PRs so I'll open an issue to track them. We should also take care of describing these conventions on the wiki & website