Skip to content

Commit

Permalink
TypeParameterUnusedInFormals: ignore methods that always return null …
Browse files Browse the repository at this point in the history
…or throw.

It's OK for a method to declare that it can return any type as long as it never
returns anything, or always returns null. We check for casts to the return type
as a heuristic.
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=84139059
  • Loading branch information
cushon committed Jan 22, 2015
1 parent 5ea7278 commit e58737a
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
Expand Up @@ -39,6 +39,7 @@
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCIdent;
import com.sun.tools.javac.tree.JCTree.JCTypeCast;
import com.sun.tools.javac.tree.JCTree.JCTypeParameter;
import com.sun.tools.javac.tree.TreeScanner;

Expand Down Expand Up @@ -110,6 +111,12 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
}
}

// Ignore cases where the type is never used in the method body. Methods that claim to return
// 'any' type are OK if they only return null, or always throw.
if (!CastFinder.find(tree.getBody(), retType)) {
return Description.NO_MATCH;
}

return attemptFix(retType, tree, state);
}

Expand Down Expand Up @@ -261,4 +268,26 @@ public Void visitType(Type type, Void unused) {
return null;
}
}

private static class CastFinder extends TreeScanner {

static boolean find(Tree tree, Type retType) {
CastFinder finder = new CastFinder(retType);
((JCTree) tree).accept(finder);
return finder.found;
}

Type retType;
boolean found = false;

private CastFinder(Type retType) {
this.retType = retType;
}

@Override
public void visitTypeCast(JCTypeCast tree) {
found |= retType.tsym.equals(ASTHelpers.getSymbol(tree.clazz));
scan(tree.expr);
}
}
}
Expand Up @@ -23,7 +23,6 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;


@RunWith(JUnit4.class)
public class TypeParameterUnusedInFormalsTest {

Expand Down Expand Up @@ -157,4 +156,35 @@ public void okNotMyParam() throws Exception {
"}")
);
}

@Test
public void okAlwaysThrows() throws Exception {
compilationHelper.assertCompileSucceeds(
compilationHelper.fileManager().forSourceLines("Test.java",
"class Test {",
" <T> T logAndThrow() { throw new RuntimeException(); }",
"}")
);
}

@Test
public void okAlwaysReturnsNull() throws Exception {
compilationHelper.assertCompileSucceeds(
compilationHelper.fileManager().forSourceLines("Test.java",
"class Test {",
" <T> T nullFactory() { return null; }",
"}")
);
}

@Test
public void badCast() throws Exception {
compilationHelper.assertCompileSucceedsWithMessages(
compilationHelper.fileManager().forSourceLines("Test.java",
"class Test {",
" // BUG: Diagnostic contains:",
" <T> T nullFactory() { return (T) null; }",
"}")
);
}
}

0 comments on commit e58737a

Please sign in to comment.