fix: use named parameter in contents() method#318
Merged
toddr merged 1 commit intocpan-authors:masterfrom Mar 23, 2026
Merged
Conversation
The contents() method already extracts $new_contents from @_ at the top of the sub, but line 2015 used $_[1] directly to assign file contents. While functionally equivalent for simple strings (which is enforced by the ref check above), $_[1] is an alias to the caller's argument — a fragile pattern that bypasses the named parameter and could break if the method signature ever changes. Replace $_[1] with $new_contents and remove the XXX comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
toddr
approved these changes
Mar 23, 2026
toddr
added a commit
that referenced
this pull request
Mar 23, 2026
…agile-arg fix: use named parameter in contents() method
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replace
$_[1]with the already-extracted$new_contentsvariable in thecontents()setter path.Why
The method signature at line 1975 already does
my ($self, $new_contents) = @_;, but line 2015 bypassed this named parameter and used$_[1]directly (with an XXX comment questioning why). While functionally equivalent for simple strings,$_[1]is an alias to the caller's original argument — fragile if the method signature ever changes.How
One-line change:
$self->{'contents'} = $_[1]→$self->{'contents'} = $new_contents. Removed the XXX comment.Testing
Full test suite passes (1553 tests). The only failure (
t/fh-ref-leak.t) is pre-existing and unrelated.🤖 Generated with Claude Code
Quality Report
Changes: 1 file changed, 1 insertion(+), 2 deletions(-)
Code scan: clean
Tests: skipped
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline