proposal: `comment_all_ignores`

[Number-3434]

comment_all_ignores

(See notes on naming in Writing Lints.)

Description

A short description suitable for display in console output.

Missing a required ignore reason. Try adding a reason for this ignore after the rule name separated by a comma.

Details

A detailed description that provides context and motivation. Can be used directly in the rule's documentation.

From the Flutter Style Guide:

comment all // ignores

Sometimes, it is necessary to write code that the analyzer is unhappy with.

If you find yourself in this situation, consider how you got there. Is the analyzer actually correct but you don’t want to admit it? Think about how you could refactor your code so that the analyzer is happy. If such a refactor would make the code better, do it. (It might be a lot of work…​ embrace the yak shave.)

If you are really really sure that you have no choice but to silence the analyzer, use // ignore: . The ignore directive should be on the same line as the analyzer warning.

If the ignore is temporary (e.g. a workaround for a bug in the compiler or analyzer, or a workaround for some known problem in Flutter that you cannot fix), then add a link to the relevant bug, as follows:

foo(); // ignore: lint_code, https://link.to.bug/goes/here

If the ignore directive is permanent, e.g. because one of our lints has some unavoidable false positives and in this case violating the lint is definitely better than all other options, then add a comment explaining why:

foo(); // ignore: lint_code, sadly there is no choice but to do
// this because we need to twiddle the quux and the bar is zorgle.

Kind

Enforces style advice.

Copied from the Flutter Style Guide.

Bad Examples

A few examples that demonstrate where this lint should fire.

foo(); // ignore: lint_code

---

// ignore: lint_code
foo();

---

// ignore_for_file: avoid_annotating_with_dynamic

void foo({dynamic e, Object f}) {  }

Good Examples

Here are a few examples that demonstrate a “good” adoption of this lint’s principle.

foo(); // ignore: lint_code, https://link.to.bug/goes/here

---

// ignore: lint_code, https://link.to.bug/goes/here
foo();

---

// ignore_for_file: avoid_annotating_with_dynamic, ignoring for clarity

void foo({dynamic e, Object f}) {  }

---

import 'package:flutter/material.dart';

extension CustomStateExtension<T extends StatefulWidget> on State<T> {
  /// Calls [setState] on the widget if it is currently [mounted].
  void setStateIfMounted(void Function() fn) {
    if (!mounted) return;

    // ignore: invalid_use_of_protected_member, necessary since we need to call setState
    setState(fn is Future<void> Function() ? () { fn(); } : fn);
  }
}

Discussion

Add any other motivation or useful context here.

Conflicts

This lint may be self-conflicting, i.e. the user may actually have to specify a comment on the ignore to ignore this rule.

Other

Once this lint gets approved, a further lint may be ignores_same_line, which will lint against multiple ignores on the same line.

Discussion checklist

• [x] This lint does not modify, complement, overlap, or conflict with existing rules.
• [x] No relevant issues (reported here, the SDK Tracker, or elsewhere).
• [x] No prior art (e.g., in other linters).
• [x] This proposal corresponds to Flutter Style Guide advice.
• [x] Motivated by real-world examples. The above example of setStateIfMounted is from a utility in one of my real-life private repositories.

[srawlins]

I'd love this rule. I miss it often, from other linters, haha.

[Number-3434]

I'd love this rule. I miss it often, from other linters, haha.

Yeah, also you would have to add an ignore comment to ignore this rule :)

// ignore: comment_all_ignores, because it's getting annoying
// ignore: some_lint

[pq]

@goderbauer: I'm guessing this'd be used at least for the flutter repo and maybe flutter recommended style?

@bwilkerson, @scheglov, @srawlins, @keertip, @kallentu: I wonder if this is something we'd consider for analyzer?

[goderbauer]

I would love to enforce this for the flutter repo.

Not so sure about putting it in the flutter recommended style set, though, since it isn't really a lint specific to flutter code. Since it is about general dart code, if we think this is useful we should align across the dart universe and put it in the general recommended set.

[srawlins]

Implemented in https://github.com/dart-lang/sdk/commit/607b3f5ddba34348f965cce99e4cac5e87a46a99

[Number-3434]

yay!