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

Prevent duplicates when overriding or shadowing methods #51

Merged
merged 2 commits into from
Aug 4, 2024
Merged

Prevent duplicates when overriding or shadowing methods #51

merged 2 commits into from
Aug 4, 2024

Conversation

simonmckenzie
Copy link
Contributor

@simonmckenzie simonmckenzie commented Jun 14, 2024

This addresses issue #50.

This is my second attempt at this - the first attempt (#49) involved excluding IsOverride methods, but this does not work with method shadowing, which becomes complex, since a new method can have different accessibility to the method it shadows, and there's no direct link in the syntax tree between a shadowing method and the method it shadows.

The approach I've used here is the same as the existing approach for deduplicating property definitions:

private static void AddPropertiesToInterface(
ITypeSymbol classSymbol,
InterfaceBuilder interfaceGenerator
)
{
classSymbol
.GetAllMembers()
.Where(x => x.Kind == SymbolKind.Property)
.OfType<IPropertySymbol>()
.Where(x => x.DeclaredAccessibility == Accessibility.Public)
.Where(x => !x.IsStatic)
.Where(x => !x.IsIndexer)
.GroupBy(x => x.Name)
.Select(g => g.First())

The only difference is that it uses ISymbol.ToDisplayString to generate the deduplication key.

The parameters I've used for the display string generation will create display strings like this, with just the method name and the parameter types and directions:

Hello(ref String, Int32, Double)

Simon McKenzie added 2 commits June 15, 2024 09:59
The current code will insert multiple definitions of a method when that method has been overridden or shadowed.

The approach is the same as that already used for property deduplication, which is to group by the method definition.
@simonmckenzie
Copy link
Contributor Author

simonmckenzie commented Jun 22, 2024

Hi @ChristianSauer, sorry to bother you, but does this PR look ok to you? I'm happy to discuss it if you'd like some changes to the approach.

@simonmckenzie
Copy link
Contributor Author

Hi @ChristianSauer,

Checking in on this PR - will you have a chance to look at it soon? My team can't use AutomaticInterface because of this problem - it'd be great to be able to use it again!

Thanks,
Simon

@ChristianSauer
Copy link
Collaborator

Ok, I finally found time to review this - sorry, was a shitty few last months for me.

@ChristianSauer ChristianSauer merged commit c3421b7 into codecentric:master Aug 4, 2024
3 checks passed
@simonmckenzie simonmckenzie deleted the fix/prevent-duplicates-when-overriding-or-shadowing branch August 5, 2024 20:31
@simonmckenzie
Copy link
Contributor Author

Thank you @ChristianSauer. Much appreciated. All the best.

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