From d26ca9f4e7561da4b9eb3a0d935b2f023aee5e17 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 6 Apr 2011 12:07:08 +0200 Subject: [PATCH] better quickfixes Use Arrays.deepEquals() on arrays with more than one dimension --- .../siyeh/InspectionGadgetsBundle.properties | 10 +-- .../ig/bugs/ArrayEqualityInspection.java | 43 ++++++++---- .../siyeh/ig/bugs/ArrayEqualsInspection.java | 69 ++++++++++++++----- 3 files changed, 86 insertions(+), 36 deletions(-) diff --git a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties index b958c1a993ee..690b6719133d 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties @@ -55,7 +55,8 @@ archaic.system.property.accessors.replace.parse.quickfix=Replace with parse meth archaic.system.property.accessors.replace.standard.quickfix=Replace with standard property access equals.called.on.array.display.name='equals()' called on array type equals.called.on.array.problem.descriptor=#ref() between arrays should probably be 'Arrays.equals()' #loc -equals.called.on.array.replace.quickfix=Replace with 'Arrays.equals()' +replace.with.arrays.equals=Replace with 'Arrays.equals()' +replace.with.arrays.deep.equals=Replace with 'Arrays.deepEquals()' assignment.to.null.display.name=Assignment to 'null' assignment.to.null.problem.descriptor=Assignment of variable #ref to null #loc assignment.to.static.field.from.instance.method.display.name=Assignment to static field from instance method @@ -887,7 +888,7 @@ nested.switch.statement.problem.descriptor=Nested #ref statement #l chained.method.call.problem.descriptor=Chained method call #ref() #loc nested.method.call.problem.descriptor=Nested method call #ref() #loc octal.literal.problem.descriptor=Octal integer #ref #loc -implicit.call.to.super.problem.descriptor=Implicit call to super() #ref #loc +implicit.call.to.super.problem.descriptor=Implicit call to 'super()' #loc negated.if.else.problem.descriptor=#ref statement with negated condition #loc negated.conditional.problem.descriptor=Conditional expression with negated condition #loc confusing.else.problem.descriptor=#ref branch may be unwrapped, as the 'if' branch never completes #loc @@ -1346,7 +1347,7 @@ flip.comparison.quickfix=Flip comparison control.flow.statement.without.braces.add.quickfix=Add braces extends.object.remove.quickfix=Remove redundant 'extends Object' implicit.call.to.super.ignore.option=Ignore for direct subclasses of java.lang.Object -implicit.call.to.super.make.explicit.quickfix=Make construction of super() explicit +implicit.call.to.super.make.explicit.quickfix=Make call to 'super()' explicit missorted.modifiers.require.option=Require annotations to be sorted before keywords missorted.modifiers.sort.quickfix=Sort modifiers nested.method.call.ignore.option=Ignore nested method calls in field initializers @@ -1598,7 +1599,7 @@ collection.contains.url.problem.decriptor={0} #ref may contain URL collection.contains.url.display.name=Map or Set may contain java.net.URL objects implicit.array.to.string.problem.descriptor=Implicit call to method 'toString()' on array #ref #loc implicit.array.to.string.method.call.problem.descriptor=Implicit call to method 'toString()' on array returned by #ref call #loc -implicit.array.to.string.display.name=Implicit call to array '.toString()' +implicit.array.to.string.display.name=Call to array '.toString()' implicit.array.to.string.quickfix=Wrap with ''{0}'' expression suspicious.indent.after.control.statement.problem.descriptor=#ref statement has suspicious indentation #loc suspicious.indent.after.control.statement.display.name=Suspicious indentation after control statement without braces @@ -1869,4 +1870,3 @@ try.finally.can.be.try.with.resources.problem.descriptor=#ref can u try.finally.can.be.try.with.resources.quickfix=Replace with 'try' with resources array.comparison.display.name=Array comparison using '==', instead of 'Arrays.equals()' array.comparison.problem.descriptor=Array objects are compared using #ref, not 'Arrays.equals()' #loc -array.comparison.quickfix=Replace with 'Arrays.equals()' diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualityInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualityInspection.java index 7e13f97ba92b..6790e89fc667 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualityInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualityInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers + * Copyright 2011 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -46,15 +46,32 @@ public class ArrayEqualityInspection extends BaseInspection { @Override public InspectionGadgetsFix buildFix(Object... infos) { - return new ArrayEqualityFix(); + final PsiArrayType type = (PsiArrayType) infos[0]; + final PsiType componentType = type.getComponentType(); + if (componentType instanceof PsiArrayType) { + return new ArrayEqualityFix(true); + } + return new ArrayEqualityFix(false); } private static class ArrayEqualityFix extends InspectionGadgetsFix { - + + private final boolean deepEquals; + + public ArrayEqualityFix(boolean deepEquals) { + this.deepEquals = deepEquals; + } + @NotNull @Override public String getName() { - return InspectionGadgetsBundle.message("array.comparison.quickfix"); + if (deepEquals) { + return InspectionGadgetsBundle.message( + "replace.with.arrays.deep.equals"); + } else { + return InspectionGadgetsBundle.message( + "replace.with.arrays.equals"); + } } @Override @@ -75,7 +92,11 @@ public class ArrayEqualityInspection extends BaseInspection { } else if (!JavaTokenType.EQEQ.equals(tokenType)) { return; } - newExpressionText.append("java.util.Arrays.equals("); + if (deepEquals) { + newExpressionText.append("java.util.Arrays.deepEquals("); + } else { + newExpressionText.append("java.util.Arrays.equals("); + } newExpressionText.append(binaryExpression.getLOperand().getText()); newExpressionText.append(','); final PsiExpression rhs = binaryExpression.getROperand(); @@ -99,18 +120,16 @@ public class ArrayEqualityInspection extends BaseInspection { @Override public void visitBinaryExpression( @NotNull PsiBinaryExpression expression) { super.visitBinaryExpression(expression); - if(!(expression.getROperand() != null)){ + final PsiExpression rhs = expression.getROperand(); + if (rhs == null) { return; } if (!ComparisonUtils.isEqualityComparison(expression)) { return; } final PsiExpression lhs = expression.getLOperand(); - if (!(lhs.getType() instanceof PsiArrayType)) { - return; - } - final PsiExpression rhs = expression.getROperand(); - if (rhs == null) { + final PsiType lhsType = lhs.getType(); + if (!(lhsType instanceof PsiArrayType)) { return; } if (!(rhs.getType() instanceof PsiArrayType)) { @@ -125,7 +144,7 @@ public class ArrayEqualityInspection extends BaseInspection { return; } final PsiJavaToken sign = expression.getOperationSign(); - registerError(sign); + registerError(sign, lhsType); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualsInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualsInspection.java index 1dd14f3c2e4c..fad4e57a6d69 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualsInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/ArrayEqualsInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2008 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2011 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,34 +29,57 @@ import org.jetbrains.annotations.NotNull; public class ArrayEqualsInspection extends BaseInspection { + @Override @NotNull public String getDisplayName(){ return InspectionGadgetsBundle.message( "equals.called.on.array.display.name"); } + @Override @NotNull public String buildErrorString(Object... infos){ return InspectionGadgetsBundle.message( "equals.called.on.array.problem.descriptor"); } + @Override public boolean isEnabledByDefault() { return true; } + @Override public InspectionGadgetsFix buildFix(Object... infos){ - return new ArrayEqualsFix(); + final PsiArrayType type = (PsiArrayType)infos[0]; + if (type != null) { + final PsiType componentType = type.getComponentType(); + if (componentType instanceof PsiArrayType) { + return new ArrayEqualsFix(true); + } + } + return new ArrayEqualsFix(false); } private static class ArrayEqualsFix extends InspectionGadgetsFix{ - @NotNull - public String getName(){ - return InspectionGadgetsBundle.message( - "equals.called.on.array.replace.quickfix"); + private final boolean deepEquals; + + public ArrayEqualsFix(boolean deepEquals) { + this.deepEquals = deepEquals; } + @NotNull + public String getName(){ + if (deepEquals) { + return InspectionGadgetsBundle.message( + "replace.with.arrays.deep.equals"); + } else { + return InspectionGadgetsBundle.message( + "replace.with.arrays.equals"); + } + } + + @Override public void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException{ final PsiIdentifier name = @@ -71,15 +94,23 @@ public class ArrayEqualsInspection extends BaseInspection { final String qualifierText = qualifier.getText(); assert call != null; final PsiExpressionList argumentList = call.getArgumentList(); - final PsiExpression[] args = argumentList.getExpressions(); - final String argText = args[0].getText(); - @NonNls final String newExpressionText = - "java.util.Arrays.equals(" + qualifierText + ", " + - argText + ')'; - replaceExpressionAndShorten(call, newExpressionText); + final PsiExpression[] arguments = argumentList.getExpressions(); + final String argumentText = arguments[0].getText(); + @NonNls final StringBuilder newExpressionText = new StringBuilder(); + if (deepEquals) { + newExpressionText.append("java.util.Arrays.deepEquals("); + } else { + newExpressionText.append("java.util.Arrays.equals("); + } + newExpressionText.append(qualifierText); + newExpressionText.append(", "); + newExpressionText.append(argumentText); + newExpressionText.append(')'); + replaceExpressionAndShorten(call, newExpressionText.toString()); } } + @Override public BaseInspectionVisitor buildVisitor(){ return new ArrayEqualsVisitor(); } @@ -95,16 +126,16 @@ public class ArrayEqualsInspection extends BaseInspection { final PsiReferenceExpression methodExpression = expression.getMethodExpression(); final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] args = argumentList.getExpressions(); - if (args.length == 0) { + final PsiExpression[] arguments = argumentList.getExpressions(); + if (arguments.length == 0) { return; } - final PsiExpression arg = args[0]; - if(arg == null){ + final PsiExpression argument = arguments[0]; + if(argument == null){ return; } - final PsiType argType = arg.getType(); - if(!(argType instanceof PsiArrayType)){ + final PsiType argumentType = argument.getType(); + if(!(argumentType instanceof PsiArrayType)){ return; } final PsiExpression qualifier = @@ -116,7 +147,7 @@ public class ArrayEqualsInspection extends BaseInspection { if(!(qualifierType instanceof PsiArrayType)){ return; } - registerMethodCallError(expression); + registerMethodCallError(expression, qualifierType); } } } \ No newline at end of file