Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auto fix unnecessary duplication of receiver #52476

Closed
bsutton opened this issue May 22, 2023 · 5 comments
Closed

auto fix unnecessary duplication of receiver #52476

bsutton opened this issue May 22, 2023 · 5 comments
Labels
analyzer-quick-fix area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@bsutton
Copy link

bsutton commented May 22, 2023

I originally reported this for Dart-Code but Dan asked for the issue to be raised here:
Dart-Code/Dart-Code#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.

@bwilkerson bwilkerson added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-quick-fix labels May 23, 2023
@bwilkerson
Copy link
Member

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.

@bsutton
Copy link
Author

bsutton commented May 23, 2023

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.

@srawlins srawlins added P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug labels May 23, 2023
@Vedsaga
Copy link

Vedsaga commented Nov 19, 2023

Would be good addition to have this...

@FMorschel
Copy link
Contributor

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
Copy link
Contributor

FMorschel commented Oct 16, 2024

After some discussion on Discord with @bwilkerson and @DanTup, taking #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-quick-fix area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants