MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IG: rewrite "'Optional.get()' without 'isPresent()' check" inspection using data flow analysis

Browse Source
This commit is contained in:
Bas Leijdekkers
2016-09-20 17:37:17 +02:00
parent 41757070ea
commit 0f2b9539d8
4 changed files with 216 additions and 203 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/OptionalGetWithoutIsPresentInspection.java
+81 -200
View File
@@ -15,14 +15,16 @@
*/
package com.siyeh.ig.bugs;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.*;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
@@ -40,9 +42,7 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
@NotNull
@Override
protected String buildErrorString(Object... infos) {
final PsiType type = (PsiType)infos[0];
final PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(type);
assert aClass != null;
final PsiClass aClass = (PsiClass)infos[0];
return InspectionGadgetsBundle.message("optional.get.without.is.present.problem.descriptor", aClass.getName());
}
@@ -54,221 +54,102 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
private static class OptionalGetWithoutIsPresentVisitor extends BaseInspectionVisitor {
@Override
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
super.visitMethodCallExpression(expression);
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
final String name = methodExpression.getReferenceName();
if (!"get".equals(name) && !"getAsDouble".equals(name) && !"getAsInt".equals(name) && !"getAsLong".equals(name)) {
return;
}
final PsiExpression qualifier = ParenthesesUtils.stripParentheses(methodExpression.getQualifierExpression());
if (qualifier == null) {
return;
}
final PsiType type = qualifier.getType();
if (!TypeUtils.isOptional(type)) {
return;
}
if (qualifier instanceof PsiReferenceExpression) {
final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier;
if (isSurroundedByIsPresentGuard(referenceExpression, expression)) {
return;
}
}
registerMethodCallError(expression, type);
public void visitMethod(PsiMethod method) {
check(method.getBody());
}
}
private static boolean isSurroundedByIsPresentGuard(PsiReferenceExpression optionalReference, PsiElement context) {
PsiStatement sibling = PsiTreeUtil.getParentOfType(context, PsiStatement.class);
sibling = PsiTreeUtil.getPrevSiblingOfType(sibling, PsiStatement.class);
final IsPresentChecker checker = new IsPresentChecker(optionalReference);
while (sibling != null) {
if (sibling instanceof PsiIfStatement) {
final PsiIfStatement ifStatement = (PsiIfStatement)sibling;
final PsiExpression condition = ifStatement.getCondition();
if (condition != null) {
final PsiElement target = optionalReference.resolve();
if (!(target instanceof PsiVariable)) {
return true;
}
final PsiVariable variable = (PsiVariable)target;
final PsiStatement thenBranch = ifStatement.getThenBranch();
if (!ControlFlowUtils.statementMayCompleteNormally(thenBranch) || VariableAccessUtils.variableIsAssigned(variable, thenBranch)) {
checker.negate = true;
if (checker.checkExpression(condition)) {
return true;
}
}
final PsiStatement elseBranch = ifStatement.getElseBranch();
if (!ControlFlowUtils.statementMayCompleteNormally(elseBranch) || VariableAccessUtils.variableIsAssigned(variable, elseBranch)) {
checker.negate = false;
if (checker.checkExpression(condition)) {
return true;
@Override
public void visitClassInitializer(PsiClassInitializer initializer) {
check(initializer.getBody());
}
@Override
public void visitField(PsiField field) {
check(field.getInitializer());
}
private void check(PsiElement element) {
if (!containsOptionalGetCall(element)) {
return;
}
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(false, true, isOnTheFly());
dfaRunner.analyzeMethod(element, new InstructionVisitor() {
@Override
public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
DfaValue dfaSource = memState.pop();
DfaValue dfaDest = memState.pop();
if (dfaDest instanceof DfaVariableValue) {
DfaVariableValue var = (DfaVariableValue)dfaDest;
final PsiModifierListOwner psi = var.getPsiVariable();
if (!(psi instanceof PsiField) || !psi.hasModifierProperty(PsiModifier.VOLATILE)) {
memState.setVarValue(var, dfaSource);
}
}
memState.push(dfaDest);
return nextInstruction(instruction, runner, memState);
}
}
else if (sibling instanceof PsiWhileStatement) {
final PsiWhileStatement whileStatement = (PsiWhileStatement)sibling;
final PsiExpression condition = whileStatement.getCondition();
checker.negate = true;
if (checker.checkExpression(condition)) {
return true;
}
}
else if (sibling instanceof PsiAssertStatement) {
final PsiAssertStatement assertStatement = (PsiAssertStatement)sibling;
final PsiExpression condition = assertStatement.getAssertCondition();
checker.negate = false;
if (checker.checkExpression(condition)) {
return true;
}
}
else if (sibling instanceof PsiExpressionStatement) {
final PsiExpressionStatement expressionStatement = (PsiExpressionStatement)sibling;
final PsiExpression expression = expressionStatement.getExpression();
if (expression instanceof PsiMethodCallExpression) {
final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression;
if (MethodCallUtils.isCallToMethod(methodCallExpression, "org.junit.Assert", PsiType.VOID, "assertTrue", null) ||
MethodCallUtils.isCallToMethod(methodCallExpression, "junit.framework.Assert", PsiType.VOID, "assertTrue", null) ||
MethodCallUtils.isCallToMethod(methodCallExpression, "org.testng.Assert", PsiType.VOID, "assertTrue", null) ||
MethodCallUtils.isCallToMethod(methodCallExpression, "org.testng.AssertJUnit", PsiType.VOID, "assertTrue", null)) {
checker.negate = false;
final PsiExpressionList argumentList = methodCallExpression.getArgumentList();
final PsiExpression[] arguments = argumentList.getExpressions();
for (PsiExpression argument : arguments) {
if (checker.checkExpression(argument)) {
return true;
@Override
public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
final DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState);
final PsiMethod targetMethod = instruction.getTargetMethod();
if (targetMethod != null) {
final PsiClass aClass = targetMethod.getContainingClass();
if (TypeUtils.isOptional(aClass)) {
final String name = targetMethod.getName();
if (name.equals("isPresent")) {
memState.pop();
memState.push(runner.getFactory().getConstFactory().getFalse());
}
else if (name.equals("get") || name.equals("getAsDouble") || name.equals("getAsInt") || name.equals("getAsLong")) {
final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)instruction.getCallExpression();
if (methodCallExpression != null) {
registerMethodCallError(methodCallExpression, aClass);
}
}
}
}
return states;
}
}
sibling = PsiTreeUtil.getPrevSiblingOfType(sibling, PsiStatement.class);
}
checker.negate = false;
PsiElement parent = PsiTreeUtil.getParentOfType(context, PsiIfStatement.class, PsiWhileStatement.class, PsiConditionalExpression.class,
PsiPolyadicExpression.class);
while (parent != null) {
if (parent instanceof PsiPolyadicExpression) {
final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)parent;
if (JavaTokenType.OROR.equals(polyadicExpression.getOperationTokenType())) {
checker.negate = true;
}
}
parent.accept(checker);
if (checker.hasIsPresentCall()) {
return true;
}
parent = PsiTreeUtil.getParentOfType(parent, PsiIfStatement.class, PsiWhileStatement.class, PsiConditionalExpression.class,
PsiPolyadicExpression.class);
}
return checker.hasIsPresentCall();
}
private static class IsPresentChecker extends JavaElementVisitor {
private final PsiReferenceExpression referenceExpression;
private boolean negate = false;
private boolean hasIsPresentCall = false;
IsPresentChecker(PsiReferenceExpression referenceExpression) {
this.referenceExpression = referenceExpression;
});
}
@Override
public void visitReferenceExpression(PsiReferenceExpression expression) {
visitExpression(expression);
private static boolean containsOptionalGetCall(PsiElement element) {
if (element == null) return false;
final OptionalGetCallChecker checker = new OptionalGetCallChecker();
element.acceptChildren(checker);
return checker.containsOptionalGetCall();
}
@Override
public void visitPolyadicExpression(PsiPolyadicExpression expression) {
final IElementType tokenType = expression.getOperationTokenType();
if (tokenType != JavaTokenType.ANDAND && tokenType != JavaTokenType.OROR) {
return;
}
for (PsiExpression operand : expression.getOperands()) {
if (PsiTreeUtil.isAncestor(operand, referenceExpression, false)) {
private static class OptionalGetCallChecker extends JavaRecursiveElementWalkingVisitor {
private boolean result = false;
@Override
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
if (result) {
return;
}
checkExpression(operand);
if (hasIsPresentCall) {
return;
}
}
}
@Override
public void visitWhileStatement(PsiWhileStatement statement) {
checkExpression(statement.getCondition());
}
@Override
public void visitIfStatement(PsiIfStatement ifStatement) {
final PsiStatement elseBranch = ifStatement.getElseBranch();
negate = elseBranch != null && PsiTreeUtil.isAncestor(elseBranch, referenceExpression, true);
final PsiStatement statement = negate ? elseBranch : ifStatement.getThenBranch();
if (statement instanceof PsiBlockStatement) {
final PsiBlockStatement blockStatement = (PsiBlockStatement)statement;
if (VariableAccessUtils.variableIsAssignedBeforeReference(referenceExpression, blockStatement)) {
return;
}
}
checkExpression(ifStatement.getCondition());
}
@Override
public void visitConditionalExpression(PsiConditionalExpression expression) {
final PsiExpression elseExpression = expression.getElseExpression();
negate = elseExpression != null && PsiTreeUtil.isAncestor(elseExpression, referenceExpression, true);
checkExpression(expression.getCondition());
}
private boolean checkExpression(PsiExpression expression) {
expression = PsiUtil.deparenthesizeExpression(expression);
if (expression instanceof PsiPrefixExpression) {
final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression;
final IElementType tokenType = prefixExpression.getOperationTokenType();
if (tokenType != JavaTokenType.EXCL) {
return false;
}
negate = !negate;
return checkExpression(prefixExpression.getOperand());
}
else if (expression instanceof PsiPolyadicExpression) {
final PsiPolyadicExpression binaryExpression = (PsiPolyadicExpression)expression;
visitPolyadicExpression(binaryExpression);
}
else if (expression instanceof PsiMethodCallExpression) {
final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression;
final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression();
super.visitMethodCallExpression(expression);
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
final String name = methodExpression.getReferenceName();
if (!"isPresent".equals(name)) {
return false;
if (!"get".equals(name) && !"getAsDouble".equals(name) && !"getAsInt".equals(name) && !"getAsLong".equals(name)) {
return;
}
final PsiExpression qualifier = ParenthesesUtils.stripParentheses(methodExpression.getQualifierExpression());
if (!(qualifier instanceof PsiReferenceExpression)) {
return false;
if (qualifier == null) {
return;
}
final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier;
hasIsPresentCall = !negate && EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(referenceExpression, this.referenceExpression);
}
else if (expression instanceof PsiReferenceExpression) {
final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression;
final PsiExpression definition = DeclarationSearchUtils.findDefinition(referenceExpression, null);
final PsiExpression optionalDefinition = DeclarationSearchUtils.findDefinition(this.referenceExpression, null);
if (definition == null || optionalDefinition == null || optionalDefinition.getTextOffset() > definition.getTextOffset()) {
return false;
final PsiType type = qualifier.getType();
if (!TypeUtils.isOptional(type)) {
return;
}
return checkExpression(definition);
result = true;
}
return hasIsPresentCall;
}
public boolean hasIsPresentCall() {
return hasIsPresentCall;
public boolean containsOptionalGetCall() {
return result;
}
}
}
}
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/TypeUtils.java
+4 -1
View File
@@ -85,7 +85,10 @@ public class TypeUtils {
}
public static boolean isOptional(@Nullable PsiType type) {
final PsiClass aClass = PsiUtil.resolveClassInClassTypeOnly(type);
return isOptional(PsiUtil.resolveClassInClassTypeOnly(type));
}
public static boolean isOptional(PsiClass aClass) {
if (aClass == null) {
return false;
}
plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/optional_get_without_is_present/OptionalGetWithoutIsPresent.java
+113 -2
View File
@@ -12,7 +12,7 @@ class OptionalWithoutIsPresent {
}
if (maybe.isPresent()) {
maybe = Optional.empty();
System.out.println(maybe.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
System.out.println(maybe.get());
}
boolean b = ((maybe.isPresent())) && maybe.get() == 1;
boolean c = (!maybe.isPresent()) || maybe.get() == 1;
@@ -34,7 +34,7 @@ class OptionalWithoutIsPresent {
final boolean present = optional.isPresent();
optional = Optional.empty();
if (present) {
final String string = optional.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>();
final String string = optional.get();
System.out.println(string);
}
}
@@ -56,8 +56,119 @@ class OptionalWithoutIsPresent {
if (! holder.isPresent()) {
holder = Optional.of("hello world");
if (!holder.isPresent()) {
return null;
}
}
return holder.get();
}
//public Collection<String> findCategories(Long articleId) {
// return java.util.stream.Stream.of(Optional.of("asdf")).filter(Optional::isPresent).map(x -> x.get()).collect(
// java.util.stream.Collectors.toList()) ;
//}
public static void main(String[] args) {
Optional<String> stringOpt;
if((stringOpt = getOptional()).isPresent()) {
stringOpt.get();
}
}
public static void main2(String[] args) {
if(getOptional().isPresent()) {
getOptional().get(); //'Optional.get()' without 'isPresent()' check
}
}
public static Optional<String> getOptional() {
return Optional.empty();
}
//void order(Optional<String> order) {
// order.ifPresent(o -> System.out.println(order.get()));
//}
public static void two(Optional<Object> o1,Optional<Object> o2) {
if (!o1.isPresent() && !o2.isPresent()) {
return;
}
System.out.println(o1.isPresent() ? o1.get() : o2.get());
}
public static void two2(Optional<Object> o1,Optional<Object> o2) {
if (!o2.isPresent()) {
return;
}
System.out.println(o1.isPresent() ? o1.get() : o2.get());
}
public static void two3(Optional<Object> o1,Optional<Object> o2) {
System.out.println(o1.isPresent() ? o1.get() : o2.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>());
}
void def(Optional<String> def) {
if (!def.isPresent()) {
throw new RuntimeException();
}
{
def.get();
}
}
private void orred(Optional<Integer> opt1, Optional<Integer> opt2) {
if (!opt1.isPresent() || !opt2.isPresent()) {
return;
}
opt1.get();
opt2.get();
}
class Range {
Optional<Integer> min = Optional.of(1);
Optional<Integer> max = Optional.of(2);
Optional<Integer> getMax() {
return max;
}
Optional<Integer> getMin() {
return min;
}
}
void useRange(Range a, Range b) {
if (!a.getMax().isPresent() || !b.getMin().isPresent()) {
}
else if (a.getMax().get() <= b.getMin().get()) {
}
}
public void foo(Optional<Long> value) {
if (!value.isPresent()) {
return;
}
try {
System.out.println(value.get()); // <- warning appears here
} finally {
System.out.println("hi");
}
}
void shortIf(Optional<String> o) {
if (true || o.isPresent()) {
o.<warning descr="'Optional.get()' without 'isPresent()' check">get</warning>();
}
}
void test(Optional<String> aOpt, Optional<String> bOpt) {
if (!aOpt.isPresent() || !bOpt.isPresent()) {
throw new RuntimeException();
}
String a = aOpt.get();
String b = bOpt.get();
}
}
plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/OptionalGetWithoutIsPresentInspectionTest.java
+18
View File
@@ -111,6 +111,24 @@ public class OptionalGetWithoutIsPresentInspectionTest extends LightInspectionTe
"}");
}
public void testNested() {
doTest("import java.util.Optional;" +
"class X {" +
" void test(Optional<String> opt, String action) {" +
" if(!opt.isPresent()) {" +
" throw new IllegalArgumentException();" +
" }" +
" switch(action) {" +
" case \"case\":" +
" System.out.println(opt.get());" +
" break;" +
" default:" +
" System.err.println(opt.get());" +
" }" +
" }" +
"}");
}
public void testOptionalGetWithoutIsPresent() {
doTest();
}