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

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

Closed
jfversluis opened this issue Sep 27, 2023 · 2 comments
Closed
Labels
Needs More Info This issue needs additional details to be actionable

Comments

@jfversluis
Copy link
Member

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

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.

@sharwell sharwell added the Needs More Info This issue needs additional details to be actionable label Feb 15, 2024
@ghost ghost closed this as completed Feb 25, 2024
@ghost
Copy link

ghost commented Feb 25, 2024

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.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info This issue needs additional details to be actionable
Projects
None yet
Development

No branches or pull requests

2 participants