Skip to content
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.

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

Ruby3 keyword argument matching #535

Closed
wants to merge 7 commits into from

Conversation

floehopper
Copy link
Member

@floehopper floehopper commented Aug 20, 2022

TODO:

  • Is there a missing unit test scenario in ExpectationTest?
  • Add acceptance tests, c.f. ParameterMatcherTest and/or OptionalParameterMatcherTest.
  • Work out whether the two test failures in StubbaExampleTest is a sign that this change might break a lot of existing tests. Do we need to put this change behind a configuration option?
  • Simplify/centralize Ruby version checking for ruby2 keywords functionality. Fixed by using RUBY_V3_PLUS.
  • Work out what to do with Ruby v1.8 - there is a problem parsing expectation_test.rb with keyword arguments. Resolved in Remove support for Ruby v1.8 #536
  • Resolve Rubocop issues in unit tests. Resolved by disabling Style/BracesAroundHashParameters cop and using hash rocket syntax in combination with Hash.ruby2_keywords_hash to designate keyword arguments. Now that we've dropped support for Ruby v1.8, we could probably change the configuration of the Style/HashSyntax cop, but that feels like a bigger change (see Use Ruby v1.9 Hash literal syntax #537).
  • Fix Yardoc for LastPositionalHash. Fixed by marking the whole class as private from a documentation point of view.
  • Find someone with a large Rails codebase using Mocha to test a pre-release version

@floehopper
Copy link
Member Author

I've just dropped support for Ruby v1.8 in #536. I'll rebase this against main now and that should resolve the issue with the keyword argument parse errors in the Ruby v1.8 builds.

@floehopper floehopper force-pushed the ruby3-keyword-argument-matching branch from ba2705e to 81ff646 Compare August 21, 2022 09:39
I'm a bit worried that this is an indication that the keyword argument
change will break a lot of existing tests.
@floehopper floehopper force-pushed the ruby3-keyword-argument-matching branch from 51bc1f2 to bf2e666 Compare August 21, 2022 10:10
Since this class is only used internally, we can make the whole class
private from a documentation point of view.
While it would be nicer to do some more fine-grained checking like in
RSpec::Support::RubyFeatures [1], I think this is probably good enough
for now. I think we are essentially using RUBY_V3_PLUS as a more generic
version of
RSpec::Support::RubyFeatures#distincts_kw_args_from_positional_hash?

[1]: https://github.com/rspec/rspec-support/blob/528d88ce6fac5f83390bf430d1c47608e9d8d29a/lib/rspec/support/ruby_features.rb
[2]: https://github.com/rspec/rspec-support/blob/528d88ce6fac5f83390bf430d1c47608e9d8d29a/lib/rspec/support/ruby_features.rb#L132-L134
floehopper pushed a commit that referenced this pull request Oct 1, 2022
When the strict keyword argument option is enabled, an expectation
expecting keyword arguments (via Expectation#with) will no longer match
an invocation passing a positional Hash argument.

Without this option, positional hash and keyword arguments are treated
the same during comparison, which can lead to false negatives in
Ruby >= v3.0 (see examples below).

For more details on keyword arguments in Ruby v3, refer to this
article [1].

Note that Hash matchers such as has_value or has_key will still treat
positional hash and keyword arguments the same, so false negatives are
still possible when they are used.

Closes #446.

See also #535 & #544 for discussions relating to this change.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0
floehopper pushed a commit that referenced this pull request Oct 9, 2022
When the strict keyword argument option is enabled, an expectation
expecting keyword arguments (via Expectation#with) will no longer match
an invocation passing a positional Hash argument.

Without this option, positional hash and keyword arguments are treated
the same during comparison, which can lead to false negatives in
Ruby >= v3.0 (see examples below).

For more details on keyword arguments in Ruby v3, refer to this
article [1].

Note that Hash matchers such as has_value or has_key will still treat
positional hash and keyword arguments the same, so false negatives are
still possible when they are used.

Closes #446.

See also #535 & #544 for discussions relating to this change.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0
floehopper pushed a commit that referenced this pull request Oct 9, 2022
When the strict keyword argument option is enabled, an expectation
expecting keyword arguments (via Expectation#with) will no longer match
an invocation passing a positional Hash argument.

Without this option, positional hash and keyword arguments are treated
the same during comparison, which can lead to false negatives in
Ruby >= v3.0 (see examples below).

For more details on keyword arguments in Ruby v3, refer to this
article [1].

Note that Hash matchers such as has_value or has_key will still treat
positional hash and keyword arguments the same, so false negatives are
still possible when they are used.

Closes #446.

See also #535 & #544 for discussions relating to this change.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0
floehopper pushed a commit that referenced this pull request Oct 12, 2022
When the strict keyword argument option is enabled, an expectation
expecting keyword arguments (via Expectation#with) will no longer match
an invocation passing a positional Hash argument.

Without this option, positional hash and keyword arguments are treated
the same during comparison, which can lead to false negatives in
Ruby >= v3.0 (see examples below).

For more details on keyword arguments in Ruby v3, refer to this
article [1].

Note that Hash matchers such as has_value or has_key will still treat
positional hash and keyword arguments the same, so false negatives are
still possible when they are used.

Closes #446.

See also #535 & #544 for discussions relating to this change.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0
floehopper pushed a commit that referenced this pull request Oct 12, 2022
When the strict keyword argument option is enabled, an expectation
expecting keyword arguments (via Expectation#with) will no longer match
an invocation passing a positional Hash argument.

Without this option, positional hash and keyword arguments are treated
the same during comparison, which can lead to false negatives in
Ruby >= v3.0 (see examples below).

For more details on keyword arguments in Ruby v3, refer to this
article [1].

Note that Hash matchers such as has_value or has_key will still treat
positional hash and keyword arguments the same, so false negatives are
still possible when they are used.

Closes #446.

See also #535 & #544 for discussions relating to this change.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0
floehopper added a commit that referenced this pull request Oct 12, 2022
Closes #562.

This introduces a new `strict_keyword_argument_matching` configuration
option. This option is only available in Ruby >= v2.7 and is disabled by
default to enable gradual adoption.

When the strict keyword argument option is enabled, an expectation
expecting keyword arguments (via `Expectation#with`) will no longer
match an invocation passing a positional Hash argument.

Without this option, positional hash and keyword arguments are treated
the same during comparison, which can lead to false negatives in
Ruby >= v3.0 (see examples below).

* Loose keyword argument matching (default)

    class Example
      def foo(a, bar:); end
    end

    example = Example.new
    example.expects(:foo).with('a', bar: 'b')
    example.foo('a', { bar: 'b' })
    # This passes the test, but would result in an ArgumentError in practice

* Strict keyword argument matching

    Mocha.configure do |c|
      c.strict_keyword_argument_matching = true
    end

    class Example
      def foo(a, bar:); end
    end

    example = Example.new
    example.expects(:foo).with('a', bar: 'b')
    example.foo('a', { bar: 'b' })
    # This now fails as expected

For more details on keyword arguments in Ruby v3, refer to this
article [1].

Note that Hash matchers such as `has_value` or `has_key` will still
treat positional hash and keyword arguments the same, so false negatives
are still possible when they are used.

Closes #446.

See also #535 & #544 for discussions relating to this change.

[1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0

Co-authored-by: Nicholas Koh <nicholas.koh@shopify.com>
@floehopper
Copy link
Member Author

Addressed in #562.

@floehopper floehopper closed this Oct 12, 2022
@floehopper floehopper deleted the ruby3-keyword-argument-matching branch October 12, 2022 17:06
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.

None yet

2 participants