Skip to content

[java] Better handle explicit receiver parameters#1017

Merged
jsotuyod merged 4 commits into
pmd:masterfrom
jsotuyod:issue-719
Apr 4, 2018
Merged

[java] Better handle explicit receiver parameters#1017
jsotuyod merged 4 commits into
pmd:masterfrom
jsotuyod:issue-719

Conversation

@jsotuyod

@jsotuyod jsotuyod commented Apr 3, 2018

Copy link
Copy Markdown
Member

jsotuyod added 3 commits April 3, 2018 03:42
 - Allow sto match all method calls, meaning UnusedPrivateMethod will
not longer produce false positives
 - Resolves pmd#719
@jsotuyod jsotuyod added this to the 6.3.0 milestone Apr 3, 2018
public int getParameterCount() {
return jjtGetNumChildren();
return jjtGetNumChildren() > 0 && getFirstChildOfType(ASTFormalParameter.class).isExplicitReceiverParameter()
? jjtGetNumChildren() - 1 : jjtGetNumChildren();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@adangel @oowekyala I know this is very opinionated, so I'd like to know your opinion.

My position is, that when asking about a parameter count you are mostly interested in the number of parameters you need to pass to the method / constructor. You don't really care about explicit receiver parameters, which are by definition, only for internal use of the method itself. We may add a new method if the need arises to get the "explicit parameter count", but so far I see no use for it.

I do wonder however, if the AST shouldn't be completely changed and have the explicit receiver param be a different kind of node than formal parameters... the JSL does define it separately after all. Under that scenario, the outward behavior of this method would remain as in my implementation, so editing the grammar to split it may be done in 7.0.0 while this implementation can be used in the meantime.

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.

@jsotuyod Yes, it makes sense to ignore explicit receiver parameters for the ParameterCount.
The code looks quite complex for determining the count - I've just created #1019 to track this (and probably other grammar changes) for PMD 7.0.0. With the grammar change in place, it should be something like this: findChildrenOfType(ASTFormalParameter.class).size();

@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.

Looks good!

public int getParameterCount() {
return jjtGetNumChildren();
return jjtGetNumChildren() > 0 && getFirstChildOfType(ASTFormalParameter.class).isExplicitReceiverParameter()
? jjtGetNumChildren() - 1 : jjtGetNumChildren();

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.

@jsotuyod Yes, it makes sense to ignore explicit receiver parameters for the ParameterCount.
The code looks quite complex for determining the count - I've just created #1019 to track this (and probably other grammar changes) for PMD 7.0.0. With the grammar change in place, it should be something like this: findChildrenOfType(ASTFormalParameter.class).size();

@jsotuyod

jsotuyod commented Apr 3, 2018

Copy link
Copy Markdown
Member Author

@adangel thanks! I'll rework it before merging

@jsotuyod jsotuyod merged commit c9621b8 into pmd:master Apr 4, 2018
@jsotuyod jsotuyod deleted the issue-719 branch April 4, 2018 00:33
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