From 3ab1b96f0bfe16b14b0e1b707a396faf3e195eec Mon Sep 17 00:00:00 2001 From: Herbert Ilhan Tanujaya Date: Tue, 22 Aug 2017 18:44:25 +0800 Subject: [PATCH] [Fix #4663] Add new CommentedKeyword cop --- CHANGELOG.md | 1 + config/enabled.yml | 4 + lib/rubocop.rb | 1 + lib/rubocop/cop/style/commented_keyword.rb | 81 ++++++++++ manual/cops.md | 1 + manual/cops_style.md | 39 +++++ spec/rubocop/cli_spec.rb | 3 +- .../cop/style/commented_keyword_spec.rb | 144 ++++++++++++++++++ 8 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/cop/style/commented_keyword.rb create mode 100644 spec/rubocop/cop/style/commented_keyword_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 30f37fa57b62..71e6c3ec6f71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/enabled.yml b/config/enabled.yml index f5436c8a87c5..223c0607997c 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -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 diff --git a/lib/rubocop.rb b/lib/rubocop.rb index c50e6b19827a..af8769dff5cb 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' diff --git a/lib/rubocop/cop/style/commented_keyword.rb b/lib/rubocop/cop/style/commented_keyword.rb new file mode 100644 index 000000000000..4ab25a006254 --- /dev/null +++ b/lib/rubocop/cop/style/commented_keyword.rb @@ -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 diff --git a/manual/cops.md b/manual/cops.md index 4a8eead8fa34..89a5c7e2bd91 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -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) diff --git a/manual/cops_style.md b/manual/cops_style.md index 0a84d666aaa8..51501df0be3c 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -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 diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 6e13d30cddf9..dab67d9c2443 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -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 diff --git a/spec/rubocop/cop/style/commented_keyword_spec.rb b/spec/rubocop/cop/style/commented_keyword_spec.rb new file mode 100644 index 000000000000..0f9919b57653 --- /dev/null +++ b/spec/rubocop/cop/style/commented_keyword_spec.rb @@ -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