Skip to content

Commit 2d31843

Browse files
HaehnchenHaehnchen
authored
Merge pull request #1917 from Haehnchen/feature/1548-tags-checking
#1548 optimize inspection for notify missing extends/implements tags …
2 parents bf8fbbc + 0b7e5c4 commit 2d31843

File tree

5 files changed

+161
-88
lines changed

5 files changed

+161
-88
lines changed

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/ServiceDeprecatedClassesInspection.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public XmlClassElementWalkingVisitor(ProblemsHolder holder, ProblemRegistrar pro
9898
}
9999

100100
@Override
101-
public void visitElement(PsiElement element) {
101+
public void visitElement(@NotNull PsiElement element) {
102102
boolean serviceArgumentAccepted = XmlHelper.getArgumentServiceIdPattern().accepts(element);
103103
if(serviceArgumentAccepted || XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) {
104104
String text = PsiElementUtils.trimQuote(element.getText());

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/TaggedExtendsInterfaceClassInspection.java‎

Lines changed: 76 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import fr.adrienbrault.idea.symfony2plugin.util.yaml.YamlHelper;
2323
import org.apache.commons.lang.StringUtils;
2424
import org.jetbrains.annotations.NotNull;
25+
import org.jetbrains.annotations.Nullable;
26+
import org.jetbrains.yaml.YAMLTokenTypes;
2527
import org.jetbrains.yaml.psi.YAMLCompoundValue;
2628
import org.jetbrains.yaml.psi.YAMLFile;
2729
import org.jetbrains.yaml.psi.YAMLKeyValue;
@@ -43,7 +45,7 @@ public PsiElementVisitor buildVisitor(final @NotNull ProblemsHolder holder, bool
4345

4446
return new PsiElementVisitor() {
4547
@Override
46-
public void visitFile(PsiFile psiFile) {
48+
public void visitFile(@NotNull PsiFile psiFile) {
4749
if(psiFile instanceof YAMLFile) {
4850
psiFile.acceptChildren(new YmlClassElementWalkingVisitor(holder, new ContainerCollectionResolver.LazyServiceCollector(holder.getProject())));
4951
} else if(psiFile instanceof XmlFile) {
@@ -63,16 +65,15 @@ public XmlClassElementWalkingVisitor(ProblemsHolder holder, ContainerCollectionR
6365
}
6466

6567
@Override
66-
public void visitElement(PsiElement element) {
67-
if(XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) {
68-
String text = PsiElementUtils.trimQuote(element.getText());
69-
PsiElement[] psiElements = element.getChildren();
70-
71-
// attach problems to string value only
72-
if(StringUtils.isNotBlank(text) && psiElements.length > 2) {
73-
XmlTag parentOfType = PsiTreeUtil.getParentOfType(element, XmlTag.class);
74-
if(parentOfType != null) {
75-
registerTaggedProblems(psiElements[1], FormUtil.getTags(parentOfType), text, holder, this.lazyServiceCollector);
68+
public void visitElement(@NotNull PsiElement element) {
69+
String className = getClassNameFromServiceDefinition(element);
70+
if (className != null) {
71+
XmlTag parentOfType = PsiTreeUtil.getParentOfType(element, XmlTag.class);
72+
if(parentOfType != null) {
73+
// attach problems to string value only
74+
PsiElement[] psiElements = element.getChildren();
75+
if (psiElements.length > 2) {
76+
registerTaggedProblems(psiElements[1], FormUtil.getTags(parentOfType), className, holder, this.lazyServiceCollector);
7677
}
7778
}
7879
}
@@ -91,20 +92,19 @@ public YmlClassElementWalkingVisitor(ProblemsHolder holder, ContainerCollectionR
9192
}
9293

9394
@Override
94-
public void visitElement(PsiElement element) {
95-
96-
if(YamlElementPatternHelper.getSingleLineScalarKey("class").accepts(element)) {
95+
public void visitElement(@NotNull PsiElement psiElement) {
96+
if(YamlElementPatternHelper.getSingleLineScalarKey("class").accepts(psiElement)) {
9797

9898
// class: '\Foo'
99-
String text = PsiElementUtils.trimQuote(element.getText());
99+
String text = PsiElementUtils.trimQuote(psiElement.getText());
100100
if(StringUtils.isBlank(text)) {
101-
super.visitElement(element);
101+
super.visitElement(psiElement);
102102
return;
103103
}
104104

105-
PsiElement yamlScalar = element.getParent();
105+
PsiElement yamlScalar = psiElement.getParent();
106106
if(!(yamlScalar instanceof YAMLScalar)) {
107-
super.visitElement(element);
107+
super.visitElement(psiElement);
108108
return;
109109
}
110110

@@ -116,51 +116,90 @@ public void visitElement(PsiElement element) {
116116
if(serviceKeyValue instanceof YAMLKeyValue) {
117117
Set<String> tags = YamlHelper.collectServiceTags((YAMLKeyValue) serviceKeyValue);
118118
if(tags.size() > 0) {
119-
registerTaggedProblems(element, tags, text, holder, this.lazyServiceCollector);
119+
registerTaggedProblems(psiElement, tags, text, holder, this.lazyServiceCollector);
120120
}
121121
}
122122
}
123123
}
124+
} else if (psiElement.getNode().getElementType() == YAMLTokenTypes.SCALAR_KEY && YamlElementPatternHelper.getServiceIdKeyValuePattern().accepts(psiElement.getParent())) {
125+
// Foobar\Foo: ~
126+
String text = PsiElementUtils.getText(psiElement);
127+
if (StringUtils.isNotBlank(text) && YamlHelper.isClassServiceId(text) && text.contains("\\")) {
128+
PsiElement yamlKeyValue = psiElement.getParent();
129+
if (yamlKeyValue instanceof YAMLKeyValue && YamlHelper.getYamlKeyValue((YAMLKeyValue) yamlKeyValue, "resource") == null && YamlHelper.getYamlKeyValue((YAMLKeyValue) yamlKeyValue, "exclude") == null) {
130+
Set<String> tags = YamlHelper.collectServiceTags((YAMLKeyValue) yamlKeyValue);
131+
if(tags.size() > 0) {
132+
registerTaggedProblems(psiElement, tags, text, holder, this.lazyServiceCollector);
133+
}
134+
}
135+
}
124136
}
125137

126-
super.visitElement(element);
138+
super.visitElement(psiElement);
127139
}
128140
}
129141

130142
private void registerTaggedProblems(@NotNull PsiElement source, @NotNull Set<String> tags, @NotNull String serviceClass, @NotNull ProblemsHolder holder, @NotNull ContainerCollectionResolver.LazyServiceCollector lazyServiceCollector) {
131-
132-
if(tags.size() == 0) {
143+
if (tags.size() == 0) {
133144
return;
134145
}
135146

136147
PhpClass phpClass = null;
137148

138149
for (String tag : tags) {
150+
String missingTagInstance = null;
139151

140-
if(!ServiceUtil.TAG_INTERFACES.containsKey(tag)) {
141-
continue;
142-
}
152+
for (String expectedClass : ServiceUtil.TAG_INTERFACES.getOrDefault(tag, new String[] {})) {
153+
// load PhpClass only if we need it, on error exit
154+
if(phpClass == null) {
155+
phpClass = ServiceUtil.getResolvedClassDefinition(holder.getProject(), serviceClass, lazyServiceCollector);
156+
if(phpClass == null) {
157+
return;
158+
}
159+
}
143160

144-
String expectedClass = ServiceUtil.TAG_INTERFACES.get(tag);
145-
if(expectedClass == null) {
146-
continue;
147-
}
161+
// skip unknown classes
162+
if (PhpElementsUtil.getClassesInterface(phpClass.getProject(), expectedClass).isEmpty()) {
163+
continue;
164+
}
148165

149-
// load PhpClass only if we need it, on error exit
150-
if(phpClass == null) {
151-
phpClass = ServiceUtil.getResolvedClassDefinition(holder.getProject(), serviceClass, lazyServiceCollector);
152-
if(phpClass == null) {
153-
return;
166+
// check interfaces
167+
if(!PhpElementsUtil.isInstanceOf(phpClass, expectedClass)) {
168+
missingTagInstance = expectedClass;
169+
continue;
154170
}
171+
172+
missingTagInstance = null;
173+
break;
155174
}
156175

157176
// check interfaces
158-
if(!PhpElementsUtil.isInstanceOf(phpClass, expectedClass)) {
159-
holder.registerProblem(source, String.format("Class needs to implement '%s' for tag '%s'", expectedClass, tag), ProblemHighlightType.WEAK_WARNING);
177+
if (missingTagInstance != null) {
178+
holder.registerProblem(source, String.format("Class needs to implement '%s' for tag '%s'", StringUtils.stripStart(missingTagInstance, "\\"), tag), ProblemHighlightType.WEAK_WARNING);
160179
}
180+
}
181+
}
161182

183+
/**
184+
* <service class="Foo\\Bar" id="required_attribute">
185+
* <service id="Foo\\Bar" />
186+
*/
187+
@Nullable
188+
private static String getClassNameFromServiceDefinition(@NotNull PsiElement element) {
189+
if (XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) {
190+
// <service class="Foo\\Bar" id="required_attribute">
191+
String text = PsiElementUtils.trimQuote(element.getText());
192+
if (StringUtils.isNotBlank(text)) {
193+
return text;
194+
}
195+
} else if (XmlHelper.getServiceIdAttributePattern().accepts(element)) {
196+
// <service id="Foo\\Bar" />
197+
String text = PsiElementUtils.trimQuote(element.getText());
198+
if (StringUtils.isNotBlank(text) && YamlHelper.isClassServiceId(text) && text.contains("\\")) {
199+
return text;
200+
}
162201
}
163202

203+
return null;
164204
}
165-
166205
}

‎src/main/java/fr/adrienbrault/idea/symfony2plugin/util/dict/ServiceUtil.java‎

Lines changed: 43 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -48,53 +48,53 @@
4848
* @author Daniel Espendiller <[email protected]>
4949
*/
5050
public class ServiceUtil {
51-
private static ServiceNameStrategyInterface[] NAME_STRATEGIES = new ServiceNameStrategyInterface[] {
51+
private static final ServiceNameStrategyInterface[] NAME_STRATEGIES = new ServiceNameStrategyInterface[] {
5252
new JavascriptServiceNameStrategy(),
5353
new DefaultServiceNameStrategy(),
5454
};
5555

56-
public static final Map<String , String> TAG_INTERFACES = new HashMap<String , String>() {{
57-
put("assetic.asset", "\\Assetic\\Filter\\FilterInterface");
58-
put("assetic.factory_worker", "\\Assetic\\Factory\\Worker\\WorkerInterface");
59-
put("assetic.filter", "\\Assetic\\Filter\\FilterInterface");
60-
put("assetic.formula_loader", "\\Assetic\\Factory\\Loader\\FormulaLoaderInterface");
61-
put("assetic.formula_resource", null);
62-
put("assetic.templating.php", null);
63-
put("assetic.templating.twig", null);
64-
put("console.command", "\\Symfony\\Component\\Console\\Command\\Command");
65-
put("data_collector", "\\Symfony\\Component\\HttpKernel\\DataCollector\\DataCollectorInterface");
66-
put("doctrine.event_listener", null);
67-
put("doctrine.event_subscriber", null);
68-
put("form.type", "\\Symfony\\Component\\Form\\FormTypeInterface");
69-
put("form.type_extension", "\\Symfony\\Component\\Form\\FormTypeExtensionInterface");
70-
put("form.type_guesser", "\\Symfony\\Component\\Form\\FormTypeGuesserInterface");
71-
put("kernel.cache_clearer", null);
72-
put("kernel.cache_warmer", "\\Symfony\\Component\\HttpKernel\\CacheWarmer\\CacheWarmerInterface");
73-
put("kernel.event_subscriber", "\\Symfony\\Component\\EventDispatcher\\EventSubscriberInterface");
74-
put("kernel.fragment_renderer", "\\Symfony\\Component\\HttpKernel\\Fragment\\FragmentRendererInterface");
75-
put("monolog.logger", null);
76-
put("monolog.processor", null);
77-
put("routing.loader", "\\Symfony\\Component\\Config\\Loader\\LoaderInterface");
56+
public static final Map<String , String[]> TAG_INTERFACES = new HashMap<>() {{
57+
put("assetic.asset", new String[]{"\\Assetic\\Filter\\FilterInterface"});
58+
put("assetic.factory_worker", new String[]{"\\Assetic\\Factory\\Worker\\WorkerInterface"});
59+
put("assetic.filter", new String[]{"\\Assetic\\Filter\\FilterInterface"});
60+
put("assetic.formula_loader", new String[]{"\\Assetic\\Factory\\Loader\\FormulaLoaderInterface"});
61+
put("assetic.formula_resource", new String[] {});
62+
put("assetic.templating.php", new String[] {});
63+
put("assetic.templating.twig", new String[] {});
64+
put("console.command", new String[]{"\\Symfony\\Component\\Console\\Command\\Command"});
65+
put("data_collector", new String[]{"\\Symfony\\Component\\HttpKernel\\DataCollector\\DataCollectorInterface"});
66+
put("doctrine.event_listener", new String[] {});
67+
put("doctrine.event_subscriber", new String[] {});
68+
put("form.type", new String[]{"\\Symfony\\Component\\Form\\FormTypeInterface"});
69+
put("form.type_extension", new String[]{"\\Symfony\\Component\\Form\\FormTypeExtensionInterface"});
70+
put("form.type_guesser", new String[]{"\\Symfony\\Component\\Form\\FormTypeGuesserInterface"});
71+
put("kernel.cache_clearer", new String[] {});
72+
put("kernel.cache_warmer", new String[]{"\\Symfony\\Component\\HttpKernel\\CacheWarmer\\CacheWarmerInterface"});
73+
put("kernel.event_subscriber", new String[]{"\\Symfony\\Component\\EventDispatcher\\EventSubscriberInterface"});
74+
put("kernel.fragment_renderer", new String[]{"\\Symfony\\Component\\HttpKernel\\Fragment\\FragmentRendererInterface"});
75+
put("monolog.logger", new String[] {});
76+
put("monolog.processor", new String[] {});
77+
put("routing.loader", new String[]{"\\Symfony\\Component\\Config\\Loader\\LoaderInterface"});
7878
//put("security.remember_me_aware", null);
79-
put("security.voter", "\\Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface");
80-
put("serializer.encoder", "\\Symfony\\Component\\Serializer\\Encoder\\EncoderInterface");
81-
put("serializer.normalizer", "\\Symfony\\Component\\Serializer\\Normalizer\\NormalizerInterface");
79+
put("security.voter", new String[]{"\\Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface"});
80+
put("serializer.encoder", new String[]{"\\Symfony\\Component\\Serializer\\Encoder\\EncoderInterface"});
81+
put("serializer.normalizer", new String[]{"\\Symfony\\Component\\Serializer\\Normalizer\\NormalizerInterface"});
8282
// Symfony\Component\Serializer\Normalizer\DenormalizerInterface
83-
put("swiftmailer.default.plugin", "\\Swift_Events_EventListener");
84-
put("templating.helper", "\\Symfony\\Component\\Templating\\Helper\\HelperInterface");
85-
put("translation.loader", "\\Symfony\\Component\\Translation\\Loader\\LoaderInterface");
86-
put("translation.extractor", "\\Symfony\\Component\\Translation\\Extractor\\ExtractorInterface");
87-
put("translation.dumper", "\\Symfony\\Component\\Translation\\Dumper\\DumperInterface");
88-
put("twig.extension", "\\Twig_ExtensionInterface");
89-
put("twig.loader", "\\Twig_LoaderInterface");
90-
put("validator.constraint_validator", "Symfony\\Component\\Validator\\ConstraintValidator");
91-
put("validator.initializer", "Symfony\\Component\\Validator\\ObjectInitializerInterface");
83+
put("swiftmailer.default.plugin", new String[]{"\\Swift_Events_EventListener"});
84+
put("templating.helper", new String[]{"\\Symfony\\Component\\Templating\\Helper\\HelperInterface"});
85+
put("translation.loader", new String[]{"\\Symfony\\Component\\Translation\\Loader\\LoaderInterface"});
86+
put("translation.extractor", new String[]{"\\Symfony\\Component\\Translation\\Extractor\\ExtractorInterface"});
87+
put("translation.dumper", new String[]{"\\Symfony\\Component\\Translation\\Dumper\\DumperInterface"});
88+
put("twig.extension", new String[]{"\\Twig\\Extension\\ExtensionInterface", "\\Twig_ExtensionInterface"});
89+
put("twig.loader", new String[]{"\\Twig\\Loader\\LoaderInterface", "\\Twig_LoaderInterface"});
90+
put("validator.constraint_validator", new String[]{"Symfony\\Component\\Validator\\ConstraintValidator"});
91+
put("validator.initializer", new String[]{"Symfony\\Component\\Validator\\ObjectInitializerInterface"});
9292

9393
// 2.6 - @TODO: how to handle duplicate interfaces; also make them weaker
94-
put("routing.expression_language_provider", "\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface");
95-
put("security.expression_language_provider", "\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface");
94+
put("routing.expression_language_provider", new String[]{"\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface"});
95+
put("security.expression_language_provider", new String[]{"\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface"});
9696

97-
put("controller.service_arguments", null);
97+
put("controller.service_arguments", new String[] {});
9898
}};
9999

100100
/**
@@ -427,19 +427,14 @@ public static PhpClass getServiceClass(@NotNull Project project, @NotNull String
427427
*/
428428
@NotNull
429429
public static Set<String> getPhpClassServiceTags(@NotNull PhpClass phpClass) {
430-
431430
Set<String> tags = new HashSet<>();
432431

433-
for (Map.Entry<String, String> entry : TAG_INTERFACES.entrySet()) {
434-
435-
if(entry.getValue() == null) {
436-
continue;
437-
}
438-
439-
if(PhpElementsUtil.isInstanceOf(phpClass, entry.getValue())) {
440-
tags.add(entry.getKey());
432+
for (Map.Entry<String, String[]> entry : TAG_INTERFACES.entrySet()) {
433+
for (String s : entry.getValue()) {
434+
if(PhpElementsUtil.isInstanceOf(phpClass, s)) {
435+
tags.add(entry.getKey());
436+
}
441437
}
442-
443438
}
444439

445440
// strong tags wins

‎src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/TaggedExtendsInterfaceClassInspectionTest.java‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,52 @@ public String getTestDataPath() {
1919
return "src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/fixtures";
2020
}
2121

22-
public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementations() {
22+
public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsOfYaml() {
2323
assertLocalInspectionContains("services.yml", "services:\n" +
2424
" foo:\n" +
2525
" class: Tag\\Instance<caret>Check\\EmptyClass\n" +
2626
" tags:\n" +
2727
" - { name: twig.extension }",
28-
"Class needs to implement '\\Twig_ExtensionInterface' for tag 'twig.extension'"
28+
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
2929
);
3030
}
3131

32+
public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsForClassAsIsOfYaml() {
33+
assertLocalInspectionContains("services.yml", "services:\n" +
34+
" Tag\\Instance<caret>Check\\EmptyClass:\n" +
35+
" tags:\n" +
36+
" - { name: twig.extension }",
37+
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
38+
);
39+
}
40+
41+
public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsOfXml() {
42+
assertLocalInspectionContains(
43+
"services.xml",
44+
"<?xml version=\"1.0\"?>\n" +
45+
"<container>\n" +
46+
" <services>\n" +
47+
" <service id=\"test\" class=\"Tag\\Instance<caret>Check\\EmptyClass\">\n" +
48+
" <tag name=\"twig.extension\"/>" +
49+
" </service>\n" +
50+
" </services>\n" +
51+
"</container>\n",
52+
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
53+
);
54+
}
55+
56+
public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsForClassAsIsOfYamlOfYml() {
57+
assertLocalInspectionContains(
58+
"services.xml",
59+
"<?xml version=\"1.0\"?>\n" +
60+
"<container>\n" +
61+
" <services>\n" +
62+
" <service id=\"Tag\\Instance<caret>Check\\EmptyClass\">\n" +
63+
" <tag name=\"twig.extension\"/>" +
64+
" </service>\n" +
65+
" </services>\n" +
66+
"</container>\n",
67+
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
68+
);
69+
}
3270
}

0 commit comments

Comments
 (0)