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

false offense for Style/CommentedKeyword #4886

Closed
chuck-john opened this issue Oct 19, 2017 · 7 comments
Closed

false offense for Style/CommentedKeyword #4886

chuck-john opened this issue Oct 19, 2017 · 7 comments

Comments

@chuck-john
Copy link

chuck-john commented Oct 19, 2017

Expected behavior

I expect rubocop to allow literal octothorpes and interpolated strings in method arguments.

Actual behavior

An offense was generated for an interpolated string in a method argument. The offense claims this is a comment, which triggered a violation of the Style/CommentedKeyword cop.

Steps to reproduce the problem

  1. Define a method with an octothorpe in a string as a default method argument.
  2. Run $ rubocop
  3. An offense similar to the screenshot below should get generated.

rubocop v0 51 0 - style commentedkeyword

RuboCop version

0.51.0

@codebycliff
Copy link

I have another example of a false positive:

image

@chuck-john chuck-john changed the title false positive for Style/CommentedKeyword false offense for Style/CommentedKeyword Oct 19, 2017
@Drenmi
Copy link
Collaborator

Drenmi commented Oct 19, 2017

Hm. I'm guessing this cop uses a regular expression instead of checking for comments outside the AST. 😅

@backus
Copy link
Contributor

backus commented Oct 19, 2017

Hm. I'm guessing this cop uses a regular expression instead of checking for comments outside the AST. 😅

We should add an internal affairs cop that bans this. Seems to be the source of a lot of bugs

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 19, 2017

@backus True. And I guess we don't review these things very carefully. 😆

@wurde
Copy link

wurde commented Oct 20, 2017

Same problem here.

app/helpers/bootstrap/alert_helper.rb:49:40: C: Do not place comments on the same line as the def keyword.
    def alert_link(name = nil, path = '#', options = {}, &block)
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^

@reitermarkus
Copy link
Contributor

Also here:

def self.tap_paths(name, taps = Dir["#{HOMEBREW_LIBRARY}/Taps/*/*/"])

@michniewicz
Copy link
Contributor

same for # inside Regexp:

foo.rb:2:20: C: Do not place comments on the same line as the def keyword.
  def foo(type = /^#{self.bar}_/)

michniewicz added a commit to michniewicz/rubocop that referenced this issue Oct 27, 2017
Commit also contains cop improvements:
Instead of going through each line of the processed_source we can easily take comments only and cop only those lines, where comments are located.
Instead of splitting line with `#` symbol we take column values from `comment.location`.
bbatsov pushed a commit that referenced this issue Oct 28, 2017
Commit also contains cop improvements:
Instead of going through each line of the processed_source we can easily take comments only and cop only those lines, where comments are located.
Instead of splitting line with `#` symbol we take column values from `comment.location`.
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

8 participants