Should prefer_collection_literals take SDK version into account?

[jamesderlin]

My pubspec.yaml has:

environment:
  sdk: ">=2.0.0-dev.68.0 <3.0.0"

Using dartanalyzer version 2.2.1-edge.a8f3a5dae6203d1064726a5953cf06a7d484249c against the following code triggers:

final mySet = Set<String>();

lint • Use collection literals when possible at ... • prefer_collection_literals

But if I change the code to:

final mySet = <String>{};

then I get:

hint • Set literals were not supported until version 2.2, but this code is required to be able to run on earlier versions at ... • sdk_version_set_literal

So I can't win. I could add // ignore: prefer_collection_literals, but then I won't get the lint when I do upgrade the minimum SDK version.

Should the prefer_collection_literals lint should take the minimum SDK version into account to restrict "when possible"?

[bwilkerson]

IMO, yes, it should.

[pq]

I've been thinking about this a bit. @bwilkerson: would this be a reasonable approach:

  @override
  void registerNodeProcessors(NodeLintRegistry registry,
      [LinterContext context]) {
    final versionConstraint = context?.analysisOptions?.sdkVersionConstraint;
    if (versionConstraint != null && before_2_2_0.intersect(versionConstraint).isEmpty) {
      return;
    }

    final visitor = new _Visitor(this);
    registry.addInstanceCreationExpression(this, visitor);
    registry.addMethodInvocation(this, visitor);
  }

where, taking the cue from analyzer, before_2_2_0 looks like:

  VersionRange get before_2_2_0 =>
      new VersionRange(max: Version.parse('2.2.0'), includeMax: false);

If it is, I can generalize a bit and propose something concrete. (We'll want to do similarly for UI-as-code features short term and new experiments moving forward.)

[bwilkerson]

I'm not sure what the effect is when a lint rule doesn't register. It probably effectively disables it, which is what I think you're going for.

@stereotype441 is planning a re-design of the sdk version checking support, so we might want to wait for that work to be done.

[pq]

It probably effectively disables it, which is what I think you're going for.

Yes. If the visitors don't get registered, the lint will never get reported.

@stereotype441: is the re-design something you have in mind for soon? And will it be API?

[pq]

I've been thinking about this a bunch over the last few days and though I have a prototype working that would solve the immediate problem, I'm more and more convinced that we should probably sit tight until the language versioning conversation shakes out (see, e.g., dart-lang/language#311). Ideally a good approach here will shake out of that and minimally we'll need to be compatible.

Thanks for your patience!

[pq]

See also the conversation in #58394.

[parlough]

Since language versions before this syntax existed aren’t supported anymore, I’m going to close this in favor of #36677 and other similar issues. They discuss a generic way to handle language versions in lints. I do think the situation is much better today now that language versioning does exist.