diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UngroupedOverloads.java b/core/src/main/java/com/google/errorprone/bugpatterns/UngroupedOverloads.java index e19ae2a4ea2..e43f8d36657 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/UngroupedOverloads.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/UngroupedOverloads.java @@ -16,16 +16,15 @@ package com.google.errorprone.bugpatterns; -import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.collect.Multimaps.toMultimap; import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.LinkType.CUSTOM; import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; +import static com.google.errorprone.matchers.Description.NO_MATCH; import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Streams; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; @@ -36,16 +35,9 @@ import com.sun.source.tree.MethodTree; import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol.MethodSymbol; -import com.sun.tools.javac.tree.JCTree; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; -import javax.annotation.Nullable; +import java.util.Objects; +import java.util.stream.Stream; import javax.lang.model.element.Name; /** @author hanuszczak@google.com (Łukasz Hanuszczak) */ @@ -62,193 +54,21 @@ ) public class UngroupedOverloads extends BugChecker implements ClassTreeMatcher { - private static final int DEFAULT_METHOD_COUNT_CUTOFF = 100; - - private final int methodCountCutoff; - - public UngroupedOverloads() { - this(DEFAULT_METHOD_COUNT_CUTOFF); - } - - /** - * @param methodCountCutoff the limit on the number of methods in the class for which fixes are no - * longer suggested (but correctness is still ensured). - */ - UngroupedOverloads(int methodCountCutoff) { - this.methodCountCutoff = methodCountCutoff; - } - - @Override - public Description matchClass(ClassTree classTree, VisitorState state) { - List classMembers = new ArrayList<>(classTree.getMembers()); - MethodFixSuggester suggester = new MethodFixSuggester(classTree, classMembers, state); - - /* - * Checking the class members for violations requires only in O(n) time whereas providing - * suggested fixes requires O(n^2) time. This is why we have a cut-off limit that depending on - * the number of class members selects feasible strategy. - */ - long methodCount = classMembers.stream().filter((tree) -> tree instanceof MethodTree).count(); - if (methodCount >= methodCountCutoff) { - checkMembers(classMembers, suggester); - } else { - orderMembers(classMembers, suggester); - } - return suggester.describeFix(); - } - - private static void checkMembers( - List classMembers, MethodFixSuggester suggester) { - Map previousOccurrences = new LinkedHashMap<>(); - for (int currentOccurrence = 0; currentOccurrence < classMembers.size(); currentOccurrence++) { - Tree memberTree = classMembers.get(currentOccurrence); - if (!(memberTree instanceof MethodTree)) { - continue; - } - - MethodTree methodTree = (MethodTree) memberTree; - Name methodName = methodTree.getName(); - - /* - * If there is a previous occurrence and it is on a position different than the previous one - * it must be the case that the methods are not ordered properly (so we report a violation). - */ - Integer previousOccurrence = previousOccurrences.get(methodName); - if (previousOccurrence != null && previousOccurrence != currentOccurrence - 1) { - suggester.justReport(currentOccurrence); - } - previousOccurrences.put(methodName, currentOccurrence); - } - } - - private static void orderMembers( - List classMembers, MethodFixSuggester suggester) { - /* - * The ordering algorithm works by bubbling (the same way as in bubble-sort) methods until they - * reach correct position. Therefore algorithm has a O(n^2) where n is number of methods within - * the class. However, this is just a worst case scenario (and classes usually don't have more - * than 1000 members anyway) that could happen only for really, really weird code. - * - * The problem itself looks like something that could be solved in O(n lg n) time but it would - * probably be much more complicated and the constant would be much greater (so for sane code - * it would be slower). - */ - Map previousOccurrences = new LinkedHashMap<>(); - for (int currentOccurrence = 0; currentOccurrence < classMembers.size(); currentOccurrence++) { - Tree memberTree = classMembers.get(currentOccurrence); - if (!(memberTree instanceof MethodTree)) { - continue; - } - - OverloadKey methodName = OverloadKey.create((MethodTree) memberTree); - - Integer previousOccurrence = previousOccurrences.get(methodName); - if (previousOccurrence != null) { - // If the block is actually moved (i.e. the `for` loop below does at least one iteration). - if (currentOccurrence - 1 > previousOccurrence) { - // We "bubble" the current occurrence until it is placed next to the previous occurrence. - for (int i = currentOccurrence - 1; i > previousOccurrence; i--) { - Tree splitterTree = classMembers.get(i); - if (!(splitterTree instanceof MethodTree)) { - continue; - } - OverloadKey splitterKey = OverloadKey.create((MethodTree) splitterTree); - - // Swapping may invalidate `previousOccurrences` so we need to shift it by one manually. - Integer splitterOccurrence = previousOccurrences.get(splitterKey); - if (splitterOccurrence != null && splitterOccurrence.equals(i)) { - previousOccurrences.put(splitterKey, i + 1); - } - - Collections.swap(classMembers, i, i + 1); - } - suggester.moveBlock(currentOccurrence); - } - previousOccurrences.put(methodName, previousOccurrence + 1); - } else { - previousOccurrences.put(methodName, currentOccurrence); - } - } - } + @AutoValue + abstract static class MemberWithIndex { - /** - * Returns a more fine-tuned starting position of a {@code current} node in the source code. - * - *

Unfortunately, {@link JCTree#getStartPosition()} doesn't account for comments which usually - * are integral part of the code. So, instead we use a heuristic: the AST node actually "starts" - * after the first newline after the end position of the previous AST node. - * - *

This assumes the relevant comments are placed either before (i.e. above) the definition or - * after the definition on the same line rather than below the definition. - * - * @param current the node for which we retrieve the position - * @param previous a node before the {@code current} one - * @return more useful starting position of the {@code current} node - */ - private static int getBroadStartPosition(VisitorState state, Tree current, Tree previous) { - int previousEndPosition = getBroadEndPosition(state, previous, current); - int currentStartPosition = ((JCTree) current).getStartPosition(); - return Math.min(previousEndPosition, currentStartPosition); - } + abstract int index(); - /** - * Returns a more fine-tuned ending position of a {@code current} node in the source code. - * - *

See {@link #getBroadStartPosition(VisitorState, Tree, Tree)} for more information. - * - * @param current the node for which we retrieve the position - * @param next a node after the {@code current} one (optional) - * @return more useful ending position of the {@code current} node - */ - private static int getBroadEndPosition(VisitorState state, Tree current, @Nullable Tree next) { - CharSequence source = state.getSourceCode(); + abstract Tree tree(); - int currentEndPosition = state.getEndPosition(current); - int nextStartPosition; - if (next != null) { - nextStartPosition = ((JCTree) next).getStartPosition(); - } else { - nextStartPosition = source.length(); + static MemberWithIndex create(int index, Tree tree) { + return new AutoValue_UngroupedOverloads_MemberWithIndex(index, tree); } - - String newline = System.lineSeparator(); - int newlinePosition = indexOf(source, newline, currentEndPosition, nextStartPosition); - return (newlinePosition < 0) ? currentEndPosition : newlinePosition; - } - - /** - * Looks for the first occurrence of {@code term} in {@code sequence}. - * - *

This is analogous to the {@link java.lang.String#indexOf(String, int)} but works with any - * {@link java.lang.CharSequence} and also supports {@code toIndex} parameter. - * - *

The algorithm has a O(nm) time complexity but it is more than enough for this use case. - * - * @param sequence the character sequence to perform the search on - * @param term the character sequence to search for - * @param fromIndex the index from which to start the search (inclusive) - * @param toIndex the index to which the search happens (exclusive) - * @return the index of the first occurrence of specified substring or -1 if not found - */ - private static int indexOf(CharSequence sequence, CharSequence term, int fromIndex, int toIndex) { - int termLength = term.length(); - for (int index = fromIndex; index + termLength - 1 < toIndex; index++) { - int i = 0; - for (; i < termLength; i++) { - if (sequence.charAt(index + i) != term.charAt(i)) { - break; - } - } - if (i == termLength) { - return index; - } - } - return -1; } @AutoValue abstract static class OverloadKey { - abstract String name(); + abstract Name name(); // TODO(cushon): re-enable this, but don't warn about interspersed static/non-static overloads // Include static-ness when grouping overloads. Static and non-static methods are still @@ -257,231 +77,68 @@ abstract static class OverloadKey { public static OverloadKey create(MethodTree methodTree) { MethodSymbol sym = ASTHelpers.getSymbol(methodTree); - return new AutoValue_UngroupedOverloads_OverloadKey(sym.getSimpleName().toString()); - } - } - - private interface OverloadViolation { - Name getMethodName(); - - void buildFix(SuggestedFix.Builder fix, VisitorState state, MethodTree target); - } - - private static class JustReport implements OverloadViolation { - private final MethodTree methodTree; - - public JustReport(MethodTree methodTree) { - this.methodTree = methodTree; - } - - @Override - public Name getMethodName() { - return methodTree.getName(); - } - - @Override - public void buildFix(SuggestedFix.Builder fix, VisitorState state, MethodTree target) { - // Do nothing, this is just for reporting violations. - } - } - - private static class MoveBlock implements OverloadViolation { - private final Name methodName; - private final int startPosition; - private final int endPosition; - - public MoveBlock(Name methodName, int startPosition, int endPosition) { - this.methodName = methodName; - this.startPosition = startPosition; - this.endPosition = endPosition; - } - - @Override - public Name getMethodName() { - return methodName; - } - - @Override - public void buildFix(SuggestedFix.Builder fix, VisitorState state, MethodTree target) { - String methodSource = getMethodSource(state.getSourceCode()); - fix.replace(startPosition, endPosition, ""); - fix.postfixWith(target, methodSource); - } - - public String getMethodSource(CharSequence sourceCode) { - return sourceCode.subSequence(startPosition, endPosition).toString(); - } - } - - private class MethodFixSuggester { - - private final ClassTree classTree; - private final ImmutableList classMembers; // Initial, unchanged members. - private final VisitorState state; - private final List violations; - private final ImmutableSet suppressed; - - public MethodFixSuggester( - ClassTree classTree, List classMembers, VisitorState state) { - this.classTree = classTree; - this.classMembers = ImmutableList.copyOf(classMembers); - this.state = state; - this.violations = new ArrayList<>(); - this.suppressed = - classMembers - .stream() - .filter(MethodTree.class::isInstance) - .map(MethodTree.class::cast) - .filter(m -> isSuppressed(m)) - .map(m -> m.getName().toString()) - .collect(toImmutableSet()); - } - - private void addViolation(OverloadViolation violation) { - if (!suppressed.contains(violation.getMethodName().toString())) { - violations.add(violation); - } - } - - public void justReport(int currentOccurrence) { - addViolation(new JustReport((MethodTree) classMembers.get(currentOccurrence))); - } - - public void moveBlock(int currentOccurrence) { - MethodTree currentTree = (MethodTree) classMembers.get(currentOccurrence); - Name currentName = currentTree.getName(); - - Tree previousTree = classMembers.get(currentOccurrence - 1); - Tree nextTree; - if (currentOccurrence + 1 < classMembers.size()) { - nextTree = classMembers.get(currentOccurrence + 1); - } else { - nextTree = null; - } - - int startPosition = getBroadStartPosition(state, currentTree, previousTree); - int endPosition = getBroadEndPosition(state, currentTree, nextTree); - addViolation(new MoveBlock(currentName, startPosition, endPosition)); - } - - public Description describeFix() { - if (violations.isEmpty()) { - return Description.NO_MATCH; - } else { - return buildAdaptedDescription().addFix(buildFix()).build(); - } - } - - private SuggestedFix buildFix() { - ImmutableMap lastGroupedOccurrences = getLastGroupedOccurrences(); - - SuggestedFix.Builder fix = SuggestedFix.builder(); - for (OverloadViolation violation : violations) { - Name methodName = violation.getMethodName(); - violation.buildFix(fix, state, lastGroupedOccurrences.get(methodName)); - } - return fix.build(); - } - - private Description.Builder buildAdaptedDescription() { - ImmutableSet methodNames = - violations.stream().map(OverloadViolation::getMethodName).collect(toImmutableSet()); - - if (methodNames.size() == 1) { - return buildMethodDescription(methodNames.iterator().next()); - } else { - return buildClassDescription(methodNames); - } - } - - private Description.Builder buildMethodDescription(Name methodName) { - MethodTree methodTree = getFirstOccurrences().get(methodName); - return buildDescription(methodTree).setMessage(getMethodFixMessage(methodTree)); - } - - private Description.Builder buildClassDescription(Collection methodNames) { - return buildDescription(classTree).setMessage(getClassFixMessage(methodNames)); - } - - /** - * Returns a mapping from a method name to the first (topmost) AST node matching this name. - * - *

For a class with methods {@code A}, {@code B} and {@code C} in the following order the - * marked nodes are considered to be "first occurrences": - * - * 

-     * AABBBABCCAB
-     * ^ ^    ^
-     *
- */ - private ImmutableMap getFirstOccurrences() { - Map firstOccurrences = new LinkedHashMap<>(); - for (Tree memberTree : classMembers) { - if (!(memberTree instanceof MethodTree)) { - continue; - } - MethodTree methodTree = (MethodTree) memberTree; - Name methodName = methodTree.getName(); - - firstOccurrences.computeIfAbsent(methodName, __ -> methodTree); - } - return ImmutableMap.copyOf(firstOccurrences); - } - - /** - * Returns a mapping from a method name to the last grouped AST node matching this name. - * - *

For a class with methods {@code A}, {@code B} and {@code C} in the following order the - * marked nodes are considered to be "last grouped occurrences": - * - * 

-     * AABBBABCCAB
-     *  ^  ^   ^
-     *
- */ - private ImmutableMap getLastGroupedOccurrences() { - Map lastGroupedOccurrences = new LinkedHashMap<>(); - for (int i = 0; i < classMembers.size(); i++) { - Tree memberTree = classMembers.get(i); - if (!(memberTree instanceof MethodTree)) { - continue; - } - MethodTree methodTree = (MethodTree) memberTree; - Name methodName = methodTree.getName(); - - Integer lastGroupedOccurrence = lastGroupedOccurrences.get(methodName); - if (lastGroupedOccurrence == null || lastGroupedOccurrence + 1 == i) { - lastGroupedOccurrences.put(methodName, i); - } - } - return transformMap(lastGroupedOccurrences, (index) -> (MethodTree) classMembers.get(index)); + return new AutoValue_UngroupedOverloads_OverloadKey(sym.getSimpleName()); } } - /** - * Transforms each value in the given {@code input} map using the {@code mapper} function. - * - * @return a new map with transformed values - */ - private static ImmutableMap transformMap( - Map input, Function mapper) { - return input - .entrySet() + @Override + public Description matchClass(ClassTree classTree, VisitorState state) { + // collect all member methods and their indices in the list of members, grouped by name + LinkedHashMultimap methods = + Streams.zip( + Stream.iterate(0, i -> i + 1), + classTree.getMembers().stream(), + MemberWithIndex::create) + .filter(m -> m.tree() instanceof MethodTree) + .collect( + toMultimap( + m -> OverloadKey.create((MethodTree) m.tree()), + x -> x, + LinkedHashMultimap::create)); + methods.asMap().forEach((key, overloads) -> checkOverloads(state, key.name(), overloads)); + return NO_MATCH; + } + + private void checkOverloads( + VisitorState state, Name name, Collection overloads) { + if (overloads.size() <= 1) { + return; + } + // check if the indices of the overloads in the member list are sequential + MemberWithIndex first = overloads.iterator().next(); + boolean grouped = + Streams.zip( + Stream.iterate(first.index(), i -> i + 1), + overloads.stream().map(m -> m.index()), + (a, b) -> Objects.equals(a, b)) + .allMatch(x -> x); + if (grouped) { + return; + } + if (overloads.stream().anyMatch(m -> isSuppressed(m.tree()))) { + return; + } + // build a fix that deletes all but the first overload, and adds them back immediately after + // the first overload + SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); + StringBuilder sb = new StringBuilder("\n"); + overloads .stream() - .collect(toImmutableMap(Map.Entry::getKey, entry -> mapper.apply(entry.getValue()))); - } - - private static String getMethodFixMessage(MethodTree methodTree) { - return String.format("Overloads of '%s' are not grouped together", methodTree.getName()); - } - - private static String getClassFixMessage(Collection methodNames) { - String methods = - methodNames - .stream() - .map(methodName -> String.format("\"%s\"", methodName.toString())) - .sorted() - .collect(Collectors.joining(", ")); - return String.format("Overloaded methods (%s) of this class are not grouped together", methods); + .filter(o -> o != first) + .forEach( + o -> { + sb.append(state.getSourceForNode(o.tree())).append('\n'); + fixBuilder.delete(o.tree()); + }); + fixBuilder.postfixWith(first.tree(), sb.toString()); + SuggestedFix fix = fixBuilder.build(); + String message = String.format("Overloads of '%s' are not grouped together.", name); + // emit findings for each overload + overloads + .stream() + .forEach( + o -> + state.reportMatch( + buildDescription(o.tree()).addFix(fix).setMessage(message).build())); } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UngroupedOverloadsTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UngroupedOverloadsTest.java index c2bdea789d6..c8c116edbe9 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/UngroupedOverloadsTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/UngroupedOverloadsTest.java @@ -16,13 +16,8 @@ package com.google.errorprone.bugpatterns; -import static com.google.errorprone.BugPattern.Category.JDK; -import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; - import com.google.errorprone.BugCheckerRefactoringTestHelper; -import com.google.errorprone.BugPattern; import com.google.errorprone.CompilationTestHelper; -import org.junit.Before; import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,84 +27,35 @@ @RunWith(JUnit4.class) public final class UngroupedOverloadsTest { - /** - * Specialized version of the {@link UngroupedOverloads} bug checker for testing with a low cutoff - * limit set. - * - *

This class needs to be public because of the limitation of {@link CompilationTestHelper}. - */ - @BugPattern( - name = "UngroupedOverloadsLowCutoff", - summary = "A specialized version of the ungrouped overloads checker with a low cutoff limit", - category = JDK, - severity = SUGGESTION - ) - public static final class UngroupedOverloadsLowCutoff extends UngroupedOverloads { - - public UngroupedOverloadsLowCutoff() { - super(5); - } - } - - private CompilationTestHelper compilationHelper; - private CompilationTestHelper cutoffCompilationHelper; - - private BugCheckerRefactoringTestHelper refactoringHelper; - private BugCheckerRefactoringTestHelper cutoffRefactoringHelper; - - @Before - public void createCompilationHelpers() { - compilationHelper = createCompilationHelper(UngroupedOverloads.class); - cutoffCompilationHelper = createCompilationHelper(UngroupedOverloadsLowCutoff.class); - } - - private CompilationTestHelper createCompilationHelper(Class bugChecker) { - return CompilationTestHelper.newInstance(bugChecker, getClass()); - } - - @Before - public void createRefactoringHelper() { - refactoringHelper = createRefactoringHelper(new UngroupedOverloads()); - cutoffRefactoringHelper = createRefactoringHelper(new UngroupedOverloadsLowCutoff()); - } + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(UngroupedOverloads.class, getClass()); - private BugCheckerRefactoringTestHelper createRefactoringHelper(BugChecker bugChecker) { - return BugCheckerRefactoringTestHelper.newInstance(bugChecker, getClass()); - } + private final BugCheckerRefactoringTestHelper refactoringHelper = + BugCheckerRefactoringTestHelper.newInstance(new UngroupedOverloads(), getClass()); @Test public void ungroupedOverloadsPositiveCasesSingle() throws Exception { - final String path = "UngroupedOverloadsPositiveCasesSingle.java"; - compilationHelper.addSourceFile(path).doTest(); - cutoffCompilationHelper.addSourceFile(path).doTest(); + compilationHelper.addSourceFile("UngroupedOverloadsPositiveCasesSingle.java").doTest(); } @Test public void ungroupedOverloadsPositiveCasesMultiple() throws Exception { - final String path = "UngroupedOverloadsPositiveCasesMultiple.java"; - compilationHelper.addSourceFile(path).doTest(); - cutoffCompilationHelper.addSourceFile(path).doTest(); + compilationHelper.addSourceFile("UngroupedOverloadsPositiveCasesMultiple.java").doTest(); } @Test public void ungroupedOverloadsPositiveCasesInterleaved() throws Exception { - final String path = "UngroupedOverloadsPositiveCasesInterleaved.java"; - compilationHelper.addSourceFile(path).doTest(); - cutoffCompilationHelper.addSourceFile(path).doTest(); + compilationHelper.addSourceFile("UngroupedOverloadsPositiveCasesInterleaved.java").doTest(); } @Test public void ungroupedOverloadsPositiveCasesCovering() throws Exception { - final String path = "UngroupedOverloadsPositiveCasesCovering.java"; - compilationHelper.addSourceFile(path).doTest(); - cutoffCompilationHelper.addSourceFile(path).doTest(); + compilationHelper.addSourceFile("UngroupedOverloadsPositiveCasesCovering.java").doTest(); } @Test public void ungroupedOverloadsNegativeCases() throws Exception { - final String path = "UngroupedOverloadsNegativeCases.java"; - compilationHelper.addSourceFile(path).doTest(); - cutoffCompilationHelper.addSourceFile(path).doTest(); + compilationHelper.addSourceFile("UngroupedOverloadsNegativeCases.java").doTest(); } @Test @@ -139,7 +85,7 @@ public void ungroupedOverloadsRefactoringInterleaved() throws Exception { @Test public void ungroupedOverloadsRefactoringBelowCutoffLimit() throws Exception { // Here we have 4 methods so refactoring should be applied. - cutoffRefactoringHelper + refactoringHelper .addInputLines( "in/BelowLimit.java", "class BelowLimit {", @@ -160,9 +106,8 @@ public void ungroupedOverloadsRefactoringBelowCutoffLimit() throws Exception { } @Test - public void ungroupedOverloadsRefactoringAboveCutoffLimit() throws Exception { - // Here we have 5 methods so refactoring should NOT be applied. - cutoffRefactoringHelper + public void ungroupedOverloadsRefactoring_fiveMethods() throws Exception { + refactoringHelper .addInputLines( "in/AboveLimit.java", "class AboveLimit {", @@ -177,8 +122,8 @@ public void ungroupedOverloadsRefactoringAboveCutoffLimit() throws Exception { "class AboveLimit {", " AboveLimit() {}", " void foo() {}", - " void bar() {}", " void foo(int x) {}", + " void bar() {}", " void baz() {}", "}") .doTest(); @@ -201,7 +146,7 @@ public void staticAndNonStatic() throws Exception { @Test public void staticAndNonStaticInterspersed() throws Exception { - cutoffCompilationHelper + compilationHelper .addSourceLines( "Test.java", "class Test {", diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesCovering.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesCovering.java index 6c4c203d22e..15d7442ad7d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesCovering.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesCovering.java @@ -17,14 +17,14 @@ package com.google.errorprone.bugpatterns.testdata; /** @author hanuszczak@google.com (Łukasz Hanuszczak) */ -// BUG: Diagnostic contains: Overloaded methods ("bar", "foo", "quux") of this class are not grouped -// together public class UngroupedOverloadsPositiveCasesCovering { + // BUG: Diagnostic contains: Overloads of 'foo' are not grouped together public void foo(int x) { System.out.println(x); } + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar() { foo(); } @@ -33,10 +33,12 @@ public void baz() { bar(); } + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x) { foo(x); } + // BUG: Diagnostic contains: Overloads of 'quux' are not grouped together private void quux() { norf(); } @@ -45,10 +47,12 @@ private void norf() { quux(); } + // BUG: Diagnostic contains: Overloads of 'quux' are not grouped together public void quux(int x) { bar(x); } + // BUG: Diagnostic contains: Overloads of 'foo' are not grouped together public void foo() { foo(42); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesInterleaved.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesInterleaved.java index d6a9232a867..3859fcf6db2 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesInterleaved.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesInterleaved.java @@ -17,12 +17,11 @@ package com.google.errorprone.bugpatterns.testdata; /** @author hanuszczak@google.com (Łukasz Hanuszczak) */ -// BUG: Diagnostic contains: Overloaded methods ("bar", "baz") of this class are not grouped -// together public class UngroupedOverloadsPositiveCasesInterleaved { private int foo; + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x, String z, int y) { System.out.println(String.format("z: %s, x: %d, y: %d", z, x, y)); } @@ -31,20 +30,24 @@ public UngroupedOverloadsPositiveCasesInterleaved(int foo) { this.foo = foo; } + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x) { bar(foo, x); } + // BUG: Diagnostic contains: Overloads of 'baz' are not grouped together public void baz(String x) { baz(x, FOO); } + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x, int y) { bar(y, FOO, x); } public static final String FOO = "foo"; + // BUG: Diagnostic contains: Overloads of 'baz' are not grouped together public void baz(String x, String y) { bar(foo, x + y, foo); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesMultiple.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesMultiple.java index ae3e920e9ac..dc735198131 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesMultiple.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesMultiple.java @@ -17,12 +17,11 @@ package com.google.errorprone.bugpatterns.testdata; /** @author hanuszczak@google.com (Łukasz Hanuszczak) */ -// BUG: Diagnostic contains: Overloaded methods ("bar", "norf", "quux") of this class are not -// grouped together public class UngroupedOverloadsPositiveCasesMultiple { private int foo; + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x, String z, int y) { System.out.println(String.format("z: %s, x: %d, y: %d", z, x, y)); } @@ -31,6 +30,7 @@ private UngroupedOverloadsPositiveCasesMultiple(int foo) { this.foo = foo; } + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x) { bar(foo, x); } @@ -39,36 +39,43 @@ public void baz(String x) { bar(42, x, 42); } + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x, int y) { bar(y, FOO, x); } public static final String FOO = "foo"; + // BUG: Diagnostic contains: Overloads of 'bar' are not grouped together public void bar(int x, int y, int z) { bar(x, String.valueOf(y), z); } + // BUG: Diagnostic contains: Overloads of 'quux' are not grouped together public int quux() { return quux(quux); } public int quux = 42; + // BUG: Diagnostic contains: Overloads of 'quux' are not grouped together public int quux(int x) { return x + quux; } private static class Quux {} + // BUG: Diagnostic contains: Overloads of 'quux' are not grouped together public int quux(int x, int y) { return quux(x + y); } + // BUG: Diagnostic contains: Overloads of 'norf' are not grouped together public int norf(int x) { return quux(x, x); } + // BUG: Diagnostic contains: Overloads of 'norf' are not grouped together public int norf(int x, int y) { return norf(x + y); } @@ -77,6 +84,7 @@ public void foo() { System.out.println("foo"); } + // BUG: Diagnostic contains: Overloads of 'norf' are not grouped together public void norf(int x, int y, int w) { norf(x + w, y + w); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesSingle.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesSingle.java index 8a77932ab2c..d7bef135a9d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesSingle.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/UngroupedOverloadsPositiveCasesSingle.java @@ -28,6 +28,7 @@ public void foo() { foo(42); } + // BUG: Diagnostic contains: Overloads of 'foo' are not grouped together public void foo(int x) { foo(x, x); } @@ -40,6 +41,7 @@ public void bar(int x) { foo(x); } + // BUG: Diagnostic contains: Overloads of 'foo' are not grouped together public void foo(int x, int y) { System.out.println(x + y); }