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.

Sign up for GitHub

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

Jump to bottom

Limitation of InlineMeInliner with lambda expressions #2347

Open
lazaroclapp opened this issue May 19, 2021 · 3 comments
Open

Limitation of InlineMeInliner with lambda expressions #2347

lazaroclapp opened this issue May 19, 2021 · 3 comments
Assignees
@nick-someone
Labels
bug

Comments

@lazaroclapp
Copy link
Contributor

lazaroclapp commented May 19, 2021 •
edited
Loading

Description of the problem / feature request:

I am seeing the following crash on InlineMeInliner for Error Prone 2.7.1.

This is on code edited after simply following the suggestions of InlineMeSuggester:

     error-prone version: 2.7.1
     BugPattern: InlineMeInliner
     Stack Trace:
     java.lang.ClassCastException: com.sun.tools.javac.code.Type$BottomType cannot be cast to com.sun.tools.javac.code.Type$ArrayType
        at com.sun.tools.javac.model.AnnotationProxyMaker$ValueVisitor.visitArray(AnnotationProxyMaker.java:186)
        at com.sun.tools.javac.code.Attribute$Array.accept(Attribute.java:327)
        at com.sun.tools.javac.model.AnnotationProxyMaker$ValueVisitor.getValue(AnnotationProxyMaker.java:167)
        at com.sun.tools.javac.model.AnnotationProxyMaker.generateValue(AnnotationProxyMaker.java:145)
        at com.sun.tools.javac.model.AnnotationProxyMaker.getAllReflectedValues(AnnotationProxyMaker.java:104)
        at com.sun.tools.javac.model.AnnotationProxyMaker.generateAnnotation(AnnotationProxyMaker.java:90)
        at com.sun.tools.javac.model.AnnotationProxyMaker.generateAnnotation(AnnotationProxyMaker.java:81)
        at com.sun.tools.javac.code.AnnoConstruct.getAnnotation(AnnoConstruct.java:185)
        at com.google.errorprone.bugpatterns.inlineme.Inliner.match(Inliner.java:148)
        at com.google.errorprone.bugpatterns.inlineme.Inliner.matchMethodInvocation(Inliner.java:126)
        at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:450)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:747)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCMethodInvocation.accept(JCTree.java:1644)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.visitMemberSelect(TreeScanner.java:680)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMemberSelect(ErrorProneScanner.java:729)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMemberSelect(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCFieldAccess.accept(JCTree.java:2112)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.visitMethodInvocation(TreeScanner.java:508)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:752)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCMethodInvocation.accept(JCTree.java:1644)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.visitExpressionStatement(TreeScanner.java:433)
        at com.google.errorprone.scanner.ErrorProneScanner.visitExpressionStatement(ErrorProneScanner.java:634)
        at com.google.errorprone.scanner.ErrorProneScanner.visitExpressionStatement(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCExpressionStatement.accept(JCTree.java:1454)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.visitBlock(TreeScanner.java:248)
        at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:521)
        at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1026)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.visitLambdaExpression(TreeScanner.java:559)
        at com.google.errorprone.scanner.ErrorProneScanner.visitLambdaExpression(ErrorProneScanner.java:703)
        at com.google.errorprone.scanner.ErrorProneScanner.visitLambdaExpression(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCLambda.accept(JCTree.java:1805)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
        at com.sun.source.util.TreeScanner.visitMethodInvocation(TreeScanner.java:509)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:752)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCMethodInvocation.accept(JCTree.java:1644)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.visitReturn(TreeScanner.java:469)
        at com.google.errorprone.scanner.ErrorProneScanner.visitReturn(ErrorProneScanner.java:818)
        at com.google.errorprone.scanner.ErrorProneScanner.visitReturn(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCReturn.accept(JCTree.java:1548)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.visitBlock(TreeScanner.java:248)
        at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:521)
        at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1026)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.visitMethod(TreeScanner.java:206)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:741)
        at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:898)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
        at com.sun.source.util.TreeScanner.visitClass(TreeScanner.java:187)
        at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:549)
        at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
        at com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
        at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
        at com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:144)
        at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:561)
        at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:151)
        at com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:591)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
        at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
        at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
        at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:152)
        at com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:120)
        at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1404)
        at com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1353)
        at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:946)
        at com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:100)
        at com.sun.tools.javac.api.JavacTaskImpl.handleExceptions(JavacTaskImpl.java:142)
        at com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:96)
        at com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:90)
        at com.facebook.buck.jvm.java.plugin.adapter.JavacTaskWrapper.call(JavacTaskWrapper.java:100)
        at com.facebook.buck.jvm.java.plugin.adapter.JavacTaskWrapper.call(JavacTaskWrapper.java:100)
        at com.facebook.buck.jvm.java.plugin.adapter.BuckJavacTask.call(BuckJavacTask.java:52)
        at com.facebook.buck.jvm.java.plugin.adapter.BuckJavacTaskProxyImpl.call(BuckJavacTaskProxyImpl.java:134)
        at com.facebook.buck.jvm.java.Jsr199JavacInvocation$CompilerWorker.lambda$startCompiler$2(Jsr199JavacInvocation.java:424)
        at com.google.common.util.concurrent.TrustedListenableFutureTask$TrustedFutureInterruptibleTask.runInterruptibly(TrustedListenableFutureTask.java:127)
        at com.google.common.util.concurrent.InterruptibleTask.run(InterruptibleTask.java:57)
        at com.google.common.util.concurrent.TrustedListenableFutureTask.run(TrustedListenableFutureTask.java:80)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

Code samples below (identifier names changed, but AST structure is similar).

This gets reported on the call-site for:

Client client = Client.client(() -> \"text\");",

Where that version of client(...) has been @Deprecated and marked with @InlineMe as follows:

  @InlineMe(replacement = "Client.client(supplier.text())", imports={"com.example.Client"})
  public static Client client(TextSupplier supplier) {
    return client(supplier.text());
  }
  public static Client client(String text) {
    return new Client(text);
  }

I can confirm, stepping through the build with a debugger, that when this line is hit when processing imports={"com.example.Client"}, a.type happens to be BottomType/<nulltype> (even though the value does show an array containing the string "com.example.Client"). We are using the EP javac on a JDK 8 JVM.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I tried to make a repro case as a test case for Error Prone here, but while there is indeed a problem in that the rewriting doesn't happen (and thus the test fails), it doesn't crash with the same exception. I'll be working on a stand alone project gradle that attempts to show the issue without our build system in the way and update this issue if that works.

What version of Error Prone are you using?

2.7.1

Have you found anything relevant by searching the web?

Nothing that quite matches, either as an Error Prone or a JDK bug.
@cushon
Copy link
Collaborator

cushon commented May 19, 2021

The unit test doesn't get refactored because the check uses compiledWithFix to filter suggested fixes, and the test doesn't compile after the refactoring is applied:

Caller.java:4: error: cannot find symbol
    Client client = Client.client(() -> "text".text());
                                              ^

I think this might be a known limitation of the handling of lambdas.

The original issue might be separate. Was the latest version of the error-prone-annotations artifact on the classpath for that compilation?

@kluever kluever added the bug label May 20, 2021
copybara-service bot pushed a commit that referenced this issue May 20, 2021
@cushon
Don't create annotation proxies for @InlineMe
d6b003f 
#2347

PiperOrigin-RevId: 374794248
@copybara-service copybara-service bot mentioned this issue May 20, 2021
Merged
copybara-service bot pushed a commit that referenced this issue May 20, 2021
@cushon
Don't create annotation proxies for @InlineMe
dc7252b 
#2347

PiperOrigin-RevId: 374915809
@cushon
Copy link
Collaborator

cushon commented May 21, 2021

The crash should be fixed by dc7252b.

I'll keep this open to track #2347 (comment).

@cushon cushon changed the title ClassCastException (BottomType cannot be cast to ArrayType) on InlineMeInliner Limitation of InlineMeInliner with lambda expressions May 21, 2021
@lazaroclapp
Copy link
Contributor Author

Few updates:

  1. Agree that those are two distinct issues 🙂
  2. I tested the current master with the patch listed above, it does solve the problem within our original build! Thanks so much! 😃 (I am happy with this resolution and will enable this checker in our build when an EP release incorporating this fix is available)
  3. I wasn't entirely able to repro the exception with a simple standalone gradle project, so there might be something from our build that is part of the issue. From seeing the fix, I suspect maybe the fact that in the real build our Client class and the class using it are in two different build targets? (See below for some info on the classpath of both targets) Not sure, though.

Was the latest version of the error-prone-annotations artifact on the classpath for that compilation?

It's not in -classpath, but it is in -processorpath for the failing javac invocation. (It would be in -classpath for the compilation of the target that actually includes the annotated method)

Note that we fork error-prone-annotations because of #2122, but our artifact is identical to the 2.7.1 error-prone-annotations except for removing usages of the @IncompatibleModifiers meta-annotation. In particular, @InlineMe should be unchanged there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nick-someone nick-someone

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cushon @nick-someone @lazaroclapp @kluever