This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
Fixed bugs found by psalm
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
When running psalm for branch 4.4, we get around 1500 errors. I browsed though them all and here is a small PR with the actual bugs I think should be fixed.
I'll make review comments about them below.
Commits
-------
0a0cd7a274 Fixed bugs found by psalm
This PR was merged into the 4.4 branch.
Discussion
----------
[Config] Fixed support for nodes not extending BaseNode
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Too many times we assume that a `NodeInterface` extends `BaseNode`. This PR adds a small test with a new `CustomNodeDefinition` and `CustomNode` that implements `NodeInterface`.
Commits
-------
68276562c3 Fixed support for nodes not extending BaseNode
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] Dont store cache misses on warmup
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38694
| License | MIT
| Doc PR | symfony/symfony-docs#15172
When we are warming the annotation cache, we are reading all annotation into an `ArrayAdapter`. When we are done we move the values to a `PhpArrayAdapter` to store them in a file.
@Seldaek [found out](https://github.com/symfony/symfony/issues/38694#issuecomment-810501066) that when you are using a custom constraint with a `Symfony\Component\Validator\Constraints\Callback`, there is a strange error like:
> Can use "yield from" only with arrays and Traversables
That is because the `Closure` in the `Symfony\Component\Validator\Constraints\Callback` cannot be serialised and saved to cache. But since the `ArrayAdapter` is also [storing misses as null](https://github.com/symfony/symfony/pull/35362), the null values are understood as real values.
When all values are moved to the `PhpArrayAdapter` and we ask the cache for a value (via Doctrine's `CacheProvider`), it will return `null` as a value instead of `false` as a cache miss. And `null` is not something one could "yield from".
Commits
-------
27a22b34af [FrameworkBundle] Dont store cache misses on warmup
This PR was merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] Remove author comments for configuration and extension
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
I really don't want to step on anyones toes. I'm also super grateful for all the time, effort and energy these contributors (and everybody else) have given to Symfony.
The `@author` tags should represent "who knows the most about this class" or to give the original author some kind of credit (more visible than `git blame`).
The FrameworkBundle's `Configuration` and `FrameworkExtension` classes are a bit special. Fist because they are well known by most recurring contributors. They are also thousands of lines long and many many users have added a line or two. The `FrameworkExtension` have over 50 people that currently authored more than 5 lines. The named authors combined have about that 12% and 25% in `Configuration` and `FrameworkExtension` respectively.
I do like the `@author` tag, but for the reasons above, I don't think they make sense for these classes.
Commits
-------
f19d72aa07 [FrameworkBundle] Remove author comments for configuration and extension
This PR was merged into the 4.4 branch.
Discussion
----------
[Validator] Delete obsolete statement in Regex::getHtmlPattern() phpDoc
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | #12235
| License | MIT
| Doc PR |
The original issue was #5307 (`match=false` not handled in `getHtmlPattern()`).
Then #5382 patched it to return null (rather than an incorrect result) and inserted a statement and a "todo" in the phpDoc.
Later (but still years ago) #12235 properly implemented pattern inversion, but only removed the "todo"; this PR removes the statement.
Commits
-------
3bdf8d7238 [Validator] Delete obsolete statement in Regex::getHtmlPattern() phpDoc
This PR was merged into the 4.4 branch.
Discussion
----------
[DependencyInjection] Fix "url" env var processor behavior when the url has no path
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Before:
```yaml
MY_URL_ENV_VAR=http://symfony.com
%env(key:path:url:MY_URL_ENV_VAR)% --> false
```
After:
```yaml
MY_URL_ENV_VAR=http://symfony.com
%env(key:path:url:MY_URL_ENV_VAR)% --> null
```
Returning `false` for the path prevents me from using the `default` env var processor that is triggered only for `''` and `null`.
(`%env(default:my_fallback_param:key:path:url:MY_URL_ENV_VAR)%`)
BTW, with PHP 8, it actually works because `substr(null, 1)` behavior changed (see https://3v4l.org/oHf6l).
Commits
-------
2876cf9cc6 [DependencyInjection] Fix "url" env var processor behavior when the url has no path
This PR was merged into the 4.4 branch.
Discussion
----------
[Cache] skip storing failure-to-save as misses in ArrayAdapter
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#38694
| License | MIT
| Doc PR | -
In addition to #40645
Commits
-------
ba6698795c [Cache] skip storing failure-to-save as misses in ArrayAdapter
This PR was merged into the 4.4 branch.
Discussion
----------
[Serializer] Allow AbstractNormalizer to use null for non-optional nullable constructor parameters without default value
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40511
| License | MIT
Serializer component AbstractNormalizer attemps to guess constructor parameters, and falls back using default values when possible. Yet, it misses one use case: nullable non-optional parameter with value not being present in incoming input, case in which null is a valid value, not the default one, yet still valid.
This PR introduce a two-line fix that forcefully set null as value for missing from input non-optional nullable constructor parameters values.
Commits
-------
d29e433ae8 [Serializer] AbstractNormalizer force null for non-optional nullable constructor parameter denormalization when not present in input
This PR was merged into the 4.4 branch.
Discussion
----------
add missing queue_name to find(id) in doctrine messenger transport
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| License | MIT
This bug was noticed by @weaverryan when reviewing the PR: https://github.com/symfony/symfony/pull/38468 related to multiple failure transports.
Commits
-------
bb26a92826 add missing queue_name to find(id) in doctrine messenger transport
This PR was merged into the 4.4 branch.
Discussion
----------
[Filesystem] Fix dumpFile() method call
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | ?
| New feature? | no
| Deprecations? | no
| Tickets | ---
| License | MIT
| Doc PR | ---
The dumpFile() method in the Filesystem component takes two arguments (filename and content). I found a test with the third argument set to null.
Commits
-------
2b33d4af65 [Filesystem] Fix dumpFile() method call
This PR was merged into the 4.4 branch.
Discussion
----------
[Form] skip intl dependent tests if the extension is missing
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
71da904de0 skip intl dependent tests if the extension is missing
This PR was merged into the 4.4 branch.
Discussion
----------
[Form] error if the input string couldn't be parsed as a date
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40597
| License | MIT
| Doc PR |
When the Intl polyfill is used instead of the PHP intl extension, the
intl_get_error_code() function always returns 0 no matter if the input
string could be parsed.
Commits
-------
5ce5300da3 error if the input string couldn't be parsed as a date
This PR was merged into the 4.4 branch.
Discussion
----------
[HttpClient] fix using stream_copy_to_stream() with responses cast to php streams
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40574
| License | MIT
| Doc PR | -
Commits
-------
cf1404a30b [HttpClient] fix using stream_copy_to_stream() with responses cast to php streams
This PR was merged into the 4.4 branch.
Discussion
----------
[VarDumper] make DateCaster tests timezone-agnostic
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
bfef4546d3 make DateCaster tests timezone-agnostic
When the Intl polyfill is used instead of the PHP intl extension, the
intl_get_error_code() function always returns 0 no matter if the input
string could be parsed.
This PR was merged into the 4.4 branch.
Discussion
----------
[Form] IntegerType: Always use en for IntegerToLocalizedStringTransformer
Fixes#40456
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40456
| License | MIT
Commits
-------
83b836dbc9 IntegerType: Always use en for IntegerToLocalizedStringTransformer Fixes#40456
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
Uses the correct assignment action for console options depending if they are short or long
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40590
| License | MIT
| Doc PR | -
Checks if the option is short or long, and assigns the corresponding "glue" for the assignment.
Commits
-------
d0a3c538f4 Uses the correct assignment action for console options depending if they are short or long
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[HttpKernel] ConfigDataCollector to return known data without the need of a Kernel
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40534
| License | MIT
| Doc PR |
Sets `$this->data` with information that can be known without the need of a `Kernel`.
Commits
-------
d919f2ce83 [HttpKernel] ConfigDataCollector to return known data without the need of a Kernel
This PR was merged into the 4.4 branch.
Discussion
----------
Fix Trying to clone an uncloneable object of class
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Since `ext-redis: 5.3..3`, the `\Redis` class is not cloneable anymore (https://github.com/phpredis/phpredis/commit/cd05a344).
PHPUnit 7.5 (used byt PHP 7.1) clones objects provided by the dataprovier, when they are cloneable.
To check if an object is cloneable, it use the `Reflexion::isCloneable` method and check if the `__clone` method is public.
The issue is, the Class generated by `createMock` is cloneable, which leads to an exception such as:
> Trying to clone an uncloneable object of class Mock_Redis_b2f171b3
This PR fake replaces the mock by a real instance of the class.
Commits
-------
f67dab95e2 FIx Trying to clone an uncloneable object of class
This PR was merged into the 4.4 branch.
Discussion
----------
[travis] remove cache of composer.lock for deps=low
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Let's hope that Composer 2 made this fast enough. Maintaining this script might not be worth it anymore.
Reverts #28146
Commits
-------
9ba015d236 [travis] remove cache of composer.lock for deps=low
This PR was merged into the 4.4 branch.
Discussion
----------
[PhpUnitBridge] fix reported class when deprecated method is static
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
aa80df609f fix reported class when deprecated method is static
This PR was merged into the 4.4 branch.
Discussion
----------
[travis] use packagist API v2
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Commits
-------
af43ca080b [travis] use packagist API v2
