Methods defined on anonymous classes cause typing errors #3609

paracycle · 2020-11-02T21:31:18Z

Input

# typed: true
Class.new do
  def match?(request)
    42
  end
end


foo = T.let(nil, T.nilable(String))
foo.match?("bar")

Observed output

editor.rb:10: Non-private call to private method Object#match? https://srb.help/7031
    10 |foo.match?("bar")
        ^^^^^^^^^^^^^^^^^
    editor.rb:3: Defined as
     3 |  def match?(request)
          ^^^^^^^^^^^^^^^^^^^
Errors: 1

Expected behavior

I would have expected match? to not have been registered as a private instance method of Object, but instead that I got this error:

editor.rb:4: Method match? does not exist on NilClass component of T.nilable(String) https://srb.help/7003
     10 |foo.match?("bar")
        ^^^^^^^^^^^^^^^^^

This seems to be related to the changes in #3505 /cc @bss

The text was updated successfully, but these errors were encountered:

bss · 2020-11-10T20:45:57Z

@paracycle This is definitely related to #3505. I'll take a look to try and figure out what's going on.

bss · 2020-11-10T22:06:25Z

So, what's going on is that classes defined by Class.new is special. There is a rewrite-pass that will rewrite this:

A = Class.new do
  def foo; end
end

To:

class A
  def foo; end
end

The bare case:

Class.new do
  def bar; end
end

Is not rewritten, instead we end up with a symbol table where #bar is defined on Object:

$ ./bazel-bin/main/sorbet -e "Class.new do def foo; end; end; nil.foo" -p symbol-table
👋 Hey there! Heads up that this is not a release build of sorbet.
Release builds are faster and more well-supported by the Sorbet team.
Check out the README to learn how to build Sorbet in release mode.
To forcibly silence this error, either pass --silence-dev-message,
or set SORBET_SILENCE_DEV_MESSAGE=1 in your shell environment.

-e:1: Non-private call to private method Object#foo https://srb.help/7031
     1 |Class.new do def foo; end; end; nil.foo
                                        ^^^^^^^
    -e:1: Defined as
     1 |Class.new do def foo; end; end; nil.foo
                     ^^^^^^^
class ::<root> < ::Object ()
  class ::<Class:<root>>[<AttachedClass>] < ::<Class:Object> ()
    method ::<Class:<root>>#<static-init> (<blk>) @ -e:1
      argument <blk><block> @ Loc {file=-e start=??? end=???}
  class ::Object < ::BasicObject (Kernel) @ https://github.com/sorbet/sorbet/tree/master/rbi/core/object.rbi#L27
    method ::Object#foo : private (<blk>) @ -e:1
      argument <blk><block> @ Loc {file=-e start=??? end=???}

I just compiled a version of sorbet locally without the private-call check, and as expected (based on the symbol table above) it unfortunately doesn't error out:

$ ./bazel-bin/main/sorbet --typed=true -e "Class.new do def foo; end; end; nil.foo" -p symbol-table
👋 Hey there! Heads up that this is not a release build of sorbet.
Release builds are faster and more well-supported by the Sorbet team.
Check out the README to learn how to build Sorbet in release mode.
To forcibly silence this error, either pass --silence-dev-message,
or set SORBET_SILENCE_DEV_MESSAGE=1 in your shell environment.

class ::<root> < ::Object ()
  class ::<Class:<root>>[<AttachedClass>] < ::<Class:Object> ()
    method ::<Class:<root>>#<static-init> (<blk>) @ -e:1
      argument <blk><block> @ Loc {file=-e start=??? end=???}
  class ::Object < ::BasicObject (Kernel) @ https://github.com/sorbet/sorbet/tree/master/rbi/core/object.rbi#L27
    method ::Object#foo : private (<blk>) @ -e:1
      argument <blk><block> @ Loc {file=-e start=??? end=???}

No errors! Great job.

So, the bug is actually that Class.new isn't handled correctly. One way to "fix" it could be to not support that style of Class.new at all, and essentially skip any class/methods defined that way in the symbol table.

paracycle · 2020-11-13T20:24:34Z

Oh wow, great find @bss, thanks for looking into this. We have some instances of bare anonymous class definitions in our codebase, that is mostly to do with some mock objects in test case. That means, those have been infecting our Object method list since the beginning 😲 Your private-call check, thankfully, has surfaced that.

@jez @elliottt Is there a way we can fix this so that bare anonymous class definitions don't end up defining their methods on Object (private or not)?

jez · 2020-11-13T21:18:43Z

I think that the solution @bss suggests of getting rid of those nodes from the AST is the only way to support this in Sorbet.

We can't stick the methods on anonymous class definitions in a rewriter pass because all the rewriter passes operate in parallel, so there's no easy way to coordinate to ensure that anonymous class names are globally unqiue.

paracycle · 2020-11-13T23:52:45Z

... there's no easy way to coordinate to ensure that anonymous class names are globally unqiue

Do we need them to be globally unique and stable? If not, we could just generate some GUIDs during the rewriting process. Not sure about the performance implications of doing that though.

jez · 2020-11-14T03:09:04Z

So actually I thought about it longer and getting unique IDs is actually easy because the pass is parallelized by file, so we could get unique IDs using a scheme like <FileRef::_id>_<Rewriterer::uniqueCounter++>

But then the problem becomes that we can't put these class defs inside method defs, because that is a syntax error in Ruby.

https://sorbet.run/#%23%20typed%3A%20true%0Adef%20before%0A%20%20x%20%3D%20Class.new(Integer)%0Aend%0A%0Adef%20after%0A%20%20%23%20AnonymousClass_%3CFileRef%3A%3A_id%3E_%3CuniqueCounter%2B%2B%3E_x%0A%20%20class%20AnonymousClass_1_1_x%0A%20%20end%0A%0A%20%20x%20%3D%20T.let(T.unsafe(Kernel).raise(%22unimplemented%22)%2C%20T.class_of(Integer))%0Aend

jez · 2020-11-14T03:12:55Z

Do we need them to be globally unique and stable?

Yes, everything that ever ends up in the SymbolTable must be stable, otherwise LSP won't work. (The whole assumption of LSP mode is that if you run Sorbet twice on an identical codebase that the symbol table doesn't change.)

jez · 2020-11-14T03:40:41Z

The other thing: almost every time that local_var = Class.new {...} occurs in a codebase, it's because it couldn't have been written with a normal class def. Specifically, it's relying on the fact that the block passed to Class.new has access to local variables that were in scope lexically. In code:

x = 0
y = Class.new {p x}

is ok, but

x = 0
class AnonymousClass_1_1_y
  p x
end

is an error, because the local variable x is not in scope inside the class def.

Sorbet only has one notion of class definitions—the class ...; end one

So the naive approach to fix this would be:

convert x = Class.new to a class def
move that class def to the end of the file (or end of the containing class)

but if that approach works in a rewriter pass, you may as well just do that yourself in the code verbatim.

The reason why people don't do that is almost always because it's not possible, and they're accessing variables from the closure, which no amount of desugaring will help with.

All of which is to say: Sorbet is somewhat fundamentally incompatible with anonymous classes, and we recommend avoiding them.

There are creative solutions that we could add to Sorbet, for example:

Rewrite x = Class.new {...} to x = T.let(..., Class), and throw away the proc body
Rewrite x = Class.new {...} to x = T.let(..., Class), and rewrite methods inside the proc body from def foo to def <defined_inside_anonymous_class_dont_shadow_things_foo> so they still show up on Object (or the containing class) but they don't interfere with real things

but I don't feel like implementing those so if someone else wants to pick one I'll review it.

magni- · 2025-02-17T02:00:29Z

From @alexevanczuk on the Sorbet slack:

Okay this I think will do the trick:

config.before do
  # In order to avoid test pollution, we can remove the `TestClass` namespace before each test.
  # This is also helpful if the same `T::Struct` is referenced multiple times, you end up with an error that looks like this:
  #
  # ArgumentError:
  #      Attempted to redefine prop :const_1 on class TestClass::ExamplePermissiveStruct
  #
  Object.send(:remove_const, :TestClass) if Object.constants.include?(:TestClass) # rubocop:disable RSpec/RemoveConst,Sorbet/ConstantsFromStrings
  Object.const_set(:TestClass, Module.new)
end

justinhoward · 2025-05-12T17:18:01Z

I've noticed that this issue occurs with any def inside a block, so it's not limited to blocks used with Class.new.

#7111 mentions this, but it was closed as a duplicate. However, I think it points out that the affected scope is a bit larger than this thread would imply.

# typed: true

class Dynamic
  def define(&block)
  end
end

dynamic = Dynamic.new.define do
  def hi
  end
end

class Greeter
end

greet = Greeter.new
greet.hi

editor.rb:17: Non-private call to private method hi on Greeter https://srb.help/7031
    17 |greet.hi
              ^^
    editor.rb:9: Defined in Object here
     9 |  def hi
          ^^^^^^
Errors: 1

playground

In the code I'm working on, this tends to occur in rspec tests when defining test helper methods. Using define_method in place of def is possible as a workaround.

paracycle added bug Something isn't working unconfirmed This issue has not yet been confirmed by the Sorbet team labels Nov 2, 2020

paracycle removed the unconfirmed This issue has not yet been confirmed by the Sorbet team label Nov 13, 2020

jez mentioned this issue Aug 8, 2022

Class.new do .. end method definitions breaks other classes #6193

Closed

jez mentioned this issue Oct 2, 2022

Incorrect code is unreachable error when a call method is defined using class_eval #6448

Closed

fcheung mentioned this issue Nov 18, 2022

Incorrect unreachable code error when using Class.new inside method body #6568

Open

froydnj mentioned this issue Jun 29, 2023

Methods defined in blocks leak into the global namespace #7111

Closed

froydnj mentioned this issue Aug 31, 2023

Dynamically defining a class with a call method affects unrelated code #7271

Closed

magni- mentioned this issue Mar 17, 2025

Add Sorbet/AnonymousClassBlock cop Shopify/rubocop-sorbet#298

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Methods defined on anonymous classes cause typing errors #3609

Methods defined on anonymous classes cause typing errors #3609

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Methods defined on anonymous classes cause typing errors #3609

Methods defined on anonymous classes cause typing errors #3609

Comments

Input

Observed output

Expected behavior

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!