Skip to content

[rspec-support] [bugfix] Properly detect kwargs hashes vs optional positional args #121

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
Merged
merged 8 commits into from
Aug 18, 2025
Merged

[rspec-support] [bugfix] Properly detect kwargs hashes vs optional positional args #121

merged 8 commits into from
Aug 18, 2025

Conversation

JonRowe
Copy link
Member

@JonRowe JonRowe commented Nov 30, 2024

This is rspec/rspec-support#594

Attempting to fix #145

Refactor has_kw_args_in?:

try to use positional parameter information to determine whether a hash is kw_args
try to leverage information passed by the ruby2_keywords annotation
fallback to inspection of keyword args to determine if they look valid or not

Added backwards compatible ruby2_keywords wrapping to the necessary methods (valid? and error_for) in the spec files like so:
ruby2_keywords(:valid?) if respond_to?(:ruby2_keywords, true)

Also needed to modify how args are passed to the MSV split_args so that the special kwargs markings are not lost.

@JonRowe JonRowe mentioned this pull request Nov 30, 2024
Closed
@JonRowe JonRowe changed the base branch from dev to main December 3, 2024 08:59
@taketo1113
Copy link

I'm facing the same issue as in #104, and it looks like this Pull Request will fix it.

@JonRowe It seems that this Pull Request will pass the tests after rebasing onto the main branch.
If the tests pass, is there anything else I can do to help with the merge?

For reference, here is the result of the rebase I tried:
https://github.com/taketo1113/rspec/actions/runs/16308333128

@taketo1113
Copy link

I rebased onto the main branch of rspec/rspec again and confirmed that the CI passes, including rubocop.
https://github.com/taketo1113/rspec/actions/runs/16929997502

@JonRowe JonRowe force-pushed the malcolmohare/rspec-expectation-1451 branch from a2bcbf3 to aeb1777 Compare August 14, 2025 13:18
@JonRowe JonRowe force-pushed the malcolmohare/rspec-expectation-1451 branch from 2d731d4 to ef47c9d Compare August 15, 2025 19:10
@JonRowe JonRowe force-pushed the malcolmohare/rspec-expectation-1451 branch from 2f05ecc to fbb5720 Compare August 17, 2025 10:21
@JonRowe JonRowe merged commit 88362f4 into main Aug 18, 2025
79 of 80 checks passed
@JonRowe JonRowe deleted the malcolmohare/rspec-expectation-1451 branch August 18, 2025 07:17
JonRowe added a commit that referenced this pull request Aug 18, 2025
@JonRowe
Merge pull request #121 from rspec/malcolmohare/rspec-expectation-1451
3ed4ec9 
[rspec-support] [bugfix] Properly detect kwargs hashes vs optional positional args
JonRowe added a commit that referenced this pull request Aug 18, 2025
@JonRowe
Changelog for #121
3fbc61b
JonRowe added a commit that referenced this pull request Aug 18, 2025
@JonRowe
Merge pull request #121 from rspec/malcolmohare/rspec-expectation-1451
3c27fed 
[rspec-support] [bugfix] Properly detect kwargs hashes vs optional positional args
JonRowe added a commit that referenced this pull request Aug 18, 2025
@JonRowe
Changelog for #121
7143095
@JonRowe JonRowe mentioned this pull request Aug 18, 2025
Merged
JonRowe added a commit that referenced this pull request Aug 18, 2025
@JonRowe
Changelog for #121
c5021bb
@pirj
Copy link
Member

pirj commented Aug 18, 2025

Fantastic, @malcolmohare , @JonRowe, thank you!

@JonRowe
Copy link
Member Author

JonRowe commented Aug 18, 2025

Released in rspec-support 3.13.5

@taketo1113
Copy link

Thank you for fixing and release gem.
I confirm the issue is resolved with rspec-support v3.13.5
https://github.com/rspec/rspec/releases/tag/rspec-support-v3.13.5

I'm facing the same issue as in #104, and it looks like this Pull Request will fix it.

@taketo1113 taketo1113 mentioned this pull request Aug 18, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression in 3.13: custom matcher hash argument improperly converted to keyword args, results in ArgumentError
4 participants
@JonRowe @taketo1113 @pirj @malcolmohare