Merge pull request #2092 from Haehnchen/feature/inspection-method-recursive

Haehnchen · web-flow · commit 6282bf965353 · 2023-03-26T10:57:54.000+02:00
replace recursive visiting for event methods inspection
diff --git a/src/main/java/fr/adrienbrault/idea/symfony2plugin/config/yaml/inspection/EventMethodCallInspection.java b/src/main/java/fr/adrienbrault/idea/symfony2plugin/config/yaml/inspection/EventMethodCallInspection.java
@@ -4,16 +4,16 @@
 import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.codeInspection.ProblemHighlightType;
 import com.intellij.codeInspection.ProblemsHolder;
+import com.intellij.lang.Language;
+import com.intellij.lang.xml.XMLLanguage;
+import com.intellij.openapi.util.NotNullLazyValue;
 import com.intellij.patterns.PlatformPatterns;
 import com.intellij.patterns.StandardPatterns;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiElementVisitor;
-import com.intellij.psi.PsiFile;
-import com.intellij.psi.PsiRecursiveElementWalkingVisitor;
 import com.intellij.psi.util.PsiTreeUtil;
-import com.intellij.psi.xml.XmlFile;
+import com.jetbrains.php.lang.PhpLanguage;
 import com.jetbrains.php.lang.parser.PhpElementTypes;
-import com.jetbrains.php.lang.psi.PhpFile;
 import com.jetbrains.php.lang.psi.elements.Method;
 import com.jetbrains.php.lang.psi.elements.PhpClass;
 import com.jetbrains.php.lang.psi.elements.PhpReturn;
@@ -33,8 +33,9 @@
 import org.apache.commons.lang.StringUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.yaml.YAMLLanguage;
 import org.jetbrains.yaml.YAMLTokenTypes;
-import org.jetbrains.yaml.psi.*;
+import org.jetbrains.yaml.psi.YAMLScalar;
 
 import java.util.Collection;
 import java.util.stream.Collectors;
@@ -52,66 +53,55 @@ public PsiElementVisitor buildVisitor(final @NotNull ProblemsHolder holder, bool
         }
 
         return new PsiElementVisitor() {
-            @Override
-            public void visitFile(@NotNull PsiFile psiFile) {
-                if(psiFile instanceof XmlFile) {
-                    visitXmlFile(psiFile, holder, new ContainerCollectionResolver.LazyServiceCollector(holder.getProject()));
-                } else if(psiFile instanceof YAMLFile) {
-                    visitYamlFile(psiFile, holder, new ContainerCollectionResolver.LazyServiceCollector(holder.getProject()));
-                } else if(psiFile instanceof PhpFile) {
-                    visitPhpFile((PhpFile) psiFile, holder);
-                }
-            }
-        };
-    }
+            private NotNullLazyValue<ContainerCollectionResolver.LazyServiceCollector> serviceCollector;
 
-    private void visitPhpFile(PhpFile psiFile, final ProblemsHolder holder) {
-        psiFile.acceptChildren(new PhpSubscriberRecursiveElementWalkingVisitor(holder));
-    }
-
-    private void visitYamlFile(PsiFile psiFile, final ProblemsHolder holder, @NotNull final ContainerCollectionResolver.LazyServiceCollector lazyServiceCollector) {
-
-        psiFile.acceptChildren(new PsiRecursiveElementWalkingVisitor() {
             @Override
             public void visitElement(@NotNull PsiElement element) {
-                annotateCallMethod(element, holder, lazyServiceCollector);
-                super.visitElement(element);
-            }
-        });
+                Language language = element.getLanguage();
 
-    }
-
-    private void visitXmlFile(@NotNull PsiFile psiFile, @NotNull final ProblemsHolder holder, @NotNull final ContainerCollectionResolver.LazyServiceCollector lazyServiceCollector) {
-
-        psiFile.acceptChildren(new PsiRecursiveElementWalkingVisitor() {
-            @Override
-            public void visitElement(@NotNull PsiElement element) {
-
-                if(XmlHelper.getTagAttributePattern("tag", "method").inside(XmlHelper.getInsideTagPattern("services")).inFile(XmlHelper.getXmlFilePattern()).accepts(element) ||
-                   XmlHelper.getTagAttributePattern("call", "method").inside(XmlHelper.getInsideTagPattern("services")).inFile(XmlHelper.getXmlFilePattern()).accepts(element)
-                  )
-                {
-
-                    // attach to text child only
-                    PsiElement[] psiElements = element.getChildren();
-                    if(psiElements.length < 2) {
-                        return;
+                if (language == YAMLLanguage.INSTANCE) {
+                    if (this.serviceCollector == null) {
+                        this.serviceCollector = NotNullLazyValue.lazy(() -> new ContainerCollectionResolver.LazyServiceCollector(holder.getProject()));
                     }
 
-                    String serviceClassValue = XmlHelper.getServiceDefinitionClass(element);
-                    if(StringUtils.isNotBlank(serviceClassValue)) {
-                        registerMethodProblem(psiElements[1], holder, serviceClassValue, lazyServiceCollector);
+                    visitYmlElement(element, holder, this.serviceCollector);
+                } else if (language == XMLLanguage.INSTANCE) {
+                    if (this.serviceCollector == null) {
+                        this.serviceCollector = NotNullLazyValue.lazy(() -> new ContainerCollectionResolver.LazyServiceCollector(holder.getProject()));
                     }
 
+                    visitXmlElement(element, holder, this.serviceCollector);
+                } else if (language == PhpLanguage.INSTANCE) {
+                    if (element instanceof StringLiteralExpression stringLiteralExpression) {
+                        visitPhpElement(stringLiteralExpression, holder);
+                    }
                 }
 
                 super.visitElement(element);
             }
-        });
+        };
+    }
+
+    public void visitXmlElement(@NotNull PsiElement element, @NotNull final ProblemsHolder holder, @NotNull NotNullLazyValue<ContainerCollectionResolver.LazyServiceCollector> collector) {
+        boolean isSupportedTag = XmlHelper.getTagAttributePattern("tag", "method").inside(XmlHelper.getInsideTagPattern("services")).inFile(XmlHelper.getXmlFilePattern()).accepts(element)
+            || XmlHelper.getTagAttributePattern("call", "method").inside(XmlHelper.getInsideTagPattern("services")).inFile(XmlHelper.getXmlFilePattern()).accepts(element);
+
+        if (isSupportedTag) {
+            // attach to text child only
+            PsiElement[] psiElements = element.getChildren();
+            if (psiElements.length < 2) {
+                return;
+            }
 
+            String serviceClassValue = XmlHelper.getServiceDefinitionClass(element);
+            if (StringUtils.isNotBlank(serviceClassValue)) {
+                registerMethodProblem(psiElements[1], holder, serviceClassValue, collector);
+            }
+
+        }
     }
 
-    private void visitYamlMethodTagKey(@NotNull final PsiElement psiElement, @NotNull ProblemsHolder holder, ContainerCollectionResolver.LazyServiceCollector collector) {
+    private void visitYamlMethodTagKey(@NotNull final PsiElement psiElement, @NotNull ProblemsHolder holder, @NotNull NotNullLazyValue<ContainerCollectionResolver.LazyServiceCollector> collector) {
 
         String methodName = PsiElementUtils.trimQuote(psiElement.getText());
         if(StringUtils.isBlank(methodName)) {
@@ -126,8 +116,7 @@ private void visitYamlMethodTagKey(@NotNull final PsiElement psiElement, @NotNul
         registerMethodProblem(psiElement, holder, classValue, collector);
     }
 
-    private void annotateCallMethod(@NotNull final PsiElement psiElement, @NotNull ProblemsHolder holder, ContainerCollectionResolver.LazyServiceCollector collector) {
-
+    private void visitYmlElement(@NotNull final PsiElement psiElement, @NotNull ProblemsHolder holder, @NotNull NotNullLazyValue<ContainerCollectionResolver.LazyServiceCollector> collector) {
         if(StandardPatterns.and(
             YamlElementPatternHelper.getInsideKeyValue("tags"),
             YamlElementPatternHelper.getSingleLineScalarKey("method")
@@ -140,10 +129,9 @@ private void annotateCallMethod(@NotNull final PsiElement psiElement, @NotNull P
         {
             visitYamlMethod(psiElement, holder, collector);
         }
-
     }
 
-    private void visitYamlMethod(PsiElement psiElement, ProblemsHolder holder, ContainerCollectionResolver.LazyServiceCollector collector) {
+    private void visitYamlMethod(@NotNull PsiElement psiElement, @NotNull ProblemsHolder holder, @NotNull NotNullLazyValue<ContainerCollectionResolver.LazyServiceCollector> collector) {
         if(YamlElementPatternHelper.getInsideKeyValue("calls").accepts(psiElement)) {
             PsiElement parent = psiElement.getParent();
             if ((parent instanceof YAMLScalar)) {
@@ -154,11 +142,11 @@ private void visitYamlMethod(PsiElement psiElement, ProblemsHolder holder, Conta
         }
     }
 
-    private void registerMethodProblem(final @NotNull PsiElement psiElement, @NotNull ProblemsHolder holder, @NotNull String classKeyValue, ContainerCollectionResolver.LazyServiceCollector collector) {
-        registerMethodProblem(psiElement, holder, ServiceUtil.getResolvedClassDefinition(psiElement.getProject(), classKeyValue, collector));
+    private void registerMethodProblem(final @NotNull PsiElement psiElement, @NotNull ProblemsHolder holder, @NotNull String classKeyValue, @NotNull NotNullLazyValue<ContainerCollectionResolver.LazyServiceCollector> collector) {
+        registerMethodProblem(psiElement, holder, ServiceUtil.getResolvedClassDefinition(psiElement.getProject(), classKeyValue, collector.get()));
     }
 
-    private void registerMethodProblem(final @NotNull PsiElement psiElement, @NotNull ProblemsHolder holder, @Nullable PhpClass phpClass) {
+    private static void registerMethodProblem(final @NotNull PsiElement psiElement, @NotNull ProblemsHolder holder, @Nullable PhpClass phpClass) {
         if(phpClass == null) {
             return;
         }
@@ -232,35 +220,20 @@ private String importIfNecessary(@NotNull PhpClass phpClass, String fqn) {
      * );
      *
      */
-    private class PhpSubscriberRecursiveElementWalkingVisitor extends PsiRecursiveElementWalkingVisitor {
-        private final ProblemsHolder holder;
-
-        PhpSubscriberRecursiveElementWalkingVisitor(ProblemsHolder holder) {
-            this.holder = holder;
-        }
-
-        @Override
-        public void visitElement(@NotNull PsiElement element) {
-            super.visitElement(element);
-
-            if(!(element instanceof StringLiteralExpression)) {
-                return;
-            }
-
-            PsiElement arrayValue = element.getParent();
-            if(arrayValue != null && arrayValue.getNode().getElementType() == PhpElementTypes.ARRAY_VALUE) {
-                PhpReturn phpReturn = PsiTreeUtil.getParentOfType(arrayValue, PhpReturn.class);
-                if(phpReturn != null) {
-                    Method method = PsiTreeUtil.getParentOfType(arrayValue, Method.class);
-                    if(method != null) {
-                        String name = method.getName();
-                        if("getSubscribedEvents".equals(name)) {
-                            PhpClass containingClass = method.getContainingClass();
-                            if(containingClass != null && PhpElementsUtil.isInstanceOf(containingClass, "\\Symfony\\Component\\EventDispatcher\\EventSubscriberInterface")) {
-                                String contents = ((StringLiteralExpression) element).getContents();
-                                if(StringUtils.isNotBlank(contents) && containingClass.findMethodByName(contents) == null) {
-                                    registerMethodProblem(element, holder, containingClass);
-                                }
+    private static void visitPhpElement(@NotNull StringLiteralExpression element, @NotNull ProblemsHolder holder) {
+        PsiElement arrayValue = element.getParent();
+        if (arrayValue != null && arrayValue.getNode().getElementType() == PhpElementTypes.ARRAY_VALUE) {
+            PhpReturn phpReturn = PsiTreeUtil.getParentOfType(arrayValue, PhpReturn.class);
+            if (phpReturn != null) {
+                Method method = PsiTreeUtil.getParentOfType(arrayValue, Method.class);
+                if (method != null) {
+                    String name = method.getName();
+                    if ("getSubscribedEvents".equals(name)) {
+                        PhpClass containingClass = method.getContainingClass();
+                        if (containingClass != null && PhpElementsUtil.isInstanceOf(containingClass, "\\Symfony\\Component\\EventDispatcher\\EventSubscriberInterface")) {
+                            String contents = element.getContents();
+                            if (StringUtils.isNotBlank(contents) && containingClass.findMethodByName(contents) == null) {
+                                registerMethodProblem(element, holder, containingClass);
                             }
                         }
                     }
diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml
@@ -333,7 +333,8 @@
         <localInspection groupPath="Symfony" shortName="EventMethodCall" displayName="Create Method"
                          groupName="Service"
                          enabledByDefault="true" level="WARNING"
-                         implementationClass="fr.adrienbrault.idea.symfony2plugin.config.yaml.inspection.EventMethodCallInspection"/>
+                         implementationClass="fr.adrienbrault.idea.symfony2plugin.config.yaml.inspection.EventMethodCallInspection"
+                         language=""/>
 
         <localInspection groupPath="Symfony" shortName="XmlServiceArgumentInspection" displayName="Symfony: XML Arguments"
                          groupName="Service"
diff --git a/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/config/yaml/inspection/EventMethodCallInspectionTest.java b/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/config/yaml/inspection/EventMethodCallInspectionTest.java
@@ -41,4 +41,59 @@ public void testThatYmlCallsProvidesMethodExistsCheck() {
             , "Missing Method");
     }
 
+    public void testThatPhpCallsProvidesMethodExistsCheck() {
+        assertLocalInspectionContains("test.php", "<?php\n" +
+                "" +
+                "use Symfony\\Component\\EventDispatcher\\EventSubscriberInterface;\n" +
+                "" +
+                "class ExceptionSubscriber implements EventSubscriberInterface\n" +
+                "{\n" +
+                "    public static function getSubscribedEvents(): array\n" +
+                "    {\n" +
+                "        // return the subscribed events, their methods and priorities\n" +
+                "        return [\n" +
+                "            KernelEvents::EXCEPTION => [\n" +
+                "                ['proces<caret>sException', 10],\n" +
+                "            ],\n" +
+                "        ];\n" +
+                "    }\n" +
+                "\n" +
+                "}",
+            "Missing Method"
+        );
+    }
+
+    public void testThatXmlCallsProvidesMethodExistsCheck() {
+        assertLocalInspectionContains("test.xml", "" +
+                "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n" +
+                "<container xmlns=\"http://symfony.com/schema/dic/services\"\n" +
+                "    xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\n" +
+                "    xsi:schemaLocation=\"http://symfony.com/schema/dic/services\n" +
+                "        https://symfony.com/schema/dic/services/services-1.0.xsd\">\n" +
+                "\n" +
+                "    <services>\n" +
+                "        <service id=\"Foo\\Service\\Method\\MyFoo\">\n" +
+                "            <call method=\"get<caret>Foos\"></call>\n" +
+                "        </service>\n" +
+                "    </services>\n" +
+                "</container>",
+            "Missing Method"
+        );
+
+        assertLocalInspectionContains("test.xml", "" +
+                "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n" +
+                "<container xmlns=\"http://symfony.com/schema/dic/services\"\n" +
+                "    xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\n" +
+                "    xsi:schemaLocation=\"http://symfony.com/schema/dic/services\n" +
+                "        https://symfony.com/schema/dic/services/services-1.0.xsd\">\n" +
+                "\n" +
+                "    <services>\n" +
+                "        <service id=\"Foo\\Service\\Method\\MyFoo\">\n" +
+                "            <tag name=\"kernel.event_listener\" method=\"get<caret>Foos\"></call>\n" +
+                "        </service>\n" +
+                "    </services>\n" +
+                "</container>",
+            "Missing Method"
+        );
+    }
 }
diff --git a/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/config/yaml/inspection/fixtures/classes.php b/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/config/yaml/inspection/fixtures/classes.php
@@ -8,3 +8,8 @@ public function getFoo() {}
     }
 }
 
+namespace Symfony\Component\EventDispatcher
+{
+    interface EventSubscriberInterface {}
+}
+

Original file line number	Diff line number	Diff line change
`@@ -8,3 +8,8 @@ public function getFoo() {}`
`8`	`8`	`}`
`9`	`9`	`}`
`10`	`10`
	`11`	`+namespace Symfony\Component\EventDispatcher`
	`12`	`+{`
	`13`	`+ interface EventSubscriberInterface {}`
	`14`	`+}`
	`15`	`+`