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

🐛 improve accuracy of MethodReference matches #97

Merged
merged 3 commits into from
Jul 3, 2024

Conversation

pranavgaikwad
Copy link
Contributor

@pranavgaikwad pranavgaikwad commented Jun 25, 2024

This PR improves accuracy of some searches:

  • METHOD_CALL / MethodReference: we try to match fqn of the method.
  • CONSTRUCTOR_CALL / MethodReference: same as above

if (declaringClass != null) {
String fullyQualifiedName = declaringClass.getQualifiedName() + "." + binding.getName();
// match fqn with query pattern
if (fullyQualifiedName.matches(getCleanedQuery(query))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what does getCleanedQuery do?

Copy link
Contributor Author

@pranavgaikwad pranavgaikwad Jun 26, 2024

Choose a reason for hiding this comment

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

@shawn-hurley updated that part of the code in my recent commit...search queries like get* are not valid regexes when it comes to Java regexes. So when matching the fqn, we need to make sure get* converts to get.* so it java regex works as expected.

added a comment

        /*
         * When comparing query pattern with an actual java element's fqn
         * we need to make sure that * not preceded with a . are replaced
         * by .* so that java regex works as expected on them
        */

@jmle
Copy link
Collaborator

jmle commented Jun 27, 2024

Looks good 👍

}
// sometimes binding or declaring class cannot be found, usually due to errors
// in source code. in that case, we will fallback and accept the match
this.symbolMatches = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will lead to false positives, but I would rather that than miss real issues (which would happen if we did the opposite). I think this is the right approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shawn-hurley yes, this was what was causing errors in our test suite where some projects are outright bad / un-buildable etc. that was about 20% of total test cases that used METHOD_CALL. I think we can live with this tradeoff

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.

4 participants