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

Test for duplicate interface implementation instead of declaration #769

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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 @@ -33,14 +33,12 @@ public INotifyPropertyChangedGenerator()
{
diagnostics = ImmutableArray<DiagnosticInfo>.Empty;

INotifyPropertyChangedInfo? info = null;

// Check if the type already implements INotifyPropertyChanged
if (typeSymbol.AllInterfaces.Any(i => i.HasFullyQualifiedMetadataName("System.ComponentModel.INotifyPropertyChanged")))
if (typeSymbol.ImplementsInterfaceMember("System.ComponentModel.INotifyPropertyChanged"))
{
diagnostics = ImmutableArray.Create(DiagnosticInfo.Create(DuplicateINotifyPropertyChangedInterfaceForINotifyPropertyChangedAttributeError, typeSymbol, typeSymbol));

goto End;
return null;
}

// Check if the type uses [INotifyPropertyChanged] or [ObservableObject] already (in the type hierarchy too)
Expand All @@ -49,15 +47,12 @@ public INotifyPropertyChangedGenerator()
{
diagnostics = ImmutableArray.Create(DiagnosticInfo.Create(InvalidAttributeCombinationForINotifyPropertyChangedAttributeError, typeSymbol, typeSymbol));

goto End;
return null;
}

bool includeAdditionalHelperMethods = attributeData.GetNamedArgument("IncludeAdditionalHelperMethods", true);

info = new INotifyPropertyChangedInfo(includeAdditionalHelperMethods);

End:
return info;
return new INotifyPropertyChangedInfo(includeAdditionalHelperMethods);
}

/// <inheritdoc/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@
// 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.Collections.Immutable;
using System.Linq;
using CommunityToolkit.Mvvm.SourceGenerators.Extensions;
using CommunityToolkit.Mvvm.SourceGenerators.Models;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using System.Collections.Immutable;
using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors;

namespace CommunityToolkit.Mvvm.SourceGenerators;
Expand All @@ -32,15 +31,15 @@ private protected override int ValidateTargetTypeAndGetInfo(INamedTypeSymbol typ
diagnostics = ImmutableArray<DiagnosticInfo>.Empty;

// Check if the type already implements INotifyPropertyChanged...
if (typeSymbol.AllInterfaces.Any(i => i.HasFullyQualifiedMetadataName("System.ComponentModel.INotifyPropertyChanged")))
if (typeSymbol.ImplementsInterfaceMember("System.ComponentModel.INotifyPropertyChanged"))
{
diagnostics = ImmutableArray.Create(DiagnosticInfo.Create(DuplicateINotifyPropertyChangedInterfaceForObservableObjectAttributeError, typeSymbol, typeSymbol));

goto End;
}

// ...or INotifyPropertyChanging
if (typeSymbol.AllInterfaces.Any(i => i.HasFullyQualifiedMetadataName("System.ComponentModel.INotifyPropertyChanging")))
if (typeSymbol.ImplementsInterfaceMember("System.ComponentModel.INotifyPropertyChanging"))
{
diagnostics = ImmutableArray.Create(DiagnosticInfo.Create(DuplicateINotifyPropertyChangingInterfaceForObservableObjectAttributeError, typeSymbol, typeSymbol));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,51 +35,51 @@ internal static class DiagnosticDescriptors
public const string AsyncVoidReturningRelayCommandMethodId = "MVVMTK0039";

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate declaration of <see cref="INotifyPropertyChanged"/> would happen.
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate implementation of <see cref="INotifyPropertyChanged"/> would happen.
/// <para>
/// Format: <c>"Cannot apply [INotifyPropertyChangedAttribute] to type {0}, as it already declares the INotifyPropertyChanged interface"</c>.
/// Format: <c>"Cannot apply [INotifyPropertyChangedAttribute] to type {0}, as it already implements the INotifyPropertyChanged interface"</c>.
/// </para>
/// </summary>
public static readonly DiagnosticDescriptor DuplicateINotifyPropertyChangedInterfaceForINotifyPropertyChangedAttributeError = new DiagnosticDescriptor(
id: "MVVMTK0001",
title: $"Duplicate {nameof(INotifyPropertyChanged)} definition",
messageFormat: $"Cannot apply [INotifyPropertyChanged] to type {{0}}, as it already declares the {nameof(INotifyPropertyChanged)} interface",
messageFormat: $"Cannot apply [INotifyPropertyChanged] to type {{0}}, as it already implements the {nameof(INotifyPropertyChanged)} interface",
category: typeof(INotifyPropertyChangedGenerator).FullName,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: $"Cannot apply [INotifyPropertyChanged] to a type that already declares the {nameof(INotifyPropertyChanged)} interface.",
description: $"Cannot apply [INotifyPropertyChanged] to a type that already implements the {nameof(INotifyPropertyChanged)} interface.",
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0001");

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate declaration of <see cref="INotifyPropertyChanged"/> would happen.
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate implementation of <see cref="INotifyPropertyChanged"/> would happen.
/// <para>
/// Format: <c>"Cannot apply [ObservableObjectAttribute] to type {0}, as it already declares the INotifyPropertyChanged interface"</c>.
/// Format: <c>"Cannot apply [ObservableObjectAttribute] to type {0}, as it already implement the INotifyPropertyChanged interface"</c>.
/// </para>
/// </summary>
public static readonly DiagnosticDescriptor DuplicateINotifyPropertyChangedInterfaceForObservableObjectAttributeError = new DiagnosticDescriptor(
id: "MVVMTK0002",
title: $"Duplicate {nameof(INotifyPropertyChanged)} definition",
messageFormat: $"Cannot apply [ObservableObject] to type {{0}}, as it already declares the {nameof(INotifyPropertyChanged)} interface",
messageFormat: $"Cannot apply [ObservableObject] to type {{0}}, as it already implements the {nameof(INotifyPropertyChanged)} interface",
category: typeof(ObservableObjectGenerator).FullName,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: $"Cannot apply [ObservableObject] to a type that already declares the {nameof(INotifyPropertyChanged)} interface.",
description: $"Cannot apply [ObservableObject] to a type that already implements the {nameof(INotifyPropertyChanged)} interface.",
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0002");

/// <summary>
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate declaration of <see cref="INotifyPropertyChanging"/> would happen.
/// Gets a <see cref="DiagnosticDescriptor"/> indicating when a duplicate implementation of <see cref="INotifyPropertyChanging"/> would happen.
/// <para>
/// Format: <c>"Cannot apply [ObservableObjectAttribute] to type {0}, as it already declares the INotifyPropertyChanging interface"</c>.
/// Format: <c>"Cannot apply [ObservableObjectAttribute] to type {0}, as it already implements the INotifyPropertyChanging interface"</c>.
/// </para>
/// </summary>
public static readonly DiagnosticDescriptor DuplicateINotifyPropertyChangingInterfaceForObservableObjectAttributeError = new DiagnosticDescriptor(
id: "MVVMTK0003",
title: $"Duplicate {nameof(INotifyPropertyChanging)} definition",
messageFormat: $"Cannot apply [ObservableObject] to type {{0}}, as it already declares the {nameof(INotifyPropertyChanging)} interface",
messageFormat: $"Cannot apply [ObservableObject] to type {{0}}, as it already implements the {nameof(INotifyPropertyChanging)} interface",
category: typeof(ObservableObjectGenerator).FullName,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: $"Cannot apply [ObservableObject] to a type that already declares the {nameof(INotifyPropertyChanging)} interface.",
description: $"Cannot apply [ObservableObject] to a type that already implements the {nameof(INotifyPropertyChanging)} interface.",
helpLinkUri: "https://aka.ms/mvvmtoolkit/errors/mvvmtk0003");

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
// 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;
using System.Linq;
using CommunityToolkit.Mvvm.SourceGenerators.Helpers;
using Microsoft.CodeAnalysis;
using System;
using System.Collections.Generic;
using System.Linq;

namespace CommunityToolkit.Mvvm.SourceGenerators.Extensions;

Expand Down Expand Up @@ -249,4 +250,23 @@ static void BuildFrom(ISymbol? symbol, in ImmutableArrayBuilder<char> builder)

BuildFrom(symbol, in builder);
}

/// <summary>
/// Checks whether a given <see cref="ITypeSymbol"/> implements at least one members of the specified interface.
/// </summary>
/// <param name="symbol">The input <see cref="ITypeSymbol"/> instance.</param>
/// <param name="fullyQualifiedName">The fully qualified name of the interface.</param>
/// <returns>Whether at least a single interface member is implemented.</returns>
public static bool ImplementsInterfaceMember(this ITypeSymbol symbol, string fullyQualifiedName)
{
INamedTypeSymbol? interfaceSymbol = symbol.AllInterfaces.FirstOrDefault(x => x.HasFullyQualifiedMetadataName(fullyQualifiedName));
if (interfaceSymbol == null)
{
return false;
}

IEnumerable<ISymbol> interfaceMembers = interfaceSymbol.GetAllMembers();

return interfaceMembers.Any(x => symbol.FindImplementationForInterfaceMember(x) != null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,22 @@ public partial class SampleViewModel : INotifyPropertyChanged
""";

VerifyGeneratedDiagnostics<INotifyPropertyChangedGenerator>(source, "MVVMTK0001");

string source_NoImplementation = """
using System.ComponentModel;
using CommunityToolkit.Mvvm.ComponentModel;

namespace MyApp
{
[INotifyPropertyChanged]
public partial class SampleViewModel : INotifyPropertyChanged
{

}
}
""";

VerifyGeneratedDiagnostics<INotifyPropertyChangedGenerator>(source_NoImplementation);
}

[TestMethod]
Expand Down Expand Up @@ -87,6 +103,22 @@ public partial class SampleViewModel : INotifyPropertyChanged
""";

VerifyGeneratedDiagnostics<ObservableObjectGenerator>(source, "MVVMTK0002");

string source_NoImplementation = """
using System.ComponentModel;
using CommunityToolkit.Mvvm.ComponentModel;

namespace MyApp
{
[ObservableObject]
public partial class SampleViewModel : INotifyPropertyChanged
{

}
}
""";

VerifyGeneratedDiagnostics<ObservableObjectGenerator>(source_NoImplementation);
}

[TestMethod]
Expand Down Expand Up @@ -134,6 +166,22 @@ public partial class SampleViewModel : INotifyPropertyChanging
""";

VerifyGeneratedDiagnostics<ObservableObjectGenerator>(source, "MVVMTK0003");

string source_NoImplementation = """
using System.ComponentModel;
using CommunityToolkit.Mvvm.ComponentModel;

namespace MyApp
{
[ObservableObject]
public partial class SampleViewModel : INotifyPropertyChanging
{

}
}
""";

VerifyGeneratedDiagnostics<ObservableObjectGenerator>(source_NoImplementation);
}

[TestMethod]
Expand Down