Merge pull request #2106 from Haehnchen/feature/routing-duplicate-visitor

duplicate routing key inspections for yaml should only visit the working context

Author: Haehnchen

src/main/java/fr/adrienbrault/idea/symfony2plugin/routing/inspection/DuplicateLocalRouteInspection.java

@@ -1,15 +1,16 @@
 package fr.adrienbrault.idea.symfony2plugin.routing.inspection;
 
 import com.intellij.codeInspection.LocalInspectionTool;
+import com.intellij.codeInspection.ProblemHighlightType;
 import com.intellij.codeInspection.ProblemsHolder;
+import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiElementVisitor;
-import com.intellij.psi.PsiFile;
-import com.intellij.psi.util.PsiTreeUtil;
 import fr.adrienbrault.idea.symfony2plugin.Symfony2ProjectComponent;
 import fr.adrienbrault.idea.symfony2plugin.util.yaml.YamlHelper;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.yaml.psi.YAMLDocument;
-import org.jetbrains.yaml.psi.YAMLValue;
+import org.jetbrains.yaml.psi.YAMLKeyValue;
+import org.jetbrains.yaml.psi.YAMLMapping;
 
 /**
  * @author Daniel Espendiller <daniel@espendiller.net>
@@ -34,24 +35,34 @@ public MyPsiElementVisitor(ProblemsHolder holder) {
         }
 
         @Override
-        public void visitFile(PsiFile file) {
-            // @TODO: detection of routing files in right way
-            // routing.yml
-            // comment.routing.yml
-            // routing/foo.yml
-            if(!YamlHelper.isRoutingFile(file)) {
-                return;
-            }
+        public void visitElement(@NotNull PsiElement element) {
+            if (element instanceof YAMLKeyValue yamlKeyValue && YamlHelper.isRoutingFile(yamlKeyValue.getContainingFile()) && yamlKeyValue.getParent() instanceof YAMLMapping yamlMapping && yamlMapping.getParent() instanceof YAMLDocument) {
+                String keyText1 = null;
 
-            YAMLDocument document = PsiTreeUtil.findChildOfType(file, YAMLDocument.class);
-            if(document == null) {
-                return;
-            }
+                int found = 0;
+                for (YAMLKeyValue keyValue : yamlMapping.getKeyValues()) {
+                    String keyText = keyValue.getKeyText();
+
+                    // lazy
+                    if (keyText1 == null) {
+                        keyText1 = yamlKeyValue.getKeyText();
+                    }
 
-            YAMLValue topLevelValue = document.getTopLevelValue();
-            if(topLevelValue != null) {
-                YamlHelper.attachDuplicateKeyInspection(topLevelValue, holder);
+                    if (keyText1.equals(keyText)) {
+                        found++;
+                    }
+
+                    if (found == 2) {
+                        final PsiElement keyElement = yamlKeyValue.getKey();
+                        assert keyElement != null;
+                        holder.registerProblem(keyElement, "Symfony: Duplicate key", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
+
+                        break;
+                    }
+                }
             }
+
+            super.visitElement(element);
         }
     }
 }

src/main/java/fr/adrienbrault/idea/symfony2plugin/util/yaml/YamlHelper.java

@@ -1,7 +1,5 @@
 package fr.adrienbrault.idea.symfony2plugin.util.yaml;
 
-import com.intellij.codeInspection.ProblemHighlightType;
-import com.intellij.codeInspection.ProblemsHolder;
 import com.intellij.openapi.application.Result;
 import com.intellij.openapi.command.WriteCommandAction;
 import com.intellij.openapi.util.Pair;
@@ -17,7 +15,6 @@
 import com.intellij.util.Consumer;
 import com.intellij.util.ObjectUtils;
 import com.intellij.util.ProcessingContext;
-import com.intellij.util.Processor;
 import com.intellij.util.containers.ContainerUtil;
 import com.jetbrains.php.lang.psi.elements.Parameter;
 import com.jetbrains.php.lang.psi.elements.PhpClass;
@@ -462,62 +459,6 @@ public static void getParentArrayKeys(YAMLKeyValue yamlKeyValue, List<String> ke
 
     }
 
-    /**
-     * Migrate to processKeysAfterRoot @TODO
-     *
-     * @param keyContext Should be Document or YAMLCompoundValueImpl which holds the key value children
-     */
-    public static void attachDuplicateKeyInspection(PsiElement keyContext, @NotNull ProblemsHolder holder) {
-
-        Map<String, PsiElement> psiElementMap = new HashMap<>();
-        Set<PsiElement> yamlKeyValues = new HashSet<>();
-
-        Collection<YAMLKeyValue> collection = PsiTreeUtil.getChildrenOfTypeAsList(keyContext, YAMLKeyValue.class);
-        for(YAMLKeyValue yamlKeyValue: collection) {
-            String keyText = PsiElementUtils.trimQuote(yamlKeyValue.getKeyText());
-            if(StringUtils.isNotBlank(keyText)) {
-                if(psiElementMap.containsKey(keyText)) {
-                    yamlKeyValues.add(psiElementMap.get(keyText));
-                    yamlKeyValues.add(yamlKeyValue);
-                } else {
-                    psiElementMap.put(keyText, yamlKeyValue);
-                }
-
-            }
-
-        }
-
-        if(yamlKeyValues.size() > 0) {
-            for(PsiElement psiElement: yamlKeyValues) {
-                if(psiElement instanceof YAMLKeyValue) {
-                    final PsiElement keyElement = ((YAMLKeyValue) psiElement).getKey();
-                    assert keyElement != null;
-                    holder.registerProblem(keyElement, "Duplicate key", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
-                }
-            }
-        }
-
-    }
-
-    /**
-     * Process yaml key in second level filtered by a root:
-     * File > roots -> "Item"
-     * TODO: visitQualifiedKeyValuesInFile
-     */
-    public static void processKeysAfterRoot(@NotNull PsiFile psiFile, @NotNull Processor<YAMLKeyValue> yamlKeyValueProcessor, @NotNull String... roots) {
-        for (String root : roots) {
-            YAMLKeyValue yamlKeyValue = YAMLUtil.getQualifiedKeyInFile((YAMLFile) psiFile, root);
-            if(yamlKeyValue != null) {
-                YAMLCompoundValue yaml = PsiTreeUtil.findChildOfType(yamlKeyValue, YAMLCompoundValue.class);
-                if(yaml != null) {
-                    for(YAMLKeyValue yamlKeyValueVisit: PsiTreeUtil.getChildrenOfTypeAsList(yaml, YAMLKeyValue.class)) {
-                        yamlKeyValueProcessor.process(yamlKeyValueVisit);
-                    }
-                }
-            }
-        }
-    }
-
     public static boolean isRoutingFile(PsiFile psiFile) {
         return psiFile.getName().contains("routing") || psiFile.getVirtualFile().getPath().contains("/routing");
     }

src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/routing/inspection/DuplicateLocalRouteInspectionTest.java

@@ -15,16 +15,25 @@ public void testDuplicateRouteKeyProvidesWarning() {
                 "  car: foo\n" +
                 "f<caret>oo:\n" +
                 "  car: foo\n",
-            "Duplicate key"
+            "Symfony: Duplicate key"
         );
 
         assertLocalInspectionContains("routing.yml", "" +
                 "fo<caret>o:\n" +
                 "  car: foo\n" +
                 "foo:\n" +
                 "  car: foo\n",
-            "Duplicate key"
+            "Symfony: Duplicate key"
         );
-    }
 
+        assertLocalInspectionNotContains("routing.yml", "" +
+                "foo:\n" +
+                "  car: foo\n" +
+                "foo<caret>bar:\n" +
+                "  car: foo\n" +
+                "foo:\n" +
+                "  car: foo\n",
+            "Symfony: Duplicate key"
+        );
+    }
 }