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

Add specs for Module.used_refinements #1028

Merged

Conversation

AI-Mozi
Copy link
Contributor

@AI-Mozi AI-Mozi commented May 15, 2023

#1016
[Feature #14332]

Module.used_refinements has been added.

@AI-Mozi AI-Mozi force-pushed the add_specs_for_module_used_refinements branch from 461e1c1 to 67a6aaa Compare May 15, 2023 12:55
@AI-Mozi AI-Mozi marked this pull request as ready for review May 15, 2023 13:24
@andrykonchin
Copy link
Member

andrykonchin commented May 15, 2023

suggestion: There are the following additional cases that I would consider:

  • a module doesn't have imported refinements at all, so used_refinements returns []
  • a module includes another module that calls using and imports some refinement - this refinement shouldn't be returned by calling #used_refinements in the child module

core/module/used_refinements_spec.rb Outdated Show resolved Hide resolved
core/module/used_refinements_spec.rb Outdated Show resolved Hide resolved
core/module/used_refinements_spec.rb Show resolved Hide resolved
core/module/used_refinements_spec.rb Show resolved Hide resolved
@AI-Mozi AI-Mozi force-pushed the add_specs_for_module_used_refinements branch from 67a6aaa to 75d4a14 Compare May 16, 2023 08:02
@AI-Mozi
Copy link
Contributor Author

AI-Mozi commented May 16, 2023

Could You please provide an example of second suggestion?

@andrykonchin
Copy link
Member

andrykonchin commented May 16, 2023

Do you mean an example of usage of ScratchPad? There is an example in another my comment #1027 (comment):

# language/next_spec.rb
ScratchPad.record []
 
[1].each do |i|
  begin
    ScratchPad << :begin
    next
  ensure
    ScratchPad << :ensure
  end
end
 
ScratchPad.recorded.should == [:begin, :ensure]

@AI-Mozi
Copy link
Contributor Author

AI-Mozi commented May 16, 2023

I was thinking about this
a module includes another module that calls using and imports some refinement - this refinement shouldn't be returned by calling #used_refinements in the child module
Sorry if previous comment was unclear.

@andrykonchin
Copy link
Member

andrykonchin commented May 16, 2023

I was thinking about this
a module includes another module that calls using and imports some refinement - this refinement shouldn't be returned by calling #used_refinements in the child module
Sorry if previous comment was unclear.

I meant the following case:

# it ignores refinements imported in a module that is included into the current one

m1 = Module.new do
  refine Integer do
  end
end

m2 = Module.new do
  using m1
end

Module.new do
  include m2

  p Module.used_refinements # => I would expect `[]`
end

There is another case that may be worth to check - when some refinements are declared in a module, that is included in a module that is passed as argument for using method:

# it returns refinements even not defined directly in a module refinements are imported from

m1 = Module.new do
  refine Integer do
  end
end

m2 = Module.new do
  include m1
end

Module.new do
  using m2

  p Module.used_refinements # => [#<refinement:Integer@#<Module:0x000000010391f3d0>>]
end

@AI-Mozi AI-Mozi force-pushed the add_specs_for_module_used_refinements branch from 75d4a14 to a5ae679 Compare May 16, 2023 13:00
@andrykonchin
Copy link
Member

Thank you for the specs!

@andrykonchin andrykonchin merged commit a3b248a into ruby:master May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants