Fix incorrect method visibility when merging with existing store cache

[4lllex]

Running Store#save with already existing cache incorrectly sets method visibility to :private on all methods. @visibility gets set during marshal_load:

rdoc/lib/rdoc/code_object/class_module.rb

Lines 440 to 448 in bb8b2d4

	visibilities.each do |visibility, methods|
	@visibility = visibility
	methods.each do |name, file|
	method = RDoc::AnyMethod.new name, singleton: type == 'class'
	method.record_location RDoc::TopLevel.new file
	add_method method
	end
	end

and is reused during merge -> add_method:

rdoc/lib/rdoc/code_object/class_module.rb

Lines 539 to 542 in bb8b2d4

	merge_collections method_list, cm.method_list, other_files do |add, meth|
	if add then
	add_method meth
	else

rdoc/lib/rdoc/code_object/context.rb

Line 494 in bb8b2d4

method.visibility = @visibility

Example:

# rdoc v7.2.0
Dir.mktmpdir do |op_dir|
  store = RDoc::Store.new(RDoc::Options.new.tap { it.op_dir = op_dir })
  top = store.add_file("file.rb")
  meth = RDoc::AnyMethod.new(nil, "method") rescue RDoc::AnyMethod.new("method")
  meth.record_location(top)
  klass = top.add_class(RDoc::NormalClass, "Object").tap { it.add_method(meth) }
  store.save # new cache
  p meth.visibility # => :public
  store.save # merge
  p meth.visibility # => :private
end

Fixes #346

[tompng]

Nice catch on the root cause - merge is definitely reusing a stale @visibility left over from marshal_load.

I'd suggest fixing this one level down instead. Visibility is lexical state, and Parser::Ruby already tracks it separately in its own @visibility - it never touches Context#visibility. So add_method overwriting method.visibility with the container's @visibility is a leftover from the old design, and every caller passing in an already-complete method has to work around it:

• marshal_load sets @visibility just so add_method picks it up
• merge (this bug) inherits what marshal_load left behind
• prepare_to_embed assigns self.visibility = code_object.visibility back onto
  the container, with a comment explaining the workaround
• Parser::Ruby orders its calls carefully:
  a.visibility = visibility # should set after adding to container

Dropping the assignment in add_method (and the matching one in add_attribute) fixes this at the root and lets all four workarounds go away.
No new argument needed - AnyMethod already defaults to :public and every caller sets visibility itself, so marshal_load just does method.visibility = visibility directly.