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

Remove unused field in LspServices #11358

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Remove unused field in LspServices #11358

merged 2 commits into from
Jan 7, 2025

Conversation

mthalman
Copy link
Member

@mthalman mthalman commented Jan 7, 2025

When using the latest daily build of the .NET 10 SDK to build this repo, it results in the following errors:

/__w/1/vmr/src/razor/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/LspServices.cs(17,56): error IDE0052: Private member 'LspServices._startupServices' can be removed as the value assigned to it is never read (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0052) [/__w/1/vmr/src/razor/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Microsoft.AspNetCore.Razor.LanguageServer.csproj]

/__w/1/vmr/src/razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/AutoInsert/RemoteAutoInsertService.cs(35,39): error IDE0052: Private member 'RemoteAutoInsertService._filePathService' can be removed as the value assigned to it is never read (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0052) [/__w/1/vmr/src/razor/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Microsoft.CodeAnalysis.Remote.Razor.csproj]

This is fixed by removing these unused fields.

@mthalman mthalman requested a review from a team as a code owner January 7, 2025 20:55
Copy link
Member

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!

One minor change needed before we can merge though

@davidwengier
Copy link
Member

Also these should have broken our build too. Probably something we need to look into.

dotnet_diagnostic.IDE0052.severity = warning

<TreatWarningsAsErrors Condition="'$(CI)' == 'true'">true</TreatWarningsAsErrors>

@mthalman mthalman requested a review from davidwengier January 7, 2025 21:37
@davidwengier
Copy link
Member

Thanks Matt!

@davidwengier davidwengier merged commit aae2a09 into dotnet:main Jan 7, 2025
12 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jan 7, 2025
@davidwengier
Copy link
Member

Created #11359 for follow up, so we don't break things in future

davidwengier added a commit that referenced this pull request Jan 8, 2025
Related to #11358

Discovered another unused field to be removed.
davidwengier added a commit that referenced this pull request Jan 8, 2025
Fixes #11359

IDE0052 wasn't being enforced in CI which means we broke the VMR build
(see #11358 and
#11361). This change makes sure this
rule is turned on and enforced in command line builds, and fixes a
couple of violations in the compiler. Should prevent future breakages
too.

Not sure why out existing .editorconfig for this diagnostic didn't work,
but I suspect its because the compiler has its own .editorconfig marked
as root.
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.

2 participants