Skip to content

[core] Merge JavaCC build scripts#2211

Merged
oowekyala merged 30 commits into
pmd:pmd/7.0.xfrom
oowekyala:master-ant-script
Jan 28, 2020
Merged

[core] Merge JavaCC build scripts#2211
oowekyala merged 30 commits into
pmd:pmd/7.0.xfrom
oowekyala:master-ant-script

Conversation

@oowekyala

@oowekyala oowekyala commented Jan 11, 2020

Copy link
Copy Markdown
Member

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:

  • Use the ParseException of pmd-core instead of duplicating a class. Error messages of that one are slightly better.
  • No JJT${langname}ParserState is 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.
  • The constant interface for token ids is turned into a final utility class. Eg JavaParserConstants become JavaTokenKinds. Lexical state IDs are not made public.
  • The constant interface for node ids is made package-private. Ids have a single purpose (implementing getXpathNodeName) and this is better-off hidden.
  • Use JavaccToken instead of generating a new Token class
  • Tokens are created by a TokenDocument. This is useful to avoid writing too much logic inline in lexical actions. Eg JavaTokenFactory is now JavaTokenDocument. Later a token document could support edition methods, to implement autofixes
  • The generated visitor interface has default methods (#1444).
  • The build file deletes the node files that were found in the main source tree. This avoids hardcoding their names as is currently done in pmd-modelica or pmd-plsql

Conventions that the build file relies on:

  • The grammar file name is the same as the language name. Eg Java.jjt, but PldocAST.jjt will be renamed PLSQL.jjt
  • The generated parser is named ${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 jjtClose the 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

@oowekyala oowekyala added the in:ast About the AST structure or API, the parsing step label Jan 11, 2020
@oowekyala oowekyala added this to the 7.0.0 milestone Jan 11, 2020
@oowekyala oowekyala changed the title Master ant script [core] Merge JavaCC build scripts Jan 11, 2020
@ghost

ghost commented Jan 13, 2020

Copy link
Copy Markdown
1 Message
📖 No java rules are changed!

Generated by 🚫 Danger

Line separators in regex are replaced
with platform independent \R. Good thing
we have that automatic windows build
@adangel adangel self-assigned this Jan 17, 2020
@adangel adangel self-requested a review January 17, 2020 17:36

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😄

Comment thread pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/CharStream.java
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java Outdated
Comment thread pmd-cpp/src/main/java/net/sourceforge/pmd/lang/cpp/CppCharStream.java Outdated
@oowekyala oowekyala mentioned this pull request Jan 18, 2020
22 tasks
@oowekyala

Copy link
Copy Markdown
Member Author

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've opened #2239 to track the process

@oowekyala oowekyala merged commit d726990 into pmd:pmd/7.0.x Jan 28, 2020
@oowekyala oowekyala deleted the master-ant-script branch January 28, 2020 13:19
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
@adangel adangel mentioned this pull request Jan 23, 2023
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in:ast About the AST structure or API, the parsing step

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants