Skip to content

Commit

Permalink
[Fix rubocop#4663] Add new CommentedKeyword cop
Browse files Browse the repository at this point in the history
  • Loading branch information
donjar committed Oct 6, 2017
1 parent 2677293 commit 3ab1b96
Show file tree
Hide file tree
Showing 8 changed files with 273 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* [#4690](https://github.com/bbatsov/rubocop/issues/4690): Add new `Lint/UnneededRequireStatement` cop. ([@koic][])
* [#4813](https://github.com/bbatsov/rubocop/pull/4813): Add new `Style/StderrPuts` cop. ([@koic][])
* [#4796](https://github.com/bbatsov/rubocop/pull/4796): Add new `Lint/RedundantWithObject` cop. ([@koic][])
* [#4663](https://github.com/bbatsov/rubocop/issues/4663): Add new `Style/CommentedKeyword` cop. ([@donjar][])

### Bug fixes

Expand Down
4 changes: 4 additions & 0 deletions config/enabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,10 @@ Style/CommentAnnotation:
StyleGuide: '#annotate-keywords'
Enabled: true

Style/CommentedKeyword:
Description: 'Do not place comments on the same line as certain keywords.'
Enabled: true

Style/ConditionalAssignment:
Description: >-
Use the return value of `if` and `case` statements for
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@
require_relative 'rubocop/cop/style/colon_method_call'
require_relative 'rubocop/cop/style/command_literal'
require_relative 'rubocop/cop/style/comment_annotation'
require_relative 'rubocop/cop/style/commented_keyword'
require_relative 'rubocop/cop/style/conditional_assignment'
require_relative 'rubocop/cop/style/copyright'
require_relative 'rubocop/cop/style/def_with_parentheses'
Expand Down
81 changes: 81 additions & 0 deletions lib/rubocop/cop/style/commented_keyword.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# This cop checks for comments put on the same line as some keywords.
# These keywords are: `begin`, `class`, `def`, `end`, `module`.
#
# Note that some comments (such as `:nodoc:` and `rubocop:disable`) are
# allowed.
#
# @example
# # bad
# if condition
# statement
# end # end if
#
# # bad
# class X # comment
# statement
# end
#
# # bad
# def x; end # comment
#
# # good
# if condition
# statement
# end
#
# # good
# class x # :nodoc:
# y
# end
class CommentedKeyword < Cop
MSG = 'Do not place comments on the same line as the ' \
'`%s` keyword.'.freeze

def investigate(processed_source)
heredoc_lines = extract_heredoc_lines(processed_source.ast)

processed_source.lines.each_with_index do |line, index|
next if heredoc_lines.any? { |r| r.include?(index + 1) }
next unless offensive?(line)

range = source_range(processed_source.buffer,
index + 1,
(line.index('#'))...(line.length))

add_offense(range, location: range)
end
end

private

KEYWORDS = %w[begin class def end module].freeze
ALLOWED_COMMENTS = %w[:nodoc: rubocop:disable].freeze

def offensive?(line)
KEYWORDS.any? { |k| line =~ /^\s*#{k}\s+.*#/ } &&
ALLOWED_COMMENTS.none? { |c| line =~ /#\s*#{c}/ }
end

def message(node)
line = node.source_line
keyword = /^\s*(\S+).*#/.match(line)[1]
format(MSG, keyword)
end

def extract_heredoc_lines(ast)
return [] unless ast
ast.each_node.with_object([]) do |node, heredocs|
next unless node.location.is_a?(Parser::Source::Map::Heredoc)
body = node.location.heredoc_body
heredocs << (body.first_line...body.last_line)
end
end
end
end
end
end
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ In the following section you find all available cops:
* [Style/ColonMethodCall](cops_style.md#stylecolonmethodcall)
* [Style/CommandLiteral](cops_style.md#stylecommandliteral)
* [Style/CommentAnnotation](cops_style.md#stylecommentannotation)
* [Style/CommentedKeyword](cops_style.md#stylecommentedkeyword)
* [Style/ConditionalAssignment](cops_style.md#styleconditionalassignment)
* [Style/Copyright](cops_style.md#stylecopyright)
* [Style/DefWithParentheses](cops_style.md#styledefwithparentheses)
Expand Down
39 changes: 39 additions & 0 deletions manual/cops_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,45 @@ Keywords | TODO, FIXME, OPTIMIZE, HACK, REVIEW

* [https://github.com/bbatsov/ruby-style-guide#annotate-keywords](https://github.com/bbatsov/ruby-style-guide#annotate-keywords)

## Style/CommentedKeyword

Enabled by default | Supports autocorrection
--- | ---
Enabled | No

This cop checks for comments put on the same line as some keywords.
These keywords are: `begin`, `class`, `def`, `end`, `module`.

Note that some comments (such as `:nodoc:` and `rubocop:disable`) are
allowed.

### Example

```ruby
# bad
if condition
statement
end # end if

# bad
class X # comment
statement
end

# bad
def x; end # comment

# good
if condition
statement
end

# good
class x # :nodoc:
y
end
```

## Style/ConditionalAssignment

Enabled by default | Supports autocorrection
Expand Down
3 changes: 2 additions & 1 deletion spec/rubocop/cli_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1523,9 +1523,10 @@ def method(a, b, c, d, e, f) end #{'#' * 57}
expect($stdout.string).to eq(<<-RESULT.strip_indent)
== example/example1.rb ==
C: 1: 11: Avoid parameter lists longer than 5 parameters. [6/5]
C: 1: 34: Do not place comments on the same line as the def keyword.
E: 1: 81: Line is too long. [90/80]
1 file inspected, 2 offenses detected
1 file inspected, 3 offenses detected
RESULT
expect($stderr.string).to eq('')
end
Expand Down
144 changes: 144 additions & 0 deletions spec/rubocop/cop/style/commented_keyword_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# frozen_string_literal: true

describe RuboCop::Cop::Style::CommentedKeyword do
let(:config) { RuboCop::Config.new }
subject(:cop) { described_class.new(config) }

it 'registers an offense when commenting on the same line as `end`' do
inspect_source(<<-RUBY.strip_indent)
if x
y
end # comment
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `end` keyword.'])
end

it 'registers an offense when commenting on the same line as `begin`' do
inspect_source(<<-RUBY.strip_indent)
begin # comment
y
end
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `begin` keyword.'])
end

it 'registers an offense when commenting on the same line as `class`' do
inspect_source(<<-RUBY.strip_indent)
class X # comment
y
end
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `class` keyword.'])
end

it 'registers an offense when commenting on the same line as `module`' do
inspect_source(<<-RUBY.strip_indent)
module X # comment
y
end
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `module` keyword.'])
end

it 'registers an offense when commenting on the same line as `def`' do
inspect_source(<<-RUBY.strip_indent)
def x # comment
y
end
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `def` keyword.'])
end

it 'registers an offense when commenting on indented keywords' do
inspect_source(<<-RUBY.strip_indent)
module X
class Y # comment
z
end
end
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `class` keyword.'])
end

it 'registers an offense when commenting after keyword with spaces' do
inspect_source(<<-RUBY.strip_indent)
def x(a, b) # comment
y
end
RUBY
expect(cop.highlights).to eq(['# comment'])
expect(cop.messages).to eq(['Do not place comments on the same line as ' \
'the `def` keyword.'])
end

it 'registers an offense for one-line cases' do
expect_offense(<<-RUBY.strip_indent)
def x; end # comment'
^^^^^^^^^^ Do not place comments on the same line as the `def` keyword.
RUBY
end

it 'does not register an offense if there are no comments after keywords' do
expect_no_offenses(<<-RUBY.strip_indent)
if x
y
end
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
class X
y
end
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
begin
x
end
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
def x
y
end
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
module X
y
end
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
# module Y # trap comment
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
'end' # comment
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
<<-HEREDOC
def # not a comment
HEREDOC
RUBY
end

it 'does not register an offense for certain comments' do
expect_no_offenses(<<-RUBY.strip_indent)
class X # :nodoc:
y
end
RUBY
expect_no_offenses(<<-RUBY.strip_indent)
def x # rubocop:disable Metrics/MethodLength
y
end
RUBY
end
end

0 comments on commit 3ab1b96

Please sign in to comment.