Need breaking change policy

[srawlins]

Maybe this needs to be filed and discussed in https://github.com/dart-lang/sdk instead of here. But I'll start here.

Two issues have brought up the idea of breaking changes recently:

• Settle on a process for deprecating lints #57794 - Settle on a process for deprecating lints
• Warn on call to Map.remove with unrelated type #57434 - my comment on making the list_remove_unrelated_type rule catch more things.

Removing a rule is a breaking change in that it causes people's analysis_options.yaml files to now parse with an error. Changing an existing rule to complain about more things causes is breaking in that it causes currently-clean codebases to no longer be lint-free.

Adding a new rule is not a breaking change since all lint rules are opt-in.

There is a concern that releasing a version of linter with breaking changes will get rolled into the SDK, into the analyzer, which will change the behavior of dartanalyzer and the analysis server for existing code.

I say (as a volunteer on the linter and analyzer projects 😛) that such a change should not be considered a breaking change in the SDK, and should not require a major version bump by the SDK.

Why?

First, a historical reason: the analyzer has always released a steady stream of new analyzer codes, which are on by default. Releases between Dart 1.0 and 2.0 consistently featured new analyzer codes, and this was a beautiful, beautiful thing. 😄 Until recently (like, since Dart 1.24 or so), many pub packages did not use presubmit hooks that ensured code was Error-, Warning-, and Hint-free, so this was not a super visible change, in terms of "breakiness." It was a super visible change in user's IDEs, much to all of our delight.

Ergo, releasing breaking-change linter releases into the SDK's analyzer is not a change in behavior. But more importantly, this leads us into the second reason:

Second, a technical reason: breaking changes hurt developers and users only in specific cases, not universally. When a breaking change causes package foo to break (e.g. by making it un-compilable, or to crash at runtime), it can hurt both foo's developers, and foo's users. This hurts the developer of the package, which, assuming they're still around and aware of the breakage, must commit new changes, perhaps solve unspecified problems, or write non-trivial fixes. It super duper hurts users of the package, who are stuck with a broken dependency until foo*'s developers release a fix.

However, breaking changes in the linter hurt no one in such a way:

• Deleting a lint rule does not cause foo to stop compiling or running, when downstream code depends on foo. Deleting a lint rule can cause foo's authors angst as they upgrade the Dart SDK, and can no longer run dartanalyzer (and perhaps analysis server?) because of the parse error in analysis_options.yaml. The fix however is straightforward and trivial (delete the rule from analysis_options.yaml).
• Improving a lint rule to report more issues in foo also does not cause it to stop compiling or running when downstream code depends on foo. It can cause foo's authors angst as they upgrade the Dart SDK, and their dartanalyzer results show new results. Again, deleting the rule from analysis_options.yaml will fix the issue. Additionally, many lint issues are not hard to fix individually, though that's not universally true.

In either case, downstream users are not affected by a breaking change in linter, and developers of an affected package have very trivial fixes. Developers of an affected package are not hindered in their workflow.

The alternative

In my view, the alternative is much worse. The alternative to me seems to be this: a breaking change in the linter (changing a rule to report more, or deleting a rule) are considered breaking changes in the SDK, and can only be landed once every few years, when the Dart SDK is bumped. This includes bug fixes, such as dart-lang/linter#1218, which extends a rule to apply to arrow functions (in addition to the original block function functionality).

[srawlins]

CC @a14n @bwilkerson @pq

[bwilkerson]

... and can no longer run dartanalyzer (and perhaps analysis server?) because of the parse error in analysis_options.yaml.

Actually, both the command-line analyzer and analysis server simply ignore rules that are specified but don't exist, and will run all of the valid rules. User's won't be broken in this sense either.

[srawlins]

Actually, both the command-line analyzer and analysis server simply ignore rules that are specified but don't exist, and will run all of the valid rules. User's won't be broken in this sense either.

Even better! 👍 👍 👍

[pq]

More food for thought from @bwilkerson (copied from mail):

Is it ok to extend an existing lint to catch related cases, especially when the language grows, as in the case that sparked this thread?
Is it ok to fix a bug in a lint if it produces violations that used to be missed?
Is it ok to split an existing lint into multiple lints (as we recently did with the deprecation hints)?
Is it ok to merge two lints that don't need to be separate?

How do we help users migrate when breaking changes need to be made?

(The issue referenced is captured in #57909.)

[pq]

As a point of reference, Flutter's breaking changes doc might serve as inspiration for a process once we've established what qualifies as breaking.