Skip to content

Commit 4612ad5

Browse files
HaehnchenHaehnchen
committed
support detection for duplicate routes in same file for xml, php attributes and docblock
1 parent 197a46e commit 4612ad5

File tree

9 files changed

+227
-14
lines changed

9 files changed

+227
-14
lines changed

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/config/xml/inspection/XmlDuplicateParameterKeyInspection.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public PsiElementVisitor buildVisitor(final @NotNull ProblemsHolder holder, bool
2323
@Override
2424
public void visitElement(@NotNull PsiElement element) {
2525
if (element instanceof XmlAttributeValue xmlAttributeValue) {
26-
visitRoot(xmlAttributeValue, holder, "parameters", "parameter", "key");
26+
visitRoot(xmlAttributeValue, holder, "parameters", "parameter", "key", "Symfony: Duplicate Key");
2727
}
2828

2929
super.visitElement(element);

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/config/xml/inspection/XmlDuplicateServiceKeyInspection.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ public PsiElementVisitor buildVisitor(final @NotNull ProblemsHolder holder, bool
2727
@Override
2828
public void visitElement(@NotNull PsiElement element) {
2929
if (element instanceof XmlAttributeValue xmlAttributeValue) {
30-
visitRoot(xmlAttributeValue, holder, "services", "service", "id");
30+
visitRoot(xmlAttributeValue, holder, "services", "service", "id", "Symfony: Duplicate Key");
3131
}
3232

3333
super.visitElement(element);
3434
}
3535
};
3636
}
3737

38-
protected void visitRoot(@NotNull XmlAttributeValue xmlAttributeValue, @NotNull ProblemsHolder holder, String root, String child, String tagName) {
38+
public static void visitRoot(@NotNull XmlAttributeValue xmlAttributeValue, @NotNull ProblemsHolder holder, @NotNull String root, @NotNull String child, @NotNull String tagName, @NotNull String message) {
3939
String value = null;
4040

4141
if (xmlAttributeValue.getParent() instanceof XmlAttribute xmlAttribute && tagName.equals(xmlAttribute.getName())) {
@@ -55,7 +55,7 @@ protected void visitRoot(@NotNull XmlAttributeValue xmlAttributeValue, @NotNull
5555
}
5656

5757
if (found == 2) {
58-
holder.registerProblem(xmlAttributeValue, "Symfony: Duplicate Key", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
58+
holder.registerProblem(xmlAttributeValue, message, ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
5959
break;
6060
}
6161
}

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/config/yaml/inspection/EventMethodCallInspection.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ private static void visitPhpElement(@NotNull StringLiteralExpression element, @N
238238
}
239239
}
240240

241-
if (parent instanceof ParameterList parameterList && PhpElementsUtil.isAttributeNamedArgumentString(element, "method", "\\Symfony\\Component\\EventDispatcher\\Attribute\\AsEventListener")) {
241+
if (parent instanceof ParameterList parameterList && PhpElementsUtil.isAttributeNamedArgumentString(element, "\\Symfony\\Component\\EventDispatcher\\Attribute\\AsEventListener", "method")) {
242242
PhpAttribute parentOfType = PsiTreeUtil.getParentOfType(parameterList, PhpAttribute.class);
243243
if (parentOfType.getOwner() instanceof PhpClass phpClass) {
244244
String contents = element.getContents();

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

Lines changed: 87 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,26 @@
55
import com.intellij.codeInspection.ProblemsHolder;
66
import com.intellij.psi.PsiElement;
77
import com.intellij.psi.PsiElementVisitor;
8+
import com.intellij.psi.util.PsiTreeUtil;
9+
import com.intellij.psi.xml.XmlAttributeValue;
10+
import com.jetbrains.php.lang.PhpLanguage;
11+
import com.jetbrains.php.lang.documentation.phpdoc.psi.PhpDocComment;
12+
import com.jetbrains.php.lang.documentation.phpdoc.psi.tags.PhpDocTag;
13+
import com.jetbrains.php.lang.psi.elements.Method;
14+
import com.jetbrains.php.lang.psi.elements.PhpAttribute;
15+
import com.jetbrains.php.lang.psi.elements.PhpClass;
16+
import com.jetbrains.php.lang.psi.elements.StringLiteralExpression;
17+
import de.espend.idea.php.annotation.dict.PhpDocCommentAnnotation;
18+
import de.espend.idea.php.annotation.dict.PhpDocTagAnnotation;
19+
import de.espend.idea.php.annotation.pattern.AnnotationPattern;
20+
import de.espend.idea.php.annotation.util.AnnotationUtil;
821
import fr.adrienbrault.idea.symfony2plugin.Symfony2ProjectComponent;
22+
import fr.adrienbrault.idea.symfony2plugin.config.xml.inspection.XmlDuplicateServiceKeyInspection;
23+
import fr.adrienbrault.idea.symfony2plugin.routing.RouteHelper;
24+
import fr.adrienbrault.idea.symfony2plugin.util.PhpElementsUtil;
925
import fr.adrienbrault.idea.symfony2plugin.util.yaml.YamlHelper;
1026
import org.jetbrains.annotations.NotNull;
27+
import org.jetbrains.yaml.YAMLLanguage;
1128
import org.jetbrains.yaml.psi.YAMLDocument;
1229
import org.jetbrains.yaml.psi.YAMLKeyValue;
1330
import org.jetbrains.yaml.psi.YAMLMapping;
@@ -16,6 +33,7 @@
1633
* @author Daniel Espendiller <daniel@espendiller.net>
1734
*/
1835
public class DuplicateLocalRouteInspection extends LocalInspectionTool {
36+
private static final String MESSAGE = "Symfony: Duplicate route name";
1937

2038
@NotNull
2139
@Override
@@ -36,7 +54,19 @@ public MyPsiElementVisitor(ProblemsHolder holder) {
3654

3755
@Override
3856
public void visitElement(@NotNull PsiElement element) {
39-
if (element instanceof YAMLKeyValue yamlKeyValue && YamlHelper.isRoutingFile(yamlKeyValue.getContainingFile()) && yamlKeyValue.getParent() instanceof YAMLMapping yamlMapping && yamlMapping.getParent() instanceof YAMLDocument) {
57+
if (element instanceof YAMLKeyValue yamlKeyValue && element.getLanguage() == YAMLLanguage.INSTANCE) {
58+
visitYaml(yamlKeyValue);
59+
} else if(element instanceof StringLiteralExpression && element.getLanguage() == PhpLanguage.INSTANCE) {
60+
visitPhp((StringLiteralExpression) element);
61+
} else if(element instanceof XmlAttributeValue xmlAttributeValue) {
62+
XmlDuplicateServiceKeyInspection.visitRoot(xmlAttributeValue, holder, "routes", "route", "id", MESSAGE);
63+
}
64+
65+
super.visitElement(element);
66+
}
67+
68+
private void visitYaml(@NotNull YAMLKeyValue yamlKeyValue) {
69+
if (YamlHelper.isRoutingFile(yamlKeyValue.getContainingFile()) && yamlKeyValue.getParent() instanceof YAMLMapping yamlMapping && yamlMapping.getParent() instanceof YAMLDocument) {
4070
String keyText1 = null;
4171

4272
int found = 0;
@@ -55,14 +85,68 @@ public void visitElement(@NotNull PsiElement element) {
5585
if (found == 2) {
5686
final PsiElement keyElement = yamlKeyValue.getKey();
5787
assert keyElement != null;
58-
holder.registerProblem(keyElement, "Symfony: Duplicate key", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
88+
holder.registerProblem(keyElement, "Symfony: Duplicate route name", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
5989

6090
break;
6191
}
6292
}
6393
}
94+
}
6495

65-
super.visitElement(element);
96+
private void visitPhp(@NotNull StringLiteralExpression element) {
97+
if (PhpElementsUtil.isAttributeNamedArgumentString(element, "\\Symfony\\Component\\Routing\\Annotation\\Route", "name")) {
98+
PhpAttribute parentOfType = PsiTreeUtil.getParentOfType(element, PhpAttribute.class);
99+
if (parentOfType.getOwner() instanceof Method method && method.getAccess().isPublic() && method.getContainingClass() != null) {
100+
int found = 0;
101+
String contents = element.getContents();
102+
103+
for (Method ownMethod : method.getContainingClass().getOwnMethods()) {
104+
for (PhpAttribute attribute : ownMethod.getAttributes("\\Symfony\\Component\\Routing\\Annotation\\Route")) {
105+
String name = PhpElementsUtil.getAttributeArgumentStringByName(attribute, "name");
106+
if (contents.equals(name)) {
107+
found++;
108+
}
109+
110+
if (found == 2) {
111+
holder.registerProblem(element, MESSAGE, ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
112+
return;
113+
}
114+
}
115+
}
116+
}
117+
}
118+
119+
if (AnnotationPattern.getPropertyIdentifierValue("name").accepts(element)) {
120+
PhpDocTag phpDocTag = PsiTreeUtil.getParentOfType(element, PhpDocTag.class);
121+
if (phpDocTag != null) {
122+
PhpClass phpClass = AnnotationUtil.getAnnotationReference(phpDocTag);
123+
if (phpClass != null && RouteHelper.ROUTE_CLASSES.contains(phpClass.getFQN())) {
124+
PhpDocComment phpDocComment = PsiTreeUtil.getParentOfType(element, PhpDocComment.class);
125+
if (phpDocComment.getNextPsiSibling() instanceof Method method && method.getAccess().isPublic() && method.getContainingClass() != null) {
126+
int found = 0;
127+
String contents = element.getContents();
128+
129+
for (Method ownMethod : method.getContainingClass().getOwnMethods()) {
130+
PhpDocCommentAnnotation phpClassContainer = AnnotationUtil.getPhpDocCommentAnnotationContainer(ownMethod.getDocComment());
131+
if(phpClassContainer != null) {
132+
PhpDocTagAnnotation firstPhpDocBlock = phpClassContainer.getFirstPhpDocBlock(RouteHelper.ROUTE_CLASSES.toArray(String[]::new));
133+
if(firstPhpDocBlock != null) {
134+
String name = firstPhpDocBlock.getPropertyValue("name");
135+
if (contents.equals(name)) {
136+
found++;
137+
}
138+
139+
if (found == 2) {
140+
holder.registerProblem(element, MESSAGE, ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
141+
return;
142+
}
143+
}
144+
}
145+
}
146+
}
147+
}
148+
}
149+
}
66150
}
67151
}
68152
}

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/util/PhpElementsUtil.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.jetbrains.php.lang.psi.elements.impl.PhpDefineImpl;
3636
import com.jetbrains.php.lang.psi.resolve.types.PhpType;
3737
import com.jetbrains.php.lang.psi.stubs.indexes.expectedArguments.PhpExpectedFunctionArgument;
38+
import com.jetbrains.php.lang.psi.stubs.indexes.expectedArguments.PhpExpectedFunctionClassConstantArgument;
3839
import com.jetbrains.php.lang.psi.stubs.indexes.expectedArguments.PhpExpectedFunctionScalarArgument;
3940
import com.jetbrains.php.phpunit.PhpUnitUtil;
4041
import com.jetbrains.php.refactoring.PhpAliasImporter;
@@ -1700,7 +1701,7 @@ public static String insertUseIfNecessary(@NotNull PsiElement phpClass, @NotNull
17001701
/**
17011702
* #[AsEventListener(method:'onBarEvent')]
17021703
*/
1703-
public static boolean isAttributeNamedArgumentString(@NotNull StringLiteralExpression element, @NotNull String namedArgument, @NotNull String fqn) {
1704+
public static boolean isAttributeNamedArgumentString(@NotNull StringLiteralExpression element, @NotNull String fqn, @NotNull String namedArgument) {
17041705
PsiElement colon = PsiTreeUtil.prevCodeLeaf(element);
17051706
if (colon == null || colon.getNode().getElementType() != PhpTokenTypes.opCOLON) {
17061707
return false;
@@ -1907,6 +1908,24 @@ public static PhpExpectedFunctionArgument findAttributeArgumentByName(@NotNull S
19071908
return null;
19081909
}
19091910

1911+
public static String getAttributeArgumentStringByName(@NotNull PhpAttribute phpAttribute, @NotNull String attributeName) {
1912+
for (PhpAttribute.PhpAttributeArgument argument : phpAttribute.getArguments()) {
1913+
if (!attributeName.equals(argument.getName())) {
1914+
continue;
1915+
}
1916+
1917+
if (argument.getArgument() instanceof PhpExpectedFunctionClassConstantArgument phpExpectedFunctionClassConstantArgument) {
1918+
return phpExpectedFunctionClassConstantArgument.getClassFqn();
1919+
}
1920+
1921+
if (argument.getArgument() instanceof PhpExpectedFunctionScalarArgument phpExpectedFunctionScalarArgument) {
1922+
return PsiElementUtils.trimQuote(phpExpectedFunctionScalarArgument.getNormalizedValue());
1923+
}
1924+
}
1925+
1926+
return null;
1927+
}
1928+
19101929
@Nullable
19111930
public static String findAttributeArgumentByNameAsString(@NotNull String attributeName, @NotNull PhpAttribute phpAttribute) {
19121931
PhpExpectedFunctionArgument attributeArgumentByName = findAttributeArgumentByName(attributeName, phpAttribute);

‎src/main/resources/META-INF/plugin.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,10 @@
284284
language="PHP"
285285
implementationClass="fr.adrienbrault.idea.symfony2plugin.routing.inspection.PhpRouteMissingInspection"/>
286286

287-
<localInspection groupPath="Symfony" shortName="Symfony2YamlRouteDuplicateInspection" displayName="Duplicate Route"
287+
<localInspection groupPath="Symfony" shortName="DuplicateLocalRouteInspection" displayName="Duplicate Route"
288288
groupName="Route"
289289
enabledByDefault="true" level="WARNING"
290-
language="yaml"
290+
language=""
291291
implementationClass="fr.adrienbrault.idea.symfony2plugin.routing.inspection.DuplicateLocalRouteInspection"/>
292292

293293
<localInspection groupPath="Symfony" shortName="Symfony2YamlDuplicateServiceKeyInspection" displayName="Duplicate Key"

‎src/main/resources/inspectionDescriptions/DuplicateLocalRouteInspection.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<html>
2+
<body>
3+
Duplicate routes are defined in same scope where only one overwrites the other
4+
<!-- tooltip end -->
5+
</body>
6+
</html>

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

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,31 @@
88
* @see fr.adrienbrault.idea.symfony2plugin.routing.inspection.DuplicateLocalRouteInspection
99
*/
1010
public class DuplicateLocalRouteInspectionTest extends SymfonyLightCodeInsightFixtureTestCase {
11+
public void setUp() throws Exception {
12+
super.setUp();
13+
14+
myFixture.copyFileToProject("DuplicateLocalRouteInspection.php");
15+
}
16+
17+
protected String getTestDataPath() {
18+
return "src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/routing/inspection/fixtures";
19+
}
1120

1221
public void testDuplicateRouteKeyProvidesWarning() {
1322
assertLocalInspectionContains("routing.yml", "" +
1423
"foo:\n" +
1524
" car: foo\n" +
1625
"f<caret>oo:\n" +
1726
" car: foo\n",
18-
"Symfony: Duplicate key"
27+
"Symfony: Duplicate route name"
1928
);
2029

2130
assertLocalInspectionContains("routing.yml", "" +
2231
"fo<caret>o:\n" +
2332
" car: foo\n" +
2433
"foo:\n" +
2534
" car: foo\n",
26-
"Symfony: Duplicate key"
35+
"Symfony: Duplicate route name"
2736
);
2837

2938
assertLocalInspectionNotContains("routing.yml", "" +
@@ -33,7 +42,83 @@ public void testDuplicateRouteKeyProvidesWarning() {
3342
" car: foo\n" +
3443
"foo:\n" +
3544
" car: foo\n",
36-
"Symfony: Duplicate key"
45+
"Symfony: Duplicate route name"
46+
);
47+
}
48+
49+
public void testDuplicateRoutingKeysForPhpAttribute() {
50+
assertLocalInspectionContains("controller.php", "<?php\n" +
51+
"\n" +
52+
"use Symfony\\Component\\Routing\\Annotation\\Route;\n" +
53+
"\n" +
54+
"class FooController\n" +
55+
"{\n" +
56+
" #[Route(name: 'foobar<caret>_index')]\n" +
57+
" #[Route(name: 'foobar_index')]\n" +
58+
" #[Route(name: 'foobar_index_1')]\n" +
59+
" public function index() {}\n" +
60+
"}\n",
61+
"Symfony: Duplicate route name"
62+
);
63+
64+
assertLocalInspectionContains("controller.php", "<?php\n" +
65+
"\n" +
66+
"use Symfony\\Component\\Routing\\Annotation\\Route;\n" +
67+
"\n" +
68+
"class FooController\n" +
69+
"{\n" +
70+
" #[Route(name: 'foobar<caret>_index')]\n" +
71+
" public function index() {}\n" +
72+
"\n" +
73+
" #[Route(name: 'foobar_index')]\n" +
74+
" public function foo() {}\n" +
75+
"}\n",
76+
"Symfony: Duplicate route name"
77+
);
78+
79+
assertLocalInspectionNotContains("controller.php", "<?php\n" +
80+
"\n" +
81+
"use Symfony\\Component\\Routing\\Annotation\\Route;\n" +
82+
"\n" +
83+
"class FooController\n" +
84+
"{\n" +
85+
" #[Route(name: 'foobar<caret>_index')]\n" +
86+
" public function index() {}\n" +
87+
"\n" +
88+
" #[Route(name: 'foobar_index_1')]\n" +
89+
" public function foo() {}\n" +
90+
"}\n",
91+
"Symfony: Duplicate route name"
92+
);
93+
}
94+
95+
public void testDuplicateRoutingKeysForPhpDocBlocks() {
96+
assertLocalInspectionContains("controller.php", "<?php\n" +
97+
"\n" +
98+
"use Symfony\\Component\\Routing\\Annotation\\Route;\n" +
99+
"\n" +
100+
"class FooController\n" +
101+
"{\n" +
102+
" /**\n" +
103+
" * @Route(name=\"test\")\n" +
104+
" */\n" +
105+
" public function index() {}\n" +
106+
" /**\n" +
107+
" * @Route(name=\"te<caret>st\")\n" +
108+
" */\n" +
109+
" public function index() {}\n" +
110+
"}\n",
111+
"Symfony: Duplicate route name"
112+
);
113+
}
114+
115+
public void testDuplicateRoutingKeysForXml() {
116+
assertLocalInspectionContains("routing.xml", "" +
117+
"<routes>\n" +
118+
" <route id=\"blog_list\" path=\"/blog\"/>\n" +
119+
" <route id=\"blog<caret>_list\" path=\"/blog1\"/>\n" +
120+
"</routes>",
121+
"Symfony: Duplicate route name"
37122
);
38123
}
39124
}

‎src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/routing/inspection/fixtures/DuplicateLocalRouteInspection.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Symfony\Component\Routing\Annotation
4+
{
5+
/**
6+
* Annotation class for @Route().
7+
*
8+
* @Annotation
9+
* @NamedArgumentConstructor
10+
* @Target({"CLASS", "METHOD"})
11+
*
12+
* @author Fabien Potencier <fabien@symfony.com>
13+
* @author Alexander M. Turek <me@derrabus.de>
14+
*/
15+
#[\Attribute(\Attribute::IS_REPEATABLE | \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD)]
16+
class Route
17+
{
18+
}
19+
}

0 commit comments

Comments
 (0)