mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
[java-analysis] Report overwritten fields via DFA CFG
Fixes IDEA-195460 Report i++ when changed value is not used afterwards doesn't work for fields Fixes IDEA-233847 "Unused assignment" does not work for field assignments in constructor GitOrigin-RevId: 6c43b53df5dfd1faf9c06bf3164f59c886db6e84
This commit is contained in:
committed by
intellij-monorepo-bot
parent
dff645ea83
commit
e5d9924dd2
+118
@@ -0,0 +1,118 @@
|
||||
// Copyright 2000-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
|
||||
package com.intellij.codeInspection.dataFlow;
|
||||
|
||||
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
|
||||
import com.intellij.codeInspection.dataFlow.instructions.ReturnInstruction;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
|
||||
import com.intellij.openapi.progress.ProgressManager;
|
||||
import com.intellij.openapi.util.Pair;
|
||||
import com.intellij.util.containers.ContainerUtil;
|
||||
import com.intellij.util.containers.FilteringIterator;
|
||||
import com.intellij.util.containers.MultiMap;
|
||||
import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
|
||||
import it.unimi.dsi.fastutil.ints.IntSet;
|
||||
import one.util.streamex.IntStreamEx;
|
||||
|
||||
import java.util.*;
|
||||
import java.util.function.BiFunction;
|
||||
|
||||
public abstract class BaseVariableAnalyzer {
|
||||
protected final Instruction[] myInstructions;
|
||||
protected final MultiMap<Instruction, Instruction> myForwardMap;
|
||||
protected final MultiMap<Instruction, Instruction> myBackwardMap;
|
||||
protected final DfaValueFactory myFactory;
|
||||
|
||||
public BaseVariableAnalyzer(ControlFlow flow) {
|
||||
myFactory = flow.getFactory();
|
||||
myInstructions = flow.getInstructions();
|
||||
myForwardMap = calcForwardMap();
|
||||
myBackwardMap = calcBackwardMap();
|
||||
}
|
||||
|
||||
private List<Instruction> getSuccessors(Instruction ins) {
|
||||
return IntStreamEx.of(LoopAnalyzer.getSuccessorIndices(ins.getIndex(), myInstructions)).elements(myInstructions).toList();
|
||||
}
|
||||
|
||||
protected MultiMap<Instruction, Instruction> calcBackwardMap() {
|
||||
MultiMap<Instruction, Instruction> result = MultiMap.create();
|
||||
for (Instruction instruction : myInstructions) {
|
||||
for (Instruction next : myForwardMap.get(instruction)) {
|
||||
result.putValue(next, instruction);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
protected MultiMap<Instruction, Instruction> calcForwardMap() {
|
||||
MultiMap<Instruction, Instruction> result = MultiMap.create();
|
||||
for (Instruction instruction : myInstructions) {
|
||||
if (isInterestingInstruction(instruction)) {
|
||||
for (Instruction next : getSuccessors(instruction)) {
|
||||
while (true) {
|
||||
if (isInterestingInstruction(next)) {
|
||||
result.putValue(instruction, next);
|
||||
break;
|
||||
}
|
||||
if (next.getIndex() + 1 >= myInstructions.length) {
|
||||
break;
|
||||
}
|
||||
next = myInstructions[next.getIndex() + 1];
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
protected abstract boolean isInterestingInstruction(Instruction instruction);
|
||||
|
||||
/**
|
||||
* @return true if completed, false if "too complex"
|
||||
*/
|
||||
protected boolean runDfa(boolean forward, BiFunction<? super Instruction, ? super BitSet, ? extends BitSet> handleState) {
|
||||
Set<Instruction> entryPoints = new HashSet<>();
|
||||
if (forward) {
|
||||
entryPoints.add(myInstructions[0]);
|
||||
}
|
||||
else {
|
||||
entryPoints.addAll(ContainerUtil.findAll(myInstructions, FilteringIterator.instanceOf(ReturnInstruction.class)));
|
||||
}
|
||||
|
||||
Deque<InstructionState> queue = new ArrayDeque<>(10);
|
||||
for (Instruction i : entryPoints) {
|
||||
queue.addLast(new InstructionState(i, new BitSet()));
|
||||
}
|
||||
|
||||
int limit = myForwardMap.size() * 100;
|
||||
Map<BitSet, IntSet> processed = new HashMap<>();
|
||||
int steps = 0;
|
||||
while (!queue.isEmpty()) {
|
||||
if (steps > limit) {
|
||||
return false;
|
||||
}
|
||||
if (steps % 1024 == 0) {
|
||||
ProgressManager.checkCanceled();
|
||||
}
|
||||
InstructionState state = queue.removeFirst();
|
||||
Instruction instruction = state.first;
|
||||
Collection<Instruction> nextInstructions = forward ? myForwardMap.get(instruction) : myBackwardMap.get(instruction);
|
||||
BitSet nextVars = handleState.apply(instruction, state.second);
|
||||
for (Instruction next : nextInstructions) {
|
||||
IntSet instructionSet = processed.computeIfAbsent(nextVars, k -> new IntOpenHashSet());
|
||||
int index = next.getIndex() + 1;
|
||||
if (!instructionSet.contains(index)) {
|
||||
instructionSet.add(index);
|
||||
queue.addLast(new InstructionState(next, nextVars));
|
||||
steps++;
|
||||
}
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
static class InstructionState extends Pair<Instruction, BitSet> {
|
||||
InstructionState(Instruction first, BitSet second) {
|
||||
super(first, second);
|
||||
}
|
||||
}
|
||||
}
|
||||
+8
-106
@@ -4,74 +4,25 @@ package com.intellij.codeInspection.dataFlow;
|
||||
import com.intellij.codeInspection.dataFlow.instructions.*;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaExpressionFactory;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaValue;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
|
||||
import com.intellij.openapi.progress.ProgressManager;
|
||||
import com.intellij.openapi.util.Pair;
|
||||
import com.intellij.psi.PsiMember;
|
||||
import com.intellij.util.containers.ContainerUtil;
|
||||
import com.intellij.util.containers.FilteringIterator;
|
||||
import com.intellij.util.containers.MultiMap;
|
||||
import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
|
||||
import it.unimi.dsi.fastutil.ints.IntSet;
|
||||
import one.util.streamex.IntStreamEx;
|
||||
import one.util.streamex.StreamEx;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
|
||||
import java.util.*;
|
||||
import java.util.function.BiFunction;
|
||||
import java.util.BitSet;
|
||||
import java.util.Collection;
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
/**
|
||||
* @author peter
|
||||
*/
|
||||
final class LiveVariablesAnalyzer {
|
||||
private final DfaValueFactory myFactory;
|
||||
private final Instruction[] myInstructions;
|
||||
private final MultiMap<Instruction, Instruction> myForwardMap;
|
||||
private final MultiMap<Instruction, Instruction> myBackwardMap;
|
||||
|
||||
final class LiveVariablesAnalyzer extends BaseVariableAnalyzer {
|
||||
LiveVariablesAnalyzer(ControlFlow flow) {
|
||||
myFactory = flow.getFactory();
|
||||
myInstructions = flow.getInstructions();
|
||||
myForwardMap = calcForwardMap();
|
||||
myBackwardMap = calcBackwardMap();
|
||||
}
|
||||
|
||||
private List<Instruction> getSuccessors(Instruction ins) {
|
||||
return IntStreamEx.of(LoopAnalyzer.getSuccessorIndices(ins.getIndex(), myInstructions)).elements(myInstructions).toList();
|
||||
}
|
||||
|
||||
private MultiMap<Instruction, Instruction> calcBackwardMap() {
|
||||
MultiMap<Instruction, Instruction> result = MultiMap.create();
|
||||
for (Instruction instruction : myInstructions) {
|
||||
for (Instruction next : myForwardMap.get(instruction)) {
|
||||
result.putValue(next, instruction);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private MultiMap<Instruction, Instruction> calcForwardMap() {
|
||||
MultiMap<Instruction, Instruction> result = MultiMap.create();
|
||||
for (Instruction instruction : myInstructions) {
|
||||
if (isInterestingInstruction(instruction)) {
|
||||
for (Instruction next : getSuccessors(instruction)) {
|
||||
while (true) {
|
||||
if (isInterestingInstruction(next)) {
|
||||
result.putValue(instruction, next);
|
||||
break;
|
||||
}
|
||||
if (next.getIndex() + 1 >= myInstructions.length) {
|
||||
break;
|
||||
}
|
||||
next = myInstructions[next.getIndex() + 1];
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
super(flow);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@@ -102,7 +53,8 @@ final class LiveVariablesAnalyzer {
|
||||
return StreamEx.empty();
|
||||
}
|
||||
|
||||
private boolean isInterestingInstruction(Instruction instruction) {
|
||||
@Override
|
||||
protected boolean isInterestingInstruction(Instruction instruction) {
|
||||
if (instruction == myInstructions[0]) return true;
|
||||
if (getReadVariables(instruction).findFirst().isPresent() || getWrittenVariable(instruction) != null) return true;
|
||||
return instruction instanceof FinishElementInstruction ||
|
||||
@@ -195,54 +147,4 @@ final class LiveVariablesAnalyzer {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return true if completed, false if "too complex"
|
||||
*/
|
||||
private boolean runDfa(boolean forward, BiFunction<? super Instruction, ? super BitSet, ? extends BitSet> handleState) {
|
||||
Set<Instruction> entryPoints = new HashSet<>();
|
||||
if (forward) {
|
||||
entryPoints.add(myInstructions[0]);
|
||||
}
|
||||
else {
|
||||
entryPoints.addAll(ContainerUtil.findAll(myInstructions, FilteringIterator.instanceOf(ReturnInstruction.class)));
|
||||
}
|
||||
|
||||
Deque<InstructionState> queue = new ArrayDeque<>(10);
|
||||
for (Instruction i : entryPoints) {
|
||||
queue.addLast(new InstructionState(i, new BitSet()));
|
||||
}
|
||||
|
||||
int limit = myForwardMap.size() * 100;
|
||||
Map<BitSet, IntSet> processed = new HashMap<>();
|
||||
int steps = 0;
|
||||
while (!queue.isEmpty()) {
|
||||
if (steps > limit) {
|
||||
return false;
|
||||
}
|
||||
if (steps % 1024 == 0) {
|
||||
ProgressManager.checkCanceled();
|
||||
}
|
||||
InstructionState state = queue.removeFirst();
|
||||
Instruction instruction = state.first;
|
||||
Collection<Instruction> nextInstructions = forward ? myForwardMap.get(instruction) : myBackwardMap.get(instruction);
|
||||
BitSet nextVars = handleState.apply(instruction, state.second);
|
||||
for (Instruction next : nextInstructions) {
|
||||
IntSet instructionSet = processed.computeIfAbsent(nextVars, k -> new IntOpenHashSet());
|
||||
int index = next.getIndex() + 1;
|
||||
if (!instructionSet.contains(index)) {
|
||||
instructionSet.add(index);
|
||||
queue.addLast(new InstructionState(next, nextVars));
|
||||
steps++;
|
||||
}
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
class InstructionState extends Pair<Instruction, BitSet> {
|
||||
InstructionState(Instruction first, BitSet second) {
|
||||
super(first, second);
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,10 @@ import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
|
||||
import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil;
|
||||
import com.intellij.codeInsight.daemon.impl.quickfix.RemoveUnusedVariableUtil;
|
||||
import com.intellij.codeInspection.*;
|
||||
import com.intellij.codeInspection.dataFlow.instructions.AssignInstruction;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaValue;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
|
||||
import com.intellij.codeInspection.ui.InspectionOptionsPanel;
|
||||
import com.intellij.java.JavaBundle;
|
||||
import com.intellij.psi.*;
|
||||
@@ -21,7 +25,6 @@ import com.siyeh.ig.psiutils.EquivalenceChecker;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import javax.swing.*;
|
||||
import java.util.List;
|
||||
import java.util.*;
|
||||
|
||||
public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
@@ -37,32 +40,31 @@ public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
return new JavaElementVisitor() {
|
||||
@Override
|
||||
public void visitMethod(PsiMethod method) {
|
||||
checkCodeBlock(method.getBody(), holder, isOnTheFly);
|
||||
checkCodeBlock(method.getBody(), holder);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void visitClassInitializer(PsiClassInitializer initializer) {
|
||||
checkCodeBlock(initializer.getBody(), holder, isOnTheFly);
|
||||
checkCodeBlock(initializer.getBody(), holder);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void visitLambdaExpression(PsiLambdaExpression expression) {
|
||||
PsiElement body = expression.getBody();
|
||||
if (body instanceof PsiCodeBlock) {
|
||||
checkCodeBlock((PsiCodeBlock)body, holder, isOnTheFly);
|
||||
checkCodeBlock((PsiCodeBlock)body, holder);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void visitField(PsiField field) {
|
||||
checkField(field, holder, isOnTheFly);
|
||||
checkField(field, holder);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
private void checkCodeBlock(final PsiCodeBlock body,
|
||||
final ProblemsHolder holder,
|
||||
final boolean isOnTheFly) {
|
||||
final ProblemsHolder holder) {
|
||||
if (body == null) return;
|
||||
final Set<PsiVariable> usedVariables = new HashSet<>();
|
||||
List<DefUseUtil.Info> unusedDefs = DefUseUtil.getUnusedDefs(body, usedVariables);
|
||||
@@ -78,7 +80,7 @@ public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
if (info.isRead() && REPORT_REDUNDANT_INITIALIZER) {
|
||||
PsiTypeElement typeElement = psiVariable.getTypeElement();
|
||||
if (typeElement == null || !typeElement.isInferredType()) {
|
||||
reportInitializerProblem(psiVariable, holder, isOnTheFly);
|
||||
reportInitializerProblem(psiVariable, holder);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -90,7 +92,7 @@ public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
// x = x = 5; reported by "Variable is assigned to itself"
|
||||
continue;
|
||||
}
|
||||
reportAssignmentProblem(psiVariable, (PsiAssignmentExpression)context, holder, isOnTheFly);
|
||||
reportAssignmentProblem(psiVariable, (PsiAssignmentExpression)context, holder);
|
||||
}
|
||||
else {
|
||||
if (context instanceof PsiPrefixExpression && REPORT_PREFIX_EXPRESSIONS ||
|
||||
@@ -101,14 +103,46 @@ public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
processFieldsViaDfa(body, holder);
|
||||
}
|
||||
|
||||
private static void reportInitializerProblem(PsiVariable psiVariable, ProblemsHolder holder, boolean isOnTheFly) {
|
||||
private void processFieldsViaDfa(PsiCodeBlock body, ProblemsHolder holder) {
|
||||
DfaValueFactory factory = new DfaValueFactory(holder.getProject(), body);
|
||||
var flow = com.intellij.codeInspection.dataFlow.ControlFlow.buildFlow(body, factory, true);
|
||||
if (flow != null) {
|
||||
Set<AssignInstruction> variables = new OverwrittenFieldAnalyzer(flow).getOverwrittenFields();
|
||||
for (AssignInstruction instruction : variables) {
|
||||
DfaValue value = instruction.getAssignedValue();
|
||||
if (!(value instanceof DfaVariableValue)) continue;
|
||||
PsiField field = ObjectUtils.tryCast(((DfaVariableValue)value).getPsiVariable(), PsiField.class);
|
||||
if (field == null) continue;
|
||||
PsiExpression lExpression = instruction.getLExpression();
|
||||
PsiExpression expression = instruction.getRExpression();
|
||||
if (lExpression == null) continue;
|
||||
PsiElement parent = PsiUtil.skipParenthesizedExprUp(lExpression.getParent());
|
||||
if (parent instanceof PsiPrefixExpression && REPORT_PREFIX_EXPRESSIONS ||
|
||||
parent instanceof PsiPostfixExpression && REPORT_POSTFIX_EXPRESSIONS) {
|
||||
holder.registerProblem(parent, JavaBundle.message("inspection.unused.assignment.problem.descriptor4", "<code>#ref</code> #loc"));
|
||||
}
|
||||
else if (parent instanceof PsiAssignmentExpression) {
|
||||
if (expression instanceof PsiArrayInitializerExpression ||
|
||||
expression instanceof PsiNewExpression && ((PsiNewExpression)expression).getArrayInitializer() != null) {
|
||||
// Due to implementation quirk, array initializers are reassigned in CFG, so false warnings appear there
|
||||
continue;
|
||||
}
|
||||
reportAssignmentProblem(field, (PsiAssignmentExpression)parent, holder);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static void reportInitializerProblem(PsiVariable psiVariable, ProblemsHolder holder) {
|
||||
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(
|
||||
isOnTheFlyOrNoSideEffects(isOnTheFly, psiVariable, psiVariable.getInitializer()) ? new RemoveInitializerFix() : null);
|
||||
isOnTheFlyOrNoSideEffects(holder.isOnTheFly(), psiVariable, psiVariable.getInitializer()) ? new RemoveInitializerFix() : null);
|
||||
holder.registerProblem(ObjectUtils.notNull(psiVariable.getInitializer(), psiVariable),
|
||||
JavaBundle.message("inspection.unused.assignment.problem.descriptor2",
|
||||
"<code>" + psiVariable.getName() + "</code>", "<code>#ref</code> #loc"),
|
||||
"<code>" + psiVariable.getName() + "</code>", "<code>#ref</code> #loc"),
|
||||
ProblemHighlightType.LIKE_UNUSED_SYMBOL,
|
||||
fixes.toArray(LocalQuickFix.EMPTY_ARRAY)
|
||||
);
|
||||
@@ -116,18 +150,17 @@ public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
|
||||
private static void reportAssignmentProblem(PsiVariable psiVariable,
|
||||
PsiAssignmentExpression assignment,
|
||||
ProblemsHolder holder,
|
||||
boolean isOnTheFly) {
|
||||
ProblemsHolder holder) {
|
||||
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(
|
||||
isOnTheFlyOrNoSideEffects(isOnTheFly, psiVariable, assignment.getRExpression()) ? new RemoveAssignmentFix() : null);
|
||||
isOnTheFlyOrNoSideEffects(holder.isOnTheFly(), psiVariable, assignment.getRExpression()) ? new RemoveAssignmentFix() : null);
|
||||
holder.registerProblem(assignment.getLExpression(),
|
||||
JavaBundle.message("inspection.unused.assignment.problem.descriptor3",
|
||||
Objects.requireNonNull(assignment.getRExpression()).getText(), "<code>#ref</code>" + " #loc"),
|
||||
Objects.requireNonNull(assignment.getRExpression()).getText(), "<code>#ref</code>" + " #loc"),
|
||||
ProblemHighlightType.LIKE_UNUSED_SYMBOL, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)
|
||||
);
|
||||
}
|
||||
|
||||
private void checkField(@NotNull PsiField field, @NotNull ProblemsHolder holder, boolean isOnTheFly) {
|
||||
private void checkField(@NotNull PsiField field, @NotNull ProblemsHolder holder) {
|
||||
if (field.hasModifierProperty(PsiModifier.FINAL)) return;
|
||||
final PsiClass psiClass = field.getContainingClass();
|
||||
if (psiClass == null) return;
|
||||
@@ -177,12 +210,12 @@ public class DefUseInspection extends AbstractBaseJavaLocalInspectionTool {
|
||||
if (wasDefinitelyAssigned) {
|
||||
if (fieldWrite.isInitializer()) {
|
||||
if (REPORT_REDUNDANT_INITIALIZER) {
|
||||
reportInitializerProblem(field, holder, isOnTheFly);
|
||||
reportInitializerProblem(field, holder);
|
||||
}
|
||||
}
|
||||
else {
|
||||
for (PsiAssignmentExpression assignment : fieldWrite.getAssignments()) {
|
||||
reportAssignmentProblem(field, assignment, holder, isOnTheFly);
|
||||
reportAssignmentProblem(field, assignment, holder);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
// Copyright 2000-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
|
||||
package com.intellij.codeInspection.defUse;
|
||||
|
||||
import com.intellij.codeInspection.dataFlow.BaseVariableAnalyzer;
|
||||
import com.intellij.codeInspection.dataFlow.ControlFlow;
|
||||
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.util.PsiUtil;
|
||||
import com.siyeh.ig.psiutils.ExpressionUtils;
|
||||
import one.util.streamex.StreamEx;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.util.BitSet;
|
||||
import java.util.Collections;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
/**
|
||||
* Analyze overwritten fields based on DFA-CFG (unlike usual CFG, it includes method calls, so we can know when field value may leak)
|
||||
*/
|
||||
final class OverwrittenFieldAnalyzer extends BaseVariableAnalyzer {
|
||||
OverwrittenFieldAnalyzer(ControlFlow flow) {
|
||||
super(flow);
|
||||
}
|
||||
|
||||
@NotNull
|
||||
private StreamEx<DfaVariableValue> getReadVariables(Instruction instruction) {
|
||||
if (instruction instanceof PushInstruction && !((PushInstruction)instruction).isReferenceWrite()) {
|
||||
DfaValue value = ((PushInstruction)instruction).getValue();
|
||||
if (value instanceof DfaVariableValue) {
|
||||
return StreamEx.of((DfaVariableValue)value);
|
||||
}
|
||||
}
|
||||
else if (instruction instanceof EndOfInitializerInstruction) {
|
||||
return StreamEx.of(myFactory.getValues()).select(DfaVariableValue.class)
|
||||
.filter(var -> var.getPsiVariable() instanceof PsiMember);
|
||||
}
|
||||
return StreamEx.empty();
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean isInterestingInstruction(Instruction instruction) {
|
||||
if (instruction == myInstructions[0]) return true;
|
||||
|
||||
if (instruction instanceof AssignInstruction && ((AssignInstruction)instruction).getAssignedValue() instanceof DfaVariableValue ||
|
||||
instruction instanceof MethodCallInstruction ||
|
||||
instruction instanceof FinishElementInstruction ||
|
||||
instruction instanceof GotoInstruction ||
|
||||
instruction instanceof ConditionalGotoInstruction ||
|
||||
instruction instanceof ControlTransferInstruction ||
|
||||
instruction instanceof FlushFieldsInstruction) {
|
||||
return true;
|
||||
}
|
||||
return getReadVariables(instruction).findFirst().isPresent();
|
||||
}
|
||||
|
||||
public Set<AssignInstruction> getOverwrittenFields() {
|
||||
Set<AssignInstruction> overwrites = StreamEx.of(myInstructions).select(AssignInstruction.class).toSet();
|
||||
if (overwrites.isEmpty()) return Collections.emptySet();
|
||||
boolean hasFieldWrite = StreamEx.of(overwrites).map(AssignInstruction::getAssignedValue)
|
||||
.select(DfaVariableValue.class)
|
||||
.map(DfaVariableValue::getPsiVariable)
|
||||
.anyMatch(f -> f instanceof PsiField);
|
||||
if (!hasFieldWrite) return Collections.emptySet();
|
||||
Set<AssignInstruction> visited = new HashSet<>();
|
||||
boolean ok = runDfa(false, (instruction, nextVars) -> {
|
||||
if (instruction instanceof AssignInstruction) {
|
||||
visited.add((AssignInstruction)instruction);
|
||||
DfaValue value = ((AssignInstruction)instruction).getAssignedValue();
|
||||
if (value instanceof DfaVariableValue) {
|
||||
int id = value.getID();
|
||||
if (!nextVars.get(id)) {
|
||||
overwrites.remove(instruction);
|
||||
BitSet clone = (BitSet)nextVars.clone();
|
||||
clone.set(id);
|
||||
return clone;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (nextVars.isEmpty()) return nextVars;
|
||||
if (instruction instanceof FlushFieldsInstruction) {
|
||||
return new BitSet();
|
||||
}
|
||||
StreamEx<DfaVariableValue> readVariables;
|
||||
if (instruction instanceof MethodCallInstruction) {
|
||||
if (!((MethodCallInstruction)instruction).getMutationSignature().isPure()) {
|
||||
return new BitSet();
|
||||
}
|
||||
// We assume that pure methods may read only static fields and fields which are passed as parameters (directly or by qualifier).
|
||||
// This might be incorrect in rare cases but allows finding many useful bugs.
|
||||
readVariables = StreamEx.of(myFactory.getValues())
|
||||
.select(DfaVariableValue.class)
|
||||
.filter(value -> value.getPsiVariable() instanceof PsiField && value.getPsiVariable().hasModifierProperty(PsiModifier.STATIC));
|
||||
}
|
||||
else if (instruction instanceof FinishElementInstruction) {
|
||||
readVariables = StreamEx.of(((FinishElementInstruction)instruction).getVarsToFlush());
|
||||
}
|
||||
else {
|
||||
readVariables = getReadVariables(instruction);
|
||||
}
|
||||
BitSet clone = (BitSet)nextVars.clone();
|
||||
boolean qualifierPush = false;
|
||||
if (instruction instanceof PushInstruction) {
|
||||
// Avoid forgetting about qualifier.field on qualifier.field = x;
|
||||
PsiExpression expression = ((PushInstruction)instruction).getExpression();
|
||||
if (expression != null && PsiUtil.skipParenthesizedExprUp(expression).getParent() instanceof PsiReferenceExpression
|
||||
&& ExpressionUtils.getCallForQualifier(expression) == null) {
|
||||
qualifierPush = true;
|
||||
}
|
||||
}
|
||||
if (!qualifierPush) {
|
||||
readVariables = readVariables.flatMap(v -> StreamEx.of(v.getDependentVariables()).prepend(v)).distinct();
|
||||
}
|
||||
readVariables.forEach(v -> clone.clear(v.getID()));
|
||||
return clone;
|
||||
});
|
||||
overwrites.retainAll(visited);
|
||||
return ok ? overwrites : Collections.emptySet();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
public class FieldOverwrite {
|
||||
int val;
|
||||
int val2;
|
||||
int[] data;
|
||||
|
||||
public FieldOverwrite(int field) {
|
||||
<warning descr="The value 123 assigned to 'this.val' is never used">this.val</warning> = 123;
|
||||
this.val = field;
|
||||
}
|
||||
|
||||
void testArray() {
|
||||
data = new int[] {10, 20, 30, 40};
|
||||
}
|
||||
|
||||
void increment() {
|
||||
<warning descr="The value changed at 'val++' is never used">val++</warning>;
|
||||
val=2;
|
||||
<warning descr="The value 3 assigned to 'val' is never used">val</warning>+=3;
|
||||
val=4;
|
||||
}
|
||||
|
||||
void use() {
|
||||
val = 1;
|
||||
val = calc(2);
|
||||
}
|
||||
|
||||
int calc(int x) {
|
||||
return val * x;
|
||||
}
|
||||
|
||||
private int getVal() {
|
||||
return val;
|
||||
}
|
||||
|
||||
void noUseInlining() {
|
||||
<warning descr="The value 1 assigned to 'val2' is never used">val2</warning> = 1;
|
||||
val2 = getVal();
|
||||
}
|
||||
|
||||
void test(FieldOverwrite fo) {
|
||||
<warning descr="The value 1 assigned to 'val' is never used">val</warning> = 1;
|
||||
val = 2;
|
||||
<warning descr="The value 3 assigned to 'fo.val' is never used">fo.val</warning> = 3;
|
||||
fo.val = 4;
|
||||
}
|
||||
|
||||
// IDEA-195460
|
||||
private int intField;
|
||||
private static int intStaticField;
|
||||
|
||||
public void main() {
|
||||
int intVar = 0;
|
||||
intVar = <warning descr="The value changed at 'intVar++' is never used">intVar++</warning>;
|
||||
System.out.println(intVar);
|
||||
|
||||
intField = 0;
|
||||
intField = <warning descr="The value changed at 'intField++' is never used">intField++</warning>;
|
||||
System.out.println(intField);
|
||||
|
||||
|
||||
intStaticField = 0;
|
||||
intStaticField = <warning descr="The value changed at 'intStaticField++' is never used">intStaticField++</warning>;
|
||||
System.out.println(intStaticField);
|
||||
}
|
||||
|
||||
void testUseStatic() {
|
||||
intStaticField = 1;
|
||||
useStatic();
|
||||
intStaticField = 2;
|
||||
}
|
||||
|
||||
static void useStatic() {
|
||||
System.out.println(intStaticField);
|
||||
}
|
||||
}
|
||||
@@ -72,6 +72,7 @@ public class DefUseTest extends LightJavaCodeInsightFixtureTestCase {
|
||||
public void testFieldInitializerChainedConstructor() { doTest(); }
|
||||
public void testUnderAlwaysFalseCondition() { doTest(); }
|
||||
public void testLastInTry() { doTest(); }
|
||||
public void testFieldOverwrite() { doTest(); }
|
||||
|
||||
@NotNull
|
||||
@Override
|
||||
|
||||
Reference in New Issue
Block a user