sdk icon indicating copy to clipboard operation
sdk copied to clipboard

auto fix unnecessary duplication of receiver

Open bsutton opened this issue 2 years ago • 5 comments

I originally reported this for Dart-Code but Dan asked for the issue to be raised here: https://github.com/Dart-Code/Dart-Code/issues/4550

The following code generates a warning:

cascade_invocations

Unnecessary duplication of receiver.\nTry using a cascade to avoid the duplication.

      final subnet = Subnet();
      subnet.name = parts[0];
      subnet.zone = parts[1];
      subnet.network = parts[2];
      subnet.range = parts[3];
      subnets.add(subnet);

It would be useful to have an auto fix which changes the code to:

     final subnet = Subnet()
        ..name = parts[0]
        ..zone = parts[1]
        ..network = parts[2]
        ..range = parts[3];
      subnets.add(subnet);

There are potentially two issues here.

In vs code if you click on the third line :

   final subnet = Subnet();
      subnet.name = parts[0];
      subnet.zone = parts[1];

an auto-fix appears for changing to cascade notation.

However, I would also expect that the 2nd line should also offer to convert to cascade notation (as it also triggers the cascade_invocation lint) but it does not.

Of course, if we implement the - fix all, and that fixes all lines as suggested, then it may not be worth fixing the fix single bug.

bsutton avatar May 22 '23 23:05 bsutton

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.

bwilkerson avatar May 23 '23 00:05 bwilkerson

My preference is for a single action that fixes all of the cascades in a single action. I really don't want to have to apply the fix for each individual line. I often have 4-6 that need to be combined and 10+ isn't uncommon.

bsutton avatar May 23 '23 01:05 bsutton

Would be good addition to have this...

Vedsaga avatar Nov 19 '23 06:11 Vedsaga

However, I would also expect that the 2nd line should also offer to convert to cascade notation (as it also triggers the cascade_invocation lint) but it does not.

The fix not triggering on that line was fixed by https://dart-review.googlesource.com/c/sdk/+/386860.

I still think this can be left open to track the bulk fix.

FMorschel avatar Sep 27 '24 16:09 FMorschel

After some discussion on Discord with @bwilkerson and @DanTup, taking https://github.com/dart-lang/sdk/issues/58662 (false positives/negatives) into consideration. This issue could mean that the multi/bulk fix for transforming every statement into a cascade could create code that didn't have the same meaning as before. So we decided that the best approach would be to create a "convert this and related to cascade" and keep both fixes so the user can decide which to use. For the above case, it would fix all together but it would still need to be manually triggered for every set of possible transformations in the file.

Here is the current CL https://dart-review.googlesource.com/c/sdk/+/390421.


E.g.:

      final subnet = Subnet();
      subnet.name = parts[0];
      subnet.zone = parts[1];
      subnet.network = parts[2];
      subnet.range = parts[3];
      subnets.add(subnet);

      final subnet2 = Subnet();
      subnet2.name = parts[0];
      subnet2.zone = parts[1];

If we select the new fix on any of the lints before the empty line, we get:

      final subnet = Subnet()
        ..name = parts[0]
        ..zone = parts[1]
        ..network = parts[2]
        ..range = parts[3];
      subnets.add(subnet);

      final subnet2 = Subnet();
      subnet2.name = parts[0];
      subnet2.zone = parts[1];

If we had selected the same after the empty line, the second set would have transformed but not the first.

FMorschel avatar Oct 16 '24 05:10 FMorschel