Skip to content

Conversation

@CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Jun 18, 2025

Have RoslynAnalyzers reference shared utilities/extensions from Roslyn layer itself.

Fix more

Update versions

Fix more

In progress

Flip

Fix

Fix

remove wrapper

Fix

Fix

in progress

Fix DeclarationKind layering

fix

One building

In progress

Breaking out refactoring helpers into core api vs service

VB side

In progress

Move type

workaround

Update src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/CSharpCompilerExtensions.projitems

Fix SubText ctor parameter verification. (dotnet#78989)

* Fix SubText ctor parameter verification.

This ctor currently throws given a zero length span at the end (and only the end) of the subtext. This evidently became far more prevalent with this (fairly) recent change to optimize newline information in our sourcetext classes. (dotnet#74728)

It doesn't make to allow zero length spans at all locations but at the end of the subtext, so the fix is just to allow construction in that case too.

Big props to Manish Vasani for providing a very actionable repro for this.

Fixes dotnet#78985.

Note there is a potentially related issue outlined in dotnet#76225 that looks attributable to the PR above too. It possible that this addresses that issue too, but I'm not completely convinced.

Extensions: bind unconditionally of LangVer (dotnet#78947)

Extensions: use specific tracking issues for different areas (second pass) (dotnet#78971)

Fix nullable analysis involving extensions and collection expressions (dotnet#78926)

Update src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/CSharpCompilerExtensions.projitems

in progrss

Update src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems

Update src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/VisualBasicCompilerExtensions.projitems

extensions

Fix

RootNamespace

Fixes

Fixes

IN progress

in progress
@dotnet-policy-service dotnet-policy-service bot added VSCode Needs API Review Needs to be reviewed by the API review council labels Jun 18, 2025
|Enabled|True|
|Severity|Warning|
|CodeFix|True|
|CodeFix|False|
Copy link
Member Author

Choose a reason for hiding this comment

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

very strange. i don't know how this doc generator works, or why it now thinks there is no codefix for some of these analyzers.

Copy link
Member Author

Choose a reason for hiding this comment

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

i wonder if it is this awful code:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

@JoeRobich Do you have any idea what might be causing this? i'm not even sure how to get this to run locally :'(

Copy link
Member

Choose a reason for hiding this comment

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

Spoke offline and I suggested it might be the transitive pinning in conjunction with the source-build specific PackageVersion removal in the GenerateDocumentationAndConfigFiles tool project. This led us to add a Directory.Packages.props to disable pinning. It apparently isn't the entire fix, but did resolve one of the reported errors.

Copy link
Member

Choose a reason for hiding this comment

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

Adding Microsoft.BCL.AsyncInterfaces dependency to the tool project was the other necessary change to fix this issue.

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review June 24, 2025 22:08
@CyrusNajmabadi CyrusNajmabadi requested review from a team as code owners June 24, 2025 22:08
@CyrusNajmabadi CyrusNajmabadi requested a review from JoeRobich June 24, 2025 22:08
catch
catch (Exception ex)
{
Console.WriteLine($"Error processing analyzer file reference: {ex.Message}\r\n{ex}");
Copy link
Member

Choose a reason for hiding this comment

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

If we keep this, it would be useful to include Path.GetFileName(analyzerFileReference.FullPath) in the message.

Copy link
Member Author

Choose a reason for hiding this comment

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

sgtm.

@CyrusNajmabadi CyrusNajmabadi changed the title WIP: Shared code layering Have RoslynAnalyzers reference shared utilities/extensions from Roslyn layer itself. Jun 25, 2025
@CyrusNajmabadi CyrusNajmabadi merged commit 19350ab into dotnet:main Jun 25, 2025
28 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the analyzerWork branch June 25, 2025 02:03
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jun 25, 2025
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Needs API Review Needs to be reviewed by the API review council VSCode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants