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

Check class collision for scaffold controller generator is broken #37543

Closed
kdiogenes opened this issue Oct 23, 2019 · 3 comments · Fixed by #37644
Closed

Check class collision for scaffold controller generator is broken #37543

kdiogenes opened this issue Oct 23, 2019 · 3 comments · Fixed by #37644
Labels

Comments

@kdiogenes
Copy link

Sorry, but I was unable to produce an executable test case for generators from the templates. I'm getting cannot load such file when require "generators/generators_test_helper".

Steps to reproduce

Add the following test case to scaffold_controller_generator_test.rb:

def test_check_class_collision
  Object.send :const_set, :UsersController, Class.new
  content = capture(:stderr) { run_generator ["User"] }
  assert_match(/The name 'UsersController' is either already used in your application or reserved/, content)
ensure
  Object.send :remove_const, :UsersController
end

Expected behavior

Test pass (a collision is detected)

Actual behavior

Test fail (no collision detected)

This generator creates a UsersControllers, but checks UserController for collision. From the check_class_collision implementation I guess that the assumption is wrong. If I understand right, respond_to?(:controller_class_name) will always return false, since controller_class_name is private. In 2017 it was protected and prior to ruby 1.9 I guess that it was returning true based on this ruby issue.

Maybe named_base_test.rb could have exposed this problem if its assert_name implementation doesn't use send. I think everything being tested here is expected to be public, right?

Let me know if my assumptions are right, I'm looking to work in a PR!

@kdiogenes kdiogenes changed the title Scaffold controller generators check class collision is broken Scaffold controller generator check class collision is broken Oct 23, 2019
@kdiogenes kdiogenes changed the title Scaffold controller generator check class collision is broken Check class collision for scaffold controller generator is broken Oct 23, 2019
@y-yagi
Copy link
Member

y-yagi commented Oct 27, 2019

Can you please provide a sample application that reproduces the issue?

@y-yagi y-yagi added the more-information-needed When reporter needs to provide more information label Oct 27, 2019
@kdiogenes
Copy link
Author

I provided an example application in https://github.com/kdiogenes/generator-class-collision. In the README there I already put the output for the commands rails g scaffold_controller name:string email:string and rails g controller Users name:string email:string.

For the first, there is no complaint about UsersController already being used in the application.

@rails-bot rails-bot bot removed the more-information-needed When reporter needs to provide more information label Oct 27, 2019
@y-yagi
Copy link
Member

y-yagi commented Oct 29, 2019

Thanks for your example application. I understood the issue.
Your understanding is correct and we need to fix that.

We don't want to expose the APIs unless really need it. In this case, it is better to specify include_all option of respond_to?. Could you fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants