Skip to content

Commit

Permalink
ClassModule#superclass= accepts a ClassModule as an argument (#1222)
Browse files Browse the repository at this point in the history
It is necessary for ClassModule's instance variable @superclass to
always be a String (or nil) so that the class can be saved with
`#marshal_dump` and loaded with `#marshal_load`.

However, there's no type checking being done, which allows a bug like
the one reported in #1221 (which was introduced in #1217) that sets
superclass to a ClassModule. That bug requires:

- setting a superclass to a NormalClass
- marshal_save
- marshal_load (which raises an exception)

With this change, passing a ClassModule to ClassModule#superclass= is
explicitly allowed by saving the full name of the ClassModule in the
@superclass instance variable.
  • Loading branch information
flavorjones authored Dec 2, 2024
1 parent a4f13d2 commit 9ced6d5
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
15 changes: 14 additions & 1 deletion lib/rdoc/code_object/class_module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -705,10 +705,23 @@ def superclass

##
# Set the superclass of this class to +superclass+
#
# where +superclass+ is one of:
#
# - +nil+
# - a String containing the full name of the superclass
# - the RDoc::ClassModule representing the superclass

def superclass=(superclass)
raise NoMethodError, "#{full_name} is a module" if module?
@superclass = superclass
case superclass
when RDoc::ClassModule
@superclass = superclass.full_name
when nil, String
@superclass = superclass
else
raise TypeError, "superclass must be a String or RDoc::ClassModule, not #{superclass.class}"
end
end

##
Expand Down
20 changes: 20 additions & 0 deletions test/rdoc/test_rdoc_class_module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,26 @@ def test_superclass
assert_equal @c3_h1, @c3_h2.superclass
end

def test_setting_superclass
@c1.superclass = nil
assert_nil(@c1.superclass)
assert_nil(@c1.instance_variable_get("@superclass")) # proxy to test marshalling

@c1.superclass = @c4_c4.full_name
assert_equal(@c1.superclass, @c4_c4)
assert_equal(@c4_c4.full_name, @c1.instance_variable_get("@superclass"))

@c1.superclass = @c4_c4
assert_equal(@c1.superclass, @c4_c4)
assert_equal(@c4_c4.full_name, @c1.instance_variable_get("@superclass"))

# we could support this if we find we need to in the future.
assert_raise(TypeError) { @c1.superclass = Object }

# but this doesn't make sense.
assert_raise(TypeError) { @c1.superclass = Object.new }
end

def test_super_classes
rdoc_c3_h1 = @xref_data.find_module_named('C3::H1')
rdoc_object = @xref_data.find_module_named('Object')
Expand Down

0 comments on commit 9ced6d5

Please sign in to comment.