mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-01-08 15:09:39 +07:00
IDEA-CR-2295: [dynamic plugins] an inspection to detect potentially leaking map keys (Language/FileType) #IDEA-245855 fixed
(cherry picked from commit 424816555af618c0727a95dab1e5c154d056737b) GitOrigin-RevId: 7952518f44a66b29739214571a156fa906c82c43
This commit is contained in:
committed by
intellij-monorepo-bot
parent
4bd450284e
commit
f0ce428844
@@ -241,6 +241,11 @@
|
||||
groupPath="Plugin DevKit" groupKey="inspections.group.code"
|
||||
enabledByDefault="true" level="WARNING"
|
||||
implementationClass="org.jetbrains.idea.devkit.inspections.IncorrectParentDisposableInspection"/>
|
||||
<localInspection language="UAST"
|
||||
key="inspections.leakable.map.key.name"
|
||||
groupPath="Plugin DevKit" groupKey="inspections.group.code"
|
||||
enabledByDefault="true" level="WARNING"
|
||||
implementationClass="org.jetbrains.idea.devkit.inspections.LeakableMapKeyInspection"/>
|
||||
|
||||
<moduleConfigurationEditorProvider implementation="org.jetbrains.idea.devkit.module.PluginModuleEditorsProvider"/>
|
||||
<implicitUsageProvider implementation="org.jetbrains.idea.devkit.inspections.DevKitImplicitUsageProvider"/>
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
<html>
|
||||
<body>
|
||||
This inspection detects passing <code>Language</code> or <code>FileType</code> as a map key in plugin code.
|
||||
<p>
|
||||
Such a usage might lead to inability to unload the plugin properly. Please consider using <code>String</code> instead.
|
||||
</p>
|
||||
</body>
|
||||
</html>
|
||||
@@ -271,6 +271,10 @@ inspections.plugin.xml.i18n.inspection.tag.family.name=Extract displayName for i
|
||||
inspections.plugin.xml.i18n.choose.bundle.4inspections.title=Choose Bundle
|
||||
inspections.plugin.xml.i18n.key=Extract text for i18n
|
||||
|
||||
inspections.leakable.map.key.name=Map key may leak
|
||||
inspections.leakable.map.key.text=Consider using ''String'' instead of ''{0}'' as the map key
|
||||
inspections.leakable.map.key.quick.fix.name=Parameterize with ''{0}''
|
||||
|
||||
line.marker.related.property.title=Related Property
|
||||
line.marker.related.property.description=Related property
|
||||
|
||||
|
||||
@@ -0,0 +1,196 @@
|
||||
// Copyright 2000-2020 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 org.jetbrains.idea.devkit.inspections;
|
||||
|
||||
import com.intellij.codeInsight.FileModificationService;
|
||||
import com.intellij.codeInspection.LocalQuickFix;
|
||||
import com.intellij.codeInspection.ProblemDescriptor;
|
||||
import com.intellij.codeInspection.ProblemsHolder;
|
||||
import com.intellij.codeInspection.util.IntentionFamilyName;
|
||||
import com.intellij.lang.Language;
|
||||
import com.intellij.lang.java.JavaLanguage;
|
||||
import com.intellij.openapi.fileTypes.FileType;
|
||||
import com.intellij.openapi.project.Project;
|
||||
import com.intellij.psi.*;
|
||||
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
|
||||
import com.intellij.psi.search.GlobalSearchScope;
|
||||
import com.intellij.uast.UastHintedVisitorAdapter;
|
||||
import org.jetbrains.annotations.NonNls;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
import org.jetbrains.idea.devkit.DevKitBundle;
|
||||
import org.jetbrains.uast.UField;
|
||||
import org.jetbrains.uast.UTypeReferenceExpression;
|
||||
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import static com.intellij.psi.CommonClassNames.JAVA_LANG_STRING_SHORT;
|
||||
import static com.intellij.psi.util.InheritanceUtil.isInheritorOrSelf;
|
||||
import static com.intellij.psi.util.PsiTypesUtil.getPsiClass;
|
||||
import static com.intellij.util.ArrayUtil.getFirstElement;
|
||||
import static com.intellij.util.containers.ContainerUtil.exists;
|
||||
import static java.util.Arrays.asList;
|
||||
|
||||
public final class LeakableMapKeyInspection extends DevKitUastInspectionBase {
|
||||
|
||||
@Override
|
||||
protected boolean isAllowed(@NotNull ProblemsHolder holder) {
|
||||
return DevKitInspectionBase.isAllowedInPluginsOnly(holder);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected @NotNull PsiElementVisitor buildInternalVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
|
||||
JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(holder.getProject());
|
||||
PsiFile file = holder.getFile();
|
||||
GlobalSearchScope resolveScope = file.getResolveScope();
|
||||
|
||||
PsiClass languageClass = psiFacade.findClass(Language.class.getName(), resolveScope);
|
||||
PsiClass fileTypeClass = psiFacade.findClass(FileType.class.getName(), resolveScope);
|
||||
PsiClass mapClass = psiFacade.findClass(Map.class.getName(), resolveScope);
|
||||
|
||||
if (languageClass == null ||
|
||||
fileTypeClass == null ||
|
||||
mapClass == null) {
|
||||
return PsiElementVisitor.EMPTY_VISITOR;
|
||||
}
|
||||
|
||||
UFieldVisitor visitor = new UFieldVisitor(
|
||||
holder,
|
||||
mapClass,
|
||||
asList(languageClass, fileTypeClass)
|
||||
);
|
||||
|
||||
//noinspection unchecked
|
||||
return UastHintedVisitorAdapter.create(
|
||||
file.getLanguage(),
|
||||
visitor,
|
||||
new Class[]{UField.class}
|
||||
);
|
||||
}
|
||||
|
||||
private static final class UFieldVisitor extends AbstractUastNonRecursiveVisitor {
|
||||
|
||||
private final @NotNull ProblemsHolder myHolder;
|
||||
private final @NotNull PsiClassType myMapType;
|
||||
private final @NotNull List<? extends PsiClass> myBaseClasses;
|
||||
|
||||
private UFieldVisitor(@NotNull ProblemsHolder holder,
|
||||
@NotNull PsiClass mapClass,
|
||||
@NotNull List<? extends PsiClass> classes) {
|
||||
myHolder = holder;
|
||||
myMapType = PsiElementFactory.getInstance(holder.getProject()).createType(mapClass);
|
||||
myBaseClasses = classes;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean visitField(@NotNull UField field) {
|
||||
PsiType fieldType = field.getType();
|
||||
|
||||
if (myMapType.isAssignableFrom(fieldType)) {
|
||||
PsiClass keyClass = getKeyClass(fieldType);
|
||||
|
||||
if (keyClass != null && isLeakable(keyClass)) {
|
||||
PsiElement typeElement = getTypeElement(field);
|
||||
|
||||
//noinspection UElementAsPsi
|
||||
PsiElement element = typeElement == null ?
|
||||
field.getNameIdentifier() :
|
||||
typeElement;
|
||||
|
||||
myHolder.registerProblem(
|
||||
element,
|
||||
DevKitBundle.message("inspections.leakable.map.key.text", keyClass.getName()),
|
||||
ReplaceKeyQuickFix.createFixesFor(element)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
private boolean isLeakable(@Nullable PsiClass keyClass) {
|
||||
return exists(
|
||||
myBaseClasses,
|
||||
baseClass -> isInheritorOrSelf(keyClass, baseClass, true)
|
||||
);
|
||||
}
|
||||
|
||||
private static @Nullable PsiClass getKeyClass(@NotNull PsiType fieldType) {
|
||||
PsiType[] parameters = fieldType instanceof PsiClassType ?
|
||||
((PsiClassType)fieldType).getParameters() :
|
||||
PsiType.EMPTY_ARRAY;
|
||||
|
||||
PsiType parameterType = getFirstElement(parameters);
|
||||
|
||||
PsiType boundType = parameterType instanceof PsiWildcardType ?
|
||||
((PsiWildcardType)parameterType).getBound() :
|
||||
null;
|
||||
|
||||
return getPsiClass(boundType == null ? parameterType : boundType);
|
||||
}
|
||||
|
||||
private static @Nullable PsiElement getTypeElement(@NotNull UField field) {
|
||||
UTypeReferenceExpression typeReference = field.getTypeReference();
|
||||
PsiElement typeReferencePsi = typeReference == null ?
|
||||
null :
|
||||
typeReference.getSourcePsi();
|
||||
|
||||
return typeReferencePsi instanceof PsiTypeElement ?
|
||||
getFirstKey(((PsiTypeElement)typeReferencePsi)) :
|
||||
typeReferencePsi;
|
||||
}
|
||||
|
||||
private static @Nullable PsiTypeElement getFirstKey(@NotNull PsiTypeElement typeElement) {
|
||||
PsiJavaCodeReferenceElement typeReference = typeElement.getInnermostComponentReferenceElement();
|
||||
if (typeReference == null) return null;
|
||||
|
||||
PsiReferenceParameterList parameterList = typeReference.getParameterList();
|
||||
return parameterList == null ?
|
||||
null :
|
||||
getFirstElement(parameterList.getTypeParameterElements());
|
||||
}
|
||||
}
|
||||
|
||||
private static final class ReplaceKeyQuickFix implements LocalQuickFix {
|
||||
|
||||
private final @NotNull @NonNls String myText;
|
||||
|
||||
private ReplaceKeyQuickFix(@NotNull String text) {
|
||||
myText = text;
|
||||
}
|
||||
|
||||
@Override
|
||||
public @IntentionFamilyName @NotNull String getFamilyName() {
|
||||
return DevKitBundle.message("inspections.leakable.map.key.quick.fix.name", myText);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
|
||||
PsiElement element = descriptor.getPsiElement();
|
||||
assert isJavaTypeElement(element);
|
||||
|
||||
if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return;
|
||||
|
||||
PsiTypeElement stringTypeElement = PsiElementFactory.getInstance(project)
|
||||
.createTypeElementFromText(myText, element.getContext());
|
||||
|
||||
JavaCodeStyleManager.getInstance(project)
|
||||
.shortenClassReferences(element.replace(stringTypeElement));
|
||||
}
|
||||
|
||||
static @NotNull LocalQuickFix @NotNull [] createFixesFor(@NotNull PsiElement element) {
|
||||
return isJavaTypeElement(element) ?
|
||||
new ReplaceKeyQuickFix[]{
|
||||
new ReplaceKeyQuickFix("? super " + JAVA_LANG_STRING_SHORT),
|
||||
new ReplaceKeyQuickFix(JAVA_LANG_STRING_SHORT)
|
||||
} :
|
||||
LocalQuickFix.EMPTY_ARRAY;
|
||||
}
|
||||
|
||||
private static boolean isJavaTypeElement(@NotNull PsiElement element) {
|
||||
return element instanceof PsiTypeElement &&
|
||||
element.getLanguage().is(JavaLanguage.INSTANCE);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
import com.intellij.lang.Language;
|
||||
import com.intellij.openapi.fileTypes.FileType;
|
||||
import com.intellij.openapi.fileTypes.LanguageFileType;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
import java.util.TreeMap;
|
||||
|
||||
public class Service {
|
||||
|
||||
private final @NotNull Map<<warning descr="Consider using 'String' instead of 'Language' as the map key">Language</warning>, Object> myLanguageMap = new HashMap<>();
|
||||
private final @NotNull HashMap<<warning descr="Consider using 'String' instead of 'Language' as the map key">Language</warning>, Object> myLanguageHashMap = new HashMap<>();
|
||||
private final @NotNull Map<<warning descr="Consider using 'String' instead of 'Language' as the map key">? super Language</warning>, Object> myLanguageMap2 = new HashMap<>();
|
||||
|
||||
private final @NotNull Map<<warning descr="Consider using 'String' instead of 'FileType' as the map key">FileType</warning>, Object> myFileTypeMap = new HashMap<>();
|
||||
private final @NotNull TreeMap<<warning descr="Consider using 'String' instead of 'FileType' as the map key">FileType</warning>, Object> myFileTypeTreeMap = new TreeMap<>();
|
||||
private final @NotNull Map<<warning descr="Consider using 'String' instead of 'FileType' as the map key">? extends FileType</warning>, Object> myFileTypeMap2 = new HashMap<>();
|
||||
private final @NotNull Map<<warning descr="Consider using 'String' instead of 'LanguageFileType' as the map key">LanguageFileType</warning>, Object> myLanguageFileTypeMap = new HashMap<>();
|
||||
|
||||
private final @NotNull Map<Object, Object> objectMap = new HashMap<>();
|
||||
private final @NotNull TreeMap<Object, Object> objectTreeMap = new TreeMap<>();
|
||||
}
|
||||
@@ -0,0 +1,9 @@
|
||||
import com.intellij.lang.Language;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.util.HashMap;
|
||||
|
||||
public class Service2 {
|
||||
|
||||
private final @NotNull HashMap<L<caret>anguage, Object> myLanguageMap = new HashMap<>();
|
||||
}
|
||||
@@ -0,0 +1,9 @@
|
||||
import com.intellij.lang.Language;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.util.HashMap;
|
||||
|
||||
public class Service2 {
|
||||
|
||||
private final @NotNull HashMap<S<caret>tring, Object> myLanguageMap = new HashMap<>();
|
||||
}
|
||||
@@ -0,0 +1,9 @@
|
||||
import com.intellij.lang.Language;
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
import java.util.HashMap;
|
||||
|
||||
public class Service2 {
|
||||
|
||||
private final @NotNull HashMap<?<caret> super String, Object> myLanguageMap = new HashMap<>();
|
||||
}
|
||||
@@ -0,0 +1,39 @@
|
||||
// Copyright 2000-2020 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 org.jetbrains.idea.devkit.inspections;
|
||||
|
||||
import com.intellij.codeInsight.intention.IntentionAction;
|
||||
import com.intellij.testFramework.TestDataPath;
|
||||
import org.jetbrains.idea.devkit.DevkitJavaTestsUtil;
|
||||
import org.junit.Assert;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
@TestDataPath("$CONTENT_ROOT/testData/inspections/leakableMapKey")
|
||||
public class LeakableMapKeyInspectionTest extends LeakableMapKeyInspectionTestBase {
|
||||
|
||||
@Override
|
||||
protected String getBasePath() {
|
||||
return DevkitJavaTestsUtil.TESTDATA_PATH + "inspections/leakableMapKey";
|
||||
}
|
||||
|
||||
@Override
|
||||
public void testHighlighting() {
|
||||
myFixture.testHighlighting("Service.java");
|
||||
}
|
||||
|
||||
public void testReplaceWithSuperString() {
|
||||
List<IntentionAction> quickFixes = myFixture.getAllQuickFixes("Service2.java");
|
||||
Assert.assertEquals(2, quickFixes.size());
|
||||
|
||||
myFixture.launchAction(quickFixes.get(0));
|
||||
myFixture.checkResultByFile("Service2.java", "Service2_withSuperString.java", true);
|
||||
}
|
||||
|
||||
public void testReplaceWithString() {
|
||||
List<IntentionAction> quickFixes = myFixture.getAllQuickFixes("Service2.java");
|
||||
Assert.assertEquals(2, quickFixes.size());
|
||||
|
||||
myFixture.launchAction(quickFixes.get(1));
|
||||
myFixture.checkResultByFile("Service2.java", "Service2_withString.java", true);
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,20 @@
|
||||
import com.intellij.lang.Language
|
||||
import com.intellij.openapi.fileTypes.FileType
|
||||
import com.intellij.openapi.fileTypes.LanguageFileType
|
||||
import java.util.*
|
||||
|
||||
class Service {
|
||||
|
||||
private val languageMap: <warning descr="Consider using 'String' instead of 'Language' as the map key">Map<Language, Any></warning> = HashMap()
|
||||
private val <warning descr="Consider using 'String' instead of 'Language' as the map key">languageHashMap</warning> = HashMap<Language, Any>()
|
||||
private val languageMutableMap: <warning descr="Consider using 'String' instead of 'Language' as the map key">MutableMap<Language, Any></warning> = HashMap()
|
||||
private val languageMap2: <warning descr="Consider using 'String' instead of 'Language' as the map key">Map<in Language, Any></warning> = HashMap()
|
||||
|
||||
private val fileTypeMap: <warning descr="Consider using 'String' instead of 'FileType' as the map key">Map<FileType, Any></warning> = HashMap()
|
||||
private val fileTypeTreeMap: <warning descr="Consider using 'String' instead of 'FileType' as the map key">TreeMap<FileType, Any></warning> = TreeMap()
|
||||
private val fileTypeMap2: <warning descr="Consider using 'String' instead of 'FileType' as the map key">Map<out FileType, Any></warning> = HashMap()
|
||||
private val languageFileTypeMap: <warning descr="Consider using 'String' instead of 'LanguageFileType' as the map key">Map<LanguageFileType, Any></warning> = HashMap()
|
||||
|
||||
private val objectMap: Map<Any, Any> = HashMap()
|
||||
private val objectTreeMap: TreeMap<Any, Any> = TreeMap()
|
||||
}
|
||||
@@ -0,0 +1,20 @@
|
||||
// Copyright 2000-2020 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 org.jetbrains.idea.devkit.kotlin.inspections;
|
||||
|
||||
import com.intellij.testFramework.TestDataPath;
|
||||
import org.jetbrains.idea.devkit.inspections.LeakableMapKeyInspectionTestBase;
|
||||
import org.jetbrains.idea.devkit.kotlin.DevkitKtTestsUtil;
|
||||
|
||||
@TestDataPath("$CONTENT_ROOT/testData/inspections/leakableMapKey")
|
||||
public class LeakableMapKeyInspectionTest extends LeakableMapKeyInspectionTestBase {
|
||||
|
||||
@Override
|
||||
protected String getBasePath() {
|
||||
return DevkitKtTestsUtil.TESTDATA_PATH + "inspections/leakableMapKey";
|
||||
}
|
||||
|
||||
@Override
|
||||
public void testHighlighting() {
|
||||
myFixture.testHighlighting("Service.kt");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,18 @@
|
||||
// Copyright 2000-2020 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 org.jetbrains.idea.devkit.inspections;
|
||||
|
||||
public abstract class LeakableMapKeyInspectionTestBase extends PluginModuleTestCase {
|
||||
|
||||
@Override
|
||||
protected void setUp() throws Exception {
|
||||
super.setUp();
|
||||
|
||||
myFixture.addClass("package com.intellij.lang; public abstract class Language {}");
|
||||
myFixture.addClass("package com.intellij.openapi.fileTypes; public interface FileType {}");
|
||||
myFixture.addClass("package com.intellij.openapi.fileTypes; public abstract class LanguageFileType implements FileType {}");
|
||||
|
||||
myFixture.enableInspections(LeakableMapKeyInspection.class);
|
||||
}
|
||||
|
||||
public abstract void testHighlighting();
|
||||
}
|
||||
Reference in New Issue
Block a user