Skip to content

Enable Roslynator#21013

Merged
abcdefg30 merged 1 commit intoOpenRA:bleedfrom
RoosterDragon:style-roslynator
Oct 30, 2023
Merged

Enable Roslynator#21013
abcdefg30 merged 1 commit intoOpenRA:bleedfrom
RoosterDragon:style-roslynator

Conversation

@RoosterDragon
Copy link
Copy Markdown
Member

@RoosterDragon RoosterDragon commented Aug 22, 2023

Remove existing rules which were not enforced and have some existing violations. Enforce a suite of useful rules that have no existing violations.


Following on from #20996.

We added some Roslynator rules to the config in previous code quality PRs, but we never added the nuget package for it so the rules were never enforced as such. Reset by removing the existing rules and enforcing some useful rules with no current violations. Follow-up PRs will look to enable ~30 other useful rules the require violations to be fixed.

Roslynator seems to have some useful rules above and beyond the existing solutions we have in place, it's the last extra rule component I currently think would be worth implementing.

See https://josefpihrt.github.io/docs/roslynator/analyzers for explanation of each rule.

@RoosterDragon RoosterDragon force-pushed the style-roslynator branch 2 times, most recently from 634a65a to 0980fee Compare August 22, 2023 17:27
@pchote
Copy link
Copy Markdown
Member

pchote commented Oct 28, 2023

This gives me a spam of errors like

CSC : error CS9057: The analyzer assembly '/Users/paul/.nuget/packages/roslynator.analyzers/4.4.0/analyzers/dotnet/cs/Roslynator.CSharp.Analyzers.dll' references version '4.4.0.0' of the compiler, which is newer than the currently running version '4.3.0.0'. [/Users/paul/src/OpenRA/OpenRA.Game/OpenRA.Game.csproj]

paul@Blackjack:OpenRA+pr/21013$ dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.404
 Commit:    be4f3ec411

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  14.0
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/6.0.404/

global.json file:
  Not found

Host:
  Version:      6.0.12
  Architecture: arm64
  Commit:       02e45a41b7

.NET SDKs installed:
  6.0.404 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.12 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.12 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

Download .NET:
  https://aka.ms/dotnet-download

Learn about .NET Runtimes and SDKs:
  https://aka.ms/dotnet/runtimes-sdk-info

@RoosterDragon
Copy link
Copy Markdown
Member Author

I've downgraded from Roslynator 4.4.0 to 4.2.0. This depends on compiler version 4.0, so should increase the compatibility. I suspect since the CI runner is using .NET 7 SDK it is avoiding running into the same issue.

@pchote
Copy link
Copy Markdown
Member

pchote commented Oct 28, 2023

LGTM, but if I switch to the files view I see a collection of "Unnecessary usage of verbatim string literal." warnings in MiniYamlTest.cs that should probably be fixed before merging.

Remove existing rules which were not enforced and have some existing violations. Enforce a suite of useful rules that have no existing violations.
@RoosterDragon
Copy link
Copy Markdown
Member Author

I've removed that rule for now. Will deal with enabling rules that require fixes in some follow-ups.

@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Oct 30, 2023

Changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants