support complex statements inside null check : RequireNonNullInspection

This commit is contained in:
Roman Ivanov
2017-09-25 09:46:06 +07:00
parent e835fa759c
commit cd4a117a49
19 changed files with 391 additions and 82 deletions
@@ -22,12 +22,10 @@ import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.psiutils.ClassUtils;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ControlFlowUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.*;
import org.jetbrains.annotations.Contract;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -35,6 +33,8 @@ import org.jetbrains.annotations.Nullable;
import static com.intellij.util.ObjectUtils.tryCast;
public class RequireNonNullInspection extends BaseJavaBatchLocalInspectionTool {
private static final EquivalenceChecker ourEquivalence = EquivalenceChecker.getCanonicalPsiEquivalence();
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
@@ -47,16 +47,18 @@ public class RequireNonNullInspection extends BaseJavaBatchLocalInspectionTool {
public void visitIfStatement(PsiIfStatement ifStatement) {
NotNullContext context = NotNullContext.from(ifStatement);
if(context == null) return;
String method = context.getMethod();
holder.registerProblem(ifStatement, "Replace condition with Objects." + method, new ReplaceWithRequireNonNullFix(method));
String method = getMethod(context.getExpression());
holder.registerProblem(ifStatement, InspectionsBundle.message("inspection.require.non.null.message", method),
new ReplaceWithRequireNonNullFix(method));
}
@Override
public void visitAssignmentExpression(PsiAssignmentExpression assignment) {
NotNullContext context = NotNullContext.from(assignment);
public void visitConditionalExpression(PsiConditionalExpression ternary) {
TernaryNotNullContext context = TernaryNotNullContext.from(ternary);
if(context == null) return;
String method = context.getMethod();
holder.registerProblem(assignment, "Replace condition with Objects." + method, new ReplaceWithRequireNonNullFix(method));
String method = getMethod(context.getNonNullExpr());
holder.registerProblem(ternary, InspectionsBundle.message("inspection.require.non.null.message", method),
new ReplaceWithRequireNonNullFix(method));
}
};
}
@@ -77,108 +79,266 @@ public class RequireNonNullInspection extends BaseJavaBatchLocalInspectionTool {
@NotNull
@Override
public String getFamilyName() {
return "Replace null checked assignment with Objects static method";
return "Replace null checked condition with Objects static method";
}
@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiElement element = descriptor.getStartElement();
final NotNullContext context;
boolean isExpr = false;
final PsiElement result;
if(element instanceof PsiIfStatement) {
context = NotNullContext.from((PsiIfStatement)element);
} else if(element instanceof PsiAssignmentExpression) {
context = NotNullContext.from((PsiAssignmentExpression)element);
isExpr = true;
NotNullContext context = NotNullContext.from((PsiIfStatement)element);
if (context == null) return;
CommentTracker tracker = new CommentTracker();
PsiExpression expression = context.getExpression();
PsiExpression requireCall = createRequireExpression(tracker, expression, project, context.getVariable(), context.getReference());
context.getReference().replace(requireCall);
result = tracker.replaceAndRestoreComments(context.getIfStatement(), context.getNullBranchStmt());
if (context.getNextToDelete() != null) {
context.getNextToDelete().delete();
}
} else if(element instanceof PsiConditionalExpression) {
TernaryNotNullContext context = TernaryNotNullContext.from((PsiConditionalExpression)element);
if(context == null) return;
CommentTracker tracker = new CommentTracker();
PsiExpression requireCall =
createRequireExpression(tracker, context.getNonNullExpr(), project, context.getVariable(), context.getNonNullExpr());
result = tracker.replace(context.getTernary(), requireCall);
} else return;
if(context == null) return;
CommentTracker tracker = new CommentTracker();
boolean isSimple = ExpressionUtils.isSimpleExpression(context.getExpression());
String expr = tracker.text(context.getExpression());
if(!isSimple) {
expr = "()->" + expr;
}
String varName = context.getVariable().getName();
String maybeSemicolon = isExpr ? "" : ";";
String replacement = varName + "=" + CommonClassNames.JAVA_UTIL_OBJECTS + "." + myMethod + "(" + varName + "," + expr + ")" + maybeSemicolon;
PsiElement result = tracker.replaceAndRestoreComments(element, replacement);
LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result);
CodeStyleManager.getInstance(project).reformat(JavaCodeStyleManager.getInstance(project).shortenClassReferences(result));
}
}
private static class NotNullContext {
private final PsiExpression myExpression;
private final PsiVariable myVariable;
@NotNull
private static PsiExpression createRequireExpression(@NotNull CommentTracker tracker,
@NotNull PsiExpression expression,
@NotNull Project project,
@NotNull PsiVariable variable,
@NotNull PsiElement context) {
boolean isSimple = ExpressionUtils.isSimpleExpression(expression);
String expr = tracker.text(expression);
if (!isSimple) {
expr = "()->" + expr;
}
String varName = variable.getName();
String requireCallText = CommonClassNames.JAVA_UTIL_OBJECTS + "." + getMethod(expression) + "(" + varName + "," + expr + ")";
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
return factory.createExpressionFromText(requireCallText, context);
}
private NotNullContext(PsiExpression expression, PsiVariable variable) {
private static class NotNullContext {
private final @NotNull PsiExpression myExpression;
private final @NotNull PsiExpression myReference;
private final @NotNull PsiStatement myNullBranchStmt;
private final @NotNull PsiVariable myVariable;
private final @NotNull PsiIfStatement myIfStatement;
private final @Nullable PsiStatement myNextToDelete;
private NotNullContext(@NotNull PsiExpression expression,
@NotNull PsiExpression reference,
@NotNull PsiStatement nullBranchStmt,
@NotNull PsiVariable variable,
@NotNull PsiIfStatement statement,
@Nullable PsiStatement nextToDelete) {
myExpression = expression;
myReference = reference;
myNullBranchStmt = nullBranchStmt;
myVariable = variable;
myIfStatement = statement;
myNextToDelete = nextToDelete;
}
@NotNull
public PsiExpression getExpression() {
return myExpression;
}
@NotNull
public PsiVariable getVariable() {
return myVariable;
}
public String getMethod() {
boolean isSimple = ExpressionUtils.isSimpleExpression(myExpression);
return isSimple? "requireNonNullElse" : "requireNonNullElseGet";
@NotNull
public PsiIfStatement getIfStatement() {
return myIfStatement;
}
@Nullable
static NotNullContext from(PsiIfStatement ifStatement) {
if(ifStatement.getElseBranch() != null) return null;
PsiStatement statement = ControlFlowUtils.stripBraces(ifStatement.getThenBranch());
public PsiStatement getNextToDelete() {
return myNextToDelete;
}
@NotNull
public PsiExpression getReference() {
return myReference;
}
@NotNull
public PsiStatement getNullBranchStmt() {
return myNullBranchStmt;
}
@Nullable
static NotNullContext from(@NotNull PsiIfStatement ifStatement) {
PsiExpression condition = ifStatement.getCondition();
if(condition == null) return null;
PsiBinaryExpression binOp = tryCast(condition, PsiBinaryExpression.class);
if(binOp == null) return null;
PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp);
PsiReferenceExpression referenceExpression = tryCast(value, PsiReferenceExpression.class);
if(referenceExpression == null) return null;
PsiVariable variable = tryCast(referenceExpression.resolve(), PsiVariable.class);
if(variable == null) return null;
PsiVariable variable = extractVariable(binOp);
if (variable == null) return null;
if(ClassUtils.isPrimitive(variable.getType())) return null;
PsiExpressionStatement expressionStatement = tryCast(statement, PsiExpressionStatement.class);
if(expressionStatement == null) return null;
PsiExpression maybeNonNull = ExpressionUtils.getAssignmentTo(expressionStatement.getExpression(), variable);
if(NullnessUtil.getExpressionNullness(maybeNonNull) != Nullness.NOT_NULL) return null;
if(!LambdaGenerationUtil.canBeUncheckedLambda(maybeNonNull)) return null;
return new NotNullContext(maybeNonNull, variable);
boolean inverted = binOp.getOperationTokenType() == JavaTokenType.NE;
PsiStatement elseBranch = ControlFlowUtils.stripBraces(ifStatement.getElseBranch());
PsiStatement thenBranch = ControlFlowUtils.stripBraces(ifStatement.getThenBranch());
if(elseBranch != null) {
PsiStatement nullBranch = inverted? thenBranch : elseBranch;
PsiStatement nonNullBranch = inverted? elseBranch : thenBranch;
return extractContext(ifStatement, variable, nullBranch, nonNullBranch, null);
} else {
PsiReturnStatement nextReturn = tryCast(PsiTreeUtil.skipWhitespacesAndCommentsForward(ifStatement), PsiReturnStatement.class);
if (nextReturn == null) return null;
if (thenBranch instanceof PsiReturnStatement) {
PsiStatement nullBranch = inverted? thenBranch : nextReturn;
PsiStatement nonNullBranch = inverted? nextReturn : thenBranch;
return extractContext(ifStatement, variable, nullBranch, nonNullBranch, nextReturn);
}
}
return null;
}
@Contract("_, _, null, _, _ -> null")
private static NotNullContext extractContext(@NotNull PsiIfStatement ifStatement,
@NotNull PsiVariable variable,
@Nullable PsiStatement nullBranch,
@Nullable PsiStatement nonNullBranch,
@Nullable PsiReturnStatement toDelete) {
if(nullBranch == null) return null;
EquivalenceChecker.Match match = ourEquivalence.statementsMatch(nullBranch, nonNullBranch);
PsiExpression nullDiff = tryCast(match.getLeftDiff(), PsiExpression.class);
PsiExpression nonNullDiff = tryCast(match.getRightDiff(), PsiExpression.class);
if(!ExpressionUtils.isReferenceTo(nullDiff, variable)) {
TopmostQualifierDiff qualifierDiff = TopmostQualifierDiff.from(nullDiff, nonNullDiff);
if(qualifierDiff == null) return null;
nullDiff = qualifierDiff.getLeft();
nonNullDiff = qualifierDiff.getRight();
if(!ExpressionUtils.isReferenceTo(nullDiff, variable)) return null;
}
if(NullnessUtil.getExpressionNullness(nonNullDiff) != Nullness.NOT_NULL) return null;
if(!LambdaGenerationUtil.canBeUncheckedLambda(nonNullDiff)) return null;
return new NotNullContext(nonNullDiff, nullDiff, nullBranch, variable, ifStatement, toDelete);
}
}
static String getMethod(PsiExpression expression) {
return ExpressionUtils.isSimpleExpression(expression) ? "requireNonNullElse" : "requireNonNullElseGet";
}
@Nullable
private static PsiVariable extractVariable(@NotNull PsiBinaryExpression binOp) {
PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp);
PsiReferenceExpression referenceExpression = tryCast(value, PsiReferenceExpression.class);
if(referenceExpression == null) return null;
PsiVariable variable = tryCast(referenceExpression.resolve(), PsiVariable.class);
if(variable == null) return null;
return variable;
}
private static class TernaryNotNullContext {
private final @NotNull PsiConditionalExpression myTernary;
private final @NotNull PsiExpression myNonNullExpr;
private final @NotNull PsiVariable myVariable;
private TernaryNotNullContext(@NotNull PsiConditionalExpression ternary, @NotNull PsiExpression expr, @NotNull PsiVariable variable) {
myTernary = ternary;
myNonNullExpr = expr;
myVariable = variable;
}
@NotNull
public PsiExpression getNonNullExpr() {
return myNonNullExpr;
}
@NotNull
public PsiConditionalExpression getTernary() {
return myTernary;
}
@NotNull
public PsiVariable getVariable() {
return myVariable;
}
@Nullable
static NotNullContext from(PsiAssignmentExpression assignment) {
PsiConditionalExpression ternary = tryCast(assignment.getRExpression(), PsiConditionalExpression.class);
if(ternary == null) return null;
PsiReferenceExpression reference = tryCast(assignment.getLExpression(), PsiReferenceExpression.class);
if(reference == null) return null;
PsiVariable variable = tryCast(reference.resolve(), PsiVariable.class);
static TernaryNotNullContext from(@NotNull PsiConditionalExpression ternary) {
PsiBinaryExpression binOp = tryCast(ternary.getCondition(), PsiBinaryExpression.class);
if(binOp == null) return null;
final boolean negated;
IElementType tokenType = binOp.getOperationTokenType();
if(tokenType == JavaTokenType.NE) {
negated = true;
} else if(tokenType == JavaTokenType.EQEQ) {
negated = false;
} else return null;
if (!ExpressionUtils.isNullLiteral(ExpressionUtils.getOtherOperand(binOp, variable))) return null;
PsiVariable variable = extractVariable(binOp);
if(variable == null) return null;
boolean negated = binOp.getOperationTokenType() == JavaTokenType.NE;
PsiExpression nonNullBranch = negated ? ternary.getElseExpression() : ternary.getThenExpression();
if(ClassUtils.isPrimitive(variable.getType())) return null;
PsiExpression nullBranch = negated ? ternary.getThenExpression() : ternary.getElseExpression();
if(!ExpressionUtils.isReferenceTo(nullBranch, variable)) return null;
if(NullnessUtil.getExpressionNullness(nonNullBranch) != Nullness.NOT_NULL) return null;
if(!LambdaGenerationUtil.canBeUncheckedLambda(nonNullBranch)) return null;
return new TernaryNotNullContext(ternary, nonNullBranch, variable);
}
}
PsiExpression main = negated? ternary.getThenExpression() : ternary.getElseExpression();
if(!ExpressionUtils.isReferenceTo(main, variable)) return null;
PsiExpression alternative = negated? ternary.getElseExpression() : ternary.getThenExpression();
if(NullnessUtil.getExpressionNullness(alternative) != Nullness.NOT_NULL) return null;
if(!LambdaGenerationUtil.canBeUncheckedLambda(alternative)) return null;
return new NotNullContext(alternative, variable);
/**
* Represents difference between o1.m1().m2() and o2.m1().m2()
* Relies that call chain and arguments are exactly the same
*/
private static class TopmostQualifierDiff {
private final @Nullable PsiExpression myLeft;
private final @Nullable PsiExpression myRight;
private TopmostQualifierDiff(@Nullable PsiExpression left, @Nullable PsiExpression right) {
myLeft = left;
myRight = right;
}
@Nullable
public PsiExpression getRight() {
return myRight;
}
@Nullable
public PsiExpression getLeft() {
return myLeft;
}
@Nullable
static TopmostQualifierDiff from(@Nullable PsiExpression left, @Nullable PsiExpression right) {
PsiMethodCallExpression leftCall = tryCast(left, PsiMethodCallExpression.class);
PsiMethodCallExpression rightCall = tryCast(right, PsiMethodCallExpression.class);
if(leftCall == null || rightCall == null) return null;
while(true) {
PsiReferenceExpression leftMethodExpression = leftCall.getMethodExpression();
PsiReferenceExpression rightMethodExpression = rightCall.getMethodExpression();
if(tryCast(leftMethodExpression.resolve(), PsiMethod.class) != tryCast(rightMethodExpression.resolve(), PsiMethod.class)) return null;
PsiExpression[] leftExpressions = leftCall.getArgumentList().getExpressions();
PsiExpression[] rightExpressions = rightCall.getArgumentList().getExpressions();
int length = leftExpressions.length;
if(length != rightExpressions.length) return null;
for (int i = 0; i < length; i++) {
if(!ourEquivalence.expressionsAreEquivalent(leftExpressions[i], rightExpressions[i])) return null;
}
PsiExpression leftQualifier = leftMethodExpression.getQualifierExpression();
leftCall = tryCast(leftQualifier, PsiMethodCallExpression.class);
PsiExpression rightQualifier = rightMethodExpression.getQualifierExpression();
rightCall = tryCast(rightQualifier, PsiMethodCallExpression.class);
if(leftCall == null || rightCall == null) {
return new TopmostQualifierDiff(leftQualifier, rightQualifier);
}
}
}
}
}
@@ -0,0 +1,14 @@
// "Replace condition with Objects.requireNonNullElseGet" "true"
import java.util.*;
class Util {
String method(String s) {return s;}
}
class Test {
public void test(Object o) {
Object another;
another = Objects.requireNonNullElseGet(o, Object::new);
}
}
@@ -0,0 +1,14 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Util {
String method(String s) {return s;}
}
class Test {
public void test(String o) {
Object another;
Util.method(Objects.requireNonNullElse(o, "Hello"));
}
}
@@ -4,6 +4,6 @@ import java.util.*;
class Test {
public void test(Object o) {
o = Objects.requireNonNullElseGet(o, Object::new);
return Objects.requireNonNullElseGet(o, Object::new);
}
}
@@ -0,0 +1,13 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Util {
String method(String s) {return s;}
}
class Test {
public void test(String o) {
return Util.method(Objects.requireNonNullElse(o, "Adasdasdas").trim()).trim();
}
}
@@ -0,0 +1,9 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Test {
public void test(String o) {
return Objects.requireNonNullElse(o, "");
}
}
@@ -4,6 +4,6 @@ import java.util.*;
class Test {
public void test(Object o) {
o = Objects.requireNonNullElseGet(o, Object::new);
o = Objects.requireNonNullElseGet(o, Object::new);
}
}
@@ -0,0 +1,13 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Test {
void work(Object o) {
}
public void test(Object o) {
work(Objects.requireNonNullElse(o, ""));
}
}
@@ -4,6 +4,6 @@ import java.util.*;
class Test {
public void test(Object o) {
o = Objects.requireNonNullElse(o, "");
o = Objects.requireNonNullElse(o, "");
}
}
@@ -0,0 +1,18 @@
// "Replace condition with Objects.requireNonNullElseGet" "true"
import java.util.*;
class Util {
String method(String s) {return s;}
}
class Test {
public void test(Object o) {
Object another;
if<caret>(o == null) {
another = new Object();
} else {
another = o;
}
}
}
@@ -0,0 +1,18 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Util {
String method(String s) {return s;}
}
class Test {
public void test(String o) {
Object another;
if<caret>(o == null) {
Util.method("Hello");
} else {
Util.method(o);
}
}
}
@@ -5,7 +5,9 @@ import java.util.*;
class Test {
public void test(Object o) {
if<caret>(o == null) {
o = new Object();
return new Object();
} else {
return o;
}
}
}
@@ -0,0 +1,17 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Util {
String method(String s) {return s;}
}
class Test {
public void test(String o) {
if<caret>(o == null) {
return Util.method("Adasdasdas".trim()).trim();
} else {
return Util.method(o.trim()).trim();
}
}
}
@@ -3,9 +3,10 @@
import java.util.*;
class Test {
public void test(Object o) {
public void test(String o) {
if<caret>(o == null) {
o = "";
return "";
}
return o;
}
}
@@ -0,0 +1,15 @@
// "Replace condition with Objects.requireNonNullElse" "false"
import java.util.*;
class Test {
void work(Object o) {}
private static Object nullable() {
return null;
}
public void test(Object o) {
work(o == null? "" <caret>: nullable());
}
}
@@ -0,0 +1,13 @@
// "Replace condition with Objects.requireNonNullElse" "true"
import java.util.*;
class Test {
void work(Object o) {
}
public void test(Object o) {
work(o <caret>== null? "" : o);
}
}
@@ -800,6 +800,8 @@ inspection.simplify.collector.message=Can be simplified using ''{0}'' collector
inspection.simplify.collector.fix.family.name=Simplify cascaded collector
inspection.simplify.collector.fix.name=Use ''Collectors.{0}'' collector
inspection.require.non.null.message=Replace condition with Objects.{0}
inspection.redundant.stream.optional.call.message=Redundant ''{0}'' call
inspection.redundant.stream.optional.call.explanation.filter=predicate is always true
inspection.redundant.stream.optional.call.explanation.sorted=subsequent ''{0}'' call makes sorting useless
@@ -2,7 +2,7 @@
<body>
This inspection suggests to replace null check with Objects static method.
Example:
<code>if(o == null) { o = ""; }</code> can be replaced with <code>o = Objects.requireNonNullElse(o, "");</code>
<p><code>if(o == null) { o = ""; }</code><p> can be replaced with </p><code>o = Objects.requireNonNullElse(o, "");</code></p>
<p><small>New in 2017.3</small></p>
</body>
</html>
+2 -2
View File
@@ -969,9 +969,9 @@
displayName="Null value for Optional type"/>
<localInspection groupPath="Java" language="JAVA" shortName="RequireNonNull"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.code.style.issues" enabledByDefault="true" level="INFORMATION"
groupKey="group.names.language.level.specific.issues.and.migration.aids9" enabledByDefault="true" level="INFORMATION"
implementationClass="com.intellij.codeInspection.RequireNonNullInspection"
displayName="Null value for Optional type"/>
displayName="Replace null checked condition with Objects static method"/>
<localInspection groupPath="Java" language="JAVA" shortName="Java9ModuleExportsPackageToItself"
groupBundle="messages.InspectionsBundle" groupKey="group.names.visibility.issues"
enabledByDefault="true" level="WARNING"