`dart fix --apply` fails to fix lint

[osa1]

Tried with sdk 7fa9a6776a34cb482056d42f8b781a2a5fe3e539.

Input:

class Test {
  List<int> lengthDelimited = <int>[];
  List<int> varints = <int>[];
  List<int> fixed32s = <int>[];
  List<int> fixed64s = <int>[];
  List<int> groups = <int>[];

  List get values => <int>[]
    ..addAll(lengthDelimited)
    ..addAll(varints)
    ..addAll(fixed32s)
    ..addAll(fixed64s)
    ..addAll(groups);
}

void main() {}

dart analyze output:

   info - protobuf_test.dart:9:7 - Use spread collections when possible. - prefer_spread_collections

File after running dart fix --apply:

class Test {
  List<int> lengthDelimited = <int>[];
  List<int> varints = <int>[];
  List<int> fixed32s = <int>[];
  List<int> fixed64s = <int>[];
  List<int> groups = <int>[];

  List get values => <int>[...lengthDelimited, ...varints, ...fixed32s, ...fixed64s]

    ..addAll(groups);
}

void main() {}

Note that groups is not moved to the list syntax, and the new file has the same lint error.

Interestingly if I remove ..addAll(groups) in the original program then dart fix --apply doesn't leave the last addAll outside the list syntax and produces lint-free output. Is there a hard-coded limit on the depth of the tree to transform maybe?

[srawlins]

It was so close! Good catch and details, @osa1!

[bwilkerson]

Is there a hard-coded limit on the depth of the tree to transform maybe?

Sort of.

When that file is analyzed there's only one diagnostic produced, as you noted. The fix fixes that one diagnostic. The dart fix command then re-analyzes the file (with the fix applied) to see whether there are any other problems that are now being reported. Once again there is one diagnostic, and the loop repeats.

But because some of the lints can be contradictory (such as prefer_double_quotes and prefer_single_quotes), we knew that it was possible for the loop to be infinite. To guard against that, we added a hard coded limit to the number of times it would repeat the attempt to fix diagnostics. That's what you're running up against.

We have some ideas for possible ways of detecting infinite loops, but they're all just heuristics and might still require a fixed limit in order to prevent the tool from spinning.

The best I can offer you at the moment is that sometimes you might be required to run dart fix more than once.

[FMorschel]

On the other hand, it seems like it ought to be fairly simple to expand the range over which the fix is being made available; in general it ought to be offered anywhere inside the diagnostic's highlight range.

Can @bwilkerson's above comment idea be applied here as well? Or am I misunderstanding this?

I see that the current highlight only applies to the first addAll name and nothing more as well. Why is that? I mean, why wouldn't it find all of the other addAll calls in this snippet?

[bwilkerson]

The lint is fairly limited. It looks for a cascade invocation of addAll applied to a list literal. The first matches that pattern; the others don't. And it sort of makes sense because you can't change any of the other invocations unless you first choose to translate the first invocation.

(That's a little different than the situation the comment applies to. In that case it's possible to change the second occurrence without changing the first.)

I suppose we could change the lint to report all of the addAll invocations that could be converted into spreads, and change the fix to do the same. It might even be the right approach.