-
Notifications
You must be signed in to change notification settings - Fork 746
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
Avoid MemberName
IOOBE on lambda parameters inside overriding methods
#3976
Avoid MemberName
IOOBE on lambda parameters inside overriding methods
#3976
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR avoids the following error new in version 2.20.0:
error-prone version: 2.20.0
BugPattern: MemberName
Stack Trace:
java.lang.IndexOutOfBoundsException: -1
at jdk.compiler/com.sun.tools.javac.util.List.get(List.java:482)
at com.google.errorprone.bugpatterns.MemberName.matchVariable(MemberName.java:156)
at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:449)
at com.google.errorprone.scanner.ErrorProneScanner.visitVariable(ErrorProneScanner.java:884)
at com.google.errorprone.scanner.ErrorProneScanner.visitVariable(ErrorProneScanner.java:150)
if (symbol.owner instanceof MethodSymbol | ||
&& symbol.getKind() == ElementKind.PARAMETER | ||
&& ASTHelpers.findEnclosingNode(state.getPath(), Tree.class).getKind() | ||
!= Kind.LAMBDA_EXPRESSION) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I wrote a fix that tests for index < 0
below, but then malformed lambda parameter names aren't flagged. I updated the test accordingly. This does make me wonder: are there possibly other expression types that should be excluded? (Nothing comes to mind, but.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
IIUC symbol.owner
for the lambda parameter is the syntactically enclosing method (toString
in the test below), which is unfortunate. I can't think of any non-lambda cases that seem likely to be affected by this either.
I wondered about explicitly handling the case where the parameter symbol isn't in the owning method symbol's parameter list, vs. checking if the parent is a lambda, but either way is probably fine for now.
No description provided.