Skip to content

fix(variable:get): avoid TypeError when selecting a variable interactively#126

Open
pjcdawkins wants to merge 1 commit into
mainfrom
fix/variable-get-interactive-null-name
Open

fix(variable:get): avoid TypeError when selecting a variable interactively#126
pjcdawkins wants to merge 1 commit into
mainfrom
fix/variable-get-interactive-null-name

Conversation

@pjcdawkins

Copy link
Copy Markdown
Contributor

What

Fixes a fatal TypeError thrown by variable:get when a variable is chosen interactively (i.e. the command is run without a name argument).

Symptom

The command runs, prompts for a variable, and displays it correctly — then crashes while printing the trailing hint:

Fatal error: Uncaught TypeError: OsUtil::escapeShellArg(): Argument #1 ($argument) must be of type string, null given
  ... in src/Command/Variable/VariableGetCommand.php on line 134

Cause

The name argument is optional. In the interactive path it stays null, but it was still passed to OsUtil::escapeShellArg() (which requires a string) when rendering the "To update the variable" hint. The crash happens after the variable has already been displayed, so the command appears to succeed and then fatals.

Fix

Use the selected variable's own name ($variable->name), which is populated in every code path, instead of the raw argument.

Notes

  • PHPStan level 9 (open PR chore(legacy): raise PHPStan to level 9 #100) detects this as escapeShellArg() expects string, mixed given but currently baselines it as noise. Once this merges, that baseline entry becomes obsolete and should be dropped on rebase.
  • No regression test added: the crash is specific to the interactive-selection path, which neither the PHP mock-API command tests nor the Go integration-test harness (PR Add integration tests for 10 new CLI commands #33) can drive via stdin today.

🤖 Generated with Claude Code

…ively

When run without a name argument, the command lets the user pick a
variable interactively. In that case the `name` argument is null, but it
was still passed to OsUtil::escapeShellArg() when printing the
"To update the variable" hint, which requires a string and threw a fatal
TypeError after the variable had already been displayed.

Use the selected variable's own name instead, which is populated in all
code paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 9, 2026 11:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a crash in the legacy PHP CLI variable:get command when running interactively (no name argument provided). The command previously displayed the selected variable successfully, then fatally errored while rendering the “update” hint due to passing null into a strictly-typed shell-escaping helper.

Changes:

  • Use the selected variable’s populated name ($variable->name) instead of the optional input argument ($name) when building the trailing variable:update hint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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