diff --git a/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersTests.cs b/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersTests.cs index c155502b09efe36a741f04a547844dcbaaf4eeb4..38d9fa4c144e8a7a766594dfcc473ce88f88e36e 100644 --- a/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersTests.cs +++ b/src/EditorFeatures/CSharpTest/GenerateFromMembers/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersTests.cs @@ -1940,6 +1940,58 @@ public bool Equals(Program other) parameters: CSharp6Implicit); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateEqualsAndGetHashCode)] + [WorkItem(25708, "https://github.com/dotnet/roslyn/issues/25708")] + public async Task TestOverrideEqualsOnRefStructReturnsFalse() + { + await TestWithPickMembersDialogAsync( +@" +ref struct Program +{ + public string s; + [||] +}", +@" +ref struct Program +{ + public string s; + + public override bool Equals(object obj) + { + return false; + } +}", +chosenSymbols: null); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateEqualsAndGetHashCode)] + [WorkItem(25708, "https://github.com/dotnet/roslyn/issues/25708")] + public async Task TestImplementIEquatableOnRefStructSkipsIEquatable() + { + await TestWithPickMembersDialogAsync( +@" +ref struct Program +{ + public string s; + [||] +}", +@" +ref struct Program +{ + public string s; + + public override bool Equals(object obj) + { + return false; + } +}", +chosenSymbols: null, +// We are forcefully enabling the ImplementIEquatable option, as that is our way +// to test that the option does nothing. The VS mode will ensure if the option +// is not available it will not be shown. +optionsCallback: options => EnableOption(options, ImplementIEquatableId)); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateEqualsAndGetHashCode)] public async Task TestImplementIEquatableOnStructInNullableContextWithUnannotatedMetadata() { diff --git a/src/Features/Core/Portable/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider.cs index 3786c98be375524c710f0023d38809e39dc6397d..102a56c1168359b99cf4981dfa7448c51ea72e11 100644 --- a/src/Features/Core/Portable/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/GenerateEqualsAndGetHashCodeFromMembers/GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable + using System; using System.Collections.Immutable; using System.Composition; @@ -35,7 +37,7 @@ internal partial class GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringPro private const string EqualsName = nameof(object.Equals); private const string GetHashCodeName = nameof(object.GetHashCode); - private readonly IPickMembersService _pickMembersService_forTestingPurposes; + private readonly IPickMembersService? _pickMembersService_forTestingPurposes; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] @@ -45,7 +47,7 @@ public GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider() } [SuppressMessage("RoslynDiagnosticsReliability", "RS0034:Exported parts should have [ImportingConstructor]", Justification = "Used incorrectly by tests")] - public GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider(IPickMembersService pickMembersService) + public GenerateEqualsAndGetHashCodeFromMembersCodeRefactoringProvider(IPickMembersService? pickMembersService) => _pickMembersService_forTestingPurposes = pickMembersService; public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) @@ -69,9 +71,9 @@ private async Task HandleNonSelectionAsync(CodeRefactoringContext context) { var (document, textSpan, cancellationToken) = context; - var syntaxFacts = document.GetLanguageService(); + var syntaxFacts = document.GetRequiredLanguageService(); var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); // We offer the refactoring when the user is either on the header of a class/struct, // or if they're between any members of a class/struct and are on a blank line. @@ -81,7 +83,7 @@ private async Task HandleNonSelectionAsync(CodeRefactoringContext context) return; } - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); // Only supported on classes/structs. var containingType = semanticModel.GetDeclaredSymbol(typeDeclaration) as INamedTypeSymbol; @@ -134,13 +136,21 @@ private bool HasOperator(INamedTypeSymbol containingType, string operatorName) private bool CanImplementIEquatable( SemanticModel semanticModel, INamedTypeSymbol containingType, - [NotNullWhen(true)] out INamedTypeSymbol constructedType) + [NotNullWhen(true)] out INamedTypeSymbol? constructedType) { - var equatableTypeOpt = semanticModel.Compilation.GetTypeByMetadataName(typeof(IEquatable<>).FullName); - if (equatableTypeOpt != null) + // A ref struct can never implement an interface, therefore never add IEquatable to the selection + // options if the type is a ref struct. + if (!containingType.IsRefLikeType) { - constructedType = equatableTypeOpt.Construct(containingType); - return !containingType.AllInterfaces.Contains(constructedType); + var equatableTypeOpt = semanticModel.Compilation.GetTypeByMetadataName(typeof(IEquatable<>).FullName!); + if (equatableTypeOpt != null) + { + constructedType = equatableTypeOpt.Construct(containingType); + + // A ref struct can never implement an interface, therefore never add IEquatable to the selection + // options if the type is a ref struct. + return !containingType.AllInterfaces.Contains(constructedType); + } } constructedType = null; @@ -174,8 +184,8 @@ private void GetExistingMemberInfo(INamedTypeSymbol containingType, out bool has GetExistingMemberInfo( info.ContainingType, out var hasEquals, out var hasGetHashCode); - var syntaxFacts = document.GetLanguageService(); - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var syntaxFacts = document.GetRequiredLanguageService(); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var typeDeclaration = syntaxFacts.GetContainingTypeDeclaration(root, textSpan.Start); return await CreateActionsAsync( @@ -240,15 +250,12 @@ private void GetExistingMemberInfo(INamedTypeSymbol containingType, out bool has Document document, SyntaxNode typeDeclaration, INamedTypeSymbol containingType, ImmutableArray members, bool generateEquals, bool generateGetHashCode, CancellationToken cancellationToken) { - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); using var _ = ArrayBuilder.GetInstance(out var pickMembersOptions); - var canImplementIEquatable = CanImplementIEquatable(semanticModel, containingType, out var equatableTypeOpt); - var hasExistingOperators = HasOperators(containingType); - - if (canImplementIEquatable) + if (CanImplementIEquatable(semanticModel, containingType, out var equatableTypeOpt)) { var value = options.GetOption(GenerateEqualsAndGetHashCodeFromMembersOptions.ImplementIEquatable); @@ -262,7 +269,7 @@ private void GetExistingMemberInfo(INamedTypeSymbol containingType, out bool has value)); } - if (!hasExistingOperators) + if (!HasOperators(containingType)) { var value = options.GetOption(GenerateEqualsAndGetHashCodeFromMembersOptions.GenerateOperators); pickMembersOptions.Add(new PickMembersOption( @@ -287,7 +294,7 @@ private void GetExistingMemberInfo(INamedTypeSymbol containingType, out bool has { // if we're generating equals for a struct, then also add IEquatable support as // well as operators (as long as the struct does not already have them). - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); implementIEquatable = CanImplementIEquatable(semanticModel, containingType, out _); generateOperators = !HasOperators(containingType); } diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/SyntaxGeneratorExtensions_CreateEqualsMethod.cs b/src/Workspaces/Core/Portable/Shared/Extensions/SyntaxGeneratorExtensions_CreateEqualsMethod.cs index be76eb3b7cc6ccbbfcdda5f8b04072833a2409e5..1cf89591c6e1bdc3d90e29602a7cd975e447d621 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/SyntaxGeneratorExtensions_CreateEqualsMethod.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/SyntaxGeneratorExtensions_CreateEqualsMethod.cs @@ -99,7 +99,16 @@ public static IMethodSymbol CreateEqualsMethod(this Compilation compilation, Imm ImmutableArray members, string localNameOpt) { - var statements = ArrayBuilder.GetInstance(); + using var _1 = ArrayBuilder.GetInstance(out var statements); + + // A ref like type can not be boxed. Because of this an overloaded Equals taking object in the general case + // can never be true, because an equivalent object can never be boxed into the object itself. Therefore only + // need to return false. + if (containingType.IsRefLikeType) + { + statements.Add(factory.ReturnStatement(factory.FalseLiteralExpression())); + return statements.ToImmutable(); + } // Come up with a good name for the local variable we're going to compare against. // For example, if the class name is "CustomerOrder" then we'll generate: @@ -113,7 +122,7 @@ public static IMethodSymbol CreateEqualsMethod(this Compilation compilation, Imm // These will be all the expressions that we'll '&&' together inside the final // return statement of 'Equals'. - using var _ = ArrayBuilder.GetInstance(out var expressions); + using var _2 = ArrayBuilder.GetInstance(out var expressions); if (factory.SupportsPatterns(parseOptions)) { @@ -189,7 +198,7 @@ public static IMethodSymbol CreateEqualsMethod(this Compilation compilation, Imm statements.Add(factory.ReturnStatement( expressions.Aggregate(factory.LogicalAndExpression))); - return statements.ToImmutableAndFree(); + return statements.ToImmutable(); } private static void AddMemberChecks(