search

google / error-prone

Catch common Java mistakes as compile-time errors
https://errorprone.info
Apache License 2.0
6.86k stars 744 forks source link

only expecting one cleanup method per class #2222

Open itzcoatl90 opened 3 years ago

itzcoatl90 commented 3 years ago

ATTENTION! Please read and follow:

  • if this is a question about Error Prone, send it to error-prone-discuss@googlegroups.com
  • if this is a bug or feature request, fill the form below as best as you can.

Description of the problem / feature request:

I was instructed to report this and post this information 

error-prone version: 2.4.0
     BugPattern: UnbalancedListener
     Stack Trace:
     java.lang.RuntimeException: only expecting one cleanup method per class
        at com.uber.errorprone.checker.leaks.UnbalancedListener$CallScanner.visitMethod(UnbalancedListener.java:270)
        at com.uber.errorprone.checker.leaks.UnbalancedListener$CallScanner.visitMethod(UnbalancedListener.java:173)
        at com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:898)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        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.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
        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.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
        at com.uber.errorprone.checker.leaks.UnbalancedListener.matchClass(UnbalancedListener.java:141)
        at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451)
        at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:549)
        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: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: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: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.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)
cushon commented 3 years ago 
java.lang.RuntimeException: only expecting one cleanup method per class
at com.uber.errorprone.checker.leaks.UnbalancedListener$CallScanner.visitMethod(UnbalancedListener.java:270)

This crash is likely a logic error in that check, which isn't one of the built-in ones.

Can you route this bug to whoever wrote the check you're using?

lazaroclapp commented 3 years ago

@itzcoatl90 If this UnbalancedListener in Android monorepo at Uber, please reach out to me internally (lazaro@) or to the Programming Systems Group (there is a public support Slack channel). This is one of ours and we should take a look at it internally first 😄

@cushon I remember the answer being "no" years ago, but... is there a way to override the url/message shown on a checker crash to point people first to our internal channels? (Without forking) It would avoid this kind of mistaken report. Even when we run into issues with first-party Google checkers, it at least allows us to take a first look an upstream an issue with extra context.

cushon commented 3 years ago

I remember the answer being "no" years ago, but... is there a way to override the url/message shown on a checker

There still isn't configuration for that, but I'm not opposed to adding one.

Another option here is to just remove the crash report URL. We customize it to point to our internal issue tracker, for OSS maybe having com.google.errorprone in the stack trace is good enough, especially since we end up catching crashes that don't original in core Error Prone (e.g. custom checks, or javac itself).

lazaroclapp commented 3 years ago

There still isn't configuration for that, but I'm not opposed to adding one.

We'd use it if it were available. If you want me to try to make a PR for it, let me know. I might not be able to get to it this week, but would definitely love having this as a configuration option.

Another option here is to just remove the crash report URL. We customize it to point to our internal issue tracker, for OSS maybe having com.google.errorprone in the stack trace is good enough, especially since we end up catching crashes that don't original in core Error Prone (e.g. custom checks, or javac itself)

You guys know better than I how Error Prone is mostly used. My assumption is that the long tail of "small" deployments benefit from the default crash report URL, and we would definitely love to have a custom message instead (over "no message" even, since even seeing com.uber.* there won't exactly tell a new hire which team to contact 🙂 )