Skip to content

Blink: fix StandaloneLineRequiredParam, BracesPosition, StatementIndentation, MethodArgumentSpace and TrailingCommaInMultiline - #291

Merged
TomasVotruba merged 4 commits into
ecsphp:mainfrom
GErpeldinger:blink-false-reports
Nov 4, 2026
Merged

TomasVotruba merged 4 commits into
ecsphp:mainfrom
GErpeldinger:blink-false-reports

Conversation

@GErpeldinger

@GErpeldinger GErpeldinger commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Blink reports these where the PHP engine reports nothing, or breaks the code.

  • StandaloneLineRequiredParam: #[Body(required: true)] counted as #[Required], on public methods.
  • StandaloneLineRequiredParam: it had no priority, so it ran after LineLength.
  • BracesPosition: new readonly class was treated as a named class.
  • BracesPosition: if ($b) // c with { on the next line became if ($b) // c {, a parse error. This ports PHP-CS-Fixer's move of the brace past comments.
  • Lexer: a line comment ended at \n only, while PHP stops before \r too, so comment rules saw the \r on CRLF files.
  • StatementIndentation: #[ opened no block, so a multi-line attribute list got the wrong indent.
  • MethodArgumentSpace: it collapsed the spaces that align comments after a comma.
  • TrailingCommaInMultiline: it added a comma to a multi-line destructuring.

Each fix commit follows a commit of failing tests, with expected output from the PHP engine.

One difference on purpose: on if ($b) /*a*/// c with { on the next line, PHP-CS-Fixer writes the brace inside the comment, a parse error. Blink keeps the input.

🤖 Generated with Claude Code

@GErpeldinger

Copy link
Copy Markdown
Contributor Author

CI here waits for a maintainer's approval, so I ran it on my fork. With the test commit alone, Blink Tests fails on Linux and Windows: https://github.com/GErpeldinger/ecs-src/actions/runs/41892226738. The fix is now pushed.

@GErpeldinger GErpeldinger changed the title Blink: stop reporting valid code in 6 cases Blink: fix StandaloneLineRequiredParam, BracesPosition, StatementIndentation, MethodArgumentSpace and TrailingCommaInMultiline Nov 4, 2026
@GErpeldinger

Copy link
Copy Markdown
Contributor Author

I added two commits that port PHP-CS-Fixer's move of the brace past comments, so the known gap in the description is gone. Failing tests alone, on my fork: https://github.com/GErpeldinger/ecs-src/actions/runs/41905778736. The fix is now pushed.

The expected output of the tables comes from the PHP engine. The fix follows.
StandaloneLineRequiredParam counted "#[Body(required: true)]" as #[Required] and ran
after LineLength. BracesPosition treated "new readonly class" as a named class, and now
keeps a brace on its own line after a "//" comment instead of writing it inside the
comment. StatementIndentation gets a block for "#[", MethodArgumentSpace keeps the spaces
that align comments, and TrailingCommaInMultiline skips destructuring by default.
…t ends

The expected output of the tables comes from the PHP engine and token_get_all. The fix
follows. The test that kept the brace in place goes away, because the fix moves it like
PHP-CS-Fixer.
…iage return

BracesPosition only rewrote the whitespace before the brace, so "if ($b) // c" with the
brace on the next line became "if ($b) // c {", a parse error. Port $moveBraceToIndex.
PHP-CS-Fixer first puts a one-line body on its own line, which blink does for closures
only, so the scan stops at code. Where PHP-CS-Fixer itself writes the brace inside a
comment, on "/*a*/// c", blink keeps the input.

The lexer ended a line comment at "\n" only. PHP stops before "\r" too, so on CRLF files
the "\r" was part of the comment, and with the move the brace could cross real code on a
file with a bare "\r".
@TomasVotruba
TomasVotruba merged commit 9e3b19a into ecsphp:main Nov 4, 2026
21 checks passed
@TomasVotruba

Copy link
Copy Markdown
Member

Thank you 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants