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

[2.3.4] NPE in TryFailRefactoring #1447

Closed
schlosna opened this issue Dec 9, 2019 · 5 comments
Closed

[2.3.4] NPE in TryFailRefactoring #1447

schlosna opened this issue Dec 9, 2019 · 5 comments

Comments

@schlosna
Copy link
Contributor

schlosna commented Dec 9, 2019

Description of the problem:

After upgrading to error-prone 2.3.4, I'm encountering a few issues that may be error-prone bugs according to their failure stacktraces. In this specific case there is a NullPointerException while running the TryFailRefactoring check.

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

I'm seeing the NullPointerException below via the TryFailRefactoring check during compilation. Example PR palantir/tritium#537 with build output at https://circleci.com/gh/palantir/tritium/5745 showing the following stacktrace. This appears to point at https://github.com/palantir/tritium/blob/ds/error-prone-javadoc/tritium-tracing/src/main/java/com/palantir/tritium/tracing/RemotingCompatibleTracingInvocationEventHandler.java#L48 with the following try/catch:

    private static final Supplier<Boolean> requiresFallback = Suppliers.memoize(() -> {
        try {
            return isUsingMultipleTracers(loadRemoting3TracersClass());
        } catch (ReflectiveOperationException e) {
            // expected case when remoting3 is not on classpath
            return false;
        }
    });
/home/circleci/project/tritium-tracing/src/main/java/com/palantir/tritium/tracing/RemotingCompatibleTracingInvocationEventHandler.java:48: An unhandled exception was thrown by the Error Prone static analysis plugin.
ERROR: An unhandled exception was thrown by the Error Prone static analysis plugin.
        try {
        ^
     Please report this at https://github.com/google/error-prone/issues/new and include the following:
  
     error-prone version: 2.3.4
     BugPattern: TryFailRefactoring
     Stack Trace:
     java.lang.NullPointerException
  	at com.google.errorprone.matchers.Matchers.lambda$methodNameStartsWith$f5bdc7b2$1(Matchers.java:825)
  	at com.google.errorprone.matchers.Matchers.lambda$allOf$93cd24cb$1(Matchers.java:127)
  	at com.google.errorprone.matchers.Matchers.lambda$anyOf$5390886b$1(Matchers.java:143)
  	at com.google.errorprone.bugpatterns.TryFailRefactoring.matchTry(TryFailRefactoring.java:62)
  	at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitTry(ErrorProneScanner.java:846)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitTry(ErrorProneScanner.java:152)
  	at com.sun.tools.javac.tree.JCTree$JCTry.accept(JCTree.java:1322)
  	at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:72)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:46)
  	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:522)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:152)
  	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:72)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:46)
  	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:704)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitLambdaExpression(ErrorProneScanner.java:152)
  	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:72)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:46)
  	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:753)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitMethodInvocation(ErrorProneScanner.java:152)
  	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:72)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:46)
  	at com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
  	at com.sun.source.util.TreeScanner.visitVariable(TreeScanner.java:223)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitVariable(ErrorProneScanner.java:887)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitVariable(ErrorProneScanner.java:152)
  	at com.sun.tools.javac.tree.JCTree$JCVariableDecl.accept(JCTree.java:968)
  	at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:72)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:46)
  	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:550)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:152)
  	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:72)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:46)
  	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:562)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:152)
  	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:56)
  	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 org.gradle.api.internal.tasks.compile.AnnotationProcessingCompileTask.call(AnnotationProcessingCompileTask.java:93)
  	at org.gradle.api.internal.tasks.compile.ResourceCleaningCompilationTask.call(ResourceCleaningCompilationTask.java:57)
  	at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:54)
  	at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:39)
  	at org.gradle.api.internal.tasks.compile.daemon.AbstractDaemonCompiler$CompilerWorkAction.execute(AbstractDaemonCompiler.java:113)
  	at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:47)
  	at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:46)
  	at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:36)
  	at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:98)
  	at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:36)
  	at org.gradle.workers.internal.FlatClassLoaderWorker.execute(FlatClassLoaderWorker.java:31)
  	at org.gradle.workers.internal.WorkerDaemonServer.execute(WorkerDaemonServer.java:56)
  	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
  	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.lang.reflect.Method.invoke(Method.java:498)
  	at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:118)
  	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
  	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.lang.reflect.Method.invoke(Method.java:498)
  	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
  	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
  	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
  	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
  	at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:412)
  	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
  	at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
  	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
  	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
  	at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56)
  	at java.lang.Thread.run(Thread.java:748)
File: tritium-tracing/src/main/java/com/palantir/tritium/tracing/RemotingCompatibleTracingInvocationEventHandler.java
Line: 48

What version of Error Prone are you using?

2.3.4

@schlosna
Copy link
Contributor Author

schlosna commented Dec 9, 2019

Seems possibly similar to #1155 . I'll see if I can create a standalone reproducer.

schlosna added a commit to palantir/tritium that referenced this issue Dec 10, 2019
@michaelhixson
Copy link
Contributor

Here's a class that reproduces the NPE for me.

package example;

class TryFailRefactoringNpe {
  static {
    try {
      Integer.parseInt("hello");
    } catch (NumberFormatException e) {
      e.printStackTrace();
    }
  }
}

That causes a problem for the first line of code in TryFailRefactoring.matchTry.

if (!JUnitMatchers.TEST_CASE.matches(state.findEnclosing(MethodTree.class), state)) {
return NO_MATCH;
}

because state.findEnclosing(MethodTree.class) is null.

Adding a null check to that block of code in my local copy of Error Prone made the NPE disappear.

MethodTree methodTree = state.findEnclosing(MethodTree.class);
if (methodTree == null || !JUnitMatchers.TEST_CASE.matches(methodTree, state)) {
  return NO_MATCH;
}

To understand why state.findEnclosing(MethodTree.class) is null in this case, I tried walking up the tree to see what the enclosing elements are.

MethodTree methodTree = state.findEnclosing(MethodTree.class);
if (methodTree == null) {
  System.out.println("NULL METHOD TREE!");
  TreePath path = state.getPath();
  while (path != null) {
    Tree leaf = path.getLeaf();
    System.out.println();
    System.out.println("----------------------------");
    System.out.println(leaf.getClass().getName());
    System.out.println("----------------------------");
    System.out.println(leaf);
    path = path.getParentPath();
  }
}

It thinks the enclosing tree elements are:

  • com.sun.tools.javac.tree.JCTree$JCTry (TryTree)
  • com.sun.tools.javac.tree.JCTree$JCBlock (BlockTree)
  • com.sun.tools.javac.tree.JCTree$JCClassDecl (ClassTree)
  • com.sun.tools.javac.tree.JCTree$JCCompilationUnit (CompilationUnitTree).

No MethodTree to be found. I guess a static initializer is considered to be a BlockTree rather than a MethodTree.

@graememorgan
Copy link
Member

Thanks for the report. I've sent a fix internally that should be out in a future release.

netdpb pushed a commit that referenced this issue Jan 6, 2020
Fixes external #1447

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=287975763
netdpb pushed a commit that referenced this issue Jan 6, 2020
Fixes external #1447

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=287975763
zregvart added a commit to syndesisio/syndesis that referenced this issue Jan 28, 2020
Most issues fixed were the UnnecessaryLambda[1], those, mostly constant
fields were converted to, mostly, static methods.

There was a need to convert some of the static initialization blocks to
methods as an issue in errorprone was triggered[2]. Refactoring those
static blocks to static methods in my opinion makes them more readable
and it allows, if one would have impetus to do so, to write tests
against them.

[1] https://errorprone.info/bugpattern/UnnecessaryLambda
[2] google/error-prone#1447
kmclarnon pushed a commit to HubSpot/error-prone that referenced this issue Feb 19, 2020
Fixes external google#1447

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=287975763
shawkins pushed a commit to shawkins/syndesis that referenced this issue Apr 16, 2020
Most issues fixed were the UnnecessaryLambda[1], those, mostly constant
fields were converted to, mostly, static methods.

There was a need to convert some of the static initialization blocks to
methods as an issue in errorprone was triggered[2]. Refactoring those
static blocks to static methods in my opinion makes them more readable
and it allows, if one would have impetus to do so, to write tests
against them.

[1] https://errorprone.info/bugpattern/UnnecessaryLambda
[2] google/error-prone#1447
@cushon
Copy link
Collaborator

cushon commented May 20, 2020

7040b51

@cushon cushon closed this as completed May 20, 2020
@schlosna
Copy link
Contributor Author

Thanks!

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

No branches or pull requests

4 participants