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

[Fix #9890] Make Colon After Comment Annotation Configurable #9899

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5722,3 +5722,4 @@
[@timlkelly]: https://github.com/timlkelly
[@AirWick219]: https://github.com/AirWick219
[@markburns]: https://github.com/markburns
[@gregfletch]: https://github.com/gregfletch
@@ -0,0 +1 @@
* [#9890](https://github.com/rubocop/rubocop/issues/9890): Make colon after comment annotation configurable. ([@gregfletch][])
4 changes: 4 additions & 0 deletions config/default.yml
Expand Up @@ -3150,6 +3150,10 @@ Style/CommentAnnotation:
- HACK
- REVIEW
- NOTE
EnforcedStyle: colon
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need an enforced style for this - we can just have an boolean options like RequireColon: true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, my initial thinking here was to leave it a bit more open with enforced style/supported styles in case somebody wanted to have a different separator, like a dash for example (i.e. TODO- some todo comment here). But I'm fine changing it to a simple boolean value indicating if a colon is required if you think that is better.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes we should just keep things simple, so let's go with the boolean value.

SupportedStyles:
- colon
- space

Style/CommentedKeyword:
Description: 'Do not place comments on the same line as certain keywords.'
Expand Down
59 changes: 53 additions & 6 deletions lib/rubocop/cop/style/comment_annotation.rb
Expand Up @@ -12,7 +12,7 @@ module Style
# incorrect registering of keywords (eg. `review`) inside a paragraph as an
# annotation.
#
# @example
# @example EnforcedStyle: colon (default)
# # bad
# # TODO make better
#
Expand All @@ -36,14 +36,37 @@ module Style
#
# # good
# # OPTIMIZE: does not work
#
# @example EnforcedStyle: space
# # bad
# # TODO: make better
#
# # good
# # TODO make better
#
# # bad
# # fixme does not work
#
# # good
# # FIXME does not work
#
# # bad
# # Optimize does not work
#
# # good
# # OPTIMIZE does not work
class CommentAnnotation < Base
include AnnotationComment
include ConfigurableEnforcedStyle
include RangeHelp
extend AutoCorrector

MSG = 'Annotation keywords like `%<keyword>s` should be all ' \
'upper case, followed by a colon, and a space, ' \
'then a note describing the problem.'
MSG_COLON_STYLE = 'Annotation keywords like `%<keyword>s` should be all ' \
'upper case, followed by a colon, and a space, ' \
'then a note describing the problem.'
MSG_SPACE_STYLE = 'Annotation keywords like `%<keyword>s` should be all ' \
'upper case, followed by a space, ' \
'then a note describing the problem.'
MISSING_NOTE = 'Annotation comment, with keyword `%<keyword>s`, is missing a note.'

def on_new_investigation
Expand All @@ -63,13 +86,19 @@ def on_new_investigation
private

def register_offense(range, note, first_word)
message = if style == :colon
MSG_COLON_STYLE
else
MSG_SPACE_STYLE
end

add_offense(
range,
message: format(note ? MSG : MISSING_NOTE, keyword: first_word)
message: format(note ? message : MISSING_NOTE, keyword: first_word)
) do |corrector|
next if note.nil?

corrector.replace(range, "#{first_word.upcase}: ")
correct_offense(corrector, range, first_word)
end
end

Expand All @@ -92,8 +121,26 @@ def concat_length(*args)
end

def correct_annotation?(first_word, colon, space, note)
return correct_colon_annotation?(first_word, colon, space, note) if style == :colon

correct_space_annotation?(first_word, colon, space, note)
end

def correct_colon_annotation?(first_word, colon, space, note)
keyword?(first_word) && (colon && space && note || !colon && !note)
end

def correct_space_annotation?(first_word, colon, space, note)
keyword?(first_word) && (!colon && space && note || !colon && !note)
end

def correct_offense(corrector, range, first_word)
if style == :colon
corrector.replace(range, "#{first_word.upcase}: ")
else
corrector.replace(range, "#{first_word.upcase} ")
end
end
end
end
end
Expand Down