Commit 4c672ef
committed
minor symfony#59995 [Config] Make ifFalse() consistent between value and closure based checks (arjenm)
This PR was squashed before being merged into the 7.3 branch.
Discussion
----------
[Config] Make ifFalse() consistent between value and closure based checks
| Q | A
| ------------- | ---
| Branch? | 7.3
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Issues | Fix symfony#59325
| License | MIT
I noticed the Config/ExpBuilder got a nice new ifFalse in Symfony 7.3.
But I think its implementation is confusing.
The behavior for `ifTrue()` was:
- If no closure is provided: if the actual value is `true` execute the then part
- If a closure is provided: if it returns `true` execute the then part
The behavior for `ifFalse()` is prior to this PR is:
- If no closure is provided: if the actual value is `false` execute the then part
- If a closure is provided: if it returns **`true`** execute the then part
With this PR it becomes:
- If no closure is provided: if the actual value is `false` execute the then part
- If a closure is provided: if it returns **`false`** execute the then part
Rationale, I think people (me included) would not expect these to be both be valid or invalid with the same input:
`$expr->ifTrue(self::valueIsNotValid(...))->thenInvalid()`
`$expr->ifFalse(self::valueIsNotValid(...))->thenInvalid()`
They/I expect to have to change it to a test for valid values (rather than invalid ones):
`$expr->ifFalse(self::valueIsValid(...))->thenInvalid()`
Commits
-------
335bdbe [Config] Make ifFalse() consistent between value and closure based checksFile tree
2 files changed
+28
-4
lines changed- src/Symfony/Component/Config
- Definition/Builder
- Tests/Definition/Builder
2 files changed
+28
-4
lines changedLines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
79 | | - | |
| 79 | + | |
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
| |||
Lines changed: 27 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
45 | 51 | | |
46 | 52 | | |
47 | 53 | | |
48 | 54 | | |
49 | 55 | | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
50 | 62 | | |
51 | 63 | | |
52 | 64 | | |
| |||
58 | 70 | | |
59 | 71 | | |
60 | 72 | | |
61 | | - | |
| 73 | + | |
62 | 74 | | |
63 | 75 | | |
64 | | - | |
| 76 | + | |
65 | 77 | | |
66 | 78 | | |
67 | | - | |
| 79 | + | |
68 | 80 | | |
69 | 81 | | |
70 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
71 | 95 | | |
72 | 96 | | |
73 | 97 | | |
| |||
0 commit comments