[Enhancement] Provide code fixes/actions for warnings

[dannoe]

I was curious how Microsoft code actions (e.g. fix "Implicit With") were implemented and if we could also extend them easily by just implementing a base class or interface. It turned out to be not that trivial, but I managed to do it anyway:

Basically every code action is just a class which implements the ICodeFixProvider interface. But they are loaded quite differently than the DiagnosticAnalyzer. Microsoft is using MEF for loading them, and it is not extendable that trivial (at least I don't know a way). So I added a reference to the Harmony library to rewrite some IL at runtime, so the BusinessCentral.LinterCop assembly is added to the list of assemblies loaded by MEF. I also implemented a basic code fix for LC0001 (FlowFields should not be editable) as an example. You can have a look at the implementation at https://github.com/dannoe/BusinessCentral.LinterCop/tree/add-code-actions

Would you accept a PR to add this into BusinessCentral.LinterCop or should I start my own repo/project for this?

[StefanMaron]

Oh please just throw in the PRs. I did not do any code fixes yet, simply because I do not understand how to implement them

But I am not quite sure about the dependency. Did you have a look at the code from MS? I know they implemented some fixes in one if their code cops

[StefanMaron]

Okay, I finally gave this is try, however I do not get it to work for me with the example you provided

[StefanMaron]

maybe that helps?

[dannoe]

I can reproduce the exception with the newest AL compiler. I'm going to take a look at it. They probably changed the code I am trying to patch with harmony.

Maybe we should ask Microsoft to add an "official" way to add code fix actions (like they did for analyzers).

[StefanMaron]

I asked, they said no :D

To come back to the original question, I think we can go this way, but only if we can create this as a separate dll to not introduce a dependency to the diagnostics part of the linter.

What do you think?

[dannoe]

It looks like Harmony currently doesn't support patching a .NET 6 dll via transpiler. At the time of writing this proof-of-concept, the default runtime of the AL Editor backend must have been .NET Framework 4.8.

I opened an issue on the harmony repo, maybe it's easy to implement. (see https://github.com/pardeike/Harmony/issues/503) If not I am thinking about using the "normal" way of patching with harmony.

I asked, they said no :D

Did they provide a reason?

To come back to the original question, I think we can go this way, but only if we can create this as a separate dll to not introduce a dependency to the diagnostics part of the linter.

What do you think?

Sounds okay. Should this extra project be a part of this repository or have its own repository?

[StefanMaron]

The reason was that the only reason to open up the class and make this kind of changes would be to officially support extending it, and they dont want to do that.

Well, the code actions project would be 100% depending on the overwriting of that class, and I am not able to support that at all :D But I think I could add this the the linter vs code extension to handle the auto download if it would be a separate repo.

[dannoe]

The problem with harmony was the following: The default .NET runtime of the AL compiler is .NET 6 but the LinterCop is still compiled with .NET Framework 4.8. As I was using harmony via NuGet, it grabs the .NET FX 4.8 version of Harmony. As a result harmony tries to use methods from the .NET Framework which are non existent in .NET 6.

If I find the time to setup a new repository for the CodeFixes I will do so and comment on this issue.

[f4n0]

I think the only way is to add the vscode handler inside the VScode Lintercop extension. just because you need to extend the vscode.CodeActionProvider ts class and register your providers.