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

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

Closed
alexrp opened this issue Apr 11, 2023 · 5 comments
Closed

Comments

@alexrp
Copy link

alexrp commented Apr 11, 2023

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

devlead commented Apr 11, 2023

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

alexrp commented Apr 11, 2023

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

devlead commented Apr 11, 2023

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

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 - cake-build/cake#4150

@alexrp
Copy link
Author

alexrp commented Nov 28, 2023

Closing since cake-build/cake#4150 was fixed.

@alexrp alexrp closed this as completed Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants