Shopify / Shopify/type_toolkit

Protect against misbehaved `inherited` hooks that don't call `super`

Open
#45 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
3
Forks
5
Avg merge
8d 23h
Merged PRs (30d)
8

Description

Relying on this inherited method means we only restore .new when user-defined self.inherited hooks call super. If an abstract class defines an inherited hook without super, concrete subclasses keep the abstract .new and can't be instantiated:

class Base
  abstract!

  def self.inherited(_subclass)
    # no super
  end

  abstract def call; end
end

class Impl < Base
  def call; end
end

Impl.new # raises CannotInstantiateAbstractClassError

I wonder if we should install this as a prepended singleton hook, or otherwise wrap/chain the existing hook, and add a regression test so subclass instantiation doesn't depend on user hooks calling super.

Originally posted by @Morriar in https://github.com/Shopify/type_toolkit/pull/3#discussion_r3826337769

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the implementation of the abstract! behavior and its self.inherited hook. Reproduce the Base and Impl example from the issue, then investigate how the hook can be protected when user-defined inherited methods omit super. Add the requested regression test and verify that the concrete subclass can be instantiated.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.