Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anonymous modules within a module body are recognized as the part of the module #1010

Open
palkan opened this issue Jun 23, 2019 · 3 comments
Labels
enhancement New feature or surprising current feature good first issue Good for newcomers

Comments

@palkan
Copy link
Contributor

palkan commented Jun 23, 2019

Input

→ View on sorbet.run

# typed: true

module A
  M = Module.new do
    def empty?
      false
    end
  end

  def empty?(val)
    val.empty?
  end
end

Observed output

editor.rb:10: Method A#empty? redefined without matching argument count. Expected: 0, got: 1 https://srb.help/4010
    10 |  def empty?(val)
          ^^^^^^^^^^^^^^^
    editor.rb:5: Previous definition
     5 |    def empty?
            ^^^^^^^^^^
Errors: 1

Expected behavior

No errors.


The provided example is the minimal reproduction.
The real-world example involved using(Module.new do ... end) and could be found here: https://github.com/palkan/rubanok/blob/b4431ac1e13600036315472f863a345675522677/lib/rubanok/rule.rb#L47-L65

@palkan palkan added bug Something isn't working unconfirmed This issue has not yet been confirmed by the Sorbet team labels Jun 23, 2019
@DarkDimius DarkDimius added good first issue Good for newcomers and removed unconfirmed This issue has not yet been confirmed by the Sorbet team labels Jun 23, 2019
@DarkDimius
Copy link
Collaborator

Thank you for a great issue report with a great reproducer!
For those interested to contribute, this should be super easy to fix:
https://github.com/sorbet/sorbet/blob/master/dsl/ClassNew.cc is already handling Class.new, it should additionally handle Module.new.

@DarkDimius DarkDimius added enhancement New feature or surprising current feature and removed bug Something isn't working labels Jun 23, 2019
@JoshCheek
Copy link

The general case for this is going to be super tricky, If you see a def in a block, it's unclear what it is defined on since that can change at run time. Eg:

M1 = Module.new do
  def empty?
    'first'
  end
end

def Module.new(&b)
  b.call
  allocate
end

M2 = Module.new do
  def empty?
    'second'
  end
end

Object.new.extend(M1).empty? # => "first"
empty?                       # => "second"
Object.new.extend(M2).empty? # => NoMethodError: private method `empty?' called for #<Object:0x00007f8098169cf0>

@searls
Copy link

searls commented Jun 17, 2023

This is also true of anonymous classes, and is preventing me from enabling types for one of the critical sections of Mocktail.

Here's a repro:

# typed: true

class ClassMaker
  def initialize
    # Initializing ClassMaker
  end

  def make
    Class.new(Object) {
      def initialize(*args, **kwargs, &blk)
        # Initializing Some totally unrelated class
      end
    }
  end
end
class_maker.rb:10: Method ClassMaker#initialize redefined without matching argument count. Expected: 0, got: 2 https://srb.help/4010
    10 |      def initialize(*args, **kwargs, &blk)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    class_maker.rb:4: Previous definition
     4 |  def initialize
          ^^^^^^^^^^^^^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or surprising current feature good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants