cake-build / cake-vscode

Contains Cake extension for Visual Studio Code.
https://marketplace.visualstudio.com/items?itemName=cake-build.cake-vscode
MIT License
42 stars 33 forks source link

Cake script is contributing unactionable diagnostics to the Problems pane #791

Closed alexrp closed 1 year ago

alexrp commented 1 year ago

image

These aren't from a real file, so clicking them takes me nowhere. I would assume that they come from code that Cake inserts?

The repo where this happens is here.

devlead commented 1 year ago

The code generation predates .NET 6 / 7, so could be that there could be optimizations to utilize newer features i.e. code like https://github.com/cake-build/cake/blob/e2ccd154a15c0cc55ec2dbc00503e8ea79e2dcd8/src/Cake.Core/Scripting/CodeGen/PropertyAliasGenerator.cs#L236-L238 Could probably use the null-coalescing assignment operator ??= to reduce the number of lines of code. I.e. something like

private Cake.Common.Build.BuildSystem _BuildSystem;
public Cake.Common.Build.BuildSystem BuildSystem
{
    [System.Diagnostics.DebuggerStepThrough]
    get
    {
        if (_BuildSystem==null)
        {
            _BuildSystem = Cake.Common.Build.BuildSystemAliases.BuildSystem(Context);
        }
        return _BuildSystem;
    }
}

could be something like (plus some handling for obsolete logging)

private Cake.Common.Build.BuildSystem _BuildSystem;
public Cake.Common.Build.BuildSystem BuildSystem
    => _BuildSystem ??= Cake.Common.Build.BuildSystemAliases.BuildSystem(Context);

Right now you could disable the warning with a pragma on top of your cake file

#pragma warning disable IDE0074

See: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0054-ide0074

alexrp commented 1 year ago

Right now you could disable the warning with a pragma on top of your cake file

This doesn't actually seem to work FWIW.

More broadly, I don't know if it ever makes sense for the user to see diagnostics from generated code that comes from Cake itself anyway. To me, it would be a bit like getting diagnostics from code created by a source generator. There's not a whole lot that I, as a user, can do about it.

devlead commented 1 year ago

This doesn't actually seem to work FWIW.

Probably because generated is inserted before your script, the code is merged with your code so Roslyn sees it as the same code, the code was written before the language feature analyzer suggests existed, it's also the same code generated regardless of the target framework. And analyzer didn't use to flag this suggestion for potential improvement.

Another possibility could be to exclude with config, something like

[*.cake]
dotnet_diagnostic.IDE0074.severity = none

https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-files

Cake now only targets net6 & net7 so future code gen could be improved in a future version, but hard to fix retroactively.

augustoproiete commented 1 year ago

Thanks @alexrp I added an issue on the Cake repo, to see what we can do to Cake itself, to avoid these problems being reported - https://github.com/cake-build/cake/issues/4150

alexrp commented 1 year ago

Closing since https://github.com/cake-build/cake/issues/4150 was fixed.