Remove the "Hint" diagnostic category

[srawlins]

Currently in the analyzer we have (approximately) the following diagnostic categories:

• CompileTimeErrorCode ("error")
• StaticWarningCode ("warning")
• HintCode ("hint")
• LintCode ("lint")

(There are others like ParserErrorCode, TodoCode, FfiCode, etc, but the 4 above are the common, user-visible ones.)

We'd like to remove the HintCode category, and move all "hints" to be either "warnings" or "lints." They will mostly be moved to "warnings."

The categories differ from each other in their default severity (one of ERROR, WARNING, or INFO), and whether they are reported by default (or must be enabled via analysis_options.yaml):

Category	Severity	Enabled-by-default?
CompileTimeErrorCode	ERROR	Yes
StaticWarningCode	WARNING	Yes
HintCode	INFO	Yes
LintCode	INFO	No

What do these mean, practically? The difference is primarily in the behavior of dart analyze (used in presubmit checks, continuous integration, and perhaps by humans):

• If no diagnostics are reported by dart analyze, its exit code is 0.
• An INFO-severity diagnostic report does not move the exit code away from 0.
• A WARNING-severity diagnostic report does move the exit code away from 0.
• An ERROR-severity diagnostic report does move the exit code away from 0.

Note also:

• The fatality of WARNING-severity diagnostics can be changed with the --[no-]fatal-warnings flag.
• The fatality of INFO-severity diagnostics can be changed with the --[no-]fatal-infos flag.
• The severity of any individual diagnostic (like DEAD_CODE or UNUSED_IMPORT or any lint rule) can be changed in analysis_options.yaml from its default to any of the others, or IGNORE.

This proposal is therefore a breaking change: "Hints" which are moved to be "warnings" will become fatal (by default) when reported by dart analyze. (Behavior will not change when --fatal-infos or --no-fatal-warnings is being used.)

Another significant change: users will also no longer get the protection from a "hint" which is moved to be an opt-in "lint" unless they explicitly enable it in their analysis_options.yaml file (or include another analysis options file which does so).

Historical note: We had more "static warnings" defined by the language in previous releases of Dart, but all but a handful of those have been dropped, and the language team has confirmed that it is "fair game" for tools like the analyzer to define their own "warnings."

[srawlins]

cc @bwilkerson @mit-mit @jacob314 @pq @lrhn @leafpetersen

[bwilkerson]

There are others like ParserErrorCode ...

Kind of a minor nit, but I consider ParserErrorCodes and FfiCodes to be errors, just like CompileTimeErrorCode. Both are user-visible and they have the same impact in terms of our tooling.

We didn't always choose the classes based on severity, sometimes it had more to do with where the diagnostics are being generated or who owns the codes. That said, what's important here is not which classes we used in the implementation, but the impact of changing the severities of some of our enabled-by-default diagnostics.

[asashour]

I would consider two things: enablement-by-default, and number of managed diagnostics. I am under the impression that generally hints are non-disabled, while lints are configurable.

A compile-time error (and similar) is an error, and the user would like to see this as early as possible to be fixed. There is no value to disable it, I guess, since the error will surely come later, either during compilation or at run-time.

Moving the current hints (around 127) to lints will make the user be overwhelmed by the number of diagnostics which are configurable. Honestly, I think this might not be adding much value, but actually preventing more productivity. Dart is taking the decision to allow combinations and configuration of very tiny details, but does the user really care?

One of the decisions the formatter took, is to almost disable configuration, so whatever the formatter team decides, everyone follows. The only option is to turn on/off the formatter, life is easy and simple.

Let's compare dead code (currently hint), why should I user configure that? A dead code will never be executed, it is better to show it while coding. Comparing this to unneeded/unnecessary (currently lint), the user may want to use the parentheses or new, but it is optional, and that's user/development team preferences.

Personally, I believe "less is more", and as the language evolves, I guess there is no need to let the users still use the old ways for long time (using new keyword, or not migrating to super parameters). Yes, this is possibly a sensitive area, but I don't think in 10 years the Dart team would still need to carry potentially old deprecated language features, just not to disturb users. Things are made to be changed, and a major change every one/two years wouldn't be that bad. Also, the severity can change, for example, using new is an info for one year, then a warning afterwards. Even if the user uses an old-written package, the diagnostics are shown only for the current package, not for the dependencies.

From the high-level, the user chould have three sets of configurable diagnostics (restrictive/all, medium, low/none). The total number of the configurable diagnostics should not be high (I would say somewhere around 100, if not less). Anything which isn't really needed to be configured, should be non-configurable by default. Configuring every diagnostic is ok, but seeing hundreds of non-categorized diagnostics is not very user-friendly.

[srawlins]

@asashour I think we agree.

Moving the current hints (around 127) to lints will make the user be overwhelmed by the number of diagnostics which are configurable.

I agree. Note that I wrote:

They will mostly be moved to "warnings."

@bwilkerson wrote up a comprehensive list, and I think it is an extreme minority of "hints" which we may rewrite as "lints." Less than 10. Examples are DIVISION_OPTIMIZATION and PACKAGE_IMPORT_CONTAINS_DOT_DOT. They're largely obscure (or are to me).

[mit-mit]

@bwilkerson wrote up a comprehensive list

Can we put that list somewhere (e.g. in a markdown gist) and link it from this issue? That would make it easier to understand the implications of this proposed change.

[srawlins]

@mit-mit I believe @bwilkerson shared these with you offline. We could move it into a public gist. The internal doc just has lots of discussion.

[srawlins]

Progress: In early December there were 140 HintCodes. Today there are 109 HintCodes, and 35 WarningCodes.

[srawlins]

Progress: Today there are 25 HintCodes, and 122 WarningCodes.

[srawlins]

Progress: Today there are 19 HintCodes, and 135 WarningCodes. This is probably what is shipping in Dart 3.0.

[srawlins]

And there are now 165 WarningCodes and 9 HintCodes. 4 are versions of "DEPRECATED_MEMBER_USE"; 2 are deprecated; the remaining are IMPORT_DEFERRED_LIBRARY_WITH_LOAD_FUNCTION, MACRO_INFO, and UNNECESSARY_IMPORT.

[bwilkerson]

@jakemac53 @scheglov

I'd be interested to get your input on MACRO_INFO. Do we want to treat this as a lint (so that it retains the INFO severity as the name applies)? Do we want to drop it (and encourage macro authors to write a plugin to produce lints)? Or some other option?

[jakemac53]

Do we want to treat this as a lint (so that it retains the INFO severity as the name applies)?

Yes

Do we want to drop it (and encourage macro authors to write a plugin to produce lints)? Or some other option?

I don't want to force macro authors into writing plugins any more than is necessary. It complicates things unnecessarily imo, both from a user experience perspective and a macro author perspective.