False negative ForbiddenComment with common TODO markers

[TWiStErRob]

The power of ForbiddenComment is increased with #5981 and therefore we can decrease the false negatives, while not introducing new false positives.

Expected Behavior

Flag all of:

// TODO: foo
// FIXME: foo
// STOPSHIP: foo
// TODO foo
// FIXME foo
// STOPSHIP foo

and by extension

// TODO : foo
// FIXME : foo
// STOPSHIP : foo

(French grammar mandates a space before the : so this is common by French speakers; I've seen it from French and Moroccan coders.)

Potentially these too:

// todo foo
// fixme foo
// stopship foo

All of the above can be achieved by using a regex: (?i)\bTODO\b similar to IDEA config. The \b is extra to make sure TODOs, mastodon, affixment words in comments are not flagged.

---

Additionally I suggest to merge the three comments into one (again this comes as a bonus of #5981 giving more power):

ForbiddenComment:
  comments:
    - value: '(?i)\b(TODO|FIXME|STOPSHIP)\b'
      reason: 'Todo marker found, fix the code and remove comment.'

so that it's easier to configure the rule (no need to copy/paste and maintain 3 rules when adding new).

Observed Behavior

These are flagged:

// TODO: foo
// FIXME: foo
// STOPSHIP: foo

nothing else.

Steps to Reproduce

Use comments in "Expected Behavior" with default Detekt config.

detekt/detekt-core/src/main/resources/default-detekt-config.yml

Lines 575 to 578 in 67d2c72

	values:
	- 'FIXME:'
	- 'STOPSHIP:'
	- 'TODO:'

Context

While working on #5981 (comment) I realized // FIXME foo would just be accepted compliant by ForbiddenComment rule. From experience with multiple codebases most of the TODOs developers write don't contain the colon (:) character.

Note: the default todo markers in IntelliJ IDEA also don't use :

https://www.jetbrains.com/help/idea/using-todo.html#add_pattern_filter_todo

Your Environment

• Version of detekt used: 1.23.0
• Version of Gradle used (if applicable): N/A
• Gradle scan link (add --scan option when running the gradle task): N/A
• Operating System and version: N/A
• Link to your project (if it's a public repository): any

[schalkms]

duplicate #3508 ?

[TWiStErRob]

No, the fix of #3508 (which is #5981) enables this improvement. Just look at the "Expected/Observed" examples, the rest is suggestions.

[BraisGabin]

I like the idea but that regex is scary. I don't think that such a difficult regex should be part of the default configuration of detekt. Nearly no one will be brave to edit it.

I think that #6075 could help to fix this.

[BraisGabin]

What do we think about this? A change in our default configuration is not a breaking change so we can make the change propesed by @TWiStErRob and if in the end we implement #6075 we can change it again at any moment. I would approve a PR doing this change.

[TWiStErRob]

Nearly no one will be brave to edit it.

I'm happy to write some docs on the rule on what the regex parts mean (in addition to a link to https://regex101.com/r/IEOm2Q/1). And I think even some programmer without any knowledge of RegEx would be able to figure out how to add/remove their own custom todo markers based on pattern recognition.

[BraisGabin]

Go for it :)

[detekt-ci]

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

[detekt-ci]

This issue was closed because it has been stalled for 7 days with no activity.