Skip to content

#1548 optimize inspection for notify missing extends/implements tags … #1917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2022
Merged

#1548 optimize inspection for notify missing extends/implements tags … #1917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...brault/idea/symfony2plugin/codeInspection/service/ServiceDeprecatedClassesInspection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
113 changes: 76 additions & 37 deletions ...ult/idea/symfony2plugin/codeInspection/service/TaggedExtendsInterfaceClassInspection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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);
}
}
}
Expand All @@ -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;
}

Expand All @@ -116,51 +116,90 @@ public void visitElement(PsiElement element) {
if(serviceKeyValue instanceof YAMLKeyValue) {
Set<String> 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<String> 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<String> 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);
}
}
}

/**
* <service class="Foo\\Bar" id="required_attribute">
* <service id="Foo\\Bar" />
*/
@Nullable
private static String getClassNameFromServiceDefinition(@NotNull PsiElement element) {
if (XmlHelper.getServiceClassAttributeWithIdPattern().accepts(element)) {
// <service class="Foo\\Bar" id="required_attribute">
String text = PsiElementUtils.trimQuote(element.getText());
if (StringUtils.isNotBlank(text)) {
return text;
}
} else if (XmlHelper.getServiceIdAttributePattern().accepts(element)) {
// <service id="Foo\\Bar" />
String text = PsiElementUtils.trimQuote(element.getText());
if (StringUtils.isNotBlank(text) && YamlHelper.isClassServiceId(text) && text.contains("\\")) {
return text;
}
}

return null;
}

}
91 changes: 43 additions & 48 deletions src/main/java/fr/adrienbrault/idea/symfony2plugin/util/dict/ServiceUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,53 +48,53 @@
* @author Daniel Espendiller <[email protected]>
*/
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<String , String> TAG_INTERFACES = new HashMap<String , String>() {{
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<String , String[]> 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[] {});
}};

/**
Expand Down Expand Up @@ -427,19 +427,14 @@ public static PhpClass getServiceClass(@NotNull Project project, @NotNull String
*/
@NotNull
public static Set<String> getPhpClassServiceTags(@NotNull PhpClass phpClass) {

Set<String> tags = new HashSet<>();

for (Map.Entry<String, String> entry : TAG_INTERFACES.entrySet()) {

if(entry.getValue() == null) {
continue;
}

if(PhpElementsUtil.isInstanceOf(phpClass, entry.getValue())) {
tags.add(entry.getKey());
for (Map.Entry<String, String[]> entry : TAG_INTERFACES.entrySet()) {
for (String s : entry.getValue()) {
if(PhpElementsUtil.isInstanceOf(phpClass, s)) {
tags.add(entry.getKey());
}
}

}

// strong tags wins
Expand Down
42 changes: 40 additions & 2 deletions ...ymfony2plugin/tests/codeInspection/service/TaggedExtendsInterfaceClassInspectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\\Instance<caret>Check\\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\\Instance<caret>Check\\EmptyClass:\n" +
" tags:\n" +
" - { name: twig.extension }",
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
);
}

public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsOfXml() {
assertLocalInspectionContains(
"services.xml",
"<?xml version=\"1.0\"?>\n" +
"<container>\n" +
" <services>\n" +
" <service id=\"test\" class=\"Tag\\Instance<caret>Check\\EmptyClass\">\n" +
" <tag name=\"twig.extension\"/>" +
" </service>\n" +
" </services>\n" +
"</container>\n",
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
);
}

public void testThatKnownTagsShouldInspectionForMissingServiceClassImplementationsForClassAsIsOfYamlOfYml() {
assertLocalInspectionContains(
"services.xml",
"<?xml version=\"1.0\"?>\n" +
"<container>\n" +
" <services>\n" +
" <service id=\"Tag\\Instance<caret>Check\\EmptyClass\">\n" +
" <tag name=\"twig.extension\"/>" +
" </service>\n" +
" </services>\n" +
"</container>\n",
"Class needs to implement 'Twig_ExtensionInterface' for tag 'twig.extension'"
);
}
}
Loading