[devkit] add alternative JvmElement-based StatefulEpInspection

2026-09-27 10:03:11 +07:00 · 2018-03-28 17:55:16 +02:00
parent 61aea15ceb
commit 5ea6cafb52
14 changed files with 278 additions and 0 deletions
@@ -151,6 +151,10 @@
                     groupKey="inspections.group.name"
                     enabledByDefault="true" level="WARNING"
                     implementationClass="org.jetbrains.idea.devkit.inspections.StatefulEpInspection"/>
+    <localInspection language="JVM" shortName="StatefulEp2" displayName="Stateful Extension 2"
+                     groupKey="inspections.group.name"
+                     enabledByDefault="false" level="WARNING"
+                     implementationClass="org.jetbrains.idea.devkit.inspections.StatefulEpInspection2"/>
    <localInspection language="UAST" shortName="UElementAsPsi" displayName="UElement as PsiElement usage"
                     groupKey="inspections.group.name"
                     enabledByDefault="true" level="WARNING"
@@ -0,0 +1,6 @@
+<html>
+<body>
+Potential memory leak detected. Please don't hold heavy objects in extensions if you're not 100% sure.
+Ideally, extensions should be stateless.
+</body>
+</html>
@@ -0,0 +1,15 @@
+// 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 org.jetbrains.idea.devkit.inspections
+
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.lang.jvm.inspection.JvmLocalInspection
+import com.intellij.psi.PsiElementVisitor
+import org.jetbrains.idea.devkit.inspections.DevKitInspectionBase.isAllowed
+
+abstract class DevKitJvmInspection : JvmLocalInspection() {
+
+  override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
+    if (!isAllowed(holder)) return PsiElementVisitor.EMPTY_VISITOR
+    return super.buildVisitor(holder, isOnTheFly)
+  }
+}
@@ -0,0 +1,83 @@
+// 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 org.jetbrains.idea.devkit.inspections
+
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.lang.jvm.DefaultJvmElementVisitor
+import com.intellij.lang.jvm.JvmClass
+import com.intellij.lang.jvm.JvmField
+import com.intellij.lang.jvm.JvmModifier
+import com.intellij.lang.jvm.types.JvmReferenceType
+import com.intellij.lang.jvm.util.JvmInheritanceUtil.isInheritor
+import com.intellij.lang.jvm.util.JvmUtil
+import com.intellij.openapi.components.ProjectComponent
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiReference
+import org.jetbrains.idea.devkit.util.ExtensionCandidate
+import org.jetbrains.idea.devkit.util.ExtensionLocator
+
+class StatefulEpInspection2 : DevKitJvmInspection() {
+
+  override fun buildJvmVisitor(project: Project, sink: HighlightSink) = object : DefaultJvmElementVisitor<Boolean?> {
+
+    override fun visitField(field: JvmField): Boolean? {
+      val clazz = field.containingClass ?: return null
+      val fieldTypeClass = JvmUtil.resolveClass(field.type as? JvmReferenceType) ?: return null
+
+      val isQuickFix by lazy(LazyThreadSafetyMode.NONE) { isInheritor(clazz, localQuickFixFqn) }
+
+      val targets = findEpCandidates(project, clazz)
+      if (targets.isEmpty() && !isQuickFix) return null
+
+      if (isInheritor(fieldTypeClass, PsiElement::class.java.canonicalName)) {
+        sink.highlight(
+          "Potential memory leak: don't hold PsiElement, use SmartPsiElementPointer instead${if (isQuickFix) "; also see LocalQuickFixOnPsiElement" else ""}"
+        )
+        return false
+      }
+
+      if (isInheritor(fieldTypeClass, PsiReference::class.java.canonicalName)) {
+        sink.highlight(message(PsiReference::class.java.simpleName, isQuickFix))
+        return false
+      }
+
+      if (!isProjectFieldAllowed(field, clazz, targets) && isInheritor(fieldTypeClass, Project::class.java.canonicalName)) {
+        sink.highlight(message(Project::class.java.simpleName, isQuickFix))
+        return false
+      }
+
+      return false
+    }
+  }
+
+  companion object {
+    private val localQuickFixFqn = LocalQuickFix::class.java.canonicalName
+    private val projectComponentFqn = ProjectComponent::class.java.canonicalName
+
+    private fun findEpCandidates(project: Project, clazz: JvmClass): Collection<ExtensionCandidate> {
+      val name = clazz.name ?: return emptyList()
+      return ExtensionLocator.byClass(project, clazz).findCandidates().filter { candidate ->
+        val forClass = candidate.pointer.element?.getAttributeValue("forClass")
+        forClass == null || !forClass.contains(name)
+      }
+    }
+
+    private fun isProjectFieldAllowed(field: JvmField, clazz: JvmClass, targets: Collection<ExtensionCandidate>): Boolean {
+      val finalField = field.hasModifier(JvmModifier.FINAL)
+      if (finalField) return true
+
+      val isProjectEP = targets.any { candidate ->
+        val name = candidate.pointer.element?.name
+        "projectService" == name || "projectConfigurable" == name
+      }
+      if (isProjectEP) return true
+
+      return isInheritor(clazz, projectComponentFqn)
+    }
+
+    private fun message(what: String, quickFix: Boolean): String {
+      val where = if (quickFix) "quick fix" else "extension"
+      return "Don't use $what as a field in $where"
+    }
+  }
+}
@@ -1,6 +1,8 @@
 // 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 org.jetbrains.idea.devkit.util;

+import com.intellij.lang.jvm.JvmClass;
+import com.intellij.lang.jvm.util.JvmClassUtil;
 import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.PsiClass;
@@ -30,6 +32,11 @@ public abstract class ExtensionLocator {
  @NotNull
  public abstract List<ExtensionCandidate> findCandidates();

+  @NotNull
+  public static ExtensionLocator byClass(@NotNull Project project, @NotNull JvmClass clazz) {
+    return new ExtensionByClassLocator(project, clazz);
+  }
+
  public static ExtensionLocator byPsiClass(PsiClass psiClass) {
    return new ExtensionByPsiClassLocator(psiClass);
  }
@@ -42,6 +49,37 @@ public abstract class ExtensionLocator {
    return new ExtensionByExtensionPointLocator(extensionPoint, extensionId);
  }

+  private static class ExtensionByClassLocator extends ExtensionLocator {
+
+    private final Project myProject;
+    private final JvmClass myClazz;
+
+    ExtensionByClassLocator(@NotNull Project project, @NotNull JvmClass clazz) {
+      myProject = project;
+      myClazz = clazz;
+    }
+
+    @NotNull
+    @Override
+    public List<ExtensionCandidate> findCandidates() {
+      String jvmName = JvmClassUtil.getJvmClassName(myClazz);
+      if (jvmName == null) {
+        return Collections.emptyList();
+      }
+
+      List<ExtensionCandidate> result = new SmartList<>();
+      processExtensionDeclarations(myClazz.getQualifiedName(), myProject, (file, startOffset, endOffset) -> {
+        XmlTag tag = getXmlTagOfTokenElement(file, startOffset, jvmName, true);
+        DomElement dom = DomUtil.getDomElement(tag);
+        if (dom instanceof Extension && ((Extension)dom).getExtensionPoint() != null) {
+          result.add(new ExtensionCandidate(SmartPointerManager.getInstance(tag.getProject()).createSmartPsiElementPointer(tag)));
+        }
+        return true; // continue processing
+      });
+
+      return result;
+    }
+  }

  private static class ExtensionByPsiClassLocator extends ExtensionLocator {
    private final PsiClass myPsiClass;
@@ -0,0 +1,12 @@
+public class Ext {
+  final com.intellij.psi.PsiElement <warning descr="Potential memory leak: don't hold PsiElement, use SmartPsiElementPointer instead">pe</warning>;
+  final com.intellij.psi.PsiReference <warning descr="Don't use PsiReference as a field in extension">r</warning>;
+  com.intellij.openapi.project.Project <warning descr="Don't use Project as a field in extension">p</warning>;
+  final com.intellij.openapi.project.Project pf;
+  public Ext() {
+    super();
+    pe = null;
+    r =null;
+    p = pf = null;
+  }
+}
@@ -0,0 +1,15 @@
+import com.intellij.openapi.project.Project;
+
+public class FakeFile {
+  final com.intellij.psi.PsiElement pe;
+  final com.intellij.psi.PsiReference r;
+  Project p;
+  final Project pf;
+
+  public FakeFile(Project project) {
+    super();
+    pe = null;
+    r = null;
+    p = pf = project;
+  }
+}
@@ -0,0 +1,12 @@
+public class Fix extends com.intellij.codeInspection.LocalQuickFix {
+  final com.intellij.psi.PsiElement <warning descr="Potential memory leak: don't hold PsiElement, use SmartPsiElementPointer instead; also see LocalQuickFixOnPsiElement">pe</warning>;
+  final com.intellij.psi.PsiReference <warning descr="Don't use PsiReference as a field in quick fix">r</warning>;
+  com.intellij.openapi.project.Project <warning descr="Don't use Project as a field in quick fix">p</warning>;
+  final com.intellij.openapi.project.Project pf;
+  public Fix() {
+    super();
+    pe = null;
+    r =null;
+    p = pf = null;
+  }
+}
@@ -0,0 +1,16 @@
+public class NonFix {
+  final com.intellij.psi.PsiElement pe;
+  final com.intellij.psi.PsiReference r;
+  com.intellij.openapi.project.Project p;
+  final com.intellij.openapi.project.Project pf;
+  public NonFix() {
+    super();
+    pe = null;
+    r =null;
+    p = pf = null;
+  }
+  
+  public static class Ext2 {
+    
+  }
+}
@@ -0,0 +1,12 @@
+public class ProjectComp implements com.intellij.openapi.components.ProjectComponent {
+  final com.intellij.psi.PsiElement <warning descr="Potential memory leak: don't hold PsiElement, use SmartPsiElementPointer instead">pe</warning>;
+  final com.intellij.psi.PsiReference <warning descr="Don't use PsiReference as a field in extension">r</warning>;
+  com.intellij.openapi.project.Project p;
+  final com.intellij.openapi.project.Project pf;
+  public ProjectComp() {
+    super();
+    pe = null;
+    r =null;
+    p = pf = null;
+  }
+}
@@ -0,0 +1,14 @@
+import com.intellij.openapi.project.Project;
+
+public class ProjectConfigurable {
+  final com.intellij.psi.PsiElement <warning descr="Potential memory leak: don't hold PsiElement, use SmartPsiElementPointer instead">pe</warning>;
+  final com.intellij.psi.PsiReference <warning descr="Don't use PsiReference as a field in extension">r</warning>;
+  Project p;
+  final Project pf;
+  public ProjectConfigurable(Project project) {
+    super();
+    pe = null;
+    r = null;
+    p = pf = project;
+  }
+}
@@ -0,0 +1,14 @@
+import com.intellij.openapi.project.Project;
+
+public class ProjectService {
+  final com.intellij.psi.PsiElement <warning descr="Potential memory leak: don't hold PsiElement, use SmartPsiElementPointer instead">pe</warning>;
+  final com.intellij.psi.PsiReference <warning descr="Don't use PsiReference as a field in extension">r</warning>;
+  Project p;
+  final Project pf;
+  public ProjectService(Project project) {
+    super();
+    pe = null;
+    r = null;
+    p = pf = project;
+  }
+}
@@ -0,0 +1,16 @@
+<idea-plugin>
+  <extensionPoints>
+    <extensionPoint name="ep" beanClass="EP"/>
+    <extensionPoint name="projectService" beanClass="SD"/>
+    <extensionPoint name="projectConfigurable" beanClass="SD"/>
+  </extensionPoints>
+  <extensions>
+    <ep implementation="Ext"/>
+    <ep implementation="NonFix$Ext2"/>
+    <ep implementation="ProjectComp"/>
+    <ep implementation="ProjectComp"/>
+    <ep forClass="FakeFile" instance="ProjectConfigurable"/>
+    <projectService serviceImplementation="ProjectService"/>
+    <projectConfigurable instance="ProjectConfigurable"/>
+  </extensions>
+</idea-plugin>
@@ -0,0 +1,21 @@
+// 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 org.jetbrains.idea.devkit.inspections;
+
+import com.intellij.openapi.application.PluginPathManager;
+import com.intellij.testFramework.TestDataPath;
+
+@TestDataPath("$CONTENT_ROOT/testData/inspections/statefulEp2")
+public class StatefulEpInspection2Test extends StatefulEpInspectionTest {
+
+  @Override
+  protected String getBasePath() {
+    return PluginPathManager.getPluginHomePathRelative("devkit") + "/testData/inspections/statefulEp2";
+  }
+
+  @Override
+  public void setUp() throws Exception {
+    super.setUp();
+    myFixture.disableInspections(new StatefulEpInspection());
+    myFixture.enableInspections(StatefulEpInspection2.class);
+  }
+}