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

Fix override implementation with generics and constraints #76223

Merged
merged 1 commit into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,13 @@ namespace Microsoft.CodeAnalysis.CSharp.ImplementAbstractClass;

[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.ImplementAbstractClass), Shared]
[ExtensionOrder(After = PredefinedCodeFixProviderNames.GenerateType)]
internal sealed class CSharpImplementAbstractClassCodeFixProvider :
AbstractImplementAbstractClassCodeFixProvider<TypeDeclarationSyntax>
[method: ImportingConstructor]
[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
internal sealed class CSharpImplementAbstractClassCodeFixProvider()
: AbstractImplementAbstractClassCodeFixProvider<TypeDeclarationSyntax>(CS0534)
{
private const string CS0534 = nameof(CS0534); // 'Program' does not implement inherited abstract member 'Goo.bar()'

[ImportingConstructor]
[SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
public CSharpImplementAbstractClassCodeFixProvider()
: base(CS0534)
{
}

protected override SyntaxToken GetClassIdentifier(TypeDeclarationSyntax classNode)
=> classNode.Identifier;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CodeStyle;
Expand Down Expand Up @@ -38,8 +39,8 @@ private OptionsCollection AllOptionsOff
};

internal Task TestAllOptionsOffAsync(
string initialMarkup,
string expectedMarkup,
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string initialMarkup,
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string expectedMarkup,
int index = 0,
OptionsCollection? options = null,
ParseOptions? parseOptions = null)
Expand Down Expand Up @@ -2553,4 +2554,180 @@ protected override void M()
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71225")]
public async Task TestConstrainedTypeParameter1()
{
await TestAllOptionsOffAsync(
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem
{
protected abstract void M<T>(I<T?> i) where T : C;
}

class [|Bad|] : Problem
{
}
""",
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem
{
protected abstract void M<T>(I<T?> i) where T : C;
}

class Bad : Problem
{
protected override void M<T>(I<T?> i) where T : class
{
throw new NotImplementedException();
}
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71225")]
public async Task TestConstrainedTypeParameter2()
{
await TestAllOptionsOffAsync(
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem<U>
{
protected abstract void M<T>(I<T?> i) where T : U;
}

class [|Bad|] : Problem<int>
{
}
""",
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem<U>
{
protected abstract void M<T>(I<T?> i) where T : U;
}

class Bad : Problem<int>
{
protected override void M<T>(I<T> i) where T : struct
{
throw new NotImplementedException();
}
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71225")]
public async Task TestConstrainedTypeParameter3()
{
await TestAllOptionsOffAsync(
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem<U>
{
protected abstract void M<T>(I<T?> i) where T : U;
}

class [|Bad|] : Problem<string>
{
}
""",
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem<U>
{
protected abstract void M<T>(I<T?> i) where T : U;
}

class Bad : Problem<string>
{
protected override void M<T>(I<T?> i) where T : class
{
throw new NotImplementedException();
}
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71225")]
public async Task TestConstrainedTypeParameter4()
{
await TestAllOptionsOffAsync(
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem<U>
{
protected abstract void M<T>(I<T?> i) where T : U;
}

class [|Bad|] : Problem<int[]>
{
}
""",
"""
#nullable enable
using System;

interface I<out T> { }

class C { }

abstract class Problem<U>
{
protected abstract void M<T>(I<T?> i) where T : U;
}

class Bad : Problem<int[]>
{
protected override void M<T>(I<T?> i) where T : class
{
throw new NotImplementedException();
}
}
""");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
context.RegisterCodeFix(
CodeAction.Create(
AnalyzersResources.Implement_abstract_class,
c => data.ImplementAbstractClassAsync(throughMember: null, canDelegateAllMembers: null, c),
cancellationToken => data.ImplementAbstractClassAsync(throughMember: null, canDelegateAllMembers: null, cancellationToken),
id),
context.Diagnostics);

Expand All @@ -63,7 +63,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
context.RegisterCodeFix(
CodeAction.Create(
string.Format(AnalyzersResources.Implement_through_0, through.Name),
c => data.ImplementAbstractClassAsync(through, canDelegateAllMembers, c),
cancellationToken => data.ImplementAbstractClassAsync(through, canDelegateAllMembers, cancellationToken),
id),
context.Diagnostics);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,7 @@ private static SyntaxList<TypeParameterConstraintClauseSyntax> GenerateDefaultCo
if (!referencedTypeParameters.Contains(typeParameter))
continue;

var constraint = typeParameter switch
{
{ HasReferenceTypeConstraint: true } => s_classConstraint,
{ HasValueTypeConstraint: true } => s_structConstraint,
_ => s_defaultConstraint
};
var constraint = GetConstraint(typeParameter);

listOfClauses.Add(TypeParameterConstraintClause(
typeParameter.Name.ToIdentifierName(),
Expand All @@ -248,6 +243,52 @@ private static SyntaxList<TypeParameterConstraintClauseSyntax> GenerateDefaultCo
return [.. listOfClauses];
}

private static TypeParameterConstraintSyntax GetConstraint(ITypeParameterSymbol typeParameter)
{
using var _ = PooledHashSet<ITypeParameterSymbol>.GetInstance(out var visited);
var constraint = GetConstraintRecursive(typeParameter);

return constraint ?? s_defaultConstraint;

TypeParameterConstraintSyntax? GetConstraintRecursive(ITypeParameterSymbol typeParameter)
{
if (visited.Add(typeParameter))
{
// If it is explicitly marked as `T : struct` or `T : class` then we want to have the same constraint on the override.
if (typeParameter.HasValueTypeConstraint)
return s_structConstraint;

if (typeParameter.HasReferenceTypeConstraint)
return s_classConstraint;

foreach (var constraintType in typeParameter.ConstraintTypes)
{
// If we ended up being constrained on a value type, then we have to have the `T : struct`
// constraint to align with that.
if (constraintType.IsValueType)
return s_structConstraint;

// For all reference types *except* interfaces, we want the `T : class` constraint. An interface
// can be implemented by a value type or a referernce type, so it adds no information to the
// constraints.
if (constraintType.IsReferenceType && constraintType.TypeKind != TypeKind.Interface)
return s_classConstraint;

// If we have `where T : U` then peek into the other contraint to see if it adds information.
if (constraintType is ITypeParameterSymbol constraintTypeParameter)
{
var constraint = GetConstraintRecursive(constraintTypeParameter);
if (constraint != null)
return constraint;
}
}
}

// We learned nothing from this constraint.
return null;
}
}

private static TypeParameterListSyntax? GenerateTypeParameterList(
IMethodSymbol method, CSharpCodeGenerationContextInfo info)
{
Expand Down
Loading