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

Fix crash when parsing activesupport's rbi #136

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dorner
Copy link

@dorner dorner commented Jul 19, 2023

Core RBI files are allowed to have multiple overloading sig definitions. For example, this is taken from activesupport's rbi in sorbet-typed:

  # these are the sigs for Date- in the stdlib
  # https://github.com/sorbet/sorbet/blob/3910f6cfd9935c9b42e2135e32e15ab8a6e5b9be/rbi/stdlib/date.rbi#L373
  # note that if more sigs are added to sorbet you should replicate them here
  # check sorbet master: https://github.com/sorbet/sorbet/blob/master/rbi/stdlib/date.rbi
  sig {params(arg0: Numeric).returns(T.self_type)}
  sig {params(arg0: Date).returns(Rational)}
  # these sigs are added for activesupport users
  sig {params(arg0: ActiveSupport::Duration).returns(T.self_type)}
  def -(arg0); end

Parlour currently can't handle this and straight out crashes with the error: node after a sig must be a method definition

This PR fixes it so at least it doesn't crash. It doesn't go further by amalgamating all the sigs together into one (e.g. trying to figure out all the possible permutations of parameters and return values). Might be worth documenting this as a known bug.

@dorner
Copy link
Author

dorner commented Jul 20, 2023

Hmm... did not realize that sorbet-typed was no longer the place to get RBI files for gems. Using rbi-central doesn't have this issue. Not sure if this is worth merging then.

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

Successfully merging this pull request may close these issues.

None yet

1 participant