Fully qualify extend T::Generic & T::Helpers

[dejmedus]

• closes Generic types called T can lead to TypeError #990

To ensure T::Generic and T::Helpers don't conflict with user defined T constants, we can fully qualify inserted T::Generic/Helpers to ::T::

[amomchilov]

I'm not sure if we should try to fix a pre-existing extend T::Helpers like this.

It's a nice feature in theory, but narrow. There are many other explicit uses of T that would be problematic (T.untyped, T::Boolean, T.any, ...), which we couldn't patch up as easily/reliably. I'm inclined err of inaction, and require people fix their own explicit usages of T.

I suppose the distinction is: who wrote the T?

If our rewriter wrote it, we should be good stewards and ::T it. If devs wrote it explicitly in their own source, they should choose ::T if they want it (or rename their generic arg to not be called T).

[amomchilov]

@Morriar wdyt?

[dejmedus]

Makes sense! I can drop those commits and open up something separately if we decide we want the behaviour later on. In that case I think it would make sense to remove already_extends? (rather than fixing) and just allow, potentially, a duplicate extends

RBS + pre-existing extends

# bin/console 
source = <<~RUBY
  # typed: true

  # @final
  #: [T]
  class Box
    extend T::Helpers
  end
RUBY

puts Spoom::Sorbet::Translate.rbs_comments_to_sorbet_sigs(
  source,
  file: "example.rb",
)

# typed: true

class Box
  extend ::T::Helpers # spoom inserted (still needed, otherwise we `extend` after `final!` unless we insert below)

  final!

  extend ::T::Generic

  T = type_member

  extend T::Helpers # source 
end

[amomchilov]

In a similar vein, I think that's too narrow of a problem-solve to be worth it. It would make their extend T::Helpers, but other code like T.nilable still fail.

I think it's a simpler (more predictable) mental model for the developer if #: [T] always caused unqualified accesses to fail, consistently

[dejmedus]

Agreed:) I removed already_extends? so it will continue to behave as it has been (inserting without checking for pre-existing) and a [T] + existing T::Helpers/Generic will error as expected

[Morriar]

Let's add a test doing this:

# @final
class Box
  extend T::Helpers
end

Do we add the extend anyway?

class Box
  extend T::Helpers
  extend ::T::Helpers
  final!
end

[dejmedus]

Yes:) Since already_extends? wasn't finding the node we've been inserting a duplicate even if there was a pre-existing. If we wanted already_extends? fixed instead of removed we'd need some logic to insert in the correct place (after the extends). Your example ends up as:

# typed: true

class Box
  extend ::T::Helpers # spoom inserted 

  final!

  extend T::Helpers # pre-existing
end

[Morriar]

Let's add this as a test to show the behavior is expected

[dejmedus]

Added here:)

[dejmedus]

I moved the already_extends? removal out of this PR to #1014 so it can be attached to its own issue, but I've left the test here as behaviour is the same