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

prepending a module into a class results in Unexpected Super #332

Open
jubishop opened this issue Mar 14, 2021 · 14 comments
Open

prepending a module into a class results in Unexpected Super #332

jubishop opened this issue Mar 14, 2021 · 14 comments

Comments

@jubishop
Copy link

jubishop commented Mar 14, 2021

module DateTimeDurationExtensions
  def -(other)
    # @type self: DateTime
    super(other)
  end
end

class DateTime < Date
  prepend DateTimeDurationExtensions
end

this results in Unexpected Super, but there is a - method inside the Date class.

@jubishop
Copy link
Author

found my answer:

# @type module: DateTime.class

@jubishop
Copy link
Author

actually then all type checking stops. hmm.

@jubishop jubishop reopened this Mar 14, 2021
@soutaro
Copy link
Owner

soutaro commented Mar 16, 2021

@jubishop Could you give me the RBS files?

Seems like you need to add module-self-type constraint in the module declaration:

interface _DateTimeMinus
  def -: (untyped) -> DateTime
end

module DateTimeDurationExtensions : _DateTimeMinus     # The module have to be mixed to classes compatible to _DateTimeMinus
  def -: (untyped) -> DateTime
end

@jubishop
Copy link
Author

thanks @soutaro for looking into this! I've attached the 3 files to show a simple example of my problem.

PrependExample.zip

@jubishop
Copy link
Author

actually your interface hint worked for me. :) I don't fully understand why tho, and wish I did :) why does having the module adhere to the interface allow super() calls, whereas just defining it in the module itself doesn't?

@jubishop
Copy link
Author

jubishop commented Mar 16, 2021

Here's a more interesting example with the Time class. Even tho my + and - signatures match the core Time signatures precisely, I still get an error that UnexpectedError: undefined method `add_call' for nil:NilClass at the super(other) line.

However if I remove the second definition for -, which is | (Time other) -> Float , then it passes just fine.

@jubishop
Copy link
Author

PrependExample.zip

@soutaro
Copy link
Owner

soutaro commented Mar 16, 2021

In the second example, the error around add_call is a bug of Steep. The module declaration in RBS is empty and the module implementation in .rb has. That inconsistency caused that unexpected error. The add_call error is a bug, but I'm not sure how it should be...

Unfortunately, type checking super calls in modules is not straightforward, and I don't think it's good enough.

I assume that you want to extend DateTime#+ to support other types, String for example. And the RBS would be something like the following:

interface _DateTimeAdd
  def +: (DateTime) -> DateTime
end

module DateTimeDurationExtensions : _DateTimeAdd
  include _DateTimeAdd

  def +: (String) -> DateTime
       | ...
end

class DateTime
  include DateTimeDurationExtensions
end

@jubishop
Copy link
Author

I can't put any combination together that works for me. I'm trying to reuse the Time signatures here, but add Duration as an option, I have this:

interface _TimeDurationExtensions
  def -: (Numeric other) -> Time
       | (Time other) -> Float
  def +: (Numeric other) -> Time
end

module TimeDurationExtensions : _TimeDurationExtensions
  def -: (Duration other) -> Time
  def +: (Duration other) -> Time
end

class Time
  prepend TimeDurationExtensions
end

And any steep check is happy if I pass a Duration but doesn't see the Numeric and Time options coming from the Interface.

@jubishop
Copy link
Author

If I try to include _TimeDurationExtensions I get an error about duplicate method definitions..

@soutaro
Copy link
Owner

soutaro commented Mar 18, 2021

How about this?

class Duration
end

interface _TimeDurationExtensions
  def +: (Numeric other) -> Time
end

module TimeDurationExtensions : _TimeDurationExtensions
  def +: (Duration other) -> Time
       | ...
end

class Time
  prepend TimeDurationExtensions
end
module TimeDurationExtensions
  def +(other)
    case other
    when Duration
      Time.new
    else
      super(other)
    end
  end
end

class Time
  prepend TimeDurationExtensions
end

Time.new + Duration.new
Time.new + 3

@jubishop
Copy link
Author

ohh I didn't understand ... was actual syntax. that works for the + case, but for the - case, which has two built in options (Numeric or Time), it fails with the undefined method "add_call", like this:

class Duration
end

interface _TimeDurationExtensions
  def -: (Numeric other) -> Time
       | (Time other) -> Float
end

module TimeDurationExtensions : _TimeDurationExtensions
  def -: (Duration other) -> Time
       | ...
end

class Time
  prepend TimeDurationExtensions
end
require 'time'

module TimeDurationExtensions
  def -(other)
    case other
    when Duration
      Time.new
    else
      super(other)
    end
  end
end

class Time
  prepend TimeDurationExtensions
end

Time.new - Time.new
Time.new - 3
Time.new - Duration.new

@soutaro
Copy link
Owner

soutaro commented Mar 19, 2021

Hmm, the add_call error looks like a bug of Steep. Or, we can just say Steep is completely broken for prepend... (Changing the prepend to include reports some errors...)

@jubishop
Copy link
Author

Haha well thanks for investigating. I wish I could commit a fix but the code is beyond me 🥲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants