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

[java] AST inconsistencies around FormalParameter #998

Closed
oowekyala opened this issue Mar 23, 2018 · 1 comment
Closed

[java] AST inconsistencies around FormalParameter #998

oowekyala opened this issue Mar 23, 2018 · 1 comment
Labels
in:ast About the AST structure or API, the parsing step
Projects
Milestone

Comments

@oowekyala
Copy link
Member

oowekyala commented Mar 23, 2018

The following are breaking API changes, that I'd like to see done sometime

FormalParameter shouldn't be an access node

FormalParameter currently implements AccessNode, even though this is only because a parameter can be final...

The problem is also real for LocalVariableDeclaration

CatchStatement shouldn't use FormalParameter as a child

Because of that, FormalParameter's grammar implements the multicatch syntax, which doesn't really make sense and makes it difficult to enrich the API of the node in a sensible way.

  • For example, methods that are only relevant to the formal parameter usage make no sense if the node is a catch statement (e.g. isVarargs), and conversely (isMultiCatch).
  • For methods that can be used in both cases, the documentation is very complicated to write
  • Most importantly, type resolution is inconsistent between the two cases

We should probably follow the JLS more closely for Catch formal parameters.

Resource shouldn't extend FormalParameter

It just doesn't make sense at all. ASTResource inherits isVarargs, isMultiCatch, all methods of AccessNode, etc...

@oowekyala
Copy link
Member Author

The last two points are fixed on the java-grammar branch, the first point is a subset of #1307, so I'll close this

@oowekyala oowekyala added this to Done in PMD 7 Mar 11, 2022
@adangel adangel mentioned this issue 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
No open projects
PMD 7
  
Done
Development

No branches or pull requests

1 participant