Add Minitest assertion annotations

vinistock commented 1 month ago

Type of Change

[x] Add RBI for a new gem
[ ] Modify RBI for an existing gem
[ ] Other:

Changes

Now that we have untyped statistics, better typing Minitest assertions is important to remove them from the data - since they don't actually indicate any lack of safety. The reason is because we invoke so many assertions in the many many tests we have that they end up polluting the results with a bunch of untypeds.

This PR adds Minitest annotations, but only for assertions. Most of the other methods defined in the Minitest gem are only invoked by the gem itself, developers almost always interact only with assertions.

Gem name: Minitest
Gem version: Latest
Gem source: https://github.com/minitest/minitest
Gem API doc: https://docs.seattlerb.org/minitest

Note

I have tested this on Core and it found maaaany cases of the incorrect usage of custom failure messages:

# Correct
assert_equal(1, 2, "One is not two!!")

# Incorrect
assert_equal(1, 2, msg: "One is not two!!")
assert_equal(1, 2, message: "One is not two!!")
assert_equal(1, 2, { message: "One is not two!!" })

The fixes are quite straight forward, but it will be a big PR.

andyw8 commented 1 month ago

Worth having a type alias for T.nilable(T.any(String, Symbol, Proc))?

vinistock commented 1 month ago

Worth having a type alias for T.nilable(T.any(String, Symbol, Proc))?

That's not possible because our CI checks to see if all constants exist in the runtime and the type alias wouldn't.

amomchilov commented 1 month ago

@Morriar Better yet, I'd rather have them in the same order as https://github.com/minitest/minitest/blob/master/lib/minitest/assertions.rb

Like so: https://github.com/Shopify/rbi-central/commit/8cf2e738d065e3972b0576ced8eaa0503c17eebb

vinistock commented 1 month ago

Sorted based on how they are declared in Minitest and improved the signature for assert_raises, which now returns the right type depending on the exception classes passed.

Shopify / rbi-central