Skip to content

Commit

Permalink
Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
rickie authored and Stephan202 committed Feb 12, 2023
1 parent 1146e2a commit 85dba89
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static com.google.errorprone.matchers.Matchers.not;
import static java.util.function.Predicate.not;
import static tech.picnic.errorprone.bugpatterns.util.ConflictDetection.findMethodRenameBlocker;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.SETUP_OR_TEARDOWN_METHOD;
import static tech.picnic.errorprone.bugpatterns.util.MoreJUnitMatchers.TEST_METHOD;
Expand All @@ -29,6 +28,7 @@
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import java.util.Optional;
import javax.lang.model.element.Modifier;
import tech.picnic.errorprone.bugpatterns.util.ConflictDetection;

/** A {@link BugChecker} that flags non-canonical JUnit method declarations. */
// XXX: Consider introducing a class-level check that enforces that test classes:
Expand Down Expand Up @@ -85,7 +85,7 @@ private void suggestTestMethodRenameIfApplicable(
tryCanonicalizeMethodName(symbol)
.ifPresent(
newName ->
findMethodRenameBlocker(symbol, newName, state)
ConflictDetection.findMethodRenameBlocker(symbol, newName, state)
.ifPresentOrElse(
blocker -> reportMethodRenameBlocker(tree, blocker, state),
() -> fixBuilder.merge(SuggestedFixes.renameMethod(tree, newName, state))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
import com.sun.tools.javac.code.Type;
import java.util.Optional;

/**
* A set of helper methods for finding conflicts which would be caused by applying certain fixes.
*/
/** A set of helper methods for detecting conflicts that would be caused when applying fixes. */
public final class ConflictDetection {
private ConflictDetection() {}

Expand All @@ -25,17 +23,16 @@ private ConflictDetection() {}
*
* <ul>
* <li>Whether the rename would merely introduce a method overload, rather than clashing with an
* existing method declaration.
* <li>Whether the rename would cause a method in a superclass to be overridden.
* existing method declaration in its class or a supertype.
* <li>Whether the rename would in fact clash with a static import. (It could be that a static
* import of the same name is only referenced from lexical scopes in which the method under
* consideration cannot be referenced directly.)
* </ul>
*
* @param method The method considered for being renamed.
* @param newName The proposed name to assign.
* @param state The {@link VisitorState} to use for searching for blockers.
* @return A human-readable argument against assigning the proposed name to an existing method, or
* @param method The method considered for renaming.
* @param newName The newly proposed name for the method.
* @param state The {@link VisitorState} to use when searching for blockers.
* @return A human-readable argument against assigning the proposed name to the given method, or
* {@link Optional#empty()} if no blocker was found.
*/
public static Optional<String> findMethodRenameBlocker(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,49 +10,48 @@
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import org.junit.jupiter.api.Test;

final class ConflictDetectionTest {
@Test
void matcher() {
CompilationTestHelper.newInstance(RenameBlockerFlagger.class, getClass())
.addSourceLines(
"/A.java",
"import static pkg.B.foo3t;",
"pkg/A.java",
"package pkg;",
"",
"import static pkg.A.B.method3t;",
"",
"class A {",
" private void foo1() {",
" foo3t();",
" void method1() {",
" method3t();",
" }",
"",
" // BUG: Diagnostic contains: a method named `foo2t` is already defined in this class or a",
" // BUG: Diagnostic contains: a method named `method2t` is already defined in this class or a",
" // supertype",
" private void foo2() {}",
" void method2() {}",
"",
" private void foo2t() {}",
" void method2t() {}",
"",
" // BUG: Diagnostic contains: `foo3t` is already statically imported",
" private void foo3() {}",
" // BUG: Diagnostic contains: `method3t` is already statically imported",
" void method3() {}",
"",
" // BUG: Diagnostic contains: `int` is not a valid identifier",
" private void in() {}",
"}")
.addSourceLines(
"/pkg/B.java",
"package pkg;",
" void in() {}",
"",
"public class B {",
" public static void foo3t() {}",
" class B {",
" static void method3t() {}",
" }",
"}")
.doTest();
}

/**
* A {@link BugChecker} that flags method rename blockers found by {@link
* ConflictDetection#findMethodRenameBlocker(Symbol.MethodSymbol, String, VisitorState)}.
* A {@link BugChecker} that uses {@link ConflictDetection#findMethodRenameBlocker(MethodSymbol,
* String, VisitorState)} to flag methods of which the name cannot be suffixed with a {@code t}.
*/
@BugPattern(summary = "Flags blockers for renaming methods", severity = ERROR)
@BugPattern(summary = "Interacts with `ConflictDetection` for testing purposes", severity = ERROR)
public static final class RenameBlockerFlagger extends BugChecker implements MethodTreeMatcher {
private static final long serialVersionUID = 1L;

Expand Down

0 comments on commit 85dba89

Please sign in to comment.