Implement abstract classes

[amomchilov]

No description provided.

[amomchilov]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• Implement abstract classes #3 👈 (View in Graphite)
• Implement interfaces #2
• Initial README #13
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Morriar]

I think there's a bug here: self inside AbstractClass#new is the abstract class itself (e.g. Widget), so self.class is Class, not the abstract class. The error message would print "Class is declared as abstract" instead of the actual class name.

I wonder if we should use name instead?

Suggested change

	raise CannotInstantiateAbstractClassError, "#{self.class.name} is declared as abstract; it cannot be instantiated"
	raise CannotInstantiateAbstractClassError, "#{name} is declared as abstract; it cannot be instantiated"

[amomchilov]

True. I have to add tests for abstract class methods, and I'll verify their name is correct in the message.

[Morriar]

My concern here is that result holds a direct reference to @__abstract_methods, so the subsequent result.merge(methods) on line 32 mutates the underlying ivar Set in place. After the first include_super: true call, @__abstract_methods would contain all ancestor methods too, meaning future calls would return incorrect (and growing) results.

I wonder if we should dup it here to avoid the mutation?

Suggested change

	result = @__abstract_methods #: Set[Symbol]?
	result = @__abstract_methods&.dup #: Set[Symbol]?

[amomchilov]

This hasn't been an issue with the current test suite, but I'll look into it!