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

Handle generic type parameters in method deduplication #58

Conversation

simonmckenzie
Copy link
Contributor

@simonmckenzie simonmckenzie commented Aug 14, 2024

This fixes an error where methods with the same parameters but different generic type parameters were being treated as identical during deduplication, resulting in the loss of interface methods.

The fix is to use genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters when constucting the SymbolDisplayFormat used for deduplication.

I have also added SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces to prevent types with the same name from being seen as identical.

Finally, I have unified MethodSignatureDisplayFormat and TypeDisplayFormat into a single FullyQualifiedDisplayFormat. These two display formats were used for method deduplication and generating type signatures from strings. It makes sense that they should follow the same rules.

The effect of the change

Given this:

public void AMethod(Func<Task<int>> getValue) {}

Deduplication key before change: AMethod(Func)
Deduplication key after change: AMethod(Func<Task<System.Int32>>)

... or, given user-defined types, the key will contain fully-qualified types:

namespace Demo;

public delegate T Func<T>();
public class Task<T>;

//...
public void AMethod(Demo.Func<Demo.Task<int>> getValue) {}

Deduplication key: AMethod(Demo.Func<Demo.Task<System.Int32>>)

I have left the global namespaces out of the examples for the sake of simplicity

Addresses #56

@ChristianSauer
Copy link
Collaborator

I think this is obsolete after your last PR for 5.0.0?

This fixes an error where methods with the same parameters but different _generic type parameters_ were being treated as identical during deduplication, resulting in the loss of interface methods.

The fix is to use `genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters` when constucting the `SymbolDisplayFormat` used for deduplication.

Before:
`public void AMethod(Func<int> getValue) {}` produced a deduplication signature of `AMethod(Func)`
After:
`public void AMethod(Func<int> getValue) {}` produced a deduplication signature of `AMethod(Func<Int32>)`
@simonmckenzie simonmckenzie force-pushed the fix/handle-generic-types-in-method-deduplication branch 3 times, most recently from 85c5aa7 to 946fc1b Compare September 1, 2024 23:06
…ng seen as identical

- Updated the test to show that nested generics work correctly
- Added test to ensure same-named generic parameters are distinguishable from one another
@simonmckenzie simonmckenzie force-pushed the fix/handle-generic-types-in-method-deduplication branch from 946fc1b to 45f3d3a Compare September 1, 2024 23:07
@simonmckenzie
Copy link
Contributor Author

simonmckenzie commented Sep 1, 2024

Hi @ChristianSauer,

This PR isn't obsolete - there's still an issue with generic type parameter deduplication. I've rebased, simplified the code, and added another test now the other PR is merged.

…ingle `FullyQualifiedDisplayFormat`

These two display formats were used for method deduplication and generating type signatures from strings. It makes sense that they should follow the same rules.
new(
genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
Copy link
Contributor Author

@simonmckenzie simonmckenzie Sep 2, 2024

Choose a reason for hiding this comment

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

The main change (which makes the deduplication key include generic type parameters in type signatures)

@simonmckenzie simonmckenzie changed the title Handle generic types in method deduplication Handle generic type parameters in method deduplication Nov 9, 2024
@ChristianSauer ChristianSauer merged commit edc3965 into codecentric:master Nov 29, 2024
3 checks passed
@ChristianSauer
Copy link
Collaborator

So, finally found time to merge

@simonmckenzie simonmckenzie deleted the fix/handle-generic-types-in-method-deduplication branch November 29, 2024 08:51
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