Skip to content

Recognize union creation members defined in an interface acting as a union member provider#83048

Open
AlekseyTs wants to merge 2 commits intodotnet:features/Unionsfrom
AlekseyTs:Unions_38
Open

Recognize union creation members defined in an interface acting as a union member provider#83048
AlekseyTs wants to merge 2 commits intodotnet:features/Unionsfrom
AlekseyTs:Unions_38

Conversation

@AlekseyTs
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs commented Apr 3, 2026

Microsoft Reviewers: Open in CodeFlow

@AlekseyTs AlekseyTs requested a review from a team April 6, 2026 02:04
@AlekseyTs
Copy link
Copy Markdown
Contributor Author

@jjonescz, @dotnet/roslyn-compiler For a second review

var definition = this.OriginalDefinition;
NamedTypeSymbol membersInterface = membersInterfaceForDefinition.AsMember(this);

foreach (var member in membersInterfaceForDefinition.GetMembers("Create"))
Copy link
Copy Markdown
Member

@jjonescz jjonescz Apr 7, 2026

Choose a reason for hiding this comment

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

"Create"

Should this be a constant in WellKnownMemberNames? #Pending

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should this be a constant in WellKnownMemberNames?

Good idea. I will add one in one of the next PRs


if (type.DeclaredAccessibility != Accessibility.Public ||
!type.IsInterface ||
!this.OriginalDefinition.AllInterfacesNoUseSiteDiagnostics.Contains(type, Symbols.SymbolEqualityComparer.AllIgnoreOptions))
Copy link
Copy Markdown
Member

@jjonescz jjonescz Apr 7, 2026

Choose a reason for hiding this comment

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

Consider testing when the interface is defined in #nullable disabled region but implemented in #nullable enabled region (and/or vice versa) to verify the effect of the comparer. #Resolved

Copy link
Copy Markdown
Contributor Author

@AlekseyTs AlekseyTs Apr 7, 2026

Choose a reason for hiding this comment

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

While it would be good to have a test like that for completeness sake. I think that adding it has lowest priority, and I am comfortable not having a test like that. Especially that this comparer is used consistently throughout the feature code.

ParameterCount: 1,
Parameters: [{ RefKind: RefKind.In or RefKind.None }]
} &&
unionType.Equals(factory.ReturnType, TypeCompareKind.AllIgnoreOptions);
Copy link
Copy Markdown
Member

@jjonescz jjonescz Apr 7, 2026

Choose a reason for hiding this comment

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

Do we have a test where the return type is an annotated nullable reference type? #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ReturnType is a TypeSymbol, therefore, any top level nullability information is not present. Also, see response for a comment above.

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.

3 participants