Add support for code actions besides ApplyChangesOperation's

[Samirat]

No description provided.

[filipw]

thanks a lot, can you please describe the scenario that you are trying to fix? due to some complexities of how to deal with editor state in such cases, the other code action types have been excluded by design.

also, in addition to that, is there some test that you can add to demonstrate what is now working better?

[Samirat]

@filipw Thanks for the review.

I have an analyzer with a non-ApplyChangesOperation code fix. Currently the behavior is that there's a little button that says 'fix it' and you click it and it just does nothing with no error. If these really aren't supported than they shouldn't show up in the UI, or they should generate an error when you try to use them.

With this change, the code fix works, although I do have to edit code to get it to refresh the state. Still a much better experience.

It seems like I should be able to add tests for this. Please confirm if you want the change, and I'll do it.

[filipw]

I am more worried about creating a corrupt state between editor and server.
at the very least some for of a new response is needed.

can you show which analyzer exactly you are using? there are analyzer tests here too, so it should be possible for you to add it as well - thanks!

[Samirat]

@filipw Analyzer I was trying to get working is here:
https://dev.azure.com/mseng/AzureDevOps/_git/AzureDevOps.Analyzers?path=%2FRegisteredUsageAnalyzer%2FAddEntryCodeAction.cs

Not open source, but maybe you can see it.

[Samirat]

If it's dangerous to do arbitrary code fixes without signaling what's changed, maybe this should emit some kind of error message instead? In any case silently ignoring the request seems like a bug. The particular code fixes I was trying to fix could probably be made into ApplyChangesOperation's if they need to.

I'm curious what kind of state corruption could occur, though. As far as I can tell, there's no feedback to either the editor or Omnisharp when random code fixes are applied. So it should be no different than opening up notepad and applying the code fix manually or something :).

[filipw]

unfortunately I get 401 when accessing this.

Can you please post the analyzer here? what is its type? In either case we will likely have to special case this - many of the CodeActionOperation like InstallWithPackageManagerCodeActionOperation or PreviewOperation would not work anyway

[Samirat]

@filipw We've got an analyzer that requires certain code patterns to be registered in files, and then we've got a code action which adds them to the file.

The code for the operation is just implemented as a custom operation, and is pretty straightforward. Since these 'registration' files are not part of the solution, it's not an ApplyChangesOperation, but it works fine to run from omnisharp.

    private class AddEntryOperation : CodeActionOperation
    {
        public AddEntryOperation(String registryPath, String entryKey)
        {
            RegistryPath = registryPath;
            EntryKey = entryKey;
        }

        public String RegistryPath { get; }

        public String EntryKey { get; }

        public override String Title => $"Add entry to registry file {Path.GetFileName(RegistryPath)}";

        public override void Apply(Workspace workspace, CancellationToken _)
        {
            String[] entries = File.ReadAllLines(RegistryPath);
            int index = Array.BinarySearch(entries, EntryKey, StringComparer.OrdinalIgnoreCase);
            if (index >= 0)
            {
                // Entry was already present.
                return;
            }

            index = ~index;
            IEnumerable<String> newEntry = new String[] { EntryKey };
            File.WriteAllLines(
                RegistryPath,
                entries.Take(index).Concat(newEntry).Concat(entries.Skip(index).Take(entries.Length - index)));
        }
    }

[Samirat]

@filipw Any more thoughts on this change? I don't know what other operations people have, but in my case, the operation is safe to apply even though it is not an ApplyChangesOperation. If it isn't going to get merged, though, I will abandon the PR. However, I think we could instead add some logic to filter out these operations from appearing in VS Code or, if this is difficult, some messaging to tell people that their code fix is not being applied. Currently you click the apply code fix button and it does nothing. I'd be happy to stick an error notification or something there.

[filipw]

@Samirat thanks, let's merge this for the time being, I think your reasoning makes sense

[Samirat]

@filipw Doesn't look like I can merge this myself.