Skip to content

Commit 37832ca

Browse files
HaehnchenHaehnchen
authored
Merge pull request #2112 from Haehnchen/feature/routes-duplicate-more
support detection for duplicate routes in same file for xml, php attributes and docblock
2 parents 3d8e94b + 8d0db1e commit 37832ca

File tree

9 files changed

+231
-14
lines changed

9 files changed

+231
-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: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,27 @@
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;
26+
import org.apache.commons.lang.StringUtils;
1027
import org.jetbrains.annotations.NotNull;
28+
import org.jetbrains.yaml.YAMLLanguage;
1129
import org.jetbrains.yaml.psi.YAMLDocument;
1230
import org.jetbrains.yaml.psi.YAMLKeyValue;
1331
import org.jetbrains.yaml.psi.YAMLMapping;
@@ -16,6 +34,7 @@
1634
* @author Daniel Espendiller <daniel@espendiller.net>
1735
*/
1836
public class DuplicateLocalRouteInspection extends LocalInspectionTool {
37+
private static final String MESSAGE = "Symfony: Duplicate route name";
1938

2039
@NotNull
2140
@Override
@@ -36,7 +55,19 @@ public MyPsiElementVisitor(ProblemsHolder holder) {
3655

3756
@Override
3857
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) {
58+
if (element instanceof YAMLKeyValue yamlKeyValue && element.getLanguage() == YAMLLanguage.INSTANCE) {
59+
visitYaml(yamlKeyValue);
60+
} else if(element instanceof StringLiteralExpression && element.getLanguage() == PhpLanguage.INSTANCE) {
61+
visitPhp((StringLiteralExpression) element);
62+
} else if(element instanceof XmlAttributeValue xmlAttributeValue) {
63+
XmlDuplicateServiceKeyInspection.visitRoot(xmlAttributeValue, holder, "routes", "route", "id", MESSAGE);
64+
}
65+
66+
super.visitElement(element);
67+
}
68+
69+
private void visitYaml(@NotNull YAMLKeyValue yamlKeyValue) {
70+
if (YamlHelper.isRoutingFile(yamlKeyValue.getContainingFile()) && yamlKeyValue.getParent() instanceof YAMLMapping yamlMapping && yamlMapping.getParent() instanceof YAMLDocument) {
4071
String keyText1 = null;
4172

4273
int found = 0;
@@ -55,14 +86,68 @@ public void visitElement(@NotNull PsiElement element) {
5586
if (found == 2) {
5687
final PsiElement keyElement = yamlKeyValue.getKey();
5788
assert keyElement != null;
58-
holder.registerProblem(keyElement, "Symfony: Duplicate key", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
89+
holder.registerProblem(keyElement, "Symfony: Duplicate route name", ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
5990

6091
break;
6192
}
6293
}
6394
}
95+
}
6496

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

‎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: 91 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,86 @@ 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+
" /**\n" +
108+
" * @Route(name=\"te<caret>st\")\n" +
109+
" */\n" +
110+
" public function index2()\n" +
111+
" {\n" +
112+
" }\n" +
113+
"}\n",
114+
"Symfony: Duplicate route name"
115+
);
116+
}
117+
118+
public void testDuplicateRoutingKeysForXml() {
119+
assertLocalInspectionContains("routing.xml", "" +
120+
"<routes>\n" +
121+
" <route id=\"blog_list\" path=\"/blog\"/>\n" +
122+
" <route id=\"blog<caret>_list\" path=\"/blog1\"/>\n" +
123+
"</routes>",
124+
"Symfony: Duplicate route name"
37125
);
38126
}
39127
}

‎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)