Resolve whether to have --strict mode, or just a set of lints

[matanlurey]

Resolved: We will have a strict mode.

Some checks could be made lints instead where it makes sense, on a case-by-case basis.

---

Meta: Should we have a set of "strict mode" lints, or a "strict mode" flag to the analyzer?

We can meet offline as well, but I'd like to post this issue for posterity and for external reference.

As seen in:

• Implement a set of "strict" static analysis rules #33749
• I would like a --strict-await static analysis option (2.1+) #33823 (comment)
• Would like a way to avoid raw generic types #33119 (comment)

In the issues above @jmesserly correctly brought up that it's unclear whether we need lints (i.e. rules, implemented in package:linter, that are opt-in via adding the individual rules to analysis_options.yaml) or hints (implemented directly in package:analyzer, that are opt-in via a potential strict flag in analysis_options.yaml.

So, the question is, do we want:

dartanalyzer --options analysis_options.yaml

analyzer:
  linter:
    rules:
      avoid_dynamic_calls: true
      avoid_inferred_casts: true
      avoid_raw_types: true

or do we want an option of either:

dartanalyzer --strict

dartanalyzer --options analysis_options.yaml

analyzer:
  strict: true

(Or, with more specificity)

dartanalyzer --strict-dynamic-calls --strict_raw_types

analyzer:
  strict:
    - dynamic_calls
    - raw_types

A bit of history

Originally when @jmesserly et al wrote the strong-mode: flag in the analyzer, this was an optional (off-by-default) mode, and she added additional opt-in flags for even stricter analysis: --no-implicit-dynamic and --no-implicit-casts. We (well, at least @srawlins and I) ❤️ these flags, but they are a little too demanding of most code bases - but they serve as an excellent way to frame this discussion.

Now that strong-mode is the default (and only option), it's less clear whether we need (or actually desire) a strict mode - i.e. a way to run stricter analysis checks to catch more errors at analysis-time and less at runtime in Dart 2.0+.

What differentiates strict flags from lints?

Nothing, though strict flags - based on discussion in #33749 are more nuanced (closer to hints):

• They do not enforce style or readability rules
• They do not change coding habits for entirely aesthetic purposes
• They do not make any decisions that could have false positives

... rather, it is possible to write a strict check as a lint, but most lints do not meet criteria for strict.

Possible flags (taken from #33749, and some offline discussion)

... to help serve as a basis of informing the decision.

• --strict-dynamic-calls: Do not allow dynamic calls (perhaps with an opt-in mechanism)
• --strict-inferred-casts: Do not allow inference to produce a top type unless top was an input
• --strict-raw-types: Require List<dynamic> instead of List implicitly being List<dynamic>

Comparative approach

Non-exhaustive - just wrote down what I could think of on top of my head.

Lints

PROs:

• Authored in the linter package, which is much more external/non-SDK developer friendly ⭐️
• Can iterate much faster
• Sit side by side with other rules trying to accomplish (sometimes) similar goals
• Can be modified, created at will without needing extra integration within the analyzer

CONs:

• Not really "on the radar" of the language/core team, i.e. most work here happens w/o much notice
• Not simple to add "quick fixes" (suggestions) for IDE users
• Needs to be rolled into the SDK

Hints

PROs:

• Easier and more natural to add "quick fixes" and suggestions for IDE users
• A strict flag is a simpler concept to wrap your head around as a concept from individual lints
• Can be more directly driven by the language/core team (i.e. positive but non-breaking changes)
• Easy to adapt 1-1 as a recommendation for "large code bases" (i.e. google3 and similar)

CONs:

• Less friendly to contributors not already experienced with developing against the SDK ⭐️
• UX issue of a strict mode flag in the analyzer/related tools (compared to "strong" mode etc)

⭐️ I don't think this is that important, given we don't expect external users to contribute these flags anymore than we expect external users to change the type system or the core libraries.

[matanlurey]

Other considerations, from an offline thread:

• Core team could use this mode as a stepping stone to Dart 3+ (like strong-mode for Dart 1->2)
  • Example might be rules that @MichaelRFairhurst tried to land in Dart 2.x, but ran out of time
• Might be performance considerations in running some of these checks in the linter v analyzer

[bwilkerson]

I have an issue with making these hints. I think that the primary distinction between hints and lints (from a user's perspective) is that hints are enabled by default and lints are disabled by default. Adding "hints" that must be enabled seems to break that pattern and make it harder to explain our already confusing plethora of "kinds" of diagnostics.

If we don't want to do this as lints (a decision I think the language team should make), I would much rather couch this in terms of a variant of (or enhancement to) the language. It seems to me to be very consistent with other language enhancements (such as super-in-mixins). If we do that, then I could even see us implementing these diagnostics as full-blown errors.

One minor correction: it's just as easy to write a quick fix for a lint as it is for a hint (or anything else). I believe that the false perception that it's "hard" to write quick fixes for lints is because we don't yet support implementing those quick fixes in the linter package, so for those not familiar with the sdk development process it appears to be a higher barrier.

[matanlurey]

Thanks! I am not tied to hints, for example if this really is a mode that the core team wants to facilitate for tighter static checks, then I imagine warning or error is totally fine to use (you already opted into non-spec tighter checks).

One minor correction: it's just as easy to write a quick fix for a lint as it is for a hint (or anything else). I believe that the false perception that it's "hard" to write quick fixes for lints is because we don't yet support implementing those quick fixes in the linter package, so for those not familiar with the sdk development process it appears to be a higher barrier.

Got it, thanks!

[davidmorgan]

FWIW I think if we had a strict flag that we would want to keep it as granular as possible, i.e. have multiple flags, to make it easier to land in large codebases. In that case, it feels a lot like we're just re-implementing linter config. So my gut feeling is to go with lints.

But I don't feel strongly either way.

[jmesserly]

Perhaps we can add the new checks individually, and then get a feel for how useful they are (in terms cost/benefit), and how much adoption we can get. Then, once we have a set of checks that looks good, we can create a "strict mode" that enforces them.

I'd be inclined to use lints for most of the checks (faster iteration cycle?). A few of the inference checks probably make more sense to implement directly in Analyzer itself, though.

[jmesserly]

Then again: if we feel confident that the set of flags in #33749 is probably what we'll end up with for strict mode, then maybe I can implement them all in the same place (as Analyzer flags).

[bwilkerson]

Removing lints is a breaking change, so I'd rather not have a plan to create temporary lints.

[jmesserly]

Removing lints is a breaking change, so I'd rather not have a plan to create temporary lints.

Is there a notion of "experimental" or "staging" for lints? (similar to language features, it can be nice to iterate & gather real world feedback before committing to an exact feature specification).

I don't think we would need to necessarily need/want to remove them, rather it would just not add any new info if you were already using the strict flag. But the lint would still be useful on its own. ("await_only_futures" is an example, it's already a lint, but it might get included in "strict mode").

EDIT: then again, this does lean us more towards an Analyzer implementation.

[bwilkerson]

Is there a notion of "experimental" or "staging" for lints?

Yes, I think there is.

[MichaelRFairhurst]

I'd like to comment on:

They do not make any decisions that could have false positives

I don't think this is true.

List y = ...;
List x = y.map(...);

Firstly, it may be intended for y and x to be List<dynamic>. You could argue that it's not a false positive, because it is flagging an area written in bad form. However, wouldn't that then contradict: "They do not change coding habits for entirely aesthetic purposes."

Secondly, it may look like the second line is not a false positive because it will always fail. Even this is not true:

class MyList<T> implements List<T> {
  ...
  List<R> map<R>(......) => new List<R>();
  ...
}

void main() {
  List x = new MyList();
  List y = x.map(...);
}

Lastly, this flag includes an opt-out mechanism, indicating false positives:

--strict-dynamic-calls: Do not allow dynamic calls (perhaps with an opt-in mechanism)

So I think you could make the case that there are no false positives, but that implies that its "good form" which implies to me that its a style preference. Or you could make the case that it's not style or "good form" but about having a stricter language on the whole, in which case it has false positives because it will restrict valid programs too.

How about the CFE?

I'd pose, rather, that these should not be lints because that assumes that all extra strictness will be static.

Counterexamples:

• invariant generics (assuming you want the same subtyping behavior at runtime)
• no awaiting non-futures (assuming you want to allow awaiting Object/dynamic, and insert a runtime check)

and of course, any --strict flag that we add to the analyzer, can and maybe should be caught at compile time in the CFE too.

[matanlurey]

Hi @MichaelRFairhurst thanks for the feedback.

We have decided (offline) that this will be a mode, not a set of lints. If there are specific checks that make more sense to implement as a lint, we can tackle those on a case by case basis. It sounds like you mostly agree with that decision though.