Commit a9152cb
committed
[TASK] Add tests and adjust ambiguity in AstConstantCommentVisitor
The `AstConstantCommentVisitor` is used to parse
`ext_conf_template.txt` files of extensions. The file provides
TypoScript comments which are parsed as constants to allow
GUI configuration mapping in the TypoScript Constant Editor.
A comment string like this:
```
# cat=Integer/101/15; type=int[-5--1]; label=Int 6 - range (neg start, neg end)
```
is parsed as syntax (yes, yuck) and specifically the argument
`type=int[-5--1]` is tokenized and parsed into a min/max range.
This was performed with ambiguous interpretation with `$start/$stop`
temporary variables and character-by-character parsing to find out
if a number is positive or negative.
The old comparison:
```
} elseif ($stop === '' && $char === '-') {
```
was never possible to trigger though, because $stop was initialized
as 'null'. This only worked before strict type declarations where
enabled in legacy code, likely.
Funnily, this oddity had no real-life implications as the newly
added tests reveal.
Before this patch, `$negativeStop` could never be set to `true`,
thus the later code:
```
if ($negativeStop) {
$stop = (int)$stop * -1;
}
```
was never triggered. So a value like `1` (from `-5--1`) would not be
transformed into `-1` theoretically. Luckily though, in this case
`$stop` was already set to `-1` (note that it would be a TWO character
string then).
When fixing this code to use a proper `$stop === null` comparison,
the end result is still the same, and `$stop` will be set to `-1`.
Tests verify that the behaviour is the same before and after the
patch. With this patch, PHPStan will no longer report the condition
to be unused, unlocking an update to PHPStan 2.1.19 without a change
in the baseline.
Resolves: #107125
Related: #98357
Related: #97816
Releases: main, 13.4
Change-Id: I6067fec7357dda1533e180f157ddd70c86ea9800
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/90112
Tested-by: Simon Schaufelberger <simonschaufi+typo3@gmail.com>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Garvin Hicking <garvin@hick.ing>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Garvin Hicking <garvin@hick.ing>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Simon Schaufelberger <simonschaufi+typo3@gmail.com>1 parent c781a30 commit a9152cb
File tree
3 files changed
+1090
-1
lines changed- typo3/sysext/core
- Classes/TypoScript/AST/Visitor
- Tests/Unit
- Fixtures
- TypoScript/AST/Visitor
3 files changed
+1090
-1
lines changedLines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
284 | 284 | | |
285 | 285 | | |
286 | 286 | | |
287 | | - | |
| 287 | + | |
288 | 288 | | |
289 | 289 | | |
290 | 290 | | |
| |||
Lines changed: 177 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
0 commit comments