Add test files for actionpack, activemodel, activerecord, and activesupport

[connorshea]

I have no idea what this does to sorbet if this is merged and you try to update sorbet-typed.

I've included an intentionally failing method to show that this is working. However, it only allows files that have no type errors, there's no way to test that sorbet catches incorrect code.

This is a simplified version of my routes.rb file that I showed in #95.

The idea to do this was started by #42 and re-upped in #102 by @panthomakos.

cc: @DarkDimius @jez I know this isn't the test harness you wanted, and I'd still like to get that eventually, but I'm curious if you think this solution would be workable? Do you see any significant flaws with this approach?

[connorshea]

I changed the git remote of sorbet-typed in my /.cache/sorbet/sorbet-typed directory, and it does not pull in anything from test/ as far as I can tell

[connorshea]

Alright, I've added more tests and explained how the tests work in the README. Plus I've fixed a few bugs I found with the existing signatures :) This should be good to review now.

[panthomakos]

I think this is a great idea. One thing that occurred to me is that if a gem has breaking changes between one version and the next, your tests may not be able to account for that. For example:

diff --git a/lib/activesupport/>=6.0.0.rc1/activesupport.rbi b/lib/activesupport/>=6.0.0.rc1/activesupport.rbi
index 9c5e29f..6a146fe 100644
--- a/lib/activesupport/>=6.0.0.rc1/activesupport.rbi
+++ b/lib/activesupport/>=6.0.0.rc1/activesupport.rbi
@@ -1,6 +1,9 @@
 # typed: strong

 class Array
+  sig { params(a: String).void }
+  def foo(a); end
+
   sig { params(elements: T.untyped).returns(T::Array[T.untyped]) }
   def excluding(*elements); end

diff --git a/lib/activesupport/all/activesupport.rbi b/lib/activesupport/all/activesupport.rbi
index dd65c02..c2af914 100644
--- a/lib/activesupport/all/activesupport.rbi
+++ b/lib/activesupport/all/activesupport.rbi
@@ -241,6 +241,9 @@ class String
 end

 class Array
+  sig { params(a: Integer).void }
+  def foo(a); end
+
   sig { returns(T::Boolean) }
   def blank?; end

diff --git a/lib/activesupport/test/activesupport_test.rb b/lib/activesupport/test/activesupport_test.rb
index f025cb8..ecf9105 100644
--- a/lib/activesupport/test/activesupport_test.rb
+++ b/lib/activesupport/test/activesupport_test.rb
@@ -1,5 +1,7 @@
 # typed: true

+Array.new.foo('hi')
+
 module ActiveSupportNumberHelperTest
   extend ActiveSupport::NumberHelper

results in:

lib/activesupport/test/activesupport_test.rb:3: Expected Integer but found String("hi") for argument a https://srb.help/7002
     3 |Array.new.foo('hi')
        ^^^^^^^^^^^^^^^^^^^
    lib/activesupport/all/activesupport.rbi:244: Method Array#foo has specified a as Integer
     244 |  sig { params(a: Integer).void }
                         ^
  Got String("hi") originating from:
    lib/activesupport/test/activesupport_test.rb:3:
     3 |Array.new.foo('hi')
                      ^^^^
Errors: 1

One way you can fix this is to include tests in each sub-directory instead and then modify the CI script. Here is an example: panthomakos@f53acfb which resolves this issue.

[DarkDimius]

This seems like a clear improvement to me & it has already discovered existing bugs.
Thank you, this is great! LGTM

If someone wants to improve on this, improvements are also welcome!

[connorshea]

@panthomakos 👍 you should open a PR to change that, it looks like a good idea (though I'd prefer if the files were named gem_name_test.rb still, and you'll have to check whether Sorbet pulls down the files properly when you organize the project like that.

You can go into .cache/sorbet/sorbet-typed/ and use git remote set-url origin URL_GOES_HERE to change the git remote that Sorbet uses to get sorbet-typed stuff, and you'll also need to merge the change into the master branch on your fork since sorbet-typed can't be configured to use a different branch. Then update the sorbet-typed files in a project and you'll get whatever is in your fork of the project.

I really should open an issue to make this easier...

[panthomakos]

Thank you for the write-up. I'll put a PR together!