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

NPE crash on auto-annotator v1.3.6 in the presence of Lombok generated constructor #185

Open
lazaroclapp opened this issue Apr 21, 2023 · 5 comments
Assignees
Labels
bug Something isn't working low priority Low priority task

Comments

@lazaroclapp
Copy link
Collaborator

I am getting an auto-annotator error due to Lombok.

In code like:

  @AllArgsConstructor
  @EqualsAndHashCode
  public static class Foo {

    private final String a;
    private final String b;
    private final String c;

    public Foo(String a, String b) {
      this.a = a;
      this.b = b;
      this.c = null;
    }

    public Foo(String c) {
      this.a = null;
      this.b = null;
      this.c = c;
    }
  }

I get the following NPE crash:

Exception in thread "main" java.lang.NullPointerException
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.lambda$collectGraphResults$1(DownstreamImpactEvaluator.java:97)
        at java.base/java.lang.Iterable.forEach(Iterable.java:75)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.lambda$collectGraphResults$2(DownstreamImpactEvaluator.java:88)
        at edu.ucr.cs.riple.injector.location.OnMethod.ifMethod(OnMethod.java:96)
        at edu.ucr.cs.riple.core.metadata.index.Fix.ifOnMethod(Fix.java:99)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.lambda$collectGraphResults$3(DownstreamImpactEvaluator.java:70)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at com.google.common.collect.CollectSpliterators$FlatMapSpliterator.lambda$forEachRemaining$1(CollectSpliterators.java:377)
        at java.base/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1693)
        at com.google.common.collect.CollectSpliterators$FlatMapSpliterator.forEachRemaining(CollectSpliterators.java:373)
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.collectGraphResults(DownstreamImpactEvaluator.java:68)
        at edu.ucr.cs.riple.core.evaluators.AbstractEvaluator.evaluate(AbstractEvaluator.java:96)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactCacheImpl.analyzeDownstreamDependencies(DownstreamImpactCacheImpl.java:114)
        at edu.ucr.cs.riple.core.Annotator.annotate(Annotator.java:134)
        at edu.ucr.cs.riple.core.Annotator.start(Annotator.java:84)
        at edu.ucr.cs.riple.core.Main.main(Main.java:46)

Which some debugging prints trace to the three argument constructor automatically injected by the lombok @AllArgsConstructor annotation (i.e. Foo(String, String, String) which isn't present in the source code, but which lombok generates). I thought we had handling in place for methods being missing from the source?

@nimakarimipour nimakarimipour added the bug Something isn't working label Apr 21, 2023
@nimakarimipour nimakarimipour self-assigned this Apr 21, 2023
@msridhar
Copy link
Member

msridhar commented May 12, 2023

To summarize what we figured out here, this issue only occurs when running AnnotatorScanner at warning level; at the error level the code gets indexed.

To be more robust against these types of issues, we should override the isSuppressed() method in AnnotatorScanner to return false. This will require depending on Error Prone 2.13.0, but we are ok with this requirement for the scanner.

@lazaroclapp
Copy link
Collaborator Author

As a note, the requirements for reproducing this is both the checker being at warning level and -XepDisableWarningsInGeneratedCode being passed to the Error Prone configuration. Avoiding either one is a reasonable workaround, but we should make the checker (and potentially NullAway as well) robust to this configuration.

@nimakarimipour
Copy link
Member

@msridhar Should we close this issue now ? It seems that running Scanner at Error level is a good workaround and we don't require a change here.

@msridhar
Copy link
Member

We should document that the scanner should always we run at error level. Still, even after that, it would be good to not crash if scanner is run at warning level. Maybe we can leave this open but mark it low priority?

@nimakarimipour nimakarimipour added the low priority Low priority task label Dec 28, 2023
@nimakarimipour
Copy link
Member

Yes, sure. Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working low priority Low priority task
Projects
None yet
Development

No branches or pull requests

3 participants