-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Introduce
ConflictDetection utility class (#478)
- Loading branch information
1 parent
da9a6dd
commit 29469cb
Showing
3 changed files
with
151 additions
and
62 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
75 changes: 75 additions & 0 deletions
75
...rone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetection.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| package tech.picnic.errorprone.bugpatterns.util; | ||
|
|
||
| import static tech.picnic.errorprone.bugpatterns.util.JavaKeywords.isValidIdentifier; | ||
|
|
||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.util.ASTHelpers; | ||
| import com.sun.source.tree.ImportTree; | ||
| import com.sun.source.tree.Tree; | ||
| import com.sun.tools.javac.code.Symbol.MethodSymbol; | ||
| import com.sun.tools.javac.code.Type; | ||
| import java.util.Optional; | ||
|
|
||
| /** A set of helper methods for detecting conflicts that would be caused when applying fixes. */ | ||
| public final class ConflictDetection { | ||
| private ConflictDetection() {} | ||
|
|
||
| /** | ||
| * If applicable, returns a human-readable argument against assigning the given name to an | ||
| * existing method. | ||
| * | ||
| * <p>This method implements imperfect heuristics. Things it currently does not consider include | ||
| * the following: | ||
| * | ||
| * <ul> | ||
| * <li>Whether the rename would merely introduce a method overload, rather than clashing with an | ||
| * 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 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( | ||
| MethodSymbol method, String newName, VisitorState state) { | ||
| if (isExistingMethodName(method.owner.type, newName, state)) { | ||
| return Optional.of( | ||
| String.format( | ||
| "a method named `%s` is already defined in this class or a supertype", newName)); | ||
| } | ||
|
|
||
| if (isSimpleNameStaticallyImported(newName, state)) { | ||
| return Optional.of(String.format("`%s` is already statically imported", newName)); | ||
| } | ||
|
|
||
| if (!isValidIdentifier(newName)) { | ||
| return Optional.of(String.format("`%s` is not a valid identifier", newName)); | ||
| } | ||
|
|
||
| return Optional.empty(); | ||
| } | ||
|
|
||
| private static boolean isExistingMethodName(Type clazz, String name, VisitorState state) { | ||
| return ASTHelpers.matchingMethods(state.getName(name), method -> true, clazz, state.getTypes()) | ||
| .findAny() | ||
| .isPresent(); | ||
| } | ||
|
|
||
| private static boolean isSimpleNameStaticallyImported(String simpleName, VisitorState state) { | ||
| return state.getPath().getCompilationUnit().getImports().stream() | ||
| .filter(ImportTree::isStatic) | ||
| .map(ImportTree::getQualifiedIdentifier) | ||
| .map(tree -> getStaticImportSimpleName(tree, state)) | ||
| .anyMatch(simpleName::contentEquals); | ||
| } | ||
|
|
||
| private static CharSequence getStaticImportSimpleName(Tree tree, VisitorState state) { | ||
| String source = SourceCode.treeToString(tree, state); | ||
| return source.subSequence(source.lastIndexOf('.') + 1, source.length()); | ||
| } | ||
| } |
73 changes: 73 additions & 0 deletions
73
...-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/util/ConflictDetectionTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package tech.picnic.errorprone.bugpatterns.util; | ||
|
|
||
| import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; | ||
|
|
||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.CompilationTestHelper; | ||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker; | ||
| import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; | ||
| 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.MethodSymbol; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| final class ConflictDetectionTest { | ||
| @Test | ||
| void matcher() { | ||
| CompilationTestHelper.newInstance(RenameBlockerFlagger.class, getClass()) | ||
| .addSourceLines( | ||
| "pkg/A.java", | ||
| "package pkg;", | ||
| "", | ||
| "import static pkg.A.B.method3t;", | ||
| "", | ||
| "import pkg.A.method4t;", | ||
| "", | ||
| "class A {", | ||
| " void method1() {", | ||
| " method3t();", | ||
| " method4(method4t.class);", | ||
| " }", | ||
| "", | ||
| " // BUG: Diagnostic contains: a method named `method2t` is already defined in this class or a", | ||
| " // supertype", | ||
| " void method2() {}", | ||
| "", | ||
| " void method2t() {}", | ||
| "", | ||
| " // BUG: Diagnostic contains: `method3t` is already statically imported", | ||
| " void method3() {}", | ||
| "", | ||
| " void method4(Object o) {}", | ||
| "", | ||
| " // BUG: Diagnostic contains: `int` is not a valid identifier", | ||
| " void in() {}", | ||
| "", | ||
| " static class B {", | ||
| " static void method3t() {}", | ||
| " }", | ||
| "", | ||
| " class method4t {}", | ||
| "}") | ||
| .doTest(); | ||
| } | ||
|
|
||
| /** | ||
| * 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 = "Interacts with `ConflictDetection` for testing purposes", severity = ERROR) | ||
| public static final class RenameBlockerFlagger extends BugChecker implements MethodTreeMatcher { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| @Override | ||
| public Description matchMethod(MethodTree tree, VisitorState state) { | ||
| return ConflictDetection.findMethodRenameBlocker( | ||
| ASTHelpers.getSymbol(tree), tree.getName() + "t", state) | ||
| .map(blocker -> buildDescription(tree).setMessage(blocker).build()) | ||
| .orElse(Description.NO_MATCH); | ||
| } | ||
| } | ||
| } |