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

IDE0018 preview presents incorrect option if blank line present #38558

Open
stephentoub opened this issue Sep 6, 2019 · 3 comments
Open

IDE0018 preview presents incorrect option if blank line present #38558

stephentoub opened this issue Sep 6, 2019 · 3 comments

Comments

@stephentoub
Copy link
Member

Version Used:
3.4.0-beta1-19426-03+b18d90790da95b6862def56802fa4249e4868bb4

Steps to Reproduce:

using System;

class Program
{
    static void Main() { }

    private static bool M1(string name)
    {
        bool result;

        Bar(name, out result);
        return result;
    }

    private static bool M2(string name)
    {
        bool result;
        Bar(name, out result);
        return result;
    }

    private static bool Bar(string name, out bool result) => throw new NotImplementedException();
}

Expected Behavior:
Invoking IDE0018 for the Document presents two cases to be fixed, one in M1 and one in M2.

Actual Behavior:
Invoking IDE0018 for the Document presents three cases to be fixed, two in M1, one of which if executed on its own would result in code that doesn't compile:
image

@sharwell
Copy link
Member

sharwell commented Sep 6, 2019

This is happening because the check boxes in the preview dialog are based on hunks of the final diff, and not based on fixes for individual issues. For cases where each fix produces exactly one distinct change hunk, the boxes correspond to fixes. However, for cases where fixes are either split across multiple locations/files and cases where fixes apply changes that are not physically separate in the file, the distinction becomes apparent.

/cc @mavasani

@stephentoub
Copy link
Member Author

Thanks, Sam. Does that mean this is by-design?

FWIW, it was a poor experience for me. I hit it because I first applied and accepted all of the changes IDE0018 made across the solution, but then as I was reviewing all the changes, I found myself manually editing / reverting a bunch of them. So I instead undid everything and tried again, the second time planning to go through and view the edits in the preview changes dialog, thinking I could just click through each change and see what the transformation was by clicking/unclicking the checkbox... but that fell apart quickly with a single refactoring split across multiple check boxes.

@mavasani
Copy link
Member

mavasani commented Sep 6, 2019

Yes, that is correct. Note that FixAll in scope computation can happen in couple of ways:

  1. Code fixer uses a batch fixer (ones exposed in Roslyn or a custom batch fixer), which computes individual fix for each diagnostic separately and then the batch fixer merges them.
  2. Code fixer uses it's own implementation of the FixAllProvider API which produces a single code action/fix with all changes. In this case, we do not even have concept of multiple code actions/fixes being accumulated, but just a single code fix that fixes multiple diagnostics.

The FixAll preview engine and application is only presented with a single merged code action with one or more text changes across one or more documents. So supporting the feature request here might not even be feasible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants