From 72176cd9b65a6ec89e4e00518a6978c93e02c612 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Thu, 20 Oct 2022 11:33:12 +0200 Subject: [PATCH] IDEA-189985 - RegistrationProblemsInspection - cleanup GitOrigin-RevId: 6015fe4ddc5fbf38a1bc18afca451a8991399fe0 --- .../messages/DevKitBundle.properties | 4 +- .../RegistrationProblemsInspection.java | 122 +++++++++--------- .../code/MyActionWrongClass.java | 2 +- ...egistrationProblemsInspectionCodeTest.java | 13 +- ...trationProblemsInspectionCodeBaseTest.java | 13 ++ 5 files changed, 78 insertions(+), 76 deletions(-) create mode 100644 plugins/devkit/devkit-tests/src/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeBaseTest.java diff --git a/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties b/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties index baed510b299d..7125b2275ea4 100644 --- a/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties +++ b/plugins/devkit/devkit-core/resources/messages/DevKitBundle.properties @@ -150,8 +150,6 @@ new.menu.project.component.text=Project Component select.plugin.module.to.patch=Select Plugin Module to Patch -keyword.extend=extend -keyword.implement=implement class.action=action class.interface=interface class.implementation=implementation @@ -254,7 +252,7 @@ inspections.registration.problems.option.check.java.code=Check Java Code inspections.registration.problems.quickfix.read-only=Class ''{0}'' is read-only inspections.registration.problems.quickfix.create.constructor=Create no-argument constructor -inspections.registration.problems.incompatible.message=According to its registration in plugin.xml, the class should {0} ''{1}'' +inspections.registration.problems.incompatible.message=According to its registration in plugin.xml, the class should extend ''{0}'' inspections.registration.problems.abstract=Plugin component class must not be abstract inspections.registration.problems.missing.noarg.ctor=Action class must have a no-argument constructor diff --git a/plugins/devkit/devkit-core/src/inspections/RegistrationProblemsInspection.java b/plugins/devkit/devkit-core/src/inspections/RegistrationProblemsInspection.java index 69385dff081b..f9f91b989df6 100644 --- a/plugins/devkit/devkit-core/src/inspections/RegistrationProblemsInspection.java +++ b/plugins/devkit/devkit-core/src/inspections/RegistrationProblemsInspection.java @@ -1,4 +1,4 @@ -// 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. +// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.idea.devkit.inspections; import com.intellij.codeInsight.daemon.impl.quickfix.ImplementOrExtendFix; @@ -32,9 +32,6 @@ import javax.swing.event.ChangeListener; import java.util.List; import java.util.Set; -/** - * @author swr - */ public class RegistrationProblemsInspection extends DevKitInspectionBase { public boolean CHECK_PLUGIN_XML = true; @@ -95,17 +92,24 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { @Override public ProblemDescriptor @Nullable [] checkFile(@NotNull PsiFile file, @NotNull InspectionManager manager, boolean isOnTheFly) { if (CHECK_PLUGIN_XML && DescriptorUtil.isPluginXml(file)) { - return checkPluginXml((XmlFile)file, manager, isOnTheFly); + return inspectPluginXml((XmlFile)file, manager, isOnTheFly); } return null; } @Override public ProblemDescriptor @Nullable [] checkClass(@NotNull PsiClass checkedClass, @NotNull InspectionManager manager, boolean isOnTheFly) { - final PsiIdentifier nameIdentifier = checkedClass.getNameIdentifier(); + if (CHECK_JAVA_CODE) { + return inspectClass(checkedClass, manager, isOnTheFly); + } + return null; + } - if (CHECK_JAVA_CODE && - nameIdentifier != null && + private ProblemDescriptor @Nullable [] inspectClass(@NotNull PsiClass checkedClass, + @NotNull InspectionManager manager, + boolean isOnTheFly) { + final PsiIdentifier nameIdentifier = checkedClass.getNameIdentifier(); + if (nameIdentifier != null && checkedClass.getQualifiedName() != null && checkedClass.getContainingFile().getVirtualFile() != null && !checkedClass.isInterface() && @@ -119,16 +123,14 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { if (componentClasses != null && !componentClasses.isEmpty()) { List problems = new SmartList<>(); - for (PsiClass compClass : componentClasses) { - if (ActionType.ACTION.myClassName.equals(compClass.getQualifiedName()) && - !checkedClass.isInheritor(compClass, true)) { + for (PsiClass componentClass : componentClasses) { + if (ActionType.ACTION.myClassName.equals(componentClass.getQualifiedName()) && + !checkedClass.isInheritor(componentClass, true)) { problems.add(manager.createProblemDescriptor(nameIdentifier, DevKitBundle.message("inspections.registration.problems.incompatible.message", - compClass.isInterface() ? - DevKitBundle.message("keyword.implement") : - DevKitBundle.message("keyword.extend"), - compClass.getQualifiedName()), isOnTheFly, - ImplementOrExtendFix.createFixes(nameIdentifier, compClass, checkedClass, isOnTheFly), + componentClass.getQualifiedName()), isOnTheFly, + ImplementOrExtendFix.createFixes(nameIdentifier, componentClass, checkedClass, + isOnTheFly), ProblemHighlightType.GENERIC_ERROR_OR_WARNING)); } } @@ -151,7 +153,7 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { return null; } - private static ProblemDescriptor @Nullable [] checkPluginXml(XmlFile xmlFile, InspectionManager manager, boolean isOnTheFly) { + private static ProblemDescriptor @Nullable [] inspectPluginXml(XmlFile xmlFile, InspectionManager manager, boolean isOnTheFly) { final XmlDocument document = xmlFile.getDocument(); if (document == null) { return null; @@ -161,11 +163,8 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { assert rootTag != null; final RegistrationChecker checker = new RegistrationChecker(manager, xmlFile, isOnTheFly); - DescriptorUtil.processComponents(rootTag, checker); - DescriptorUtil.processActions(rootTag, checker); - return checker.getProblems(); } @@ -173,19 +172,6 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { return checkedClass.hasModifierProperty(PsiModifier.ABSTRACT); } - @Nullable - private static PsiElement getAttValueToken(@NotNull XmlAttribute attribute) { - final XmlAttributeValue valueElement = attribute.getValueElement(); - if (valueElement == null) return null; - - final PsiElement[] children = valueElement.getChildren(); - if (children.length == 3 && children[1] instanceof XmlToken) { - return children[1]; - } - if (children.length == 1 && children[0] instanceof PsiErrorElement) return null; - return valueElement; - } - private static final class RegistrationChecker implements ComponentType.Processor, ActionType.Processor { private List myList; private final InspectionManager myManager; @@ -311,35 +297,38 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { if (attribute != null) { final PsiElement token = getAttValueToken(attribute); if (token != null) { - final String actionClassName = attribute.getValue().trim(); - final PsiClass actionClass = findClass(actionClassName); - if (actionClass == null) { - addProblem(token, - DevKitBundle.message("inspections.registration.problems.cannot.resolve.class", - DevKitBundle.message("class.action")), - ProblemHighlightType.LIKE_UNKNOWN_SYMBOL, myOnTheFly, ((LocalQuickFix)QuickFixFactory.getInstance() - .createCreateClassOrInterfaceFix(token, actionClassName, true, AnAction.class.getName()))); - } - else { - if (!type.isOfType(actionClass)) { - final PsiClass psiClass = findClass(type.myClassName); - if (psiClass != null && !actionClass.isInheritor(psiClass, true)) { + String attributeValue = attribute.getValue(); + if (attributeValue != null) { + final String actionClassName = attributeValue.trim(); + final PsiClass actionClass = findClass(actionClassName); + if (actionClass == null) { + addProblem(token, + DevKitBundle.message("inspections.registration.problems.cannot.resolve.class", + DevKitBundle.message("class.action")), + ProblemHighlightType.LIKE_UNKNOWN_SYMBOL, myOnTheFly, ((LocalQuickFix)QuickFixFactory.getInstance() + .createCreateClassOrInterfaceFix(token, actionClassName, true, AnAction.class.getName()))); + } + else { + if (!type.isOfType(actionClass)) { + final PsiClass psiClass = findClass(type.myClassName); + if (psiClass != null && !actionClass.isInheritor(psiClass, true)) { + addProblem(token, + DevKitBundle.message("inspections.registration.problems.action.incompatible.class", type.myClassName), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myOnTheFly, + ImplementOrExtendFix.createFixes(token, psiClass, actionClass, myOnTheFly)); + } + } + final ConstructorType noArgCtor = ConstructorType.getNoArgCtor(actionClass); + if (noArgCtor == null) { addProblem(token, - DevKitBundle.message("inspections.registration.problems.action.incompatible.class", type.myClassName), - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myOnTheFly, - ImplementOrExtendFix.createFixes(token, psiClass, actionClass, myOnTheFly)); + DevKitBundle.message("inspections.registration.problems.missing.noarg.ctor"), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myOnTheFly, new CreateConstructorFix(actionClass, myOnTheFly)); + } + if (isAbstract(actionClass)) { + addProblem(token, + DevKitBundle.message("inspections.registration.problems.abstract"), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myOnTheFly); } - } - final ConstructorType noArgCtor = ConstructorType.getNoArgCtor(actionClass); - if (noArgCtor == null) { - addProblem(token, - DevKitBundle.message("inspections.registration.problems.missing.noarg.ctor"), - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myOnTheFly, new CreateConstructorFix(actionClass, myOnTheFly)); - } - if (isAbstract(actionClass)) { - addProblem(token, - DevKitBundle.message("inspections.registration.problems.abstract"), - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, myOnTheFly); } } } @@ -347,6 +336,19 @@ public class RegistrationProblemsInspection extends DevKitInspectionBase { return true; } + @Nullable + private static PsiElement getAttValueToken(@NotNull XmlAttribute attribute) { + final XmlAttributeValue valueElement = attribute.getValueElement(); + if (valueElement == null) return null; + + final PsiElement[] children = valueElement.getChildren(); + if (children.length == 3 && children[1] instanceof XmlToken) { + return children[1]; + } + if (children.length == 1 && children[0] instanceof PsiErrorElement) return null; + return valueElement; + } + private void addProblem(XmlTagValue impl, @InspectionMessage String problem, ProblemHighlightType type, diff --git a/plugins/devkit/devkit-java-tests/testData/inspections/registrationProblems/code/MyActionWrongClass.java b/plugins/devkit/devkit-java-tests/testData/inspections/registrationProblems/code/MyActionWrongClass.java index 18c87be1aeaa..91cf9eab4471 100644 --- a/plugins/devkit/devkit-java-tests/testData/inspections/registrationProblems/code/MyActionWrongClass.java +++ b/plugins/devkit/devkit-java-tests/testData/inspections/registrationProblems/code/MyActionWrongClass.java @@ -1 +1 @@ -public class MyActionWrongClass extends java.lang.String {} \ No newline at end of file +public class MyActionWrongClass extends java.util.ArrayList {} \ No newline at end of file diff --git a/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeTest.java b/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeTest.java index 9c4884761d08..d99aace23b95 100644 --- a/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeTest.java +++ b/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeTest.java @@ -5,24 +5,13 @@ import com.intellij.testFramework.TestDataPath; import org.jetbrains.idea.devkit.DevkitJavaTestsUtil; @TestDataPath("$CONTENT_ROOT/testData/inspections/registrationProblems/code") -public class RegistrationProblemsInspectionCodeTest extends PluginModuleTestCase { +public class RegistrationProblemsInspectionCodeTest extends RegistrationProblemsInspectionCodeBaseTest { @Override protected String getBasePath() { return DevkitJavaTestsUtil.TESTDATA_PATH + "inspections/registrationProblems/code"; } - @Override - protected void setUp() throws Exception { - super.setUp(); - - myFixture.addClass("package com.intellij.openapi.actionSystem; public class AnAction {}"); - - myFixture.addClass("package com.intellij.openapi.components; public interface BaseComponent {}"); - - myFixture.enableInspections(new RegistrationProblemsInspection()); - } - public void testComponentAbstractImplementation() { setPluginXml("ComponentAbstractImplementation.xml"); myFixture.testHighlighting("AbstractApplicationComponent.java"); diff --git a/plugins/devkit/devkit-tests/src/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeBaseTest.java b/plugins/devkit/devkit-tests/src/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeBaseTest.java new file mode 100644 index 000000000000..05832bf1f5fc --- /dev/null +++ b/plugins/devkit/devkit-tests/src/org/jetbrains/idea/devkit/inspections/RegistrationProblemsInspectionCodeBaseTest.java @@ -0,0 +1,13 @@ +// 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; + +public abstract class RegistrationProblemsInspectionCodeBaseTest extends PluginModuleTestCase { + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(new RegistrationProblemsInspection()); + myFixture.addClass("package com.intellij.openapi.actionSystem; public class AnAction {}"); + myFixture.addClass("package com.intellij.openapi.components; public interface BaseComponent {}"); + } +}