`have_public_method` and `have_protected_method` matchers are missing

[trinistr]

There are matchers for pretty much all other variations of method-having, but there are none for #public_methods and #protected_methods:

$ ls lib/mspec/matchers/have*method*
lib/mspec/matchers/have_instance_method.rb
lib/mspec/matchers/have_method.rb
lib/mspec/matchers/have_private_instance_method.rb
lib/mspec/matchers/have_private_method.rb
lib/mspec/matchers/have_protected_instance_method.rb
lib/mspec/matchers/have_public_instance_method.rb
lib/mspec/matchers/have_singleton_method.rb

I would like to add these matchers for completeness.

Rationale:

• I think anyone can reasonably expect to have these matchers available.
• There are tests that would benefit (see for example core/argf/shared/each_codepoint.rb).

[trinistr]

I would like to add these matchers for completeness.

A PR is already in the works, mostly just wanted to get opinion if this is something that shouldn't be added.

[eregon]

• There are tests that would benefit (see for example core/argf/shared/each_codepoint.rb).

Like https://github.com/ruby/spec/blob/b833642f457a047aea80bb8157e343d10c80c3ea/core/argf/shared/each_codepoint.rb#L13

I think that should be this rather:

@argf.public_methods(false).should.include?(@method)

In general I don't like those have*method matchers because they are rather unclear what they actually do/check, and just using e.g. public_methods(false) seems far clearer.
https://github.com/ruby/mspec/blob/bd8efcf2951b99c9c2235d1ddb7ddb06f37aa332/lib/mspec/matchers/have_private_method.rb for example just seems unnecessary boilerplate, it just calls .private_methods, so why a matcher jut for that? It's also error-prone as it calls private_methods with default true which is typically not what we want in specs.

For these reasons I think we should actually deprecate all these have*method matchers, but unsure it's worth the effort.

One step would be to update https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#predicate-matchers to no longer mention them.

Sorry, I guess that's not what you were looking for but yeah in the current state I see these matchers as legacy stuff that's just waiting to be deprecated. Probably the same for all have_ matchers, all of these seem trivially expressive without a custom matcher, and the error message would still be good enough when it fails.

[eregon]

We should probably also deprecate be_kind_of and be_an_instance_of, they are redundant with .should.foo form:

obj.should be_kind_of(Enumerable)
obj.should.is_a?(Enumerable)

obj.should be_an_instance_of(Array)
obj.should.instance_of?(Array)

[trinistr]

In my opinion, matchers that improve readability should be left in (and expanded). (But I'm also a fan of RSpec.)

have*method matchers, while making reading tests easier, indeed have somewhat of a problem with "what does this do?", though I would argue that "it just calls object.*methods(include_super).include?" is quite clear, if it is documented.

just seems unnecessary boilerplate

Eh, if the base class was rewritten, I'm pretty sure all the matchers could be one-liners, basically just HavePrivateMethodMatcher = MethodMatcher.build(:private_methods, "private method"), or even one call with a hash to create all of them. (Separation of concerns is questionable, but 'tis a reasonable trade-off.)

But I agree that these matchers may be superfluous, and the benefit is not that large to have them.

It's also error-prone as it calls private_methods with default true which is typically not what we want in specs.

Today I learned ™️ that SomeClass.private_methods(false) doesn't actually return only singleton methods of that class, but also of singleton superclasses (https://bugs.ruby-lang.org/issues/8743), and so usually also isn't what we actually want to test. Maybe there should instead be matchers have_own*class_method or something? It's currently not obvious that private_methods(false) is not the right tool (because documentation is wrong) and how to test this correctly.

(Going off on a tangent, how did it happen that private_class_method and private_instance_method do completely different things? Method-related methods remind me of PHP, and that is sad.)

We should probably also deprecate be_kind_of and be_an_instance_of, they are redundant with .should.foo form

I feel that obj.should be_kind_of(Enumerable) reads much easier than obj.should.is_a?(Enumerable), which is plain awkward to look at. should.instance_of? is more or less in the same boat.

On that note, are be_true and be_false deprecated? They seem completely unnecessary, though I still use them due to muscle memory from RSpec.