Java: Look for redundant assignment of a field in class and field initializers (IDEA-149587)

This commit is contained in:
Pavel Dolgov
2017-06-20 17:36:32 +03:00
parent efec0ad0f1
commit 0aa8430cb4
8 changed files with 248 additions and 17 deletions

View File

@@ -220,7 +220,7 @@ public class HighlightControlFlowUtil {
* see JLS chapter 16
* @return true if variable assigned (maybe more than once)
*/
private static boolean variableDefinitelyAssignedIn(@NotNull PsiVariable variable, @NotNull PsiElement context) {
public static boolean variableDefinitelyAssignedIn(@NotNull PsiVariable variable, @NotNull PsiElement context) {
try {
ControlFlow controlFlow = getControlFlow(context);
return ControlFlowUtil.isVariableDefinitelyAssigned(variable, controlFlow);

View File

@@ -16,10 +16,12 @@
package com.intellij.codeInspection.defUse;
import com.intellij.codeInsight.daemon.GroupNames;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInsight.daemon.impl.quickfix.RemoveUnusedVariableUtil;
import com.intellij.codeInspection.*;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.DefUseUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.ui.JBUI;
@@ -62,6 +64,11 @@ public class DefUseInspectionBase extends BaseJavaBatchLocalInspectionTool {
checkCodeBlock((PsiCodeBlock)body, holder, isOnTheFly);
}
}
@Override
public void visitField(PsiField field) {
checkField(field, holder, isOnTheFly);
}
};
}
@@ -89,25 +96,11 @@ public class DefUseInspectionBase extends BaseJavaBatchLocalInspectionTool {
if (context instanceof PsiDeclarationStatement || context instanceof PsiResourceVariable) {
if (info.isRead() && REPORT_REDUNDANT_INITIALIZER) {
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(
isOnTheFlyOrNoSideEffects(isOnTheFly, psiVariable, psiVariable.getInitializer()) ? createRemoveInitializerFix() : null);
holder.registerProblem(ObjectUtils.notNull(psiVariable.getInitializer(), psiVariable),
InspectionsBundle.message("inspection.unused.assignment.problem.descriptor2",
"<code>" + psiVariable.getName() + "</code>", "<code>#ref</code> #loc"),
ProblemHighlightType.LIKE_UNUSED_SYMBOL,
fixes.toArray(new LocalQuickFix[fixes.size()])
);
reportInitializerProblem(psiVariable, holder, isOnTheFly);
}
}
else if (context instanceof PsiAssignmentExpression) {
final PsiAssignmentExpression assignment = (PsiAssignmentExpression)context;
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(
isOnTheFlyOrNoSideEffects(isOnTheFly, psiVariable, assignment.getRExpression()) ? createRemoveAssignmentFix() : null);
holder.registerProblem(assignment.getLExpression(),
InspectionsBundle.message("inspection.unused.assignment.problem.descriptor3",
ObjectUtils.assertNotNull(assignment.getRExpression()).getText(), "<code>#ref</code>" + " #loc"),
ProblemHighlightType.LIKE_UNUSED_SYMBOL, fixes.toArray(new LocalQuickFix[fixes.size()])
);
reportAssignmentProblem(psiVariable, (PsiAssignmentExpression)context, holder, isOnTheFly);
}
else {
if (context instanceof PsiPrefixExpression && REPORT_PREFIX_EXPRESSIONS ||
@@ -120,6 +113,96 @@ public class DefUseInspectionBase extends BaseJavaBatchLocalInspectionTool {
}
}
private void reportInitializerProblem(PsiVariable psiVariable, ProblemsHolder holder, boolean isOnTheFly) {
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(
isOnTheFlyOrNoSideEffects(isOnTheFly, psiVariable, psiVariable.getInitializer()) ? createRemoveInitializerFix() : null);
holder.registerProblem(ObjectUtils.notNull(psiVariable.getInitializer(), psiVariable),
InspectionsBundle.message("inspection.unused.assignment.problem.descriptor2",
"<code>" + psiVariable.getName() + "</code>", "<code>#ref</code> #loc"),
ProblemHighlightType.LIKE_UNUSED_SYMBOL,
fixes.toArray(LocalQuickFix.EMPTY_ARRAY)
);
}
private void reportAssignmentProblem(PsiVariable psiVariable,
PsiAssignmentExpression assignment,
ProblemsHolder holder,
boolean isOnTheFly) {
List<LocalQuickFix> fixes = ContainerUtil.createMaybeSingletonList(
isOnTheFlyOrNoSideEffects(isOnTheFly, psiVariable, assignment.getRExpression()) ? createRemoveAssignmentFix() : null);
holder.registerProblem(assignment.getLExpression(),
InspectionsBundle.message("inspection.unused.assignment.problem.descriptor3",
ObjectUtils.assertNotNull(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) {
if (field.hasModifierProperty(PsiModifier.FINAL)) return;
final PsiClass psiClass = field.getContainingClass();
if (psiClass == null) return;
final PsiClassInitializer[] classInitializers = psiClass.getInitializers();
if (classInitializers.length == 0) return;
final boolean isStatic = field.hasModifierProperty(PsiModifier.STATIC);
final boolean fieldHasInitializer = field.hasInitializer();
final PsiClassInitializer initializerBeforeField = PsiTreeUtil.getPrevSiblingOfType(field, PsiClassInitializer.class);
final List<FieldWrite> fieldWrites = new ArrayList<>(); // class initializers and field initializer in the program order
if (fieldHasInitializer && initializerBeforeField == null) {
fieldWrites.add(FieldWrite.createInitializer());
}
for (PsiClassInitializer classInitializer : classInitializers) {
if (classInitializer.hasModifierProperty(PsiModifier.STATIC) == isStatic) {
final List<PsiAssignmentExpression> assignments = collectAssignments(field, classInitializer);
if (!assignments.isEmpty()) {
boolean isDefinitely = HighlightControlFlowUtil.variableDefinitelyAssignedIn(field, classInitializer.getBody());
fieldWrites.add(FieldWrite.createAssignments(isDefinitely, assignments));
}
}
if (fieldHasInitializer && initializerBeforeField == classInitializer) {
fieldWrites.add(FieldWrite.createInitializer());
}
}
Collections.reverse(fieldWrites);
boolean wasDefinitelyAssigned = false;
for (final FieldWrite fieldWrite : fieldWrites) {
if (wasDefinitelyAssigned) {
if (fieldWrite.isInitializer()) {
reportInitializerProblem(field, holder, isOnTheFly);
}
else {
for (PsiAssignmentExpression assignment : fieldWrite.getAssignments()) {
reportAssignmentProblem(field, assignment, holder, isOnTheFly);
}
}
}
else if (fieldWrite.isDefinitely()) {
wasDefinitelyAssigned = true;
}
}
}
@NotNull
private static List<PsiAssignmentExpression> collectAssignments(@NotNull PsiField field, @NotNull PsiClassInitializer classInitializer) {
final List<PsiAssignmentExpression> assignmentExpressions = new ArrayList<>();
classInitializer.accept(new JavaRecursiveElementVisitor() {
@Override
public void visitAssignmentExpression(PsiAssignmentExpression expression) {
final PsiExpression lExpression = expression.getLExpression();
if (lExpression instanceof PsiJavaReference && ((PsiJavaReference)lExpression).isReferenceTo(field)) {
final PsiExpression rExpression = expression.getRExpression();
if (rExpression != null) {
assignmentExpressions.add(expression);
}
}
super.visitAssignmentExpression(expression);
}
});
return assignmentExpressions;
}
private static boolean isOnTheFlyOrNoSideEffects(boolean isOnTheFly,
PsiVariable psiVariable,
PsiExpression initializer) {
@@ -193,4 +276,37 @@ public class DefUseInspectionBase extends BaseJavaBatchLocalInspectionTool {
public String getShortName() {
return SHORT_NAME;
}
private static class FieldWrite {
final boolean myDefinitely;
final List<PsiAssignmentExpression> myAssignments;
private FieldWrite(boolean definitely, List<PsiAssignmentExpression> assignments) {
myDefinitely = definitely;
myAssignments = assignments;
}
public boolean isDefinitely() {
return myDefinitely;
}
public boolean isInitializer() {
return myAssignments == null;
}
public List<PsiAssignmentExpression> getAssignments() {
return myAssignments != null ? myAssignments : Collections.emptyList();
}
@NotNull
public static FieldWrite createInitializer() {
return new FieldWrite(true, null);
}
@NotNull
public static FieldWrite createAssignments(boolean definitely, @NotNull List<PsiAssignmentExpression> assignmentExpressions) {
return new FieldWrite(definitely, assignmentExpressions);
}
}
}

View File

@@ -0,0 +1,7 @@
// "Remove redundant assignment" "true"
class A {
static int n;
static {
}
static { n = 2; }
}

View File

@@ -0,0 +1,5 @@
// "Remove redundant initializer" "true"
class A {
int n;
{ n = 1; }
}

View File

@@ -0,0 +1,6 @@
// "Remove redundant assignment" "true"
class A {
static int n;
static { <caret>n = 1; }
static { n = 2; }
}

View File

@@ -0,0 +1,5 @@
// "Remove redundant initializer" "true"
class A {
int n = <caret>0;
{ n = 1; }
}

View File

@@ -0,0 +1,91 @@
class C {
static boolean b = System.getProperty("foo") != null;
static class C1 {
{ <warning descr="The value \"a\" assigned to 's' is never used">s</warning> = "a"; }
String s = "b";
}
static class C2 {
String s = <warning descr="Variable 's' initializer '\"b\"' is redundant">"b"</warning>;
{ s = "c"; }
}
static class C3 {
{ <warning descr="The value \"a\" assigned to 's' is never used">s</warning> = "a"; }
String s;
{ s = "c"; }
}
static class C4 {
{ if (b) <warning descr="The value \"a\" assigned to 's' is never used">s</warning> = "a"; }
String s = "b";
}
static class C5 {
String s = "b";
{ if (b) s = "c"; }
}
static class C6 {
String s = <warning descr="Variable 's' initializer '\"b\"' is redundant">"b"</warning>;
{ if (b) s = "c"; else s = "d"; }
}
static class C7 {
String s;
{ s = "c"; }
}
static class C8 {
{ s = "a"; }
String s;
}
static class C9 {
String s;
{
<warning descr="The value \"b\" assigned to 's' is never used">s</warning> = "b";
if (b) <warning descr="The value \"c\" assigned to 's' is never used">s</warning> = "c";
}
{ s = "d"; }
}
static class S1 {
static { <warning descr="The value \"a\" assigned to 's' is never used">s</warning> = "a"; }
static String s = "b";
}
static class S2 {
static String s = <warning descr="Variable 's' initializer '\"b\"' is redundant">"b"</warning>;
static { s = "c"; }
}
static class S3 {
static { <warning descr="The value \"a\" assigned to 's' is never used">s</warning> = "a"; }
static String s;
static { s = "c"; }
}
static class S4 {
static { if (b) <warning descr="The value \"a\" assigned to 's' is never used">s</warning> = "a"; }
static String s = "b";
}
static class S5 {
static String s = "b";
static { if (b) s = "c"; }
}
static class S6 {
static String s = <warning descr="Variable 's' initializer '\"b\"' is redundant">"b"</warning>;
static { if (b) s = "c"; else s = "d"; }
}
static class S7 {
static String s;
static { s = "c"; }
}
static class S8 {
static { s = "a"; }
static String s;
}
static class S9 {
static String s;
static {
<warning descr="The value \"b\" assigned to 's' is never used">s</warning> = "b";
if (b) <warning descr="The value \"c\" assigned to 's' is never used">s</warning> = "c";
}
static { s = "d"; }
}
static class S10 {
static String s = "a";
{ s = "b"; }
}
}

View File

@@ -56,6 +56,7 @@ public class DefUseTest extends LightCodeInsightFixtureTestCase {
public void testAssignmentsInLambdaBody() { doTest(); }
public void testNestedTryFinallyInEndlessLoop() { doTest(); }
public void testNestedTryFinallyInForLoop() { doTest(); }
public void testFieldInitializer() { doTest(); }
private void doTest() {
myFixture.enableInspections(new DefUseInspection());