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 new in subclasses of abstract classes when tap is redeclared #7536

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

Bo98
Copy link
Contributor

@Bo98 Bo98 commented Dec 14, 2023

Motivation

I have an abstract class that happened to redeclare the tap method to be something else entirely. This used to work until 544f173. Sorbet seems to generally work around tampering like this (see Object.instance_method(:method) a few lines prior) so I reckon the same should happen here.

I could have done:

Object.instance_method(:tap).bind_call(super(*args, &blk)) do |result|
  if result.instance_of?(mod)
    raise "#{mod} is declared as abstract; it cannot be instantiated"
  end
end

but the code-readability benefits from tap are largely lost so it's IMO clearer to just avoid it entirely.

Test plan

See included test.

@Bo98 Bo98 requested a review from a team as a code owner December 14, 2023 16:20
@Bo98 Bo98 requested review from jez and removed request for a team December 14, 2023 16:20
Copy link
Collaborator

@jez jez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jez jez merged commit d151da3 into sorbet:master Dec 14, 2023
15 checks passed
@Bo98 Bo98 deleted the abstract-tap-fix branch December 14, 2023 17:40
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

2 participants