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

build(analyzer): replace FxCopAnalyzers with NetAnalyzers #34780

Merged
merged 5 commits into from
Mar 21, 2023

Conversation

archerzz
Copy link
Member

@archerzz archerzz commented Mar 8, 2023

FxCopAnalyzers is obsolete and it's reocmmended to use NetAnalyzers. See correponding issue for details.

resolve #34779

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

`FxCopAnalyzers` is obsolete and it's reocmmended to use `NetAnalyzers`. See correponding issue for details.

resolve Azure#34779
@archerzz
Copy link
Member Author

archerzz commented Mar 8, 2023

@dvbb helped to verify that this PR can fix the false positive of CA1825. But replacing analyzers triggers more errors, probably due to newly added rules. Need further work on this.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@archerzz
Copy link
Member Author

archerzz commented Mar 8, 2023

I randomly check some SDKs and the new analyzer will not fail the build.

@jsquire jsquire requested a review from heaths March 8, 2023 15:09
@jsquire
Copy link
Member

jsquire commented Mar 8, 2023

@heaths: Would you please review?

@jsquire jsquire requested a review from pallavit March 8, 2023 15:10
@archerzz
Copy link
Member Author

@heaths I've run dotnet build ./eng/service/proj which should cover all projects. I've got 7 errors:

C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\eventhub\Microsoft.Azure.EventHubs\tests\ServiceFabricProcessor\OptionsTests.cs(64,13): error xUnit2000: The literal or con
stant value tag should be passed as the 'expected' argument in the call to 'Assert.Equal(expected, actual)' in method 'SimpleOptionsTest' on type 'OptionsTests'. Swap the parameter values
. [C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\eventhub\Microsoft.Azure.EventHubs\tests\Microsoft.Azure.EventHubs.Tests.csproj::TargetFramework=net6.0]
C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\eventhub\Microsoft.Azure.EventHubs\tests\ServiceFabricProcessor\OptionsTests.cs(64,13): error xUnit2000: The literal or con
stant value tag should be passed as the 'expected' argument in the call to 'Assert.Equal(expected, actual)' in method 'SimpleOptionsTest' on type 'OptionsTests'. Swap the parameter values
. [C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\eventhub\Microsoft.Azure.EventHubs\tests\Microsoft.Azure.EventHubs.Tests.csproj::TargetFramework=net7.0]
C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\eventhub\Microsoft.Azure.EventHubs\tests\ServiceFabricProcessor\OptionsTests.cs(64,13): error xUnit2000: The literal or con
stant value tag should be passed as the 'expected' argument in the call to 'Assert.Equal(expected, actual)' in method 'SimpleOptionsTest' on type 'OptionsTests'. Swap the parameter values
. [C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\eventhub\Microsoft.Azure.EventHubs\tests\Microsoft.Azure.EventHubs.Tests.csproj::TargetFramework=net461]
C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\purview\Azure.Analytics.Purview.Administration\src\Generated\PurviewAdministrationClientBuilderExtensions.cs(32,173): error
 CS1503: Argument 3: cannot convert from 'Azure.Analytics.Purview.Administration.PurviewAccountClientOptions' to 'Azure.Analytics.Purview.Administration.PurviewMetadataClientOptions' [C:\
Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\purview\Azure.Analytics.Purview.Administration\src\Azure.Analytics.Purview.Administration.csproj::TargetFramework=netstandard2
.0]
C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\purview\Azure.Analytics.Purview.Administration\src\Generated\PurviewAdministrationClientBuilderExtensions.cs(42,191): error
 CS1503: Argument 4: cannot convert from 'Azure.Analytics.Purview.Administration.PurviewAccountClientOptions' to 'Azure.Analytics.Purview.Administration.PurviewMetadataClientOptions' [C:\
Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\purview\Azure.Analytics.Purview.Administration\src\Azure.Analytics.Purview.Administration.csproj::TargetFramework=netstandard2
.0]
C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\webpubsub\Azure.Messaging.WebPubSub\src\WebPubSubServiceClientBuilderExtensions.cs(25,98): error CS0111: Type 'WebPubSubSer
viceClientBuilderExtensions' already defines a member called 'AddWebPubSubServiceClient' with the same parameter types [C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\web
pubsub\Azure.Messaging.WebPubSub\src\Azure.Messaging.WebPubSub.csproj::TargetFramework=netstandard2.0]
C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\webpubsub\Azure.Messaging.WebPubSub\src\WebPubSubServiceClientBuilderExtensions.cs(52,98): error CS0111: Type 'WebPubSubSer
viceClientBuilderExtensions' already defines a member called 'AddWebPubSubServiceClient' with the same parameter types [C:\Users\mingzhehuang\workspaces\archerzz\azure-sdk-for-net\sdk\web
pubsub\Azure.Messaging.WebPubSub\src\Azure.Messaging.WebPubSub.csproj::TargetFramework=netstandard2.0]

  • 3 errors are about xUnit styles which are reported by xunit.analyzers which probably be brought in as dependency of xunit.
  • the other 4 errors are irrelevant. they are newly generated ASP.Net extension codes which have not been checked in.

So looks like we can directly apply this change and monitor any future break?

@heaths
Copy link
Member

heaths commented Mar 20, 2023

Do any of those errors happen without your changes? If they are regressed by your change, they'll need to be fixed. If not - if they already exist without your changes - then we can go ahead and merge.

@archerzz
Copy link
Member Author

Do any of those errors happen without your changes? If they are regressed by your change, they'll need to be fixed. If not - if they already exist without your changes - then we can go ahead and merge.

@heaths They are not regressions by my change. The latter 4 have already been fixed on main branch. The former 3 (e.g. xunit style errors) exists on main branch as well. They are not detected probably because those test codes have not been touched for quite a long time. Other more active projects have worked around it:

#pragma warning disable xUnit2000 // Literal or constant should be the first argument to Assert.NotEqual
Assert.NotEqual(maybeSeven, null, new ModelComparer<int?>());
Assert.NotEqual(maybePi, null, new ModelComparer<double?>());
Assert.NotEqual(maybeTrue, null, new ModelComparer<bool?>());
#pragma warning restore xUnit2000 // Literal or constant should be the first argument to Assert.NotEqual

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

Successfully merging this pull request may close these issues.

[FEATURE REQ] Replace Microsoft.CodeAnalysis.FxCopAnalyzers with Microsoft.CodeAnalysis.NetAnalyzers
6 participants