8000 [DependencyInjection] Make `DefinitionErrorExceptionPass` consider `IGNORE_ON_UNINITIALIZED_REFERENCE` and `RUNTIME_EXCEPTION_ON_INVALID_REFERENCE` the same by MatTheCat · Pull Request #60423 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DependencyInjection] Make DefinitionErrorExceptionPass consider IGNORE_ON_UNINITIALIZED_REFERENCE and RUNTIME_EXCEPTION_ON_INVALID_REFERENCE the same #60423

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 1 commit into from
May 17, 2025

Conversation

MatTheCat
Copy link
Contributor
@MatTheCat MatTheCat commented May 14, 2025
Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #54570
License MIT

The test container can make runtime errors appear at compile-time: say you have a private definition referenced by a controller. Such a reference would be configured with RUNTIME_EXCEPTION_ON_INVALID_REFERENCE, so if the private definition references an errored one, you’d get no exception at compile-time.

Now, if this private definition is also referenced by the test container, it would be as IGNORE_ON_UNINITIALIZED_REFERENCE. This would change DefinitionErrorExceptionPass::isErrorForRuntime() result and trigger a compile-time exception.

Following #60423 (comment), this PR makes the DefinitionErrorExceptionPass consider IGNORE_ON_UNINITIALIZED_REFERENCE the same way than RUNTIME_EXCEPTION_ON_INVALID_REFERENCE to fix this issue.

@nicolas-grekas
Copy link
Member

Would it make sense to say that if IGNORE_ON_UNINITIALIZED_REFERENCE is used, then RUNTIME_EXCEPTION_ON_INVALID_REFERENCE is implied? Would that fix the issue?

@MatTheCat
Copy link
Contributor Author

Don’t know if it makes sense but it would also solve the issue yes.

@nicolas-grekas
Copy link
Member

It would to me yes!

@MatTheCat MatTheCat changed the title [DependencyInjection] Make the DefinitionErrorExceptionPass ignore the test container [DependencyInjection] Make DefinitionErrorExceptionPass consider IGNORE_ON_UNINITIALIZED_REFERENCE and RUNTIME_EXCEPTION_ON_INVALID_REFERENCE the same May 16, 2025
@nicolas-grekas
Copy link
Member

It'd be great to have the test case in the DI component instead. That'd solve the deps=low failure also.

…GNORE_ON_UNINITIALIZED_REFERENCE` and `RUNTIME_EXCEPTION_ON_INVALID_REFERENCE` the same
@MatTheCat
Copy link
Contributor Author

Tests updated; failures are now unrelated!

@fabpot
Copy link
Member
fabpot commented May 17, 2025

Thank you @MatTheCat.

@fabpot fabpot merged commit 334bd4f into symfony:6.4 May 17, 2025
9 of 11 checks passed
@MatTheCat MatTheCat deleted the ticket_54570 branch May 17, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0