Skip to content

Comments

Extensions: fix RemoveUnusedMember scenarios#81754

Merged
jcouv merged 2 commits intodotnet:mainfrom
jcouv:extension-unused
Dec 18, 2025
Merged

Extensions: fix RemoveUnusedMember scenarios#81754
jcouv merged 2 commits intodotnet:mainfrom
jcouv:extension-unused

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented Dec 18, 2025

Fixes #80645 and related issues #81213 and #81657

The principle of AbstractRemoveUnusedMembersDiagnosticAnalyzer is that we respond to member declarations by registering them (AnalyzeSymbolDeclaration), then analyze various references (invocations, member references, deconstructions, nameof, cref, etc), then when we exit the type we can report private unused members.

The change is that we treat extension members as part of the enclosing static class:

  • we don't report unused private members until we exit the enclosing static class
  • we enumerate the extension members along with members of the enclosing static class

Additionally, any usage of disambiguation syntax marks the corresponding extension member as "used".

@jcouv jcouv self-assigned this Dec 18, 2025
@jcouv jcouv added Area-IDE Feature - Extension Everything The extension everything feature labels Dec 18, 2025
@jcouv jcouv marked this pull request as ready for review December 18, 2025 20:57
@jcouv jcouv requested a review from a team as a code owner December 18, 2025 20:57
Comment on lines +279 to +281
{
symbolStartContext.RegisterSymbolEndAction(symbolEndContext => OnSymbolEnd(symbolEndContext, hasUnsupportedOperation));
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

Suggested change
{
symbolStartContext.RegisterSymbolEndAction(symbolEndContext => OnSymbolEnd(symbolEndContext, hasUnsupportedOperation));
}
symbolStartContext.RegisterSymbolEndAction(symbolEndContext => OnSymbolEnd(symbolEndContext, hasUnsupportedOperation));
``` #ByDesign

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 kept those braces as neither the condition nor the branch are simple, so find this this easier to read

Comment on lines 290 to 292
{
return false;
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

Suggested change
{
return false;
}
return false;

Also, consider a comment above the if. but ok if not present. #Resolved

Comment on lines 746 to 748
{
yield return extensionMember;
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

Suggested change
{
yield return extensionMember;
}
yield return extensionMember;
``` #Resolved

Comment on lines 830 to 833
if (extensionBlockMethod.AssociatedSymbol is { } associatedSymbol)
{
AddIfCandidateSymbol(builder, associatedSymbol);
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

Suggested change
if (extensionBlockMethod.AssociatedSymbol is { } associatedSymbol)
{
AddIfCandidateSymbol(builder, associatedSymbol);
}
AddIfCandidateSymbol(builder, extensionBlockMethod.AssociatedSymbol);
``` #Resolved

Comment on lines 39 to 42
foreach (var extensionMember in extensionBlock.Members)
{
yield return extensionMember;
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

Suggested change
foreach (var extensionMember in extensionBlock.Members)
{
yield return extensionMember;
}
foreach (var extensionMember in extensionBlock.Members)
yield return extensionMember;
``` #Resolved

FixedCode = code,
LanguageVersion = LanguageVersion.CSharp14,
}.RunAsync();
}
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

you can just inline 'code' and remove the 'fixedcode=code' line in all of these. then make the methods not async/awit. but instead just return the new Test { ... }.RunAsync(); #Resolved

Comment on lines 37 to 38
if (associated is null)
continue;
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Dec 18, 2025

Choose a reason for hiding this comment

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

Suggested change
if (associated is null)
continue;
``` #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I'd misunderstood the suggestion in previous PR because of how the diff is shown in CodeFlow...

@jcouv jcouv requested a review from CyrusNajmabadi December 18, 2025 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extension blocks: extension method called from constructor is marked as unused

3 participants