diff --git a/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/ServiceDeprecatedClassesInspection.java b/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/ServiceDeprecatedClassesInspection.java index d903d7130..d535fc3de 100644 --- a/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/ServiceDeprecatedClassesInspection.java +++ b/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/ServiceDeprecatedClassesInspection.java @@ -98,7 +98,7 @@ public XmlClassElementWalkingVisitor(ProblemsHolder holder, ProblemRegistrar pro } @Override - public void visitElement(PsiElement element) { + public void visitElement(@NotNull PsiElement element) { boolean serviceArgumentAccepted = XmlHelper.getArgumentServiceIdPattern().accepts(element); if(serviceArgumentAccepted || XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) { String text = PsiElementUtils.trimQuote(element.getText()); diff --git a/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/TaggedExtendsInterfaceClassInspection.java b/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/TaggedExtendsInterfaceClassInspection.java index 0e6404ad9..6de500880 100644 --- a/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/TaggedExtendsInterfaceClassInspection.java +++ b/src/main/java/fr/adrienbrault/idea/symfony2plugin/codeInspection/service/TaggedExtendsInterfaceClassInspection.java @@ -22,6 +22,8 @@ import fr.adrienbrault.idea.symfony2plugin.util.yaml.YamlHelper; import org.apache.commons.lang.StringUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.yaml.YAMLTokenTypes; import org.jetbrains.yaml.psi.YAMLCompoundValue; import org.jetbrains.yaml.psi.YAMLFile; import org.jetbrains.yaml.psi.YAMLKeyValue; @@ -43,7 +45,7 @@ public PsiElementVisitor buildVisitor(final @NotNull ProblemsHolder holder, bool return new PsiElementVisitor() { @Override - public void visitFile(PsiFile psiFile) { + public void visitFile(@NotNull PsiFile psiFile) { if(psiFile instanceof YAMLFile) { psiFile.acceptChildren(new YmlClassElementWalkingVisitor(holder, new ContainerCollectionResolver.LazyServiceCollector(holder.getProject()))); } else if(psiFile instanceof XmlFile) { @@ -63,16 +65,15 @@ public XmlClassElementWalkingVisitor(ProblemsHolder holder, ContainerCollectionR } @Override - public void visitElement(PsiElement element) { - if(XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) { - String text = PsiElementUtils.trimQuote(element.getText()); - PsiElement[] psiElements = element.getChildren(); - - // attach problems to string value only - if(StringUtils.isNotBlank(text) && psiElements.length > 2) { - XmlTag parentOfType = PsiTreeUtil.getParentOfType(element, XmlTag.class); - if(parentOfType != null) { - registerTaggedProblems(psiElements[1], FormUtil.getTags(parentOfType), text, holder, this.lazyServiceCollector); + public void visitElement(@NotNull PsiElement element) { + String className = getClassNameFromServiceDefinition(element); + if (className != null) { + XmlTag parentOfType = PsiTreeUtil.getParentOfType(element, XmlTag.class); + if(parentOfType != null) { + // attach problems to string value only + PsiElement[] psiElements = element.getChildren(); + if (psiElements.length > 2) { + registerTaggedProblems(psiElements[1], FormUtil.getTags(parentOfType), className, holder, this.lazyServiceCollector); } } } @@ -91,20 +92,19 @@ public YmlClassElementWalkingVisitor(ProblemsHolder holder, ContainerCollectionR } @Override - public void visitElement(PsiElement element) { - - if(YamlElementPatternHelper.getSingleLineScalarKey("class").accepts(element)) { + public void visitElement(@NotNull PsiElement psiElement) { + if(YamlElementPatternHelper.getSingleLineScalarKey("class").accepts(psiElement)) { // class: '\Foo' - String text = PsiElementUtils.trimQuote(element.getText()); + String text = PsiElementUtils.trimQuote(psiElement.getText()); if(StringUtils.isBlank(text)) { - super.visitElement(element); + super.visitElement(psiElement); return; } - PsiElement yamlScalar = element.getParent(); + PsiElement yamlScalar = psiElement.getParent(); if(!(yamlScalar instanceof YAMLScalar)) { - super.visitElement(element); + super.visitElement(psiElement); return; } @@ -116,51 +116,90 @@ public void visitElement(PsiElement element) { if(serviceKeyValue instanceof YAMLKeyValue) { Set tags = YamlHelper.collectServiceTags((YAMLKeyValue) serviceKeyValue); if(tags.size() > 0) { - registerTaggedProblems(element, tags, text, holder, this.lazyServiceCollector); + registerTaggedProblems(psiElement, tags, text, holder, this.lazyServiceCollector); } } } } + } else if (psiElement.getNode().getElementType() == YAMLTokenTypes.SCALAR_KEY && YamlElementPatternHelper.getServiceIdKeyValuePattern().accepts(psiElement.getParent())) { + // Foobar\Foo: ~ + String text = PsiElementUtils.getText(psiElement); + if (StringUtils.isNotBlank(text) && YamlHelper.isClassServiceId(text) && text.contains("\\")) { + PsiElement yamlKeyValue = psiElement.getParent(); + if (yamlKeyValue instanceof YAMLKeyValue && YamlHelper.getYamlKeyValue((YAMLKeyValue) yamlKeyValue, "resource") == null && YamlHelper.getYamlKeyValue((YAMLKeyValue) yamlKeyValue, "exclude") == null) { + Set tags = YamlHelper.collectServiceTags((YAMLKeyValue) yamlKeyValue); + if(tags.size() > 0) { + registerTaggedProblems(psiElement, tags, text, holder, this.lazyServiceCollector); + } + } + } } - super.visitElement(element); + super.visitElement(psiElement); } } private void registerTaggedProblems(@NotNull PsiElement source, @NotNull Set tags, @NotNull String serviceClass, @NotNull ProblemsHolder holder, @NotNull ContainerCollectionResolver.LazyServiceCollector lazyServiceCollector) { - - if(tags.size() == 0) { + if (tags.size() == 0) { return; } PhpClass phpClass = null; for (String tag : tags) { + String missingTagInstance = null; - if(!ServiceUtil.TAG_INTERFACES.containsKey(tag)) { - continue; - } + for (String expectedClass : ServiceUtil.TAG_INTERFACES.getOrDefault(tag, new String[] {})) { + // load PhpClass only if we need it, on error exit + if(phpClass == null) { + phpClass = ServiceUtil.getResolvedClassDefinition(holder.getProject(), serviceClass, lazyServiceCollector); + if(phpClass == null) { + return; + } + } - String expectedClass = ServiceUtil.TAG_INTERFACES.get(tag); - if(expectedClass == null) { - continue; - } + // skip unknown classes + if (PhpElementsUtil.getClassesInterface(phpClass.getProject(), expectedClass).isEmpty()) { + continue; + } - // load PhpClass only if we need it, on error exit - if(phpClass == null) { - phpClass = ServiceUtil.getResolvedClassDefinition(holder.getProject(), serviceClass, lazyServiceCollector); - if(phpClass == null) { - return; + // check interfaces + if(!PhpElementsUtil.isInstanceOf(phpClass, expectedClass)) { + missingTagInstance = expectedClass; + continue; } + + missingTagInstance = null; + break; } // check interfaces - if(!PhpElementsUtil.isInstanceOf(phpClass, expectedClass)) { - holder.registerProblem(source, String.format("Class needs to implement '%s' for tag '%s'", expectedClass, tag), ProblemHighlightType.WEAK_WARNING); + if (missingTagInstance != null) { + holder.registerProblem(source, String.format("Class needs to implement '%s' for tag '%s'", StringUtils.stripStart(missingTagInstance, "\\"), tag), ProblemHighlightType.WEAK_WARNING); } + } + } + /** + * + * + */ + @Nullable + private static String getClassNameFromServiceDefinition(@NotNull PsiElement element) { + if (XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) { + // + String text = PsiElementUtils.trimQuote(element.getText()); + if (StringUtils.isNotBlank(text)) { + return text; + } + } else if (XmlHelper.getServiceIdAttributePattern().accepts(element)) { + // + String text = PsiElementUtils.trimQuote(element.getText()); + if (StringUtils.isNotBlank(text) && YamlHelper.isClassServiceId(text) && text.contains("\\")) { + return text; + } } + return null; } - } diff --git a/src/main/java/fr/adrienbrault/idea/symfony2plugin/util/dict/ServiceUtil.java b/src/main/java/fr/adrienbrault/idea/symfony2plugin/util/dict/ServiceUtil.java index 64f363ac1..916f13dfa 100644 --- a/src/main/java/fr/adrienbrault/idea/symfony2plugin/util/dict/ServiceUtil.java +++ b/src/main/java/fr/adrienbrault/idea/symfony2plugin/util/dict/ServiceUtil.java @@ -48,53 +48,53 @@ * @author Daniel Espendiller */ public class ServiceUtil { - private static ServiceNameStrategyInterface[] NAME_STRATEGIES = new ServiceNameStrategyInterface[] { + private static final ServiceNameStrategyInterface[] NAME_STRATEGIES = new ServiceNameStrategyInterface[] { new JavascriptServiceNameStrategy(), new DefaultServiceNameStrategy(), }; - public static final Map TAG_INTERFACES = new HashMap() {{ - put("assetic.asset", "\\Assetic\\Filter\\FilterInterface"); - put("assetic.factory_worker", "\\Assetic\\Factory\\Worker\\WorkerInterface"); - put("assetic.filter", "\\Assetic\\Filter\\FilterInterface"); - put("assetic.formula_loader", "\\Assetic\\Factory\\Loader\\FormulaLoaderInterface"); - put("assetic.formula_resource", null); - put("assetic.templating.php", null); - put("assetic.templating.twig", null); - put("console.command", "\\Symfony\\Component\\Console\\Command\\Command"); - put("data_collector", "\\Symfony\\Component\\HttpKernel\\DataCollector\\DataCollectorInterface"); - put("doctrine.event_listener", null); - put("doctrine.event_subscriber", null); - put("form.type", "\\Symfony\\Component\\Form\\FormTypeInterface"); - put("form.type_extension", "\\Symfony\\Component\\Form\\FormTypeExtensionInterface"); - put("form.type_guesser", "\\Symfony\\Component\\Form\\FormTypeGuesserInterface"); - put("kernel.cache_clearer", null); - put("kernel.cache_warmer", "\\Symfony\\Component\\HttpKernel\\CacheWarmer\\CacheWarmerInterface"); - put("kernel.event_subscriber", "\\Symfony\\Component\\EventDispatcher\\EventSubscriberInterface"); - put("kernel.fragment_renderer", "\\Symfony\\Component\\HttpKernel\\Fragment\\FragmentRendererInterface"); - put("monolog.logger", null); - put("monolog.processor", null); - put("routing.loader", "\\Symfony\\Component\\Config\\Loader\\LoaderInterface"); + public static final Map TAG_INTERFACES = new HashMap<>() {{ + put("assetic.asset", new String[]{"\\Assetic\\Filter\\FilterInterface"}); + put("assetic.factory_worker", new String[]{"\\Assetic\\Factory\\Worker\\WorkerInterface"}); + put("assetic.filter", new String[]{"\\Assetic\\Filter\\FilterInterface"}); + put("assetic.formula_loader", new String[]{"\\Assetic\\Factory\\Loader\\FormulaLoaderInterface"}); + put("assetic.formula_resource", new String[] {}); + put("assetic.templating.php", new String[] {}); + put("assetic.templating.twig", new String[] {}); + put("console.command", new String[]{"\\Symfony\\Component\\Console\\Command\\Command"}); + put("data_collector", new String[]{"\\Symfony\\Component\\HttpKernel\\DataCollector\\DataCollectorInterface"}); + put("doctrine.event_listener", new String[] {}); + put("doctrine.event_subscriber", new String[] {}); + put("form.type", new String[]{"\\Symfony\\Component\\Form\\FormTypeInterface"}); + put("form.type_extension", new String[]{"\\Symfony\\Component\\Form\\FormTypeExtensionInterface"}); + put("form.type_guesser", new String[]{"\\Symfony\\Component\\Form\\FormTypeGuesserInterface"}); + put("kernel.cache_clearer", new String[] {}); + put("kernel.cache_warmer", new String[]{"\\Symfony\\Component\\HttpKernel\\CacheWarmer\\CacheWarmerInterface"}); + put("kernel.event_subscriber", new String[]{"\\Symfony\\Component\\EventDispatcher\\EventSubscriberInterface"}); + put("kernel.fragment_renderer", new String[]{"\\Symfony\\Component\\HttpKernel\\Fragment\\FragmentRendererInterface"}); + put("monolog.logger", new String[] {}); + put("monolog.processor", new String[] {}); + put("routing.loader", new String[]{"\\Symfony\\Component\\Config\\Loader\\LoaderInterface"}); //put("security.remember_me_aware", null); - put("security.voter", "\\Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface"); - put("serializer.encoder", "\\Symfony\\Component\\Serializer\\Encoder\\EncoderInterface"); - put("serializer.normalizer", "\\Symfony\\Component\\Serializer\\Normalizer\\NormalizerInterface"); + put("security.voter", new String[]{"\\Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface"}); + put("serializer.encoder", new String[]{"\\Symfony\\Component\\Serializer\\Encoder\\EncoderInterface"}); + put("serializer.normalizer", new String[]{"\\Symfony\\Component\\Serializer\\Normalizer\\NormalizerInterface"}); // Symfony\Component\Serializer\Normalizer\DenormalizerInterface - put("swiftmailer.default.plugin", "\\Swift_Events_EventListener"); - put("templating.helper", "\\Symfony\\Component\\Templating\\Helper\\HelperInterface"); - put("translation.loader", "\\Symfony\\Component\\Translation\\Loader\\LoaderInterface"); - put("translation.extractor", "\\Symfony\\Component\\Translation\\Extractor\\ExtractorInterface"); - put("translation.dumper", "\\Symfony\\Component\\Translation\\Dumper\\DumperInterface"); - put("twig.extension", "\\Twig_ExtensionInterface"); - put("twig.loader", "\\Twig_LoaderInterface"); - put("validator.constraint_validator", "Symfony\\Component\\Validator\\ConstraintValidator"); - put("validator.initializer", "Symfony\\Component\\Validator\\ObjectInitializerInterface"); + put("swiftmailer.default.plugin", new String[]{"\\Swift_Events_EventListener"}); + put("templating.helper", new String[]{"\\Symfony\\Component\\Templating\\Helper\\HelperInterface"}); + put("translation.loader", new String[]{"\\Symfony\\Component\\Translation\\Loader\\LoaderInterface"}); + put("translation.extractor", new String[]{"\\Symfony\\Component\\Translation\\Extractor\\ExtractorInterface"}); + put("translation.dumper", new String[]{"\\Symfony\\Component\\Translation\\Dumper\\DumperInterface"}); + put("twig.extension", new String[]{"\\Twig\\Extension\\ExtensionInterface", "\\Twig_ExtensionInterface"}); + put("twig.loader", new String[]{"\\Twig\\Loader\\LoaderInterface", "\\Twig_LoaderInterface"}); + put("validator.constraint_validator", new String[]{"Symfony\\Component\\Validator\\ConstraintValidator"}); + put("validator.initializer", new String[]{"Symfony\\Component\\Validator\\ObjectInitializerInterface"}); // 2.6 - @TODO: how to handle duplicate interfaces; also make them weaker - put("routing.expression_language_provider", "\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface"); - put("security.expression_language_provider", "\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface"); + put("routing.expression_language_provider", new String[]{"\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface"}); + put("security.expression_language_provider", new String[]{"\\Symfony\\Component\\ExpressionLanguage\\ExpressionFunctionProviderInterface"}); - put("controller.service_arguments", null); + put("controller.service_arguments", new String[] {}); }}; /** @@ -427,19 +427,14 @@ public static PhpClass getServiceClass(@NotNull Project project, @NotNull String */ @NotNull public static Set getPhpClassServiceTags(@NotNull PhpClass phpClass) { - Set tags = new HashSet<>(); - for (Map.Entry entry : TAG_INTERFACES.entrySet()) { - - if(entry.getValue() == null) { - continue; - } - - if(PhpElementsUtil.isInstanceOf(phpClass, entry.getValue())) { - tags.add(entry.getKey()); + for (Map.Entry entry : TAG_INTERFACES.entrySet()) { + for (String s : entry.getValue()) { + if(PhpElementsUtil.isInstanceOf(phpClass, s)) { + tags.add(entry.getKey()); + } } - } // strong tags wins diff --git a/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/TaggedExtendsInterfaceClassInspectionTest.java b/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/TaggedExtendsInterfaceClassInspectionTest.java index 92e8b743a..fb513b703 100644 --- a/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/TaggedExtendsInterfaceClassInspectionTest.java +++ b/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/TaggedExtendsInterfaceClassInspectionTest.java @@ -19,14 +19,52 @@ public String getTestDataPath() { return "src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/fixtures"; } - public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementations() { + public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsOfYaml() { assertLocalInspectionContains("services.yml", "services:\n" + " foo:\n" + " class: Tag\\InstanceCheck\\EmptyClass\n" + " tags:\n" + " - { name: twig.extension }", - "Class needs to implement '\\Twig_ExtensionInterface' for tag 'twig.extension'" + "Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'" ); } + public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsForClassAsIsOfYaml() { + assertLocalInspectionContains("services.yml", "services:\n" + + " Tag\\InstanceCheck\\EmptyClass:\n" + + " tags:\n" + + " - { name: twig.extension }", + "Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'" + ); + } + + public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsOfXml() { + assertLocalInspectionContains( + "services.xml", + "\n" + + "\n" + + " \n" + + " Check\\EmptyClass\">\n" + + " " + + " \n" + + " \n" + + "\n", + "Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'" + ); + } + + public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsForClassAsIsOfYamlOfYml() { + assertLocalInspectionContains( + "services.xml", + "\n" + + "\n" + + " \n" + + " Check\\EmptyClass\">\n" + + " " + + " \n" + + " \n" + + "\n", + "Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'" + ); + } } diff --git a/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/fixtures/classes.php b/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/fixtures/classes.php index 79d749d7e..e4e0c7b9c 100644 --- a/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/fixtures/classes.php +++ b/src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/codeInspection/service/fixtures/classes.php @@ -19,6 +19,7 @@ function get(); namespace { class Twig_Extension {} + interface Twig_ExtensionInterface {} } namespace Tag\InstanceCheck