Skip to content

Fix internal variable bleeding in REPL eval context #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix internal variable bleeding in REPL eval context #80

Copilot wants to merge 3 commits into from

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 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