search

dotnet / format

Home for the dotnet-format command
MIT License
1.92k stars 174 forks source link

CA1822 not reported by VS or locally is mistakenly "fixed" in GitHub Action #1960

Closed jfversluis closed 6 months ago

jfversluis commented 11 months ago

In the .NET MAUI repo we have a curious case regarding dotnet format.

We are using the formatting tool to run a daily check on anything that might be merged that needs formatting/improvement. Recently we have started to add the enforcement of CA findings as well, and in this case it's about this particular set. Note that the file MauiScrollView.cs is now included and in this same PR no changes have been made to this particular file.

Looking at this file from Visual Studio (Code) no CA is triggered. However, now our daily dotnet format run comes along and decides that there is a CA1822 in there and a method needs to be made static. By doing that, it will break the code and thus our build, so fixing the warning is not something we should want.

I've added a screenshot below because the commit on our repo is likely to disappear, but the PR can be found here.

image

This code can be found here: https://github.com/dotnet/maui/blob/main/src/Core/src/Platform/Android/MauiScrollView.cs#L294 and is the IOnScrollChangeListener.OnScrollChange

Additionally, trying to reproduce this with running dotnet format locally, it also does not try and fix this instance, it seems to only happen on our GitHub Action.

So not sure if this is some kind of bug or rather maybe a question to determine why there is this difference. To support the theory that this does not happen in an IDE, but does happen when running this through GitHub Actions: this PR, that adds #pragma warning disable CA1822 for this method signature, prevents dotnet format from making this change.

I'd be curious to know why there is this difference in how this is handled locally vs Visual Studio vs in a GitHub Action so that I know this is a proper fix or if we should go about this any other way.

From what I can tell the tooling version is the same on all machines and at the time of writing is .NET 8 RC1. If you need any more details please let me know or you can reach out directly on Teams.

Thanks!

sharwell commented 7 months ago

It is unlikely that this issue is caused by dotnet format directly. It is likely caused by version differences in the analyzer assemblies getting used, e.g. if different SDK versions are involved. A binlog comparing local execution of dotnet format with CI execution would be helpful, although #2094 suggests this may not be working currently.

ghost commented 6 months ago

Closing this issue as we've seen no reply to the request for more information. If you are able to get the requested information, please add it to the issue and we will retriage it.