search

dart-code-checker / dart-code-metrics

Software analytics tool that helps developers analyse and improve software quality.
https://dcm.dev
Other
860 stars 266 forks source link

[BUG] `format-comment` false positive #1232

Open FMorschel opened 1 year ago

FMorschel commented 1 year ago

Environment and configuration

DCM version: 5.7.3 Dart SDK version: 3.0.0

Configuration ```yaml dart_code_metrics: rules: - double-literal-format - format-comment - member-ordering-extended: alphabetize: false order: - constructors - named-constructors - factory-constructors - static-getters-setters - static-const-public-fields - static-const-private-fields - static-final-public-fields - static-final-private-fields - static-late-final-public-fields - static-late-final-private-fields - static-late-public-fields - static-late-private-fields - static-public-fields - static-private-fields - const-public-fields - const-private-fields - final-public-fields - final-private-fields - late-final-public-fields - late-final-private-fields - late-public-fields - late-private-fields - public-fields - private-fields - overridden-public-fields - overriden-public-methods - static-public-methods - static-private-methods - public-methods - private-methods - getters-setters - prefer-commenting-analyzer-ignores - prefer-first - prefer-immediate-return - prefer-last - always-remove-listener - avoid-border-all - avoid-returning-widgets: ignored-names: - testFunction ignored-annotations: - allowedAnnotation - avoid-unnecessary-setstate - avoid-wrapping-in-padding - prefer-const-border-radius ```

What did you do?

  // TODO: home.
  // TODO: login.

What did you expect to happen?

No line should trigger since I'm actually formatting them like sentences.

What actually happened?

When I have two lines of comment one after another, this lint triggers. When there is only one, the lint doesn't trigger.

Participation

Additional comments

No response

incendial commented 1 year ago

@FMorschel for multiline comments the rule treats TODO: as separate parts of the sentences (which is probably incorrect) and expects an uppercase letter to be the first after the :.

So this version won't trigger the lint:

// TODO: Home.
// TODO: Login.
incendial commented 1 year ago

The more I look at various examples of todos, the more I'm not sure whether any change should be done here. Most of them start with the uppercase after :.

@FMorschel and you expected for the rule to accept a lowercase letter as the first sentence letter?

FMorschel commented 1 year ago

Yes, but obviously this is just team standards. This could be solved by, for example, the lint warning about that. That would make us self aware and we could solve that by simply changing the first letter. If we are willing to use DCM, we should be considered of it's boundaries.