Skip to content

[java] Rename visitor methods to reduce overloading#2706

Merged
oowekyala merged 12 commits into
pmd:pmd/7.0.xfrom
oowekyala:generic-visitor3
Aug 23, 2020
Merged

[java] Rename visitor methods to reduce overloading#2706
oowekyala merged 12 commits into
pmd:pmd/7.0.xfrom
oowekyala:generic-visitor3

Conversation

@oowekyala

@oowekyala oowekyala commented Aug 7, 2020

Copy link
Copy Markdown
Member

Describe the PR

Follow the conventions of #2661 for the Java and JSP modules. This is important as this changes the wrapper script, so all javacc modules depend on this PR for #880. It's also important that it lands in the 7.0.x branch so that java-grammar can be updated early, before we touch rules

Since this updates the wrapper, parts of other Javacc modules needed to be updated too.

I'll also rename acceptVisitor but not in a PR because the diff is uninteresting.

TODO on master

  • Deprecate ASTJspDocument, ASTJspDeclarations -> those nodes don't exist, they're not in the grammar. Probably leftovers from a refactoring. See 2673fee
  • Deprecate ScalaParserVisitorAdapter::zero and ::combine -> those are removed from AstVisitorBase for reasons explained in 1171afb

--> #2720

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by travis)
  • Added (in-code) documentation (if needed)

This way you don't need to upcast a node to eg JavaNode
in order to call the next method to delegate:

```java
visit((JavaNode) node, data) -> visitJavaNode(node, data)
```

This prevents writing `visit(node, data)` and not being
sure what overload is called.
This is just not so useful, most visitors use
side effects on the parameter, or, return the
parameter, which is only possible if `P = R`,
and so only known in the specific subclass.
@oowekyala oowekyala added this to the 7.0.0 milestone Aug 7, 2020
Idk why they were there, but the grammar
does not produce them anywhere. Compilation
was failing because they expected a visit
method on the visitor, which is not generated
@oowekyala oowekyala mentioned this pull request Aug 11, 2020
10 tasks
@ghost

ghost commented Aug 11, 2020

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

Generated by 🚫 Danger

@oowekyala oowekyala marked this pull request as ready for review August 11, 2020 12:54

@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 created a PR for the deprecations on master: #2720

@oowekyala oowekyala self-assigned this Aug 23, 2020
@oowekyala oowekyala merged commit f203d98 into pmd:pmd/7.0.x Aug 23, 2020
@oowekyala oowekyala deleted the generic-visitor3 branch August 23, 2020 14:54
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants