Skip to content

Commit

Permalink
[Fix #6630] Updated Style/CommentAnnotation to be able to handle mu…
Browse files Browse the repository at this point in the history
…ltiword keyword phrases.
  • Loading branch information
dvandersluis authored and bbatsov committed Aug 25, 2021
1 parent 150f285 commit 2bab715
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 12 deletions.
1 change: 1 addition & 0 deletions changelog/fix_updated_annotationcomment_to_be_able_to.md
@@ -0,0 +1 @@
* [#6630](https://github.com/rubocop/rubocop/issues/6630): Updated `Style/CommentAnnotation` to be able to handle multiword keyword phrases. ([@dvandersluis][])
2 changes: 1 addition & 1 deletion config/default.yml
Expand Up @@ -3156,7 +3156,7 @@ Style/CommentAnnotation:
StyleGuide: '#annotate-keywords'
Enabled: true
VersionAdded: '0.10'
VersionChanged: '1.3'
VersionChanged: '<<next>>'
Keywords:
- TODO
- FIXME
Expand Down
19 changes: 12 additions & 7 deletions lib/rubocop/cop/mixin/annotation_comment.rb
Expand Up @@ -21,7 +21,8 @@ def annotation?
end

def correct?(colon:)
return false unless keyword?(keyword) && space && note
return false unless keyword && space && note
return false unless keyword == keyword.upcase

self.colon.nil? == !colon
end
Expand All @@ -38,18 +39,22 @@ def bounds
attr_reader :keywords

def split_comment(comment)
match = comment.text.match(/^(# ?)([A-Za-z]+)(\s*:)?(\s+)?(\S+)?/)
# Sort keywords by reverse length so that if a keyword is in a phrase
# but also on its own, both will match properly.
keywords_regex = Regexp.new(
Regexp.union(keywords.sort_by { |w| -w.length }).source,
Regexp::IGNORECASE
)
regex = /^(# ?)(\b#{keywords_regex}\b)(\s*:)?(\s+)?(\S+)?/i

match = comment.text.match(regex)
return false unless match

match.captures
end

def keyword?(word)
keywords.include?(word)
end

def keyword_appearance?
keyword && keyword?(keyword.upcase) && (colon || space)
keyword && (colon || space)
end

def just_keyword_of_sentence?
Expand Down
3 changes: 3 additions & 0 deletions lib/rubocop/cop/style/comment_annotation.rb
Expand Up @@ -6,6 +6,9 @@ module Style
# This cop checks that comment annotation keywords are written according
# to guidelines.
#
# Annotation keywords can be specified by overriding the cop's `Keywords`
# configuration. Keywords are allowed to be single words or phrases.
#
# NOTE: With a multiline comment block (where each line is only a
# comment), only the first line will be able to register an offense, even
# if an annotation keyword starts another line. This is done to prevent
Expand Down
40 changes: 36 additions & 4 deletions spec/rubocop/cop/annotation_comment_spec.rb
Expand Up @@ -3,7 +3,7 @@
RSpec.describe RuboCop::Cop::AnnotationComment do
subject(:annotation) { described_class.new(comment, keywords) }

let(:keywords) { %w[TODO FIXME] }
let(:keywords) { ['TODO', 'FOR LATER', 'FIXME'] }
let(:comment) { instance_double('Parser::Source::Comment', text: "# #{text}") }

describe '#annotation?' do
Expand All @@ -27,6 +27,12 @@
it { is_expected.to eq(true) }
end

context 'when the keyword is multiple words' do
let(:text) { 'FOR LATER: note' }

it { is_expected.to eq(true) }
end

context 'when annotated with a non keyword' do
let(:text) { 'SOMETHING: note' }

Expand All @@ -47,6 +53,8 @@
end

describe '#correct?' do
subject { annotation.correct?(colon: colon) }

shared_examples_for 'correct' do |text|
let(:text) { text }

Expand All @@ -59,11 +67,12 @@
it { is_expected.to be_falsey }
end

context 'when a colon is required' do
subject { annotation.correct?(colon: true) }
let(:colon) { true }

context 'when a colon is required' do
it_behaves_like 'correct', 'TODO: text'
it_behaves_like 'correct', 'FIXME: text'
it_behaves_like 'correct', 'FOR LATER: text'
it_behaves_like 'incorrect', 'TODO: '
it_behaves_like 'incorrect', 'TODO '
it_behaves_like 'incorrect', 'TODO'
Expand All @@ -74,13 +83,15 @@
it_behaves_like 'incorrect', 'todo text'
it_behaves_like 'incorrect', 'UPDATE: text'
it_behaves_like 'incorrect', 'UPDATE text'
it_behaves_like 'incorrect', 'FOR LATER text'
end

context 'when no colon is required' do
subject { annotation.correct?(colon: false) }
let(:colon) { false }

it_behaves_like 'correct', 'TODO text'
it_behaves_like 'correct', 'FIXME text'
it_behaves_like 'correct', 'FOR LATER text'
it_behaves_like 'incorrect', 'TODO: '
it_behaves_like 'incorrect', 'TODO '
it_behaves_like 'incorrect', 'TODO'
Expand All @@ -91,6 +102,27 @@
it_behaves_like 'incorrect', 'todo text'
it_behaves_like 'incorrect', 'UPDATE: text'
it_behaves_like 'incorrect', 'UPDATE text'
it_behaves_like 'incorrect', 'FOR LATER: text'
end

context 'when there is duplication in the keywords' do
context 'when the longer keyword is given first' do
let(:keywords) { ['TODO LATER', 'TODO'] }

it_behaves_like 'correct', 'TODO: text'
it_behaves_like 'correct', 'TODO LATER: text'
it_behaves_like 'incorrect', 'TODO text'
it_behaves_like 'incorrect', 'TODO LATER text'
end

context 'when the shorter keyword is given first' do
let(:keywords) { ['TODO', 'TODO LATER'] }

it_behaves_like 'correct', 'TODO: text'
it_behaves_like 'correct', 'TODO LATER: text'
it_behaves_like 'incorrect', 'TODO text'
it_behaves_like 'incorrect', 'TODO LATER text'
end
end
end
end
13 changes: 13 additions & 0 deletions spec/rubocop/cop/style/comment_annotation_spec.rb
Expand Up @@ -153,6 +153,19 @@ class ToBeDone
RUBY
end
end

context 'with multiword keywords' do
let(:cop_config) { { 'Keywords' => ['TODO', 'DO SOMETHING', 'TODO LATER'] } }

it 'registers an offense for each matching keyword' do
cop_config['Keywords'].each do |keyword|
expect_offense(<<~RUBY, keyword: keyword)
# #{keyword} blah blah blah
^{keyword}^ Annotation keywords like `#{keyword}` should be all upper case, followed by a colon, and a space, then a note describing the problem.
RUBY
end
end
end
end

context 'with RequireColon configuration set to false' do
Expand Down

0 comments on commit 2bab715

Please sign in to comment.