diff --git a/plugins/devkit/resources/META-INF/plugin.xml b/plugins/devkit/resources/META-INF/plugin.xml index 47716e22cfa5..c16651089337 100644 --- a/plugins/devkit/resources/META-INF/plugin.xml +++ b/plugins/devkit/resources/META-INF/plugin.xml @@ -151,6 +151,10 @@ groupKey="inspections.group.name" enabledByDefault="true" level="WARNING" implementationClass="org.jetbrains.idea.devkit.inspections.StatefulEpInspection"/> + + +Potential memory leak detected. Please don't hold heavy objects in extensions if you're not 100% sure. +Ideally, extensions should be stateless. + + \ No newline at end of file diff --git a/plugins/devkit/src/inspections/DevKitJvmInspection.kt b/plugins/devkit/src/inspections/DevKitJvmInspection.kt new file mode 100644 index 000000000000..298fdfea1d95 --- /dev/null +++ b/plugins/devkit/src/inspections/DevKitJvmInspection.kt @@ -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) + } +} diff --git a/plugins/devkit/src/inspections/StatefulEpInspection2.kt b/plugins/devkit/src/inspections/StatefulEpInspection2.kt new file mode 100644 index 000000000000..8aaa1d007764 --- /dev/null +++ b/plugins/devkit/src/inspections/StatefulEpInspection2.kt @@ -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 { + + 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 { + 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): 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" + } + } +} diff --git a/plugins/devkit/src/util/ExtensionLocator.java b/plugins/devkit/src/util/ExtensionLocator.java index 959169559884..2656cae641d9 100644 --- a/plugins/devkit/src/util/ExtensionLocator.java +++ b/plugins/devkit/src/util/ExtensionLocator.java @@ -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 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 findCandidates() { + String jvmName = JvmClassUtil.getJvmClassName(myClazz); + if (jvmName == null) { + return Collections.emptyList(); + } + + List 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; diff --git a/plugins/devkit/testData/inspections/statefulEp2/Ext.java b/plugins/devkit/testData/inspections/statefulEp2/Ext.java new file mode 100644 index 000000000000..1ca1e9ccd69d --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/Ext.java @@ -0,0 +1,12 @@ +public class Ext { + 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 Ext() { + super(); + pe = null; + r =null; + p = pf = null; + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/FakeFile.java b/plugins/devkit/testData/inspections/statefulEp2/FakeFile.java new file mode 100644 index 000000000000..2f09502bdbbd --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/FakeFile.java @@ -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; + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/Fix.java b/plugins/devkit/testData/inspections/statefulEp2/Fix.java new file mode 100644 index 000000000000..7de0523ce2cb --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/Fix.java @@ -0,0 +1,12 @@ +public class Fix extends com.intellij.codeInspection.LocalQuickFix { + 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 Fix() { + super(); + pe = null; + r =null; + p = pf = null; + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/NonFix.java b/plugins/devkit/testData/inspections/statefulEp2/NonFix.java new file mode 100644 index 000000000000..8c2bd3b464be --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/NonFix.java @@ -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 { + + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/ProjectComp.java b/plugins/devkit/testData/inspections/statefulEp2/ProjectComp.java new file mode 100644 index 000000000000..5bfa62a40e66 --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/ProjectComp.java @@ -0,0 +1,12 @@ +public class ProjectComp implements com.intellij.openapi.components.ProjectComponent { + 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 ProjectComp() { + super(); + pe = null; + r =null; + p = pf = null; + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/ProjectConfigurable.java b/plugins/devkit/testData/inspections/statefulEp2/ProjectConfigurable.java new file mode 100644 index 000000000000..89933d160ddd --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/ProjectConfigurable.java @@ -0,0 +1,14 @@ +import com.intellij.openapi.project.Project; + +public class ProjectConfigurable { + final com.intellij.psi.PsiElement pe; + final com.intellij.psi.PsiReference r; + Project p; + final Project pf; + public ProjectConfigurable(Project project) { + super(); + pe = null; + r = null; + p = pf = project; + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/ProjectService.java b/plugins/devkit/testData/inspections/statefulEp2/ProjectService.java new file mode 100644 index 000000000000..764105760712 --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/ProjectService.java @@ -0,0 +1,14 @@ +import com.intellij.openapi.project.Project; + +public class ProjectService { + final com.intellij.psi.PsiElement pe; + final com.intellij.psi.PsiReference r; + Project p; + final Project pf; + public ProjectService(Project project) { + super(); + pe = null; + r = null; + p = pf = project; + } +} \ No newline at end of file diff --git a/plugins/devkit/testData/inspections/statefulEp2/plugin.xml b/plugins/devkit/testData/inspections/statefulEp2/plugin.xml new file mode 100644 index 000000000000..76e4aafa9deb --- /dev/null +++ b/plugins/devkit/testData/inspections/statefulEp2/plugin.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/plugins/devkit/testSources/inspections/StatefulEpInspection2Test.java b/plugins/devkit/testSources/inspections/StatefulEpInspection2Test.java new file mode 100644 index 000000000000..7180c0d05a07 --- /dev/null +++ b/plugins/devkit/testSources/inspections/StatefulEpInspection2Test.java @@ -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); + } +}