Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot 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 and others added 3 commits September 4, 2025 17:35
Co-authored-by: 333fred <2371880+333fred@users.noreply.github.com>
Co-authored-by: 333fred <2371880+333fred@users.noreply.github.com>
Co-authored-by: 333fred <2371880+333fred@users.noreply.github.com>
@Copilot 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 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.

Co-authored-by: 333fred <2371880+333fred@users.noreply.github.com>
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 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?

Co-authored-by: 333fred <2371880+333fred@users.noreply.github.com>
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.

Co-authored-by: 333fred <2371880+333fred@users.noreply.github.com>
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.

@333fred
Copy link
Member

333fred commented Sep 11, 2025

@dotnet/roslyn-compiler for reviews please.

* upstream/main: (201 commits)
  Handle extension blocks in CLS compliance checker (#80251)
  Fix
  Simplify
  Simplify
  Simplify
  Simplify
  Simplify
  Add doc for GetInferredNullableAnnotation (#80245)
  Rename files to match type within
  Implement ref local hoisting in runtime async. Closes #79763.
  Pull ref initialization hoisting into a reusable class for use in runtime async
  REvert
  Break cycle for lambda using field keyword with inferred return type (#79995)
  Simplify tests
  Simplify
  Fixup
  New extension
  Extensions: add SyntaxGenerator support and AssociatedExtensionImplementation API (#80170)
  In process
  Support 'add using' for modern extension methods
  ...
@jcouv jcouv self-assigned this Sep 16, 2025
@jcouv jcouv mentioned this pull request Sep 16, 2025
37 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?

@333fred 333fred enabled auto-merge (squash) September 19, 2025 16:12
@333fred 333fred merged commit 84133cc into main Sep 19, 2025
27 of 28 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 19, 2025
@333fred 333fred deleted the copilot/fix-77001 branch September 19, 2025 18:08
@akhera99 akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025
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