Ensure only constants that extend Module are parsed from requested paths

[hamishmorgan]

Motivation

I ran into a subtle bug when running tapioca dsl on specific file path containing an arbitrary non-Module constant. e.g:

bin/tapioca dsl path/to.my/file.rb

Where file.rb contains:

require "active_resource"

class Post < ActiveResource::Base
  SOME_CONSTANT = 123
end

Would blow up like this:

tapioca/lib/tapioca/dsl/compiler.rb:65:in `<': comparison of Integer with Class failed (ArgumentError)

                k < klass && !k.singleton_class?
                    ^^^^^
        from tapioca/lib/tapioca/dsl/compiler.rb:65:in `block in descendants_of'
        from tapioca/lib/tapioca/dsl/compiler.rb:64:in `select'
        from tapioca/lib/tapioca/dsl/compiler.rb:64:in `descendants_of'
        ⋮
        from tapioca/lib/tapioca/dsl/compilers/active_resource.rb:78:in `gather_constants'
        ⋮
        from tapioca/lib/tapioca/dsl/compiler.rb:44:in `processable_constants'
        ⋮

This would happen if any loaded compiler used the Tapioca::Dsl::Compiler#descendants_of method in it's #gather_constants implementation. (The ActiveResource compiler from the example above is one such).

It only occurs when requesting specific paths from the CLI interface; not when run on the whole project, or when requesting specific symbols.

The cause of the issue is that Taioca::Dsl::Compiler#descendants_of implementation assumes everything in @@requested_constants class attribute implements #<, which should be true because @@requested_constants is typed Array[Module]. However, when requesting paths from CLI, @@requested_constants is populated (in part) by Tapioca::Commands::AbstractDsl#constantize using Object.const_get, which grabs all constants in the requested file, regardless of their type. Object.const_get is T.untyped so the type mismatch is never caught.

Implementation

The solution I'm proposing here is to silently throw away all constants that don't implement Module.

An alternative solution I considered was changing the logic in Taioca::Dsl::Compiler#descendants_of to be resilient to other types, but since the @@requested_constants attribute is clearly typed Array[Module] I figured we shouldn't have allowed other types in there in first place.

Given we should only be producing Module instances, the choice is whether to throw them away or fail when they are encountered. Failing would make it so you could never mix constants with DSL, which is silly so I went with throwing them away.

I also considered adding a warning message, but generally there's nothing wrong with using constants in DSLS, so there's
nothing to complain about. Hence, they a thrown away silently.

I'm not convinced this is the best solution. I'm not terribly experienced with Ruby so there's likely a better way. My priority was communicate the issue and make it concrete with a failing test. I'm very open to suggestion on a better fix.

Tests

Added a test that fails without these changes. It generates RBIs using the CLI by file reference, for an ActiveResource implementation that contains an arbitrary constant.

[bitwise-aiden]

I'll let others more versed in Ruby object model comment on how to filter non-modules, but this makes a lot of sense to me.

In terms of the offending code with k < klass would it make sense to invert that to minimize the likelihood of k having overridden #<?

[hamishmorgan]

In terms of the offending code with k < klass would it make sense to invert that to minimize the likelihood of k having overridden #<?

I actually tried that. Flipping it around you get

`>': compared with non class/module (TypeError)

[hamishmorgan]

@KaanOzkan ready for re-review 🙂