IDEA-160239 Warn about using Integer::min, Integer::max where Comparator expected

This commit is contained in:
Tagir Valeev
2016-08-25 12:33:03 +03:00
parent cda735ca81
commit f96b5696c3
9 changed files with 215 additions and 0 deletions

View File

@@ -0,0 +1,100 @@
/*
* Copyright 2000-2016 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;
import com.intellij.codeInsight.FileModificationService;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
public class InvalidComparatorMethodReferenceInspection extends BaseJavaBatchLocalInspectionTool {
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, boolean isOnTheFly) {
return new JavaElementVisitor() {
@Override
public void visitMethodReferenceExpression(PsiMethodReferenceExpression expression) {
PsiExpression qualifierExpression = expression.getQualifierExpression();
PsiElement referenceNameElement = expression.getReferenceNameElement();
if (!(qualifierExpression instanceof PsiReference) || referenceNameElement == null) {
return;
}
String name = referenceNameElement.getText();
if (!name.equals("min") && !name.equals("max")) {
return;
}
PsiType functionalInterfaceType = expression.getFunctionalInterfaceType();
if (!(functionalInterfaceType instanceof PsiClassType)) {
return;
}
PsiClass targetType = ((PsiClassType)functionalInterfaceType).resolve();
if (targetType == null) {
return;
}
PsiElement refType = ((PsiReference)qualifierExpression).resolve();
if (!(refType instanceof PsiClass)) {
return;
}
String className = ((PsiClass)refType).getQualifiedName();
if (className == null || (!className.equals(Integer.class.getName()) && !className.equals(Math.class.getName()))) {
return;
}
//noinspection DialogTitleCapitalization
holder
.registerProblem(expression,
"Method reference mapped to Comparator interface does not fulfill the Comparator contract",
new ReplaceWithComparatorQuickFix(name.equals("min")));
}
};
}
private static class ReplaceWithComparatorQuickFix implements LocalQuickFix {
private final boolean reverse;
public ReplaceWithComparatorQuickFix(boolean reverse) {
this.reverse = reverse;
}
@Nls
@NotNull
@Override
public String getName() {
return "Replace with " + (reverse ? "Comparator.reverseOrder()" : "Comparator.naturalOrder()");
}
@Nls
@NotNull
@Override
public String getFamilyName() {
return "Replace with comparator";
}
@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiElement element = descriptor.getPsiElement();
if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return;
PsiElement parent = element.getParent();
if (parent != null) {
PsiExpression newMethodExpression = JavaPsiFacade.getElementFactory(project)
.createExpressionFromText(CommonClassNames.JAVA_UTIL_COMPARATOR + "." + (reverse ? "reverseOrder()" : "naturalOrder()"), parent);
PsiElement shortMethodExpression = JavaCodeStyleManager.getInstance(project).shortenClassReferences(newMethodExpression);
element.replace(shortMethodExpression);
}
}
}
}

View File

@@ -0,0 +1,12 @@
// "Replace with Comparator.naturalOrder()" "true"
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
public class Main {
public static void main(String[] args) {
List<Integer> ints = Arrays.asList(3,12,-2,3,-1,-4,4,args.length);
System.out.println(ints.stream().max(Comparator.naturalOrder()));
}
}

View File

@@ -0,0 +1,13 @@
// "Replace with Comparator.reverseOrder()" "true"
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
public class Main {
public static void main(String[] args) {
List<Integer> ints = Arrays.asList(3,12,-2,3,-1,-4,4,args.length);
ints.sort(Comparator.reverseOrder());
System.out.println(ints);
}
}

View File

@@ -0,0 +1,16 @@
// "Replace with Comparator.naturalOrder()" "false"
import java.util.Arrays;
import java.util.List;
public class Main {
static class Integer {
static int max(int a, int b) {
return a == b ? 0 : a < b ? -1 : 1;
}
}
public static void main(String[] args) {
List<java.lang.Integer> ints = Arrays.asList(3,12,-2,3,-1,-4,4,args.length);
System.out.println(ints.stream().max(Int<caret>eger::max));
}
}

View File

@@ -0,0 +1,11 @@
// "Replace with Comparator.naturalOrder()" "true"
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
public class Main {
public static void main(String[] args) {
List<Integer> ints = Arrays.asList(3,12,-2,3,-1,-4,4,args.length);
System.out.println(ints.stream().max(Int<caret>eger::max));
}
}

View File

@@ -0,0 +1,12 @@
// "Replace with Comparator.reverseOrder()" "true"
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
public class Main {
public static void main(String[] args) {
List<Integer> ints = Arrays.asList(3,12,-2,3,-1,-4,4,args.length);
ints.sort(Math::m<caret>in);
System.out.println(ints);
}
}

View File

@@ -0,0 +1,38 @@
/*
* Copyright 2000-2016 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.codeInsight.daemon.quickFix;
import com.intellij.codeInspection.InvalidComparatorMethodReferenceInspection;
import com.intellij.codeInspection.LocalInspectionTool;
import org.jetbrains.annotations.NotNull;
public class InvalidComparatorMethodReferenceInspectionTest extends LightQuickFixParameterizedTestCase {
@NotNull
@Override
protected LocalInspectionTool[] configureLocalInspectionTools() {
return new LocalInspectionTool[]{
new InvalidComparatorMethodReferenceInspection(),
};
}
public void test() throws Exception { doAllTests(); }
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/invalidComparatorMethodReference";
}
}

View File

@@ -0,0 +1,10 @@
<html>
<body>
This inspection reports method references mapped to Comparator interface which don't fulfill its contract.
<!-- tooltip end -->
<p>
Some method references like <code>Integer::max</code> can be mapped to <code>Comparator</code> interface.
However using them as <code>Comparator</code> is useless and result might be unpredictable.
</p>
</body>
</html>

View File

@@ -755,6 +755,9 @@
<localInspection groupPath="Java" language="JAVA" shortName="Convert2MethodRef" displayName="Lambda can be replaced with method reference"
groupKey="group.names.language.level.specific.issues.and.migration.aids" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.LambdaCanBeMethodReferenceInspection" />
<localInspection groupPath="Java" language="JAVA" shortName="InvalidComparatorMethodReference" displayName="Invalid method reference used for Comparator"
groupKey="group.names.probable.bugs" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.InvalidComparatorMethodReferenceInspection"/>
<localInspection groupPath="Java" language="JAVA" shortName="TrivialMethodReference" displayName="Method reference can be replaced with its qualifier"
groupKey="group.names.declaration.redundancy" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.TrivialMethodReferenceInspection"/>