From c3bd56a80ced32940bc45227db76bf073f2ea3a8 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Tue, 28 Jun 2016 14:56:28 +0300 Subject: [PATCH] Java inspection: convert the intention for "Make Type Generic" into an INFORMATION-level inspection (IDEA-157727) --- .../MakeTypeGenericInspection.java | 128 ++++++++++++++++++ .../intention/impl/MakeTypeGenericAction.java | 104 -------------- .../makeTypeGeneric/Field.java} | 0 .../makeTypeGeneric/Field_after.java} | 0 .../makeTypeGeneric/ImplementedRaw.java} | 0 .../makeTypeGeneric/LocalVariable.java | 7 + .../makeTypeGeneric/LocalVariable_after.java | 5 +- .../daemon/quickFix/MakeTypeGenericTest.java | 26 ---- .../codeInspection/MakeTypeGenericTest.java | 76 +++++++++++ .../src/messages/CodeInsightBundle.properties | 2 - .../src/messages/InspectionsBundle.properties | 3 + .../MakeTypeGeneric.html} | 2 +- .../before.java.template | 8 -- resources/src/META-INF/IdeaPlugin.xml | 8 +- 14 files changed, 221 insertions(+), 148 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/miscGenerics/MakeTypeGenericInspection.java delete mode 100644 java/java-impl/src/com/intellij/codeInsight/intention/impl/MakeTypeGenericAction.java rename java/java-tests/testData/{codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/before1.java => codeInspection/makeTypeGeneric/Field.java} (100%) rename java/java-tests/testData/{codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/after1.java => codeInspection/makeTypeGeneric/Field_after.java} (100%) rename java/java-tests/testData/{codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/beforeImplementedRaw.java => codeInspection/makeTypeGeneric/ImplementedRaw.java} (100%) create mode 100644 java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable.java rename resources-en/src/intentionDescriptions/MakeTypeGenericAction/after.java.template => java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable_after.java (50%) delete mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/MakeTypeGenericTest.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInspection/MakeTypeGenericTest.java rename resources-en/src/{intentionDescriptions/MakeTypeGenericAction/description.html => inspectionDescriptions/MakeTypeGeneric.html} (60%) delete mode 100644 resources-en/src/intentionDescriptions/MakeTypeGenericAction/before.java.template diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/miscGenerics/MakeTypeGenericInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/miscGenerics/MakeTypeGenericInspection.java new file mode 100644 index 000000000000..9b02882aa85d --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/miscGenerics/MakeTypeGenericInspection.java @@ -0,0 +1,128 @@ +/* + * 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.miscGenerics; + +import com.intellij.codeInspection.*; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.util.TypeConversionUtil; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * @author dsl + */ +public class MakeTypeGenericInspection extends BaseJavaBatchLocalInspectionTool { + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, + boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitVariable(PsiVariable variable) { + super.visitVariable(variable); + visitVariableImpl(variable); + } + + @Override + public void visitJavaToken(PsiJavaToken token) { + super.visitJavaToken(token); + if (token.getTokenType() == JavaTokenType.EQ) { + final PsiElement parent = token.getParent(); + if (parent instanceof PsiVariable) { + visitVariableImpl((PsiVariable)parent); + } + } + } + + private void visitVariableImpl(@NotNull PsiVariable variable) { + if (variable.getTypeElement() != null) { + final PsiType type = getSuggestedType(variable); + if (type != null) { + final String typeText = type.getCanonicalText(); + final String message = + InspectionsBundle.message("inspection.raw.variable.type.make.generic.text", variable.getName(), typeText); + holder.registerProblem(variable, message, new MyLocalQuickFix(message)); + } + } + } + }; + } + + @Nullable + private static PsiType getSuggestedType(@NotNull PsiVariable variable) { + final PsiExpression initializer = variable.getInitializer(); + if (initializer == null) return null; + final PsiType variableType = variable.getType(); + final PsiType initializerType = initializer.getType(); + if (!(variableType instanceof PsiClassType)) return null; + final PsiClassType variableClassType = (PsiClassType) variableType; + if (!variableClassType.isRaw()) return null; + if (!(initializerType instanceof PsiClassType)) return null; + final PsiClassType initializerClassType = (PsiClassType) initializerType; + if (initializerClassType.isRaw()) return null; + final PsiClassType.ClassResolveResult variableResolveResult = variableClassType.resolveGenerics(); + final PsiClassType.ClassResolveResult initializerResolveResult = initializerClassType.resolveGenerics(); + if (initializerResolveResult.getElement() == null) return null; + PsiClass variableResolved = variableResolveResult.getElement(); + if (variableResolved == null) return null; + PsiSubstitutor targetSubstitutor = TypeConversionUtil.getClassSubstitutor(variableResolved, initializerResolveResult.getElement(), initializerResolveResult.getSubstitutor()); + if (targetSubstitutor == null) return null; + PsiType type = JavaPsiFacade.getInstance(variable.getProject()).getElementFactory().createType(variableResolved, targetSubstitutor); + if (variableType.equals(type)) return null; + return type; + } + + private static class MyLocalQuickFix implements LocalQuickFix { + private String myName; + + public MyLocalQuickFix(@NotNull String name) { + myName = name; + } + + @Nls + @NotNull + @Override + public String getName() { + return myName; + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.raw.variable.type.make.generic.family"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + final PsiElement element = descriptor.getPsiElement(); + if (element instanceof PsiVariable) { + final PsiVariable variable = (PsiVariable)element; + final PsiTypeElement typeElement = variable.getTypeElement(); + if (typeElement != null) { + final PsiType type = getSuggestedType(variable); + if (type != null) { + final PsiElementFactory factory = JavaPsiFacade.getInstance(variable.getProject()).getElementFactory(); + typeElement.replace(factory.createTypeElement(type)); + } + } + } + } + } +} diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/MakeTypeGenericAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/MakeTypeGenericAction.java deleted file mode 100644 index 5a902c8ecd10..000000000000 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/MakeTypeGenericAction.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright 2000-2009 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.intention.impl; - -import com.intellij.codeInsight.CodeInsightBundle; -import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction; -import com.intellij.openapi.editor.CaretModel; -import com.intellij.openapi.editor.Editor; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Pair; -import com.intellij.psi.*; -import com.intellij.psi.util.PsiUtil; -import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.util.IncorrectOperationException; -import org.jetbrains.annotations.NotNull; - -/** - * @author dsl - */ -public class MakeTypeGenericAction extends PsiElementBaseIntentionAction { - private String variableName; - private String newTypeName; - - @Override - @NotNull - public String getFamilyName() { - return CodeInsightBundle.message("intention.make.type.generic.family"); - } - - @Override - @NotNull - public String getText() { - return CodeInsightBundle.message("intention.make.type.generic.text", variableName, newTypeName); - } - - @Override - public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) { - if (!PsiUtil.isLanguageLevel5OrHigher(element)) return false; - if (!element.isWritable()) return false; - return findVariable(element) != null; - } - - private Pair findVariable(final PsiElement element) { - PsiVariable variable = null; - PsiElement elementParent = element.getParent(); - if (element instanceof PsiIdentifier) { - if (elementParent instanceof PsiVariable) { - variable = (PsiVariable)elementParent; - } - } - else if (element instanceof PsiJavaToken) { - final PsiJavaToken token = (PsiJavaToken)element; - if (token.getTokenType() != JavaTokenType.EQ) return null; - if (elementParent instanceof PsiVariable) { - variable = (PsiVariable)elementParent; - } - } - if (variable == null) return null; - variableName = variable.getName(); - final PsiExpression initializer = variable.getInitializer(); - if (initializer == null) return null; - final PsiType variableType = variable.getType(); - final PsiType initializerType = initializer.getType(); - if (!(variableType instanceof PsiClassType)) return null; - final PsiClassType variableClassType = (PsiClassType) variableType; - if (!variableClassType.isRaw()) return null; - if (!(initializerType instanceof PsiClassType)) return null; - final PsiClassType initializerClassType = (PsiClassType) initializerType; - if (initializerClassType.isRaw()) return null; - final PsiClassType.ClassResolveResult variableResolveResult = variableClassType.resolveGenerics(); - final PsiClassType.ClassResolveResult initializerResolveResult = initializerClassType.resolveGenerics(); - if (initializerResolveResult.getElement() == null) return null; - PsiClass variableResolved = variableResolveResult.getElement(); - PsiSubstitutor targetSubstitutor = TypeConversionUtil.getClassSubstitutor(variableResolved, initializerResolveResult.getElement(), initializerResolveResult.getSubstitutor()); - if (targetSubstitutor == null) return null; - PsiType type = JavaPsiFacade.getInstance(variable.getProject()).getElementFactory().createType(variableResolved, targetSubstitutor); - if (variableType.equals(type)) return null; - newTypeName = type.getCanonicalText(); - return Pair.create(variable, type); - } - - @Override - public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException { - Pair pair = findVariable(element); - if (pair == null) return; - PsiVariable variable = pair.getFirst(); - PsiType type = pair.getSecond(); - - variable.getTypeElement().replace(JavaPsiFacade.getInstance(variable.getProject()).getElementFactory().createTypeElement(type)); - } -} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/before1.java b/java/java-tests/testData/codeInspection/makeTypeGeneric/Field.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/before1.java rename to java/java-tests/testData/codeInspection/makeTypeGeneric/Field.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/after1.java b/java/java-tests/testData/codeInspection/makeTypeGeneric/Field_after.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/after1.java rename to java/java-tests/testData/codeInspection/makeTypeGeneric/Field_after.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/beforeImplementedRaw.java b/java/java-tests/testData/codeInspection/makeTypeGeneric/ImplementedRaw.java similarity index 100% rename from java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric/beforeImplementedRaw.java rename to java/java-tests/testData/codeInspection/makeTypeGeneric/ImplementedRaw.java diff --git a/java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable.java b/java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable.java new file mode 100644 index 000000000000..c90b6d9ac298 --- /dev/null +++ b/java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable.java @@ -0,0 +1,7 @@ +import java.util.*; + +public class F { + void f() { + List list = new ArrayList(); + } +} \ No newline at end of file diff --git a/resources-en/src/intentionDescriptions/MakeTypeGenericAction/after.java.template b/java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable_after.java similarity index 50% rename from resources-en/src/intentionDescriptions/MakeTypeGenericAction/after.java.template rename to java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable_after.java index dcf819e7bd82..4178b2d99a5e 100644 --- a/resources-en/src/intentionDescriptions/MakeTypeGenericAction/after.java.template +++ b/java/java-tests/testData/codeInspection/makeTypeGeneric/LocalVariable_after.java @@ -1,7 +1,6 @@ -import java.util.ArrayList; -import java.util.List; +import java.util.*; -public class X { +public class F { void f() { List list = new ArrayList(); } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/MakeTypeGenericTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/MakeTypeGenericTest.java deleted file mode 100644 index 75d9cab9941e..000000000000 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/MakeTypeGenericTest.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2000-2012 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.codeInsight.daemon.LightIntentionActionTestCase; - -public class MakeTypeGenericTest extends LightIntentionActionTestCase { - public void test() throws Exception { doAllTests(); } - - @Override - protected String getBasePath() { - return "/codeInsight/daemonCodeAnalyzer/quickFix/makeTypeGeneric"; - } -} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/MakeTypeGenericTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/MakeTypeGenericTest.java new file mode 100644 index 000000000000..e8a9e75447d0 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/MakeTypeGenericTest.java @@ -0,0 +1,76 @@ +/* + * 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.JavaTestUtil; +import com.intellij.codeInsight.intention.IntentionAction; +import com.intellij.codeInspection.miscGenerics.MakeTypeGenericInspection; +import com.intellij.openapi.roots.ModuleRootModificationUtil; +import com.intellij.testFramework.IdeaTestUtil; +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; + +import java.util.List; + +public class MakeTypeGenericTest extends LightCodeInsightFixtureTestCase { + private MakeTypeGenericInspection myInspection = new MakeTypeGenericInspection(); + + @Override + protected String getBasePath() { + return JavaTestUtil.getRelativeJavaTestDataPath() + "/codeInspection/makeTypeGeneric"; + } + + @Override + protected void setUp() throws Exception { + super.setUp(); + ModuleRootModificationUtil.setModuleSdk(myModule, IdeaTestUtil.getMockJdk18()); + myFixture.enableInspections(myInspection); + } + + @Override + protected void tearDown() throws Exception { + try { + myFixture.disableInspections(myInspection); + } + finally { + super.tearDown(); + } + } + + public void testField() { + doTest("Change type of TT to java.util.Comparator"); + } + + public void testLocalVariable() { + doTest("Change type of list to java.util.List"); + } + + public void testImplementedRaw() { + assertIntentionNotAvailable("Change type of"); + } + + private void doTest(String intentionName) { + myFixture.configureByFiles(getTestName(false) + ".java"); + final IntentionAction singleIntention = myFixture.findSingleIntention(intentionName); + myFixture.launchAction(singleIntention); + myFixture.checkResultByFile(getTestName(false) + ".java", getTestName(false) + "_after.java", true); + } + + private void assertIntentionNotAvailable(String intentionName) { + myFixture.configureByFiles(getTestName(false) + ".java"); + final List intentionActions = myFixture.filterAvailableIntentions(intentionName); + assertEmpty(intentionName + " is not expected", intentionActions); + } +} diff --git a/platform/platform-resources-en/src/messages/CodeInsightBundle.properties b/platform/platform-resources-en/src/messages/CodeInsightBundle.properties index a9bbd0123a39..4f4c29ff3ce9 100644 --- a/platform/platform-resources-en/src/messages/CodeInsightBundle.properties +++ b/platform/platform-resources-en/src/messages/CodeInsightBundle.properties @@ -170,8 +170,6 @@ intention.join.declaration.text=Join declaration and assignment intention.split.declaration.assignment.text=Split into declaration and assignment intention.add.override.annotation=Add '@Override' annotation intention.add.override.annotation.family=Add Override Annotation -intention.make.type.generic.family=Make Type Generic -intention.make.type.generic.text=Change type of {0} to {1} intention.split.if.family=Split If intention.split.if.text=Split into 2 if's intention.split.filter.text=Split into filter's chain diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index eb0869dd2c1b..4776a3ebdbb0 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -215,6 +215,9 @@ inspection.suspicious.collections.method.calls.display.name=Suspicious collectio inspection.suspicious.collections.method.calls.problem.descriptor=''{0}'' may not contain objects of type ''{1}'' inspection.suspicious.collections.method.calls.problem.descriptor1=Suspicious call to ''{0}'' +inspection.raw.variable.type.make.generic.family=Make Type Generic +inspection.raw.variable.type.make.generic.text=Change type of {0} to {1} + inspection.reference.invalid=element no longer exists inspection.reference.default.package=default package inspection.reference.implicit.constructor.name=implicit constructor of {0} diff --git a/resources-en/src/intentionDescriptions/MakeTypeGenericAction/description.html b/resources-en/src/inspectionDescriptions/MakeTypeGeneric.html similarity index 60% rename from resources-en/src/intentionDescriptions/MakeTypeGenericAction/description.html rename to resources-en/src/inspectionDescriptions/MakeTypeGeneric.html index 952bf189f5fa..0c2c900e406a 100644 --- a/resources-en/src/intentionDescriptions/MakeTypeGenericAction/description.html +++ b/resources-en/src/inspectionDescriptions/MakeTypeGeneric.html @@ -1,6 +1,6 @@ -This intention considers variable declaration with initializer and adjusts variable type +This inspection considers variable declaration with initializer and adjusts variable type if it was declared with raw type whereas initializer has fully parameterized generic type. diff --git a/resources-en/src/intentionDescriptions/MakeTypeGenericAction/before.java.template b/resources-en/src/intentionDescriptions/MakeTypeGenericAction/before.java.template deleted file mode 100644 index 9746dcfbade4..000000000000 --- a/resources-en/src/intentionDescriptions/MakeTypeGenericAction/before.java.template +++ /dev/null @@ -1,8 +0,0 @@ -import java.util.ArrayList; -import java.util.List; - -public class X { - void f() { - List list = new ArrayList(); - } -} \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index ca43d4c44eda..ba9be7dbbc98 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -636,6 +636,10 @@ key="inspection.suspicious.collections.method.calls.display.name" groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.miscGenerics.SuspiciousCollectionsMethodCallsInspection"/> + @@ -949,10 +953,6 @@ com.intellij.codeInsight.daemon.impl.quickfix.AddRuntimeExceptionToThrowsAction Java/Declaration - - com.intellij.codeInsight.intention.impl.MakeTypeGenericAction - Java/Declaration - com.intellij.codeInsight.intention.impl.AddOverrideAnnotationAction Java/Annotations