mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
Java: Tests for fixes for duplicate expressions inspection (IDEA-136761)
This commit is contained in:
+33
@@ -2,7 +2,9 @@
|
||||
package com.intellij.codeInspection.duplicateExpressions;
|
||||
|
||||
import com.intellij.psi.*;
|
||||
import com.intellij.psi.tree.IElementType;
|
||||
import com.intellij.psi.tree.TokenSet;
|
||||
import com.intellij.psi.util.PsiUtil;
|
||||
import com.intellij.util.containers.ObjectIntHashMap;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
@@ -69,6 +71,19 @@ class ComplexityCalculator {
|
||||
int c = NEGATIONS.contains(unary.getOperationTokenType()) ? SIMPLE_OPERATOR : OPERATOR;
|
||||
return getComplexity(unary.getOperand()) + c;
|
||||
}
|
||||
if (e instanceof PsiBinaryExpression) {
|
||||
PsiBinaryExpression binary = (PsiBinaryExpression)e;
|
||||
IElementType token = binary.getOperationTokenType();
|
||||
PsiExpression left = binary.getLOperand();
|
||||
PsiExpression right = binary.getROperand();
|
||||
int c = BOOLEAN_OPERATION_TOKENS.contains(token) ||
|
||||
JavaTokenType.PLUS.equals(token) && (isLiteral(left, "1") || isLiteral(right, "1")) ||
|
||||
JavaTokenType.MINUS.equals(token) && isLiteral(right, "1") ||
|
||||
JavaTokenType.ASTERISK.equals(token) && (isLiteral(left, "2") || isLiteral(right, "2")) ||
|
||||
JavaTokenType.DIV.equals(token) && isLiteral(right, "2")
|
||||
? SIMPLE_OPERATOR : OPERATOR;
|
||||
return c + getComplexity(left) + getComplexity(right);
|
||||
}
|
||||
if (e instanceof PsiPolyadicExpression) {
|
||||
PsiPolyadicExpression polyadic = (PsiPolyadicExpression)e;
|
||||
int c = BOOLEAN_OPERATION_TOKENS.contains(polyadic.getOperationTokenType()) ? SIMPLE_OPERATOR : OPERATOR;
|
||||
@@ -148,4 +163,22 @@ class ComplexityCalculator {
|
||||
}
|
||||
return c;
|
||||
}
|
||||
|
||||
private static boolean isLiteral(@Nullable PsiExpression expression, @NotNull String withText) {
|
||||
expression = PsiUtil.skipParenthesizedExprDown(expression);
|
||||
return expression instanceof PsiLiteral && expression.textMatches(withText);
|
||||
}
|
||||
|
||||
/**
|
||||
* Quick check to filter out the obvious things early
|
||||
*/
|
||||
static boolean isDefinitelySimple(@Nullable PsiExpression expression, int threshold) {
|
||||
if (expression instanceof PsiLiteral) {
|
||||
return CONSTANT < threshold;
|
||||
}
|
||||
if (expression instanceof PsiReferenceExpression && ((PsiReferenceExpression)expression).getQualifierExpression() == null) {
|
||||
return REFERENCE < threshold;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
+6
-2
@@ -44,8 +44,8 @@ class DuplicateExpressionsContext {
|
||||
}
|
||||
|
||||
@Nullable
|
||||
static DuplicateExpressionsContext getOrCreateContext(PsiExpression expression, @NotNull UserDataHolder session) {
|
||||
PsiCodeBlock nearestBody = ObjectUtils.tryCast(ControlFlowUtil.findCodeFragment(expression), PsiCodeBlock.class);
|
||||
static DuplicateExpressionsContext getOrCreateContext(@NotNull PsiExpression expression, @NotNull UserDataHolder session) {
|
||||
PsiCodeBlock nearestBody = findNearestBody(expression);
|
||||
if (nearestBody != null) {
|
||||
Map<PsiCodeBlock, DuplicateExpressionsContext> contexts = session.getUserData(CONTEXTS_KEY);
|
||||
if (contexts == null) {
|
||||
@@ -61,4 +61,8 @@ class DuplicateExpressionsContext {
|
||||
Map<PsiCodeBlock, DuplicateExpressionsContext> contexts = session.getUserData(CONTEXTS_KEY);
|
||||
return contexts != null ? contexts.get(body) : null;
|
||||
}
|
||||
|
||||
static PsiCodeBlock findNearestBody(@NotNull PsiExpression expression) {
|
||||
return ObjectUtils.tryCast(ControlFlowUtil.findCodeFragment(expression), PsiCodeBlock.class);
|
||||
}
|
||||
}
|
||||
|
||||
+272
-16
@@ -1,25 +1,35 @@
|
||||
// Copyright 2000-2018 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.duplicateExpressions;
|
||||
|
||||
import com.intellij.codeInspection.InspectionsBundle;
|
||||
import com.intellij.codeInspection.LocalInspectionTool;
|
||||
import com.intellij.codeInspection.LocalInspectionToolSession;
|
||||
import com.intellij.codeInspection.ProblemsHolder;
|
||||
import com.intellij.codeInspection.*;
|
||||
import com.intellij.codeInspection.ui.SingleIntegerFieldOptionsPanel;
|
||||
import com.intellij.openapi.application.ApplicationManager;
|
||||
import com.intellij.openapi.editor.Editor;
|
||||
import com.intellij.openapi.fileEditor.FileEditorManager;
|
||||
import com.intellij.openapi.fileEditor.OpenFileDescriptor;
|
||||
import com.intellij.openapi.project.Project;
|
||||
import com.intellij.openapi.util.Ref;
|
||||
import com.intellij.openapi.util.UserDataHolder;
|
||||
import com.intellij.openapi.vfs.VirtualFile;
|
||||
import com.intellij.psi.*;
|
||||
import com.intellij.psi.controlFlow.*;
|
||||
import com.intellij.psi.util.PsiTreeUtil;
|
||||
import com.intellij.psi.util.PsiUtil;
|
||||
import com.intellij.refactoring.introduceVariable.InputValidator;
|
||||
import com.intellij.refactoring.introduceVariable.IntroduceVariableHandler;
|
||||
import com.intellij.refactoring.introduceVariable.IntroduceVariableSettings;
|
||||
import com.intellij.refactoring.ui.TypeSelectorManagerImpl;
|
||||
import com.intellij.refactoring.util.RefactoringUtil;
|
||||
import com.intellij.util.IncorrectOperationException;
|
||||
import com.siyeh.ig.psiutils.CommentTracker;
|
||||
import gnu.trove.THashMap;
|
||||
import gnu.trove.THashSet;
|
||||
import org.jetbrains.annotations.Nls;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
|
||||
import javax.swing.*;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.*;
|
||||
|
||||
/**
|
||||
* @author Pavel.Dolgov
|
||||
@@ -40,10 +50,32 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool {
|
||||
if (expression instanceof PsiParenthesizedExpression) {
|
||||
return;
|
||||
}
|
||||
visitExpressionImpl(expression);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void visitReferenceExpression(PsiReferenceExpression expression) {
|
||||
super.visitReferenceExpression(expression);
|
||||
|
||||
if (expression.getParent() instanceof PsiCallExpression) {
|
||||
return;
|
||||
}
|
||||
visitExpressionImpl(expression);
|
||||
}
|
||||
|
||||
public void visitExpressionImpl(PsiExpression expression) {
|
||||
if (ComplexityCalculator.isDefinitelySimple(expression, complexityThreshold) ||
|
||||
SideEffectCalculator.isDefinitelyWithSideEffect(expression)) {
|
||||
return;
|
||||
}
|
||||
DuplicateExpressionsContext context = DuplicateExpressionsContext.getOrCreateContext(expression, session);
|
||||
if (context == null || context.mayHaveSideEffect(expression)) {
|
||||
return;
|
||||
}
|
||||
PsiType type = RefactoringUtil.getTypeByExpressionWithExpectedType(expression);
|
||||
if (type == null || PsiType.VOID.equals(type)) {
|
||||
return;
|
||||
}
|
||||
if (context.getComplexity(expression) > complexityThreshold) {
|
||||
context.addOccurrence(expression);
|
||||
}
|
||||
@@ -55,7 +87,7 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool {
|
||||
|
||||
PsiCodeBlock body = method.getBody();
|
||||
if (body != null) {
|
||||
filterExpressions(body, session);
|
||||
registerProblemsForExpressions(body, session);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -63,7 +95,7 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool {
|
||||
public void visitClassInitializer(PsiClassInitializer initializer) {
|
||||
super.visitClassInitializer(initializer);
|
||||
|
||||
filterExpressions(initializer.getBody(), session);
|
||||
registerProblemsForExpressions(initializer.getBody(), session);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -72,11 +104,11 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool {
|
||||
|
||||
PsiElement body = expression.getBody();
|
||||
if (body instanceof PsiCodeBlock) {
|
||||
filterExpressions((PsiCodeBlock)body, session);
|
||||
registerProblemsForExpressions((PsiCodeBlock)body, session);
|
||||
}
|
||||
}
|
||||
|
||||
public void filterExpressions(@NotNull PsiCodeBlock body, @NotNull UserDataHolder session) {
|
||||
public void registerProblemsForExpressions(@NotNull PsiCodeBlock body, @NotNull UserDataHolder session) {
|
||||
DuplicateExpressionsContext context = DuplicateExpressionsContext.getContext(body, session);
|
||||
if (context == null) return;
|
||||
|
||||
@@ -84,14 +116,34 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool {
|
||||
context.forEach((pattern, occurrences) -> {
|
||||
if (!processed.contains(pattern)) {
|
||||
processed.addAll(occurrences);
|
||||
if (occurrences.size() > 1 && areSafeToExtract(occurrences, body)) {
|
||||
for (PsiExpression occurrence : occurrences) {
|
||||
holder.registerProblem(occurrence, InspectionsBundle.message("inspection.duplicate.expressions.message"));
|
||||
}
|
||||
}
|
||||
registerProblems(occurrences, body);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
public void registerProblems(@NotNull List<PsiExpression> occurrences, @NotNull PsiCodeBlock body) {
|
||||
if (occurrences.size() > 1 && areSafeToExtract(occurrences, body)) {
|
||||
Map<PsiExpression, List<PsiVariable>> reusableVariables = collectReusableVariables(occurrences);
|
||||
for (PsiExpression occurrence : occurrences) {
|
||||
List<LocalQuickFix> fixes = new ArrayList<>();
|
||||
List<PsiVariable> variables = reusableVariables.get(occurrence);
|
||||
if (variables != null) {
|
||||
for (PsiVariable variable : variables) {
|
||||
fixes.add(new ReuseVariableFix(occurrence, variable));
|
||||
}
|
||||
}
|
||||
PsiVariable variable = findVariableByInitializer(occurrence);
|
||||
if (variable != null && canReplaceOtherOccurrences(occurrence, occurrences, variable)) {
|
||||
fixes.add(new ReplaceOtherOccurrencesFix(occurrence, variable));
|
||||
}
|
||||
else if (isOnTheFly) {
|
||||
fixes.add(new IntroduceVariableFix(occurrence));
|
||||
}
|
||||
holder.registerProblem(occurrence, InspectionsBundle.message("inspection.duplicate.expressions.message"),
|
||||
fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -154,10 +206,214 @@ public class DuplicateExpressionsInspection extends LocalInspectionTool {
|
||||
return variables;
|
||||
}
|
||||
|
||||
@NotNull
|
||||
private static Map<PsiExpression, List<PsiVariable>> collectReusableVariables(@NotNull List<PsiExpression> occurrences) {
|
||||
if (occurrences.size() <= 1) {
|
||||
return Collections.emptyMap();
|
||||
}
|
||||
Map<PsiVariable, PsiExpression> initializers = new THashMap<>();
|
||||
for (PsiExpression occurrence : occurrences) {
|
||||
PsiVariable variable = findVariableByInitializer(occurrence);
|
||||
if (variable != null) {
|
||||
initializers.put(variable, occurrence);
|
||||
}
|
||||
}
|
||||
if (initializers.isEmpty()) {
|
||||
return Collections.emptyMap();
|
||||
}
|
||||
|
||||
Map<PsiExpression, List<PsiVariable>> result = new THashMap<>();
|
||||
initializers.forEach((variable, initializer) -> {
|
||||
for (PsiExpression occurrence : occurrences) {
|
||||
if (occurrence != initializer && canReplaceWith(occurrence, variable)) {
|
||||
result.computeIfAbsent(occurrence, unused -> new ArrayList<>()).add(variable);
|
||||
}
|
||||
}
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
private static boolean canReplaceOtherOccurrences(@NotNull PsiExpression originalOccurrence,
|
||||
@NotNull List<PsiExpression> occurrences,
|
||||
@NotNull PsiVariable variable) {
|
||||
return occurrences.stream().anyMatch(occurrence -> occurrence != originalOccurrence && canReplaceWith(occurrence, variable));
|
||||
}
|
||||
|
||||
private static boolean canReplaceWith(@NotNull PsiExpression occurrence, @NotNull PsiVariable variable) {
|
||||
String variableName = variable.getName();
|
||||
if (variableName == null) {
|
||||
return false;
|
||||
}
|
||||
PsiElementFactory factory = JavaPsiFacade.getInstance(variable.getProject()).getElementFactory();
|
||||
PsiExpression refExpr;
|
||||
try {
|
||||
refExpr = factory.createExpressionFromText(variableName, occurrence);
|
||||
}
|
||||
catch (IncorrectOperationException e) {
|
||||
return false;
|
||||
}
|
||||
return refExpr instanceof PsiReferenceExpression && ((PsiReferenceExpression)refExpr).resolve() == variable;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
private static PsiVariable findVariableByInitializer(@NotNull PsiExpression expression) {
|
||||
PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
|
||||
if (parent instanceof PsiVariable) {
|
||||
PsiVariable variable = (PsiVariable)parent;
|
||||
if (PsiTreeUtil.isAncestor(variable.getInitializer(), expression, false)) {
|
||||
return variable;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
@Override
|
||||
public JComponent createOptionsPanel() {
|
||||
return new SingleIntegerFieldOptionsPanel(
|
||||
InspectionsBundle.message("inspection.duplicate.complexity.threshold"), this, "complexityThreshold", 3);
|
||||
InspectionsBundle.message("inspection.duplicate.expressions.complexity.threshold"), this, "complexityThreshold", 3);
|
||||
}
|
||||
|
||||
private static class IntroduceVariableFix implements LocalQuickFix {
|
||||
private final String myExpressionText;
|
||||
|
||||
private IntroduceVariableFix(@NotNull PsiExpression expression) {myExpressionText = expression.getText();}
|
||||
|
||||
@Nls(capitalization = Nls.Capitalization.Sentence)
|
||||
@NotNull
|
||||
@Override
|
||||
public String getFamilyName() {
|
||||
return InspectionsBundle.message("inspection.duplicate.expressions.introduce.variable.fix.family.name");
|
||||
}
|
||||
|
||||
@Nls(capitalization = Nls.Capitalization.Sentence)
|
||||
@NotNull
|
||||
@Override
|
||||
public String getName() {
|
||||
return InspectionsBundle.message("inspection.duplicate.expressions.introduce.variable.fix.name", myExpressionText);
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean startInWriteAction() {
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
|
||||
PsiElement element = descriptor.getPsiElement();
|
||||
if (element instanceof PsiExpression) {
|
||||
VirtualFile virtualFile = element.getContainingFile().getVirtualFile();
|
||||
Editor editor = FileEditorManager.getInstance(project).openTextEditor(new OpenFileDescriptor(project, virtualFile), true);
|
||||
if (editor != null) {
|
||||
new MyIntroduceVariableHandler().invoke(project, editor, (PsiExpression)element);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static class MyIntroduceVariableHandler extends IntroduceVariableHandler {
|
||||
@Override
|
||||
public IntroduceVariableSettings getSettings(Project project, Editor editor, PsiExpression expr,
|
||||
PsiExpression[] occurrences, TypeSelectorManagerImpl typeSelectorManager,
|
||||
boolean declareFinalIfAll, boolean anyAssignmentLHS, InputValidator validator,
|
||||
PsiElement anchor, JavaReplaceChoice replaceChoice) {
|
||||
if (replaceChoice == null && ApplicationManager.getApplication().isUnitTestMode()) {
|
||||
replaceChoice = JavaReplaceChoice.ALL;
|
||||
}
|
||||
return super.getSettings(project, editor, expr, occurrences, typeSelectorManager,
|
||||
declareFinalIfAll, anyAssignmentLHS, validator, anchor, replaceChoice);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static class ReuseVariableFix implements LocalQuickFix {
|
||||
private final String myExpressionText;
|
||||
private final String myVariableName;
|
||||
|
||||
private ReuseVariableFix(@NotNull PsiExpression expression, @NotNull PsiVariable variable) {
|
||||
myExpressionText = expression.getText();
|
||||
myVariableName = variable.getName();
|
||||
}
|
||||
|
||||
@Nls(capitalization = Nls.Capitalization.Sentence)
|
||||
@NotNull
|
||||
@Override
|
||||
public String getFamilyName() {
|
||||
return InspectionsBundle.message("inspection.duplicate.expressions.reuse.variable.fix.family.name");
|
||||
}
|
||||
|
||||
@Nls(capitalization = Nls.Capitalization.Sentence)
|
||||
@NotNull
|
||||
@Override
|
||||
public String getName() {
|
||||
return InspectionsBundle.message("inspection.duplicate.expressions.reuse.variable.fix.name", myVariableName, myExpressionText);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
|
||||
PsiElement element = descriptor.getPsiElement();
|
||||
if (element instanceof PsiExpression) {
|
||||
new CommentTracker().replaceAndRestoreComments(element, myVariableName);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static class ReplaceOtherOccurrencesFix implements LocalQuickFix {
|
||||
private final String myExpressionText;
|
||||
private final String myVariableName;
|
||||
|
||||
private ReplaceOtherOccurrencesFix(@NotNull PsiExpression expression, @NotNull PsiVariable variable) {
|
||||
myExpressionText = expression.getText();
|
||||
myVariableName = variable.getName();
|
||||
}
|
||||
|
||||
@Nls(capitalization = Nls.Capitalization.Sentence)
|
||||
@NotNull
|
||||
@Override
|
||||
public String getFamilyName() {
|
||||
return InspectionsBundle.message("inspection.duplicate.expressions.replace.other.occurrences.fix.family.name");
|
||||
}
|
||||
|
||||
@Nls(capitalization = Nls.Capitalization.Sentence)
|
||||
@NotNull
|
||||
@Override
|
||||
public String getName() {
|
||||
return InspectionsBundle.message("inspection.duplicate.expressions.replace.other.occurrences.fix.name", myVariableName, myExpressionText);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
|
||||
PsiElement element = descriptor.getPsiElement();
|
||||
if (element instanceof PsiExpression) {
|
||||
List<PsiExpression> occurrences = collectReplaceableOccurrences((PsiExpression)element);
|
||||
for (PsiExpression occurrence : occurrences) {
|
||||
new CommentTracker().replaceAndRestoreComments(occurrence, myVariableName);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@NotNull
|
||||
private static List<PsiExpression> collectReplaceableOccurrences(@NotNull PsiExpression originalExpr) {
|
||||
PsiVariable variable = findVariableByInitializer(originalExpr);
|
||||
PsiCodeBlock nearestBody = DuplicateExpressionsContext.findNearestBody(originalExpr);
|
||||
if (variable != null && nearestBody != null) {
|
||||
List<PsiExpression> replaceableOccurrences = new ArrayList<>();
|
||||
nearestBody.accept(new JavaRecursiveElementWalkingVisitor() {
|
||||
final ExpressionHashingStrategy hashingStrategy = new ExpressionHashingStrategy();
|
||||
|
||||
@Override
|
||||
public void visitExpression(PsiExpression occurrence) {
|
||||
super.visitExpression(occurrence);
|
||||
|
||||
if (occurrence != originalExpr &&
|
||||
hashingStrategy.equals(occurrence, originalExpr) &&
|
||||
canReplaceWith(occurrence, variable)) {
|
||||
replaceableOccurrences.add(occurrence);
|
||||
}
|
||||
}
|
||||
});
|
||||
return replaceableOccurrences;
|
||||
}
|
||||
return Collections.emptyList();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+2
-24
@@ -1,44 +1,22 @@
|
||||
// Copyright 2000-2018 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.duplicateExpressions;
|
||||
|
||||
import com.intellij.psi.*;
|
||||
import com.intellij.psi.tree.IElementType;
|
||||
import com.intellij.psi.PsiAssignmentExpression;
|
||||
import com.intellij.psi.PsiUnaryExpression;
|
||||
import com.intellij.psi.util.PsiUtil;
|
||||
import com.siyeh.ig.psiutils.EquivalenceChecker;
|
||||
import com.siyeh.ig.psiutils.SideEffectChecker;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
/**
|
||||
* @author Pavel.Dolgov
|
||||
*/
|
||||
class NoSideEffectExpressionEquivalenceChecker extends EquivalenceChecker {
|
||||
@Override
|
||||
protected Match newExpressionsMatch(@NotNull PsiNewExpression newExpression1,
|
||||
@NotNull PsiNewExpression newExpression2) {
|
||||
return EXACT_MISMATCH;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Match methodCallExpressionsMatch(@NotNull PsiMethodCallExpression methodCallExpression1,
|
||||
@NotNull PsiMethodCallExpression methodCallExpression2) {
|
||||
if (SideEffectChecker.mayHaveSideEffects(methodCallExpression1)) {
|
||||
return EXACT_MISMATCH;
|
||||
}
|
||||
return super.methodCallExpressionsMatch(methodCallExpression1, methodCallExpression2);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Match assignmentExpressionsMatch(@NotNull PsiAssignmentExpression assignmentExpression1,
|
||||
@NotNull PsiAssignmentExpression assignmentExpression2) {
|
||||
return EXACT_MISMATCH;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Match arrayInitializerExpressionsMatch(@NotNull PsiArrayInitializerExpression arrayInitializerExpression1,
|
||||
@NotNull PsiArrayInitializerExpression arrayInitializerExpression2) {
|
||||
return EXACT_MISMATCH;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Match unaryExpressionsMatch(@NotNull PsiUnaryExpression unaryExpression1, @NotNull PsiUnaryExpression unaryExpression2) {
|
||||
if (PsiUtil.isIncrementDecrementOperation(unaryExpression1)) {
|
||||
|
||||
+8
-1
@@ -5,7 +5,6 @@ import com.intellij.codeInspection.dataFlow.CommonDataflow;
|
||||
import com.intellij.codeInspection.dataFlow.DfaFactType;
|
||||
import com.intellij.psi.*;
|
||||
import com.intellij.psi.util.PsiUtil;
|
||||
import com.intellij.util.ObjectUtils;
|
||||
import com.intellij.util.containers.ObjectIntHashMap;
|
||||
import com.siyeh.ig.psiutils.ClassUtils;
|
||||
import com.siyeh.ig.psiutils.MethodUtils;
|
||||
@@ -165,4 +164,12 @@ class SideEffectCalculator {
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Quick check to filter out the obvious things early
|
||||
*/
|
||||
static boolean isDefinitelyWithSideEffect(@Nullable PsiExpression expression) {
|
||||
return expression instanceof PsiAssignmentExpression ||
|
||||
PsiUtil.isIncrementDecrementOperation(expression);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
class C {
|
||||
int foo(String s) {
|
||||
if (<weak_warning descr="Multiple occurrences of 's.split(\",\").length'">s.split(",").length</weak_warning> > 2) {
|
||||
return (<weak_warning descr="Multiple occurrences of 's.split(\",\").length'">s.split(",").length</weak_warning>);
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
+11
@@ -0,0 +1,11 @@
|
||||
class C {
|
||||
String foo(String a, String b, StringBuilder s) {
|
||||
if (b.equals(<weak_warning descr="Multiple occurrences of 'a.substring(2) + b.substring(a.length())'">a.substring(2) + b.substring(a.length())</weak_warning>)) {
|
||||
return <weak_warning descr="Multiple occurrences of 'a.substring(2) + b.substring(a.length())'">a.substring(2) + b.substring(a.length())</weak_warning>;
|
||||
}
|
||||
if (s.append(a).append(b.substring(a.length())) != null) {
|
||||
return s.append(a).append(b.substring(a.length())).toString();
|
||||
}
|
||||
return a;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,17 @@
|
||||
class C {
|
||||
String foo(String s) {
|
||||
String t = s;
|
||||
int len = s.length();
|
||||
if (len >= 2 && (s.charAt(0) == '(' && s.charAt(len - 1) == ')')) {
|
||||
t = s.substring(1, len);
|
||||
}
|
||||
len--;
|
||||
if (len >= 2 && (s.charAt(0) == '{' && s.charAt(len - 1) == '}')) {
|
||||
t = s.substring(1, len);
|
||||
}
|
||||
if (len >= 2 && (s.charAt(0) == '[' && s.charAt(len - 1) == ']')) {
|
||||
t = s.substring(1, len);
|
||||
}
|
||||
return t;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
class C {
|
||||
String foo(String s) {
|
||||
String t = s;
|
||||
int len = s.length();
|
||||
if (len >= 3 && (s.charAt(0) == '(' && <weak_warning descr="Multiple occurrences of 's.charAt(len - 2)'">s.charAt(len - 2)</weak_warning> == ')')) {
|
||||
t = s.substring(1, len);
|
||||
}
|
||||
if (len >= 3 && (s.charAt(0) == '{' && <weak_warning descr="Multiple occurrences of 's.charAt(len - 2)'">s.charAt(len - 2)</weak_warning> == '}')) {
|
||||
t = s.substring(1, len);
|
||||
}
|
||||
if (len >= 3 && (s.charAt(0) == '[' && <weak_warning descr="Multiple occurrences of 's.charAt(len - 2)'">s.charAt(len - 2)</weak_warning> == ']')) {
|
||||
t = s.substring(1, len);
|
||||
}
|
||||
return t;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,15 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
if (ret == null) {
|
||||
ret = s.substring(s.length() - 9);
|
||||
}
|
||||
else {
|
||||
ret += "," + <caret>s.substring(s.length() - 9);
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
+17
@@ -0,0 +1,17 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substr = s.substring(s.length() - 9);
|
||||
if (ret == null) {
|
||||
ret = s.substring(s.length() - 9);
|
||||
}
|
||||
else {
|
||||
String substr2 = <caret>s.substring(s.length() - 9);
|
||||
ret += "," + substr2;
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
+18
@@ -0,0 +1,18 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substring = s.substring(s.length() - 9);
|
||||
String substr = substring;
|
||||
if (ret == null) {
|
||||
ret = substring;
|
||||
}
|
||||
else {
|
||||
String substr2 = substring;
|
||||
ret += "," + substr2;
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
+16
@@ -0,0 +1,16 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substring = s.substring(s.length() - 9);
|
||||
if (ret == null) {
|
||||
ret = substring;
|
||||
}
|
||||
else {
|
||||
ret += "," + substring;
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substr = <caret>s.substring(s.length() - 9);
|
||||
if (ret == null) {
|
||||
ret = s.substring(s.length() - 9);
|
||||
}
|
||||
else {
|
||||
ret += "," + s.substring(s.length() - 9);
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substr = s.substring(s.length() - 9);
|
||||
if (ret == null) {
|
||||
ret = substr;
|
||||
}
|
||||
else {
|
||||
ret += "," + substr;
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substr = s.substring(s.length() - 9);
|
||||
if (ret == null) {
|
||||
ret = <caret>s.substring(s.length() - 9);
|
||||
}
|
||||
else {
|
||||
ret += "," + s.substring(s.length() - 9);
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,16 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
String substr = s.substring(s.length() - 9);
|
||||
if (ret == null) {
|
||||
ret = substr;
|
||||
}
|
||||
else {
|
||||
ret += "," + s.substring(s.length() - 9);
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
+18
@@ -0,0 +1,18 @@
|
||||
class C {
|
||||
private static String toString(float... x) {
|
||||
String ret = null;
|
||||
for (float v : x) {
|
||||
String s = String.format("%f", v);
|
||||
{
|
||||
String substr = <caret>s.substring(s.length() - 9);
|
||||
}
|
||||
if (ret == null) {
|
||||
ret = s.substring(s.length() - 9);
|
||||
}
|
||||
else {
|
||||
ret += "," + s.substring(s.length() - 9);
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
+62
@@ -0,0 +1,62 @@
|
||||
// Copyright 2000-2018 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.java.codeInspection
|
||||
|
||||
import com.intellij.JavaTestUtil
|
||||
import com.intellij.codeInsight.intention.IntentionAction
|
||||
import com.intellij.codeInspection.InspectionsBundle
|
||||
import com.intellij.codeInspection.duplicateExpressions.DuplicateExpressionsInspection
|
||||
import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase
|
||||
|
||||
/**
|
||||
* @author Pavel.Dolgov
|
||||
*/
|
||||
class DuplicateExpressionsFixTest : LightCodeInsightFixtureTestCase() {
|
||||
val inspection = DuplicateExpressionsInspection()
|
||||
|
||||
override fun setUp() {
|
||||
super.setUp()
|
||||
myFixture.enableInspections(inspection)
|
||||
}
|
||||
|
||||
override fun getBasePath() = JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/duplicateExpressionsFix"
|
||||
|
||||
fun testIntroduceVariable() = doTest(introduce("s.substring(s.length() - 9)"))
|
||||
fun testReuseVariable() = doTest(reuse("substr", "s.substring(s.length() - 9)"))
|
||||
fun testReplaceOthers() = doTest(replace("substr", "s.substring(s.length() - 9)"))
|
||||
fun testIntroduceVariableOtherVariableNotInScope() = doTest(introduce("s.substring(s.length() - 9)"))
|
||||
fun testVariableNotInScopeCantReplaceOthers() = doNegativeTest(replace("substr", "s.substring(s.length() - 9)"))
|
||||
|
||||
private fun doTest(message: String, threshold: Int = 50) =
|
||||
withThreshold(threshold) {
|
||||
myFixture.configureByFile("${getTestName(false)}.java")
|
||||
myFixture.launchAction(myFixture.findSingleIntention(message))
|
||||
myFixture.checkResultByFile("${getTestName(false)}_after.java")
|
||||
}
|
||||
|
||||
private fun doNegativeTest(message: String, threshold: Int = 50) =
|
||||
withThreshold(threshold) {
|
||||
myFixture.configureByFile("${getTestName(false)}.java")
|
||||
val intentions = myFixture.filterAvailableIntentions(message)
|
||||
assertEquals(emptyList<IntentionAction>(), intentions)
|
||||
}
|
||||
|
||||
private fun withThreshold(threshold: Int, block: () -> Unit) {
|
||||
val oldThreshold = inspection.complexityThreshold
|
||||
try {
|
||||
inspection.complexityThreshold = threshold
|
||||
block()
|
||||
}
|
||||
finally {
|
||||
inspection.complexityThreshold = oldThreshold
|
||||
}
|
||||
}
|
||||
|
||||
private fun introduce(expr: String) =
|
||||
InspectionsBundle.message("inspection.duplicate.expressions.introduce.variable.fix.name", expr)!!
|
||||
|
||||
private fun reuse(name: String, expr: String) =
|
||||
InspectionsBundle.message("inspection.duplicate.expressions.reuse.variable.fix.name", name, expr)!!
|
||||
|
||||
private fun replace(name: String, expr: String) =
|
||||
InspectionsBundle.message("inspection.duplicate.expressions.replace.other.occurrences.fix.name", name, expr)!!
|
||||
}
|
||||
+6
-2
@@ -19,12 +19,16 @@ class DuplicateExpressionsTest : LightCodeInsightFixtureTestCase() {
|
||||
override fun getBasePath() = JavaTestUtil.getRelativeJavaTestDataPath() + "/inspection/duplicateExpressions"
|
||||
|
||||
fun testUpdateInWhileLoop() = doTest(70)
|
||||
fun testIntFloatMixedMath() = doTest(60)
|
||||
fun testIntFloatMixedMath() = doTest(50)
|
||||
fun testTernaryIf() = doTest(60)
|
||||
fun testStringStartsWith() = doTest(60)
|
||||
fun testFinalFieldsOfParameter() = doTest(100)
|
||||
fun testStringCharAt() = doTest(70)
|
||||
fun testStringCharAt() = doTest(60)
|
||||
fun testEquals() = doTest(70)
|
||||
fun testVariableModified() = doTest(50)
|
||||
fun testVariableNotModified() = doTest(50)
|
||||
fun testCompositeQualifier() = doTest(40)
|
||||
fun testMethodCallWithSideEffect() = doTest(70)
|
||||
|
||||
private fun doTest(threshold: Int = 50) {
|
||||
val oldThreshold = inspection.complexityThreshold
|
||||
|
||||
@@ -1006,4 +1006,10 @@ inspection.objects.equals.can.be.simplified.fix.name=Replace 'Objects.equals()'
|
||||
|
||||
inspection.duplicate.expressions.display.name=Multiple occurrences of the same expression
|
||||
inspection.duplicate.expressions.message=Multiple occurrences of <code>#ref</code> #loc
|
||||
inspection.duplicate.complexity.threshold=Expression complexity threshold
|
||||
inspection.duplicate.expressions.complexity.threshold=Expression complexity threshold
|
||||
inspection.duplicate.expressions.introduce.variable.fix.family.name=Introduce variable
|
||||
inspection.duplicate.expressions.introduce.variable.fix.name=Introduce variable for ''{0}''
|
||||
inspection.duplicate.expressions.reuse.variable.fix.family.name=Reuse variable
|
||||
inspection.duplicate.expressions.reuse.variable.fix.name=Reuse variable ''{0}'' for ''{1}''
|
||||
inspection.duplicate.expressions.replace.other.occurrences.fix.family.name=Replace with variable other occurrences of expression
|
||||
inspection.duplicate.expressions.replace.other.occurrences.fix.name=Replace with ''{0}'' other occurrences of ''{1}''
|
||||
Reference in New Issue
Block a user