Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 4, 2025

This PR removes the CS1998 (WRN_AsyncLacksAwaits) warning entirely from the C# compiler and removes all dependent C# infrastructure, based on the team decision that this warning is no longer beneficial.

Relates to test plan #75960.
Closes #77001.

Changes Made

Core compiler changes:

  • Removed CS1998 diagnostic emission from DefiniteAssignment.cs
  • Commented out WRN_AsyncLacksAwaits enum definition in ErrorCode.cs
  • Commented out references in ErrorFacts.cs and generated files
  • Deleted resource strings from CSharpResources.resx and all XLF translation files

Infrastructure cleanup:

  • Completely removed C# code fix providers (CSharpRemoveAsyncModifierCodeFixProvider and CSharpMakeMethodSynchronousCodeFixProvider) and their tests
  • Preserved abstract base classes and Visual Basic implementations for future VB consolidation
  • Updated telemetry descriptions to remove C# code fix provider references

Test updates:

  • Updated 32+ test files across the compiler test suite to remove CS1998 expectations
  • Removed C#-specific test classes for the deleted code fix providers

The warning infrastructure is now fully disabled for C# and won't be used again. Abstract base classes remain available for Visual Basic, which may continue to benefit from these diagnostics.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@333fred 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI changed the title [WIP] Consider not emitting CS1998 for interface implementations / method overrides Remove CS1998 warning entirely for async methods without await operators Sep 4, 2025
Copilot AI requested a review from 333fred September 4, 2025 17:49
Copilot finished work on behalf of 333fred September 4, 2025 17:49
@333fred
Copy link
Member

333fred commented Sep 4, 2025

@copilot please comment out the warning so we don't use it again in the future, and delete the resources associated with it.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot please comment out the warning so we don't use it again in the future, and delete the resources associated with it.

Done! I've commented out the CS1998 warning definition and deleted the associated resources:

Changes made (ca2c367):

  • Commented out WRN_AsyncLacksAwaits = 1998 in ErrorCode.cs to prevent future usage
  • Commented out references in ErrorFacts.cs and Generated files
  • Removed resource strings from CSharpResources.resx
  • Disabled both CSharpRemoveAsyncModifierCodeFixProvider and CSharpMakeMethodSynchronousCodeFixProvider (they can't function without the warning)
  • Updated scripting tests and disabled test classes that relied on CS1998

The warning infrastructure is now fully disabled and won't be used again in the future.

Copilot AI changed the title Remove CS1998 warning entirely for async methods without await operators Remove CS1998 warning entirely and disable dependent infrastructure Sep 4, 2025
Copilot finished work on behalf of 333fred September 4, 2025 18:05
@333fred
Copy link
Member

333fred commented Sep 4, 2025

@copilot there are errors:

D:\a\_work\1\s\.packages\microsoft.dotnet.xlifftasks\10.0.0-beta.25427.2\build\Microsoft.DotNet.XliffTasks.targets(85,5): error : 'xlf\CSharpResources.cs.xlf' is out-of-date with 'CSharpResources.resx'. Run `msbuild /t:UpdateXlf` to update .xlf files or set UpdateXlfOnBuild=true to update them on every build, but note that it is strongly discouraged to set UpdateXlfOnBuild=true in official/CI build environments as they should not modify source code during the build. [D:\a\_work\1\s\src\Compilers\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.csproj::TargetFramework=net9.0]
D:\a\_work\1\s\.packages\microsoft.dotnet.xlifftasks\10.0.0-beta.25427.2\build\Microsoft.DotNet.XliffTasks.targets(85,5): error : 'xlf\CSharpResources.cs.xlf' is out-of-date with 'CSharpResources.resx'. Run `msbuild /t:UpdateXlf` to update .xlf files or set UpdateXlfOnBuild=true to update them on every build, but note that it is strongly discouraged to set UpdateXlfOnBuild=true in official/CI build environments as they should not modify source code during the build. [D:\a\_work\1\s\src\Compilers\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.csproj::TargetFramework=netstandard2.0]

@333fred
Copy link
Member

333fred commented Sep 4, 2025

@CyrusNajmabadi what do you want to do about CSharpRemoveAsyncModifierCodeFixProvider and CSharpMakeMethodSynchronousCodeFixProvider?

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot there are errors:

D:\a\_work\1\s\.packages\microsoft.dotnet.xlifftasks\10.0.0-beta.25427.2\build\Microsoft.DotNet.XliffTasks.targets(85,5): error : 'xlf\CSharpResources.cs.xlf' is out-o...

Fixed! I've updated all XLF translation files to remove the WRN_AsyncLacksAwaits and WRN_AsyncLacksAwaits_Title entries that were causing the out-of-sync errors. The build errors should now be resolved (a2dde98).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 4zjvsblobprodcus390.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • 6s7vsblobprodcus313.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • 80zvsblobprodcus35.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • ckzvsblobprodcus347.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • l49vsblobprodcus358.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • m16vsblobprodcus374.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • pe3vsblobprodcus354.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)
  • ytvvsblobprodcus310.vsblob.vsassets.io
    • Triggering command: dotnet build src/Compilers/CSharp/Portable/Microsoft.CodeAnalysis.CSharp.csproj --no-restore (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@333fred
Copy link
Member

333fred commented Sep 4, 2025

@copilot remove those unrelated changes in global.json and build.cmd

Copilot finished work on behalf of 333fred September 4, 2025 18:25
@CyrusNajmabadi
Copy link
Member

@333fred are we removing the diagnostic entirely?

If so, we can just remove these fixers.

I assume we're doing the same for VB for consistency? Or are we intending divergence here?

@333fred
Copy link
Member

333fred commented Sep 4, 2025

@333fred are we removing the diagnostic entirely?

Yes.

I assume we're doing the same for VB for consistency? Or are we intending divergence here?

We'll need to clarify with folks. VB is not getting runtime async, so the benefits would be more murky.

Copy link
Contributor Author

Copilot AI commented Sep 4, 2025

@copilot remove those unrelated changes in global.json and build.cmd

Removed the unrelated changes to global.json (newline) and Build.cmd (file permissions) in commit 800934c. The files are now restored to their original state.

@jcouv jcouv self-assigned this Sep 16, 2025
@jcouv jcouv mentioned this pull request Sep 16, 2025
40 tasks
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (commit 15)

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (commit 15)

* upstream/main: (206 commits)
  Remove bogus xlf tag (#80357)
  Fix missing type argument checks
  Add tests
  Use dotnet run file for generating compiler code (#80248)
  Only restore based on assets file changes if the actual content changed (#80341)
  make expressionbody analyzer use semanticspananalysis (#80339)
  [EnC] Use ignoreAssemblyKey: false to resolve symbol keys (#80342)
  Properly populate ExportedType metadata table in presence of extension block. (#80311)
  Propagate `params` to lambdas and local functions (#79880)
  Change 17.15 to VS 2026 preview. (#80325)
  Improve virtualproject support for older .NET SDKs (#80324)
  Update dependencies from https://github.com/dotnet/dotnet build 283666 (#80344)
  Update dependencies from https://github.com/dotnet/arcade build 20250917.6 (#80343)
  Simplifying
  Fix tests
  Fix tests
  Fix introduce variable placement in top level statements
  move to immutable types in signature help
  move to immutable types in signature help
  Fix check
  ...
@333fred
Copy link
Member

333fred commented Sep 18, 2025

@jjonescz any other comments?

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.

Consider not emitting CS1998 for interface implementations / method overrides

9 participants