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

cop ColonMethodCall shouldn't forbid constructor like calls #425

Closed
markijbema opened this issue Aug 10, 2013 · 3 comments · Fixed by #426
Closed

cop ColonMethodCall shouldn't forbid constructor like calls #425

markijbema opened this issue Aug 10, 2013 · 3 comments · Fixed by #426

Comments

@markijbema
Copy link
Contributor

Ruby has the tradition of adding constructor-like methods to instantiate classes, for instance Array() to construct an instance of Array. Some libraries offer this as well, like Nokogiri. Their readme suggests using colons, and I do think it makes sense in this case, and should be excepted from the ColonMethodCall:

doc = Nokogiri::HTML(open('http://www.google.com/search?q=sparklemotion'))

Opinions?

My suggestion would be to allow method calls which start with a capital letter.

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 10, 2013

:: will probably be removed from Ruby in 3.0 (not sure if Nokogiri's maintainers are aware of this). I generally dislike this style, since some people are initially confused that they are dealing with a class instead of method call, but I'm ok to have this as an option of the cop, disabled by default.

@markijbema
Copy link
Contributor Author

That's not how I would interpret it:

http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/54883

Matz is saying it's not happening before 3.0 at least, but the argument he makes against deprecation is specifically about the exception I'm adding support for here. I'm not against making it optional, but I think it's here to stay.

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 10, 2013

Can't argue with that, no option needed :-) I guess you should update the relevant section of the style guide as well.

kulte pushed a commit to laurelandwolf/onebox that referenced this issue Aug 13, 2014
this is fixed in the latest rubocop (head), so the
current version might still give errors about this.

For more information: rubocop/rubocop#425
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

Successfully merging a pull request may close this issue.

2 participants