-
Notifications
You must be signed in to change notification settings - Fork 745
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
Fix to FallThrough to skip generated/mutated AST nodes (e.g. Lombok) #1573
Fix to FallThrough to skip generated/mutated AST nodes (e.g. Lombok) #1573
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
(e.g. Lombok) Fixes #1573 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=311375182
(e.g. Lombok) Fixes #1573 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=311375182
While building orca with upgraded spring boot version 2.3.12, google error-prone package throwing IndexOutOfBoundException as given below: orca/orca-api/src/main/java/com/netflix/spinnaker/orca/api/pipeline/TaskResult.java:32: error: An unhandled exception was thrown by the Error Prone static analysis plugin. @builder ^ Please report this at https://github.com/google/error-prone/issues/new and include the following: error-prone version: 2.4.0 BugPattern: FallThrough Stack Trace: java.lang.IndexOutOfBoundsException at java.base/java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:633) at java.base/java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:41) at com.google.errorprone.bugpatterns.FallThrough.matchSwitch(FallThrough.java:70) at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451) at com.google.errorprone.scanner.ErrorProneScanner.visitSwitch(ErrorProneScanner.java:825) at com.google.errorprone.scanner.ErrorProneScanner.visitSwitch(ErrorProneScanner.java:152) at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCSwitch.accept(JCTree.java:1229) at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82) The error is due to Lombok lib upgrade from 1.18.16 to 1.18.20 as transitive dependency of Spring boot. Similar to the issue mentioned below google/error-prone#2575 Further investigation reveal that bug pattern "FallThrough" for lombok annotations (like @Singular) could be a false positive, because Lombok modifies the AST on the fly causing trouble to errorprone checks, as mentioned in below issue and PR. google/error-prone#613 google/error-prone#1573 Considering the above points, we can suppress warning of "FallThrough" bug pattern reported by errorprone for @Builder/@Singular annotations.
…#4194) * Remove @Singular annotation While building orca with upgraded spring boot version 2.3.12, google error-prone package throwing IndexOutOfBoundException as given below: orca/orca-api/src/main/java/com/netflix/spinnaker/orca/api/pipeline/TaskResult.java:32: error: An unhandled exception was thrown by the Error Prone static analysis plugin. @builder ^ Please report this at https://github.com/google/error-prone/issues/new and include the following: error-prone version: 2.4.0 BugPattern: FallThrough Stack Trace: java.lang.IndexOutOfBoundsException at java.base/java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:633) at java.base/java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:41) at com.google.errorprone.bugpatterns.FallThrough.matchSwitch(FallThrough.java:70) at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451) at com.google.errorprone.scanner.ErrorProneScanner.visitSwitch(ErrorProneScanner.java:825) at com.google.errorprone.scanner.ErrorProneScanner.visitSwitch(ErrorProneScanner.java:152) at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCSwitch.accept(JCTree.java:1229) at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82) The error is due to Lombok lib upgrade from 1.18.16 to 1.18.20 as transitive dependency of Spring boot. Similar to the issue mentioned below google/error-prone#2575 Fix: To remove @Singular annotation from classes using Lombok builder pattern. Reference: projectlombok/lombok#2221 * Revert "Remove @Singular annotation" This reverts commit e0d57a2. * Suppress warning for "FallThrough" bug pattern reported by errorprone While building orca with upgraded spring boot version 2.3.12, google error-prone package throwing IndexOutOfBoundException as given below: orca/orca-api/src/main/java/com/netflix/spinnaker/orca/api/pipeline/TaskResult.java:32: error: An unhandled exception was thrown by the Error Prone static analysis plugin. @builder ^ Please report this at https://github.com/google/error-prone/issues/new and include the following: error-prone version: 2.4.0 BugPattern: FallThrough Stack Trace: java.lang.IndexOutOfBoundsException at java.base/java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:633) at java.base/java.nio.HeapCharBuffer.subSequence(HeapCharBuffer.java:41) at com.google.errorprone.bugpatterns.FallThrough.matchSwitch(FallThrough.java:70) at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451) at com.google.errorprone.scanner.ErrorProneScanner.visitSwitch(ErrorProneScanner.java:825) at com.google.errorprone.scanner.ErrorProneScanner.visitSwitch(ErrorProneScanner.java:152) at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCSwitch.accept(JCTree.java:1229) at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82) The error is due to Lombok lib upgrade from 1.18.16 to 1.18.20 as transitive dependency of Spring boot. Similar to the issue mentioned below google/error-prone#2575 Further investigation reveal that bug pattern "FallThrough" for lombok annotations (like @Singular) could be a false positive, because Lombok modifies the AST on the fly causing trouble to errorprone checks, as mentioned in below issue and PR. google/error-prone#613 google/error-prone#1573 Considering the above points, we can suppress warning of "FallThrough" bug pattern reported by errorprone for @Builder/@Singular annotations.
There is a general issue with Lombok in that, by default, it modifies the AST on the fly.
In particular, this causes trouble for EP checks which query
state. getSourceCode()
andtry to match it to
Tree
objects they see. This is a hard problem to fix in general, butthis PR fixes a particular instance of this involving the
FallThrough
checker and@lombok.Builder
on@lombok.Singular
map fields.In particular, we skip the check whenever we see that the end position of a case statement's
Tree
seems to be less than 0 (e.g. not in the source).I tried to construct a repro/test case for this, but it's not easy to include within
core/src/test
as it would require the code to be built using the Lombok annotation processor in addition to
the EP test compiler.
For reference, the code showing the issue looks something like this:
Where Lombok constructs the initializer for
m
as something including a new switchstatement at the AST level (without matching source).