Skip to content

Fix internal variable bleeding in REPL eval context#80

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-internal-variables-conflict
Open

Fix internal variable bleeding in REPL eval context#80
Copilot wants to merge 3 commits intomainfrom
copilot/fix-internal-variables-conflict

Conversation

Copy link
Contributor

Copilot AI commented Jan 25, 2026 •
edited
Loading

Internal variables $line, $out, and $evl in the REPL class were visible in the eval context, causing conflicts when users defined variables with the same names.

// Before: Internal $line bleeds through
wp> $line = 'test';
=> string(4) "test"
wp> $line;
=> string(13) "return $line;"  // Shows internal value

// After: User variable works correctly
wp> $line = 'test';
=> string(4) "test"
wp> $line;
=> string(4) "test"  // Shows user value

Changes

  • Renamed internal variables to __repl_ prefixed equivalents:

    • $line → $__repl_input_line
    • $out → $__repl_output
    • $evl → $__repl_eval_result

  • Added test coverage for all three variable names to prevent regression

The __repl_ prefix follows PHP internal variable conventions and eliminates naming collisions with typical user code.

Original prompt

This section details on the original issue you should resolve

<issue_title>Internal variables bleed over and cause conflicts</issue_title>
<issue_description>I haven't had the chance to dig into this further, but just wanted to log this somewhere to prevent forgetting about it.

Assigning a value to $line in wp shell gives very unexpected results, causing some pretty confusing behavior and bugs.

Behavior:

wp shell

wp> $line = 'this wont work';
=> string(14) 'this wont work'
wp> $line;
=> string(13) "return $line;"

wp> $linetest = 'this will';
=> string(9) "this will"
wp> $linetest;
=> string(9) "this will"
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@schlessera</author><body>
`$line` is one of the variables that bleed over from the context that evaluates the code that was typed in.

While I don't think we can provide a completely clean context with `eval()`, we can certainly make sure we're not using something that is as easy to run into conflicts with than `$line`.</body></comment_new>
</comments>

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@codecov
Copy link

codecov bot commented Jan 25, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/WP_CLI/Shell/REPL.php 91.66% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI and others added 2 commits January 25, 2026 17:04
@swissspidy
Rename internal variables to prevent bleeding into eval context
31bf815 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
@swissspidy
Add test for $out and $evl variables
ecaf2e7 
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix internal variables bleed over issue in wp shell Fix internal variable bleeding in REPL eval context Jan 25, 2026
Copilot AI requested a review from swissspidy January 25, 2026 17:05
@swissspidy swissspidy requested a review from Copilot January 25, 2026 17:13
Copilot AI reviewed Jan 25, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical bug where internal REPL variables ($line, $out, and $evl) were bleeding into the eval context, causing unexpected behavior when users defined variables with these common names. The fix renames these internal variables to use a __repl_ prefix, making them highly unlikely to collide with user code.

Changes:

  • Renamed three internal variables to prefixed equivalents: $line → $__repl_input_line, $out → $__repl_output, $evl → $__repl_eval_result
  • Added comprehensive test coverage to prevent regression for all three variable names

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/WP_CLI/Shell/REPL.php Systematically renamed internal variables across all 24 occurrences to use __repl_ prefix, preventing scope bleeding into eval context
features/shell.feature Added two test scenarios verifying users can define variables with previously problematic names ($line, $out, $evl)

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

@swissspidy swissspidy marked this pull request as ready for review January 25, 2026 17:20
@swissspidy swissspidy requested a review from a team as a code owner January 25, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@swissspidy swissspidy Awaiting requested review from swissspidy

At least 1 approving review is required to merge this pull request.

Assignees

@swissspidy swissspidy

Copilot code review Copilot

Labels

command:shell scope:testing state:unconfirmed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Internal variables bleed over and cause conflicts

2 participants

@swissspidy