Skip to content
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

[FrameworkBundle] Removed eval() from KernelShutdownOnTearDownTrait #30364

Merged
merged 1 commit into from Feb 24, 2019

Conversation

skalpa
Copy link
Contributor

@skalpa skalpa commented Feb 24, 2019

Q A
Branch? 4.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

Apart from triggering the #30362 bug, the eval() block from #30124 also broke my workflow: static code analyzers don't like this, and the trait even crashes PHPStan.

It may also bring up other compatibility issues to other people (ie: I know companies that completely disable eval() on their servers).

As it was only required to keep the trait compatible with PHP 5.x, it is unnecessary on 4.x that requires PHP 7.1+, and this PR removes it on the 4.2 branch.

@skalpa
Copy link
Contributor Author

skalpa commented Feb 24, 2019

The fabpot.io failure seems to be a false positive (the script reads the typehint from the first definition and complains about the docblock of the second definition).

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Feb 24, 2019
@nicolas-grekas
Copy link
Member

You should report the crash to phpstan then.

@nicolas-grekas
Copy link
Member

Thank you @skalpa.

@nicolas-grekas nicolas-grekas merged commit 324b70a into symfony:4.2 Feb 24, 2019
nicolas-grekas added a commit that referenced this pull request Feb 24, 2019
…arDownTrait (skalpa)

This PR was merged into the 4.2 branch.

Discussion
----------

[FrameworkBundle] Removed eval() from KernelShutdownOnTearDownTrait

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

Apart from triggering the #30362 bug, the `eval()` block from #30124 also broke my workflow: static code analyzers don't like this, and the trait even crashes PHPStan.

It may also bring up other compatibility issues to other people (ie: I know companies that completely disable `eval()` on their servers).

As it was only required to keep the trait compatible with PHP 5.x, it is unnecessary on 4.x that requires PHP 7.1+, and this PR removes it on the 4.2 branch.

Commits
-------

324b70a Removed eval() from KernelShutdownOnTearDownTrait
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.

None yet

4 participants