8000 [RFC] Check types validity in the service graph at compile time - for currently loaded classes only · Issue #27744 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[RFC] Check types validity in the service graph at compile time - for currently loaded classes only #27744

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

Jump to bottom
Closed
nicolas-grekas opened this issue Jun 27, 2018 · 3 comments
Closed

[RFC] Check types validity in the service graph at compile time - for currently loaded classes only #27744

nicolas-grekas opened this issue Jun 27, 2018 · 3 comments
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 27, 2018 •
edited
Loading

While wiring services, nothing prevents one from doing mistakes and break type hints, e.g. injecting a Foo object into an incompatible BarInterface-hinted argument.

When this happens, the error is discovered at runtime. A common practice to detect this before prod is to create a smoke test that instantiates all services and does nothing else. This should work even better with the new test container in 4.1. Maybe something we could add to the MakerBundle? Of course, this strategy doesn't work for lazy services / locators / iterators. Not sure about false-positives also.

We're not verifying these references at compile-time because we do not want to load all PHP classes, as doing so takes time. Also because doing the verification itself might slow down the compilation too much and hurt DX.

Yet, I think we could still have a compiler pass that does these checks. In order to make it affordable, it would have a mode where it would check references only for currently loaded classes/interfaces, using class|interface_exists() with false as 2nd argument, and maybe opcache_is_script_cached() also.

If the overhead of checking only the types (but not loading the code) is low enough, we could enable this pass by default in the dev env.

Worth a try?
@xabbuh xabbuh added DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Jun 27, 2018
@nicolas-grekas nicolas-grekas added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Jun 27, 2018
@linaori
Copy link
Contributor
linaori commented Jun 27, 2018

Will my build process be slower? Yes. Does it guarantee a more stable run-time or even stop the build process and let me know I made a mistake? For sure!

I think this would be worth implementing.

@nicolas-grekas
Copy link
Member Author
nicolas-grekas commented Jun 27, 2018 •
edited
Loading

I don't know if that's possible, but we could also try running this pass in a dedicated test case!

alcalyn added a commit to alcalyn/symfony that referenced this issue Jun 29, 2018
@alcalyn
[symfony#27744] Add compiler pass to check arguments type hint
62b5042
alcalyn added a commit to alcalyn/symfony that referenced this issue Jul 3, 2018
@alcalyn
[symfony#27744] Add compiler pass to check arguments type hint
9ff1945
alcalyn added a commit to alcalyn/symfony that referenced this issue Jul 3, 2018
@alcalyn
[symfony#27744] Add compiler pass to check arguments type hint
ee016ec
@alcalyn alcalyn mentioned this issue Jul 3, 2018
Closed
alcalyn added a commit to alcalyn/symfony that referenced this issue Jul 4, 2018
@alcalyn
[symfony#27744] Add compiler pass to check arguments type hint
b563760
alcalyn added a commit to alcalyn/symfony that referenced this issue Jul 9, 2018
@alcalyn
[symfony#27744] Add compiler pass to check arguments type hint
7a48a03
alcalyn added a commit to alcalyn/symfony that referenced this issue Jul 9, 2018
@alcalyn
[symfony#27744] Add compiler pass to check arguments type hint
4b59201
@Tobion
Copy link
Contributor
Tobion commented Jun 5, 2019

Why not a new command that does this and that people can add to their CI?

GuilhemN pushed a commit to GuilhemN/symfony that referenced this issue Jun 28, 2019
@alcalyn @GuilhemN
[symfony#27744] Add compiler pass to check arguments type hint
4c4ef9b
@GuilhemN GuilhemN mentioned this issue Jun 28, 2019
Merged
GuilhemN pushed a commit to GuilhemN/symfony that referenced this issue Jun 28, 2019
@alcalyn @GuilhemN
[symfony#27744] Add compiler pass to check arguments type hint
a21e408
GuilhemN pushed a commit to GuilhemN/symfony that referenced this issue Oct 2, 2019
@alcalyn @GuilhemN
[symfony#27744] Add compiler pass to check arguments type hint
3cd3c99
nicolas-grekas pushed a commit that referenced this issue Oct 2, 2019
@alcalyn @nicolas-grekas
[#27744] Add compiler pass to check arguments type hint
a020bc8
nicolas-grekas added a commit that referenced this issue Nov 4, 2019
@nicolas-grekas
feature #32256 [DI] Add compiler pass and command to check that servi…
29fd51f 
…ces wiring matches type declarations (alcalyn, GuilhemN, nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[DI] Add compiler pass and command to check that services wiring matches type declarations

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27744
| License       | MIT
| Doc PR        |

PR replacing #27825.

It adds a `lint:container` command asserting the type hints used in your code are correct.

Commits
-------

8230a15 Make it really work on real apps
4b3e9d4 Fix comments, improve the feature
a6292b9 [DI] Add compiler pass to check arguments type hint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-grekas @Tobion @linaori @xabbuh
0