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

Enhance style/filename to support RubyGems convention #1569

Closed
e2 opened this issue Jan 13, 2015 · 6 comments
Closed

Enhance style/filename to support RubyGems convention #1569

e2 opened this issue Jan 13, 2015 · 6 comments

Comments

@e2
Copy link
Contributor

e2 commented Jan 13, 2015

I'm just putting this up for discussion (and since I might be implementing this anyway).

Style would be based on this: http://guides.rubygems.org/name-your-gem

Sadly, RuboCop itself doesn't follow the convention (should be 'rubo_cop', not 'rubocop').

Personally, it took me a while to learn to write RuboCop instead of Rubocop (because I assume the name based on the file).

Historically, Rails made the transition in the past (e.g. now it's require 'active_record').

The value is mostly when mapping a module/class name to a file and vice verse.

Not following the convention can create ambiguity (although rare), e.g. without the underscore in filenames, both NoWhere and NowHere would resolve to 'nowhere'. (Instead of 'no_where' and 'now_here' respectively).

And it creates a mess when doing the reverse (e.g. what's the module name in the file 'nowhere.rb' ? - now we have to search for all matching constants).

Either way, there's probably little reason to not follow the RubyGems convention in new projects.

@e2
Copy link
Contributor Author

e2 commented Jan 13, 2015

To make this less boring, here are some examples of ambiguity: childrenslaughter, everyoneshavingbabies, needsatan, nowthatcherisdead, kidsexchange.

Sure, not having to put the underscore is "convenient", but there's a price tag on it.

@e2
Copy link
Contributor Author

e2 commented Jan 13, 2015

A reasonable exception would be if the word has a single letter, e.g. 'RSpec', (because 'r_spec' would really be pushing it).

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 17, 2015

I'm pretty sure the cop follows the RubyGems conventions as it was derived from it. You can specify certain exceptions like RSpec and RuboCop in the cop's config. I understand RuboCop violates the convention, but I don't care that much about this - it's hard for me to write rubo_cop and changing the filenames will break too much related code, anyways.

I'm not sure what exactly would you like to be changed in the cop.

@e2
Copy link
Contributor Author

e2 commented Jan 17, 2015

It's about matching the module name to the file name, so it's not about checking just the file name, but taking the class name into account.

Maybe it would be best to just start with extreme cases, e.g. with the RubyGems example:

require 'net/http/digest_auth' # -> Net::HTTP::DigestAuth

.. having a file 'digest-auth' would be an offense. So would 'http_digest_auth' (because the class is not 'HttpDigestAuth'.

If a file name has many parts to it (using '-' or '_') - the filename shouldn't contain a definition of a class that doesn't match - because requiring that file should define the expected, namespaced class/module name.

I'm pretty sure being strict here (like with RuboCop) would break too much code - so I'm wondering if "obviously misleading" examples can't be covered at least (where '-' and '_' are provided, but incorrectly used).

So I think this should be a new cop at least, probably more related to this one: https://github.com/bbatsov/rubocop/blob/master/lib/rubocop/cop/style/class_and_module_camel_case.rb

The problem is it would have to ideally take the whole path, and not just the filename. So 'net/http/foo' should be expected to define a class or module Net::Http::Foo (which is usually the case).

Sticking with the convention is most useful with auto loading "plugins" where a module can be loaded/initialized based on the file name or module name. Without a convention, the filename and module would have to provided - just like e.g. for Kernel.autoload calls.

And in general, if I need to use 'Foo::BarBaz', I expect `require "foo/bar_baz" to work - or I suddenly have to know both the class name and namespace ... and the filename.

Do you think this is worth tackling with PR? Or is it unlikely to be accepted?

@alexdowad
Copy link
Contributor

Maintainers: do you want this? It seems to be a fairly common request (there are other similar issues in the issue tracker). Perhaps disabled by default?

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 23, 2015

Yeah, I'm fine with this.

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

No branches or pull requests

3 participants