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

OptionalGetWithoutIsPresentInspection: removed IsPresentCheck (instead used two instruction states) (IDEA-CR-18052)

Browse Source
This commit is contained in:
Tagir Valeev
2017-02-10 11:55:43 +07:00
parent 415c664c85
commit 123e339952
7 changed files with 67 additions and 177 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaBoxedValue.java
+2 -2
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
* Copyright 2000-2009 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -23,7 +23,7 @@ import org.jetbrains.annotations.Nullable;
import java.util.Map;
public class DfaBoxedValue extends DfaValue implements DfaComparableValue {
public class DfaBoxedValue extends DfaValue {
private final DfaValue myWrappedValue;
private DfaBoxedValue(DfaValue valueToWrap, DfaValueFactory factory) {
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaComparableValue.java
-24
View File
@@ -1,24 +0,0 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.intellij.codeInspection.dataFlow.value;
/**
* Marker interface for automatic relation creation
*
* @author Tagir Valeev
*/
public interface DfaComparableValue {
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaRelationValue.java
+4 -3
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
* Copyright 2000-2009 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -54,8 +54,9 @@ public class DfaRelationValue extends DfaValue {
public DfaRelationValue createRelation(DfaValue dfaLeft, DfaValue dfaRight, IElementType relation, boolean negated) {
if (PLUS == relation) return null;
if (dfaLeft instanceof DfaComparableValue || dfaRight instanceof DfaComparableValue) {
if (!(dfaLeft instanceof DfaComparableValue)) {
if (dfaLeft instanceof DfaVariableValue || dfaLeft instanceof DfaBoxedValue || dfaLeft instanceof DfaUnboxedValue
|| dfaRight instanceof DfaVariableValue || dfaRight instanceof DfaBoxedValue || dfaRight instanceof DfaUnboxedValue) {
if (!(dfaLeft instanceof DfaVariableValue || dfaLeft instanceof DfaBoxedValue || dfaLeft instanceof DfaUnboxedValue)) {
return createRelation(dfaRight, dfaLeft, getSymmetricOperation(relation), negated);
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaUnboxedValue.java
+2 -2
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2017 JetBrains s.r.o.
* Copyright 2000-2009 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,7 +17,7 @@ package com.intellij.codeInspection.dataFlow.value;
import org.jetbrains.annotations.NonNls;
public class DfaUnboxedValue extends DfaValue implements DfaComparableValue {
public class DfaUnboxedValue extends DfaValue {
private final DfaVariableValue myVariable;
DfaUnboxedValue(DfaVariableValue valueToWrap, DfaValueFactory factory) {
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java
+1 -1
View File
@@ -46,7 +46,7 @@ import java.util.List;
import static com.intellij.patterns.PsiJavaPatterns.*;
public class DfaVariableValue extends DfaValue implements DfaComparableValue {
public class DfaVariableValue extends DfaValue {
private static final ElementPattern<? extends PsiModifierListOwner> MEMBER_OR_METHOD_PARAMETER =
or(psiMember(), psiParameter().withSuperParent(2, psiMember()));
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/OptionalGetWithoutIsPresentInspection.java
+51 -142
View File
@@ -17,7 +17,9 @@ package com.siyeh.ig.bugs;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ObjectUtils;
@@ -27,7 +29,6 @@ import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import gnu.trove.TIntObjectHashMap;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -92,12 +93,13 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
private void analyze(PsiElement context) {
final DataFlowRunner dfaRunner = new StandardDataFlowRunner(false, true, isOnTheFly()) {
private final OptionalValueFactory myOptionalFactory = new OptionalValueFactory(getFactory());
private final DfaOptionalValue myPresentValue = new DfaOptionalValue(getFactory(), true);
private final DfaOptionalValue myAbsentValue = new DfaOptionalValue(getFactory(), false);
@NotNull
@Override
protected DfaMemoryState createMemoryState() {
return new OptionalMemoryState(getFactory(), myOptionalFactory);
return new OptionalMemoryState(getFactory(), myPresentValue, myAbsentValue);
}
};
dfaRunner.analyzeMethod(context, new StandardInstructionVisitor() {
@@ -110,8 +112,22 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
OptionalMemoryState optionalMemState = (OptionalMemoryState)memState;
if (qualifier != null && TypeUtils.isOptional(qualifier.getType())) {
if ("isPresent".equals(methodName)) {
DfaValue result = optionalMemState.createIsPresentCheckResult(memState.peek());
return replaceResult(instruction, runner, memState, result);
DfaValue qualifierValue = optionalMemState.peek();
ThreeState state = optionalMemState.checkOptional(qualifierValue);
if(state == ThreeState.UNSURE) {
DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState);
if(states.length != 1) return states;
OptionalMemoryState trueState = (OptionalMemoryState)states[0].getMemoryState();
trueState.pop();
OptionalMemoryState falseState = trueState.createCopy();
trueState.push(optionalMemState.getBoolean(true));
trueState.applyIsPresentCheck(true, qualifierValue);
falseState.push(optionalMemState.getBoolean(false));
falseState.applyIsPresentCheck(false, qualifierValue);
return new DfaInstructionState[] {states[0], new DfaInstructionState(states[0].getInstruction(), falseState)};
} else {
return replaceResult(instruction, runner, memState, optionalMemState.getBoolean(state.toBoolean()));
}
}
else if (isOptionalGetMethodName(methodName)) {
ThreeState state = optionalMemState.checkOptional(memState.peek());
@@ -121,7 +137,7 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
if ("of".equals(methodName)) {
PsiMethod method = call.resolveMethod();
if (method != null && TypeUtils.isOptional(method.getContainingClass())) {
return replaceResult(instruction, runner, memState, optionalMemState.getOptionalFactory().getOptional(true));
return replaceResult(instruction, runner, memState, optionalMemState.getOptional(true));
}
}
if ("ofNullable".equals(methodName)) {
@@ -130,14 +146,14 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
TypeUtils.isOptional(method.getContainingClass()) &&
call.getArgumentList().getExpressions().length == 1) {
if(memState.isNotNull(memState.peek())) {
return replaceResult(instruction, runner, memState, optionalMemState.getOptionalFactory().getOptional(true));
return replaceResult(instruction, runner, memState, optionalMemState.getOptional(true));
}
}
}
if ("empty".equals(methodName)) {
PsiMethod method = call.resolveMethod();
if (method != null && TypeUtils.isOptional(method.getContainingClass())) {
return replaceResult(instruction, runner, memState, optionalMemState.getOptionalFactory().getOptional(false));
return replaceResult(instruction, runner, memState, optionalMemState.getOptional(false));
}
}
}
@@ -175,169 +191,62 @@ public class OptionalGetWithoutIsPresentInspection extends BaseInspection {
return "get".equals(name) || "getAsDouble".equals(name) || "getAsInt".equals(name) || "getAsLong".equals(name);
}
static class OptionalValueFactory {
private final DfaOptionalValue myPresentOptional, myAbsentOptional;
private final DfaValueFactory myFactory;
private final TIntObjectHashMap<IsPresentCheck> myPresentChecks = new TIntObjectHashMap<>();
OptionalValueFactory(DfaValueFactory factory) {
myPresentOptional = new DfaOptionalValue(factory, true);
myAbsentOptional = new DfaOptionalValue(factory, false);
myFactory = factory;
}
IsPresentCheck getIsPresentCheck(DfaValue optional) {
int id = optional.getID();
IsPresentCheck check = myPresentChecks.get(id);
if(check == null) {
myPresentChecks.put(id, check = new IsPresentCheck(myFactory, optional, null));
}
return check;
}
DfaOptionalValue getOptional(boolean present) {
return present ? myPresentOptional : myAbsentOptional;
}
}
static class OptionalMemoryState extends DfaMemoryStateImpl {
private final OptionalValueFactory myOptionalFactory;
private final DfaOptionalValue myPresentValue;
private final DfaOptionalValue myAbsentValue;
protected OptionalMemoryState(DfaValueFactory factory,
OptionalValueFactory optionalFactory) {
public OptionalMemoryState(DfaValueFactory factory,
DfaOptionalValue presentValue,
DfaOptionalValue absentValue) {
super(factory);
myOptionalFactory = optionalFactory;
myPresentValue = presentValue;
myAbsentValue = absentValue;
}
protected OptionalMemoryState(OptionalMemoryState toCopy) {
super(toCopy);
myOptionalFactory = toCopy.myOptionalFactory;
public OptionalMemoryState(OptionalMemoryState state) {
super(state);
myPresentValue = state.myPresentValue;
myAbsentValue = state.myAbsentValue;
}
public OptionalValueFactory getOptionalFactory() {
return myOptionalFactory;
public DfaValue getBoolean(boolean val) {
return val ? getFactory().getConstFactory().getTrue() : getFactory().getConstFactory().getFalse();
}
@Override
public boolean applyCondition(final DfaValue dfaCond) {
if (dfaCond instanceof DfaRelationValue) {
DfaRelationValue relation = (DfaRelationValue)dfaCond;
if (relation.isEquality() || relation.isNonEquality()) {
DfaValue left = relation.getLeftOperand();
DfaValue right = relation.getRightOperand();
DfaConstValue constValue = null;
DfaValue nonConst = null;
if (left instanceof DfaConstValue) {
constValue = (DfaConstValue)left;
nonConst = right;
}
else if (right instanceof DfaConstValue) {
constValue = (DfaConstValue)right;
nonConst = left;
}
if (constValue != null) {
IsPresentCheck check = unwrapValue(nonConst, IsPresentCheck.class);
Object value = constValue.getValue();
if (value instanceof Boolean && check != null) {
boolean present = ((Boolean)value).booleanValue() ^ relation.isNonEquality();
applyIsPresentCheck(present ? check : check.createNegated());
}
}
}
}
IsPresentCheck check = unwrapValue(dfaCond, IsPresentCheck.class);
if (check != null) {
return applyIsPresentCheck(check);
}
return super.applyCondition(dfaCond);
}
@NotNull
DfaValue createIsPresentCheckResult(DfaValue qualifierValue) {
ThreeState state = checkOptional(qualifierValue);
switch (state) {
case YES:
return getFactory().getConstFactory().getTrue();
case NO:
return getFactory().getConstFactory().getFalse();
case UNSURE:
return myOptionalFactory.getIsPresentCheck(qualifierValue);
}
throw new IllegalStateException();
public DfaValue getOptional(boolean present) {
return present ? myPresentValue : myAbsentValue;
}
@NotNull
@Override
protected DfaVariableState createVariableState(@NotNull DfaVariableValue var) {
if (var.isNegated()) {
DfaVariableState negatedState = getVariableState(var.createNegated());
DfaValue negatedValue = negatedState.getValue();
if (negatedValue != null) {
return negatedState.withValue(negatedValue.createNegated());
}
}
return new ValuableDataFlowRunner.ValuableDfaVariableState(var);
}
private <T extends DfaValue> T unwrapValue(DfaValue dfaCond, Class<T> aClass) {
if (dfaCond instanceof DfaVariableValue) {
DfaVariableValue var = (DfaVariableValue)dfaCond;
public ThreeState checkOptional(DfaValue value) {
if (value instanceof DfaVariableValue) {
DfaVariableValue var = (DfaVariableValue)value;
DfaValue newCond = getVariableState(var).getValue();
if (newCond == null) {
newCond = getVariableState(var.createNegated()).getValue();
T check = unwrapValue(newCond, aClass);
return check == null ? null : ObjectUtils.tryCast(check.createNegated(), aClass);
return checkOptional(getVariableState(var.createNegated()).getValue());
}
return unwrapValue(newCond, aClass);
return checkOptional(newCond);
}
return ObjectUtils.tryCast(dfaCond, aClass);
return value instanceof DfaOptionalValue ? ThreeState.fromBoolean(((DfaOptionalValue)value).myPresent) : ThreeState.UNSURE;
}
private boolean applyIsPresentCheck(IsPresentCheck check) {
DfaOptionalValue optional = unwrapValue(check.myOptional, DfaOptionalValue.class);
if (optional != null) {
return optional.myPresent == check.myNegated;
void applyIsPresentCheck(boolean present, DfaValue qualifier) {
if (qualifier instanceof DfaVariableValue) {
DfaVariableValue optionalVar = (DfaVariableValue)qualifier;
setVariableState(optionalVar, getVariableState(optionalVar).withValue(getOptional(present)));
}
if (check.myOptional instanceof DfaVariableValue) {
DfaVariableValue optionalVar = (DfaVariableValue)check.myOptional;
setVariableState(optionalVar, getVariableState(optionalVar)
.withValue(myOptionalFactory.getOptional(!check.myNegated)));
}
return true;
}
@NotNull
@Override
public DfaMemoryStateImpl createCopy() {
public OptionalMemoryState createCopy() {
return new OptionalMemoryState(this);
}
public ThreeState checkOptional(DfaValue value) {
DfaOptionalValue optional = unwrapValue(value, DfaOptionalValue.class);
return optional == null ? ThreeState.UNSURE : ThreeState.fromBoolean(optional.myPresent);
}
}
}
static class IsPresentCheck extends DfaValue implements DfaComparableValue {
final @NotNull DfaValue myOptional;
final boolean myNegated;
final IsPresentCheck myInverted;
protected IsPresentCheck(DfaValueFactory factory, @NotNull DfaValue optional, IsPresentCheck positiveCheck) {
super(factory);
myOptional = optional;
myNegated = positiveCheck != null;
myInverted = positiveCheck == null ? new IsPresentCheck(factory, optional, this) : positiveCheck;
}
@Override
public IsPresentCheck createNegated() {
return myInverted;
}
public String toString() {
return (myNegated ? "!" : "") + myOptional + ".isPresent()";
}
}
plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/optional_get_without_is_present/OptionalGetWithoutIsPresent.java
+7 -3
View File
@@ -84,15 +84,19 @@ class OptionalWithoutIsPresent {
}
private void checkAsserts1() {
Optional<String> o1 = Optional.empty();
Optional<String> o1 = getOptional();
assert o1.isPresent();
System.out.println(o1.get());
Optional<String> o2 = Optional.empty();
Optional<String> o2 = getOptional();
org.junit.Assert.assertTrue(o2.isPresent());
System.out.println(o2.get());
Optional<String> o3 = Optional.empty();
Optional<String> o3 = getOptional();
org.testng.Assert.assertTrue(o3.isPresent());
System.out.println(o3.get());
o2 = getOptional();
org.junit.Assert.assertTrue(!o2.isPresent());
System.out.println(o2.<warning descr="'Optional.get()' will definitely fail as Optional is empty here">get</warning>());
}
private void checkAsserts2() {