Skip to content

Added service name reference contributor for YAML DIC files #1919

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.

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 2 commits into from
May 9, 2022

Conversation

adamwojs
Copy link
Contributor

@adamwojs adamwojs commented May 7, 2022

This PR brings support for service references detection over the YAML based dependency injection container configuration files. In practice this allows to:

  • finding usages of given service
  • renaming service name with all references at once

image

References are detected in:

  • Aliases
  • Configurators
  • Constructor injection
  • Decorators
  • Factories
  • Method injection
  • Parent definition
  • Properties injection

@adamwojs adamwojs force-pushed the service_ref_provider branch from a2aeb5f to a861cb0 Compare May 7, 2022 10:11
@adamwojs adamwojs force-pushed the service_ref_provider branch 2 times, most recently from a94c5bb to f105c81 Compare May 8, 2022 14:19
Comment on lines +192 to +223
PlatformPatterns
.psiElement(YAMLScalar.class)
.withParent(
PlatformPatterns
.psiElement(YAMLSequenceItem.class)
.withParent(
PlatformPatterns
.psiElement(YAMLSequence.class)
.withParent(
PlatformPatterns
.psiElement(YAMLKeyValue.class)
.withParent(
PlatformPatterns
.psiElement(YAMLMapping.class)
.withParent(
PlatformPatterns
.psiElement(YAMLSequenceItem.class)
.withParent(
PlatformPatterns
.psiElement(YAMLSequence.class)
.withParent(
PlatformPatterns
.psiElement(YAMLKeyValue.class)
.withName("calls")
)
)
)
)

)
)
),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestion how this could be simplified will be more the welcome 🤔

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as long as its tested its fine, thats how its nested :)

i also have no idea to simplify this.

@adamwojs adamwojs force-pushed the service_ref_provider branch from c4e0d12 to 595899b Compare May 8, 2022 14:38
@adamwojs adamwojs marked this pull request as ready for review May 8, 2022 14:44
@adamwojs adamwojs force-pushed the service_ref_provider branch from 595899b to d7281c2 Compare May 8, 2022 14:53
@adamwojs
Copy link
Contributor Author

adamwojs commented May 8, 2022

PR is now ready for code review @Haehnchen 😉

assertReferenceMatchOnParent(
YAMLFileType.YML,
"services:\n" +
" app.service.bar:\n" +
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are keys not supported by references provider?

so that we can provide support for this also later:

App\\BarService: ~
App\\BarService:
  arguments:
    $test: "%foobar%"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for feedback!

are keys not supported by references provider?

No. I planned to work on this in separate PR. Work in progress branch: https://github.com/Haehnchen/idea-php-symfony2-plugin/compare/master...adamwojs:class_fqn_ref_provider?expand=1

Case when service name is FQCN is a little bit more tricky. What I would expect as Symfony user is that whenever class/namespace is renamed the service name and it's references are updated. This would be a huge time saver.

Good idea with parameters references 👍

@Haehnchen
Copy link
Owner

👍

@Haehnchen Haehnchen merged commit eb13d45 into Haehnchen:master May 9, 2022
@adamwojs adamwojs deleted the service_ref_provider branch May 9, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants