Allow exceptions to `prefer_const_constructors`

[ghost]

@dnfield commented on Apr 24, 2019, 5:04 AM UTC:

Flutter uses the analyzer lint prefer_const_constructors. We love it!

Occasionally, we are able to refactor a class that wasn't const to be const. When this class lives in dart:ui, the lint lets us know where in the framework to update the constructors.

Our problem comes when we have internal customers who get the latest Flutter framework but sometimes are behind by some commits on dart:ui (because they have a custom embedder that doesn't update quite in sync with google3 for example). In a recent case, there are over 800 callsites for the changed constructor, and adding one-off ignores for all of them isn't practical. Turning off the lint also isn't desirable.

It would be really great if we could add a list of class exceptions to prefer_const_constructors, maybe something like

  const_constructor_excludes:
    - Rect.fromLTRB
    - RRect.whatever

So that we could disable the lint for just those constructors while our consumers have time to catch up, but also wouldn't require us to turn the lint completely off for large refactors like this.

/cc @cbracken who probably has more ideas about this.

This issue was moved by pq from dart-lang/sdk#36726.

[ghost]

@kevmoo commented on Apr 24, 2019, 5:48 AM UTC:

CC @pq – wonder if this should be moved to the linter repo...

[srawlins]

I don't see how the ignore list would allow consumers to catch up? Wouldn't it just hide all lint reports that they should be catching up on?

[srawlins]

(Also, for google3 we have custom whitelisting mechanisms for times when Dart adds swaths of new static analysis reports)

[cbracken]

@srawlins in this case, the lint is to allow the Flutter team to decouple making a backward-compatible change to dart:ui (constifying a constructor) and applying that change in the framework. Currently, because we enforce this lint on the framework, we're forced to tightly couple the dart:ui change with the framework update.

You could imagine a situation in which a repo contained multiple custom Flutter runtimes, but when the framework is rolled into the repo, apps build against the updated framework code (which uses const), then run on a very slightly older custom Flutter embedder that hasn't yet been updated to have the dart:ui change that constifies the constructor, which causes breakage.

This change would help with the specific case of making constructors const but as you say, this doesn't solve the overall problem of decoupling a backward-compatible change from framework updates that use that change. For such cases, something like our internal whitelisting mechanism is necessary. It just happens that making constructors const has been the biggest source of this issue for us so far. ;-)