google / error-prone

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

ErrorProne crashed: MixedMutabilityReturnType #1234

Closed shevek closed 4 years ago

shevek commented 5 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:

ErrorProne crashed

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

We don't have anything self-contained, we just can't build our code.

What version of Error Prone are you using?

     error-prone version: 2.3.3
     BugPattern: MixedMutabilityReturnType
     Stack Trace:
     java.lang.IllegalArgumentException: Replacement{range=[2200..2253), replaceWith=ImmutableSet.copyOf(new ArrayUnenforcedSet<>(Collections.singleton(edge)))} conflicts with existing replacement Replacement{range=[2200..2253), replaceWith=ImmutableList.copyOf(new ArrayUnenforcedSet<>(Collections.singleton(edge)))}
        at com.google.errorprone.fixes.Replacements.add(Replacements.java:109)
        at com.google.errorprone.fixes.SuggestedFix.getReplacements(SuggestedFix.java:90)
        at com.google.errorprone.fixes.AppliedFix$Applier.apply(AppliedFix.java:71)
        at com.google.errorprone.JavacErrorDescriptionListener.lambda$new$0(JavacErrorDescriptionListener.java:71)
        at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
        at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175)
        at java.util.Collections$2.tryAdvance(Collections.java:4717)
        at java.util.Collections$2.forEachRemaining(Collections.java:4725)
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
        at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
        at com.google.errorprone.JavacErrorDescriptionListener.onDescribed(JavacErrorDescriptionListener.java:84)
        at com.google.errorprone.ErrorProneAnalyzer.lambda$finished$1(ErrorProneAnalyzer.java:136)
        at com.google.errorprone.VisitorState.reportMatch(VisitorState.java:282)
        at com.google.errorprone.bugpatterns.MixedMutabilityReturnType$ReturnTypesScanner.visitMethod(MixedMutabilityReturnType.java:217)
        at com.google.errorprone.bugpatterns.MixedMutabilityReturnType$ReturnTypesScanner.visitMethod(MixedMutabilityReturnType.java:190)
        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.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.bugpatterns.MixedMutabilityReturnType$ReturnTypesScanner.visitClass(MixedMutabilityReturnType.java:205)
        at com.google.errorprone.bugpatterns.MixedMutabilityReturnType$ReturnTypesScanner.visitClass(MixedMutabilityReturnType.java:190)
        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.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.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:591)
        at com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
        at com.google.errorprone.bugpatterns.MixedMutabilityReturnType.matchCompilationUnit(MixedMutabilityReturnType.java:157)
        at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:433)
        at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:541)
eaftan commented 5 years ago

@graememorgan have you seen anything like this?

shevek commented 5 years ago

Additional annoyance was that `@SuppressWarnings("MixedMutabilityReturnType") did not work around the bug, and we actually had to downgrade errorprone. If we suppress a warning, can we also suppress the checker, and associated costs and bugs?

It's not a real day's coding unless I get an unexpected internal error out of some compiler or other.

graememorgan commented 5 years ago

I haven't seen anything like this, and I would have expected to while developing it.

Do you have a repro of any sort?

shevek commented 5 years ago

I'm sorry, we're on a proprietary codebase and I have no insight whatsoever into how to construct something for this. I can give you the fragment in question, which uses jgrapht APIs:

    public Set<E> getEdges(V sourceVertex, V targetVertex) {
        // ...
        if (edge == null)
            return Collections.emptySet();
        else
            return new ArrayUnenforcedSet<>(Collections.singleton(edge));
    }
cushon commented 4 years ago

Closing since I don't think we can do much more than https://github.com/google/error-prone/commit/97767bbe295dfe3bd5a346ba0b086f043a950259 without a repro.

orischwartz commented 3 years ago

I got the same error on the latest release.

     error-prone version: 2.5.1
     BugPattern: MixedMutabilityReturnType
     Stack Trace:
     java.lang.IllegalArgumentException: Replacement{range=[439..445), replaceWith=ImmutableMap.copyOf(result)} conflicts with existing replacement Replacement{range=[439..445), replaceWith=ImmutableBiMap.copyOf(result)}
        at com.google.errorprone.fixes.Replacements.add(Replacements.java:109)
        at com.google.errorprone.fixes.SuggestedFix.getReplacements(SuggestedFix.java:92)
        at com.google.errorprone.fixes.AppliedFix$Applier.apply(AppliedFix.java:71)
        at com.google.errorprone.JavacErrorDescriptionListener.lambda$new$1(JavacErrorDescriptionListener.java:86)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
        at java.base/java.util.Collections$2.tryAdvance(Collections.java:4747)
        at java.base/java.util.Collections$2.forEachRemaining(Collections.java:4755)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)

Minimally reproducible with

    private BiMap<String, String> triggerMixedMutabilityReturnTypeCrash(Collection<String> input) {

        if (input.isEmpty()) {
            return ImmutableBiMap.of();
        }

        HashBiMap<String, String> result = HashBiMap.create();
        return result;
    }