Skip to content

fix(CommandHelpToAttributeRector): support concatenated string in setHelp()#932

Merged
TomasVotruba merged 1 commit into
rectorphp:mainfrom
androshchuk:fix/command-help-concat-string
May 7, 2026
Merged

fix(CommandHelpToAttributeRector): support concatenated string in setHelp()#932
TomasVotruba merged 1 commit into
rectorphp:mainfrom
androshchuk:fix/command-help-concat-string

Conversation

@androshchuk
Copy link
Copy Markdown
Contributor

Previously the rule only handled plain String_ literals. When setHelp() received a Concat expression (e.g. "line one\n" . "line two") it silently dropped the call without migrating the text to #[AsCommand(help: ...)], leaving the help text permanently lost.

Introduce resolveStringExpr() that recursively folds a Concat tree of String_ literals into a single string value. Non-literal sub-expressions (variables, function calls, …) cause the method to return null, which now also prevents the setHelp() removal — fixing the silent data-loss bug.

Problem

CommandHelpToAttributeRector only handles a plain String_ literal as the argument to setHelp(). When the argument is a concatenated string (a tree of Concat nodes), the rule exhibits two bugs:

  1. Silent data losssetHelp() is removed from the method chain (via return $node->var in the traversal callback) even though $helpString is never set, so the help text is permanently discarded.
  2. No migration — because $helpString stays null, refactor() returns null and nothing is added to #[AsCommand(help: ...)].

Example that triggered the bug:

$this
    ->setHelp("First line.\n"
        . "Second line.\n"
        . "Third line.")
    ->addOption(...)

Fix

  • Add resolveStringExpr(Expr): ?string — recursively folds a Concat tree of String_ literals into a single string value. Returns null for any non-literal sub-expression (variables, function calls, …).
  • Replace the inline instanceof String_ check with a call to resolveStringExpr().
  • Crucially, return $node->var (which removes setHelp()) now only executes when $resolvedValue !== null — so non-resolvable expressions are left untouched instead of being silently dropped.

Test

Added add_help_concatenated_string.php.inc fixture covering a setHelp() call with a multi-line concatenated string alongside another chained method call (addOption).

…Help()

Previously the rule only handled plain String_ literals. When setHelp()
received a Concat expression (e.g. "line one\n" . "line two") it silently
dropped the call without migrating the text to #[AsCommand(help: ...)],
leaving the help text permanently lost.

Introduce resolveStringExpr() that recursively folds a Concat tree of
String_ literals into a single string value. Non-literal sub-expressions
(variables, function calls, …) cause the method to return null, which
now also prevents the setHelp() removal — fixing the silent data-loss bug.
@TomasVotruba TomasVotruba merged commit b48ee99 into rectorphp:main May 7, 2026
7 checks passed
@TomasVotruba
Copy link
Copy Markdown
Member

Looks good, thank you 👍

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants