diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/AnalyzerReleases.Shipped.md b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/AnalyzerReleases.Shipped.md index edf5a5d5f..8b88e411d 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/AnalyzerReleases.Shipped.md +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/AnalyzerReleases.Shipped.md @@ -27,3 +27,4 @@ WCTDP0017 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenera WCTDP0018 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Error | WCTDP0019 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Error | WCTDP0020 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Warning | +WCTDP0021 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Warning | diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Constants/WellKnownPropertyNames.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Constants/WellKnownPropertyNames.cs index 7d88ca86a..f5f81c523 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Constants/WellKnownPropertyNames.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Constants/WellKnownPropertyNames.cs @@ -13,4 +13,9 @@ internal static class WellKnownPropertyNames /// The MSBuild property to control the XAML mode. /// public const string DependencyPropertyGeneratorUseWindowsUIXaml = nameof(DependencyPropertyGeneratorUseWindowsUIXaml); + + /// + /// The MSBuild property to control whether the project is a WinRT component. + /// + public const string CsWinRTComponent = nameof(CsWinRTComponent); } diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationAnalyzer.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationAnalyzer.cs new file mode 100644 index 000000000..845ad0b7d --- /dev/null +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationAnalyzer.cs @@ -0,0 +1,103 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.GeneratedDependencyProperty.Constants; +using CommunityToolkit.GeneratedDependencyProperty.Extensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using static CommunityToolkit.GeneratedDependencyProperty.Diagnostics.DiagnosticDescriptors; + +namespace CommunityToolkit.GeneratedDependencyProperty; + +/// +/// A diagnostic analyzer that generates a warning whenever a dependency property is declared as a property. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class UseFieldDeclarationAnalyzer : DiagnosticAnalyzer +{ + /// + public override ImmutableArray SupportedDiagnostics { get; } = [DependencyPropertyFieldDeclaration]; + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(static context => + { + // Get the XAML mode to use + bool useWindowsUIXaml = context.Options.AnalyzerConfigOptionsProvider.GlobalOptions.GetMSBuildBooleanPropertyValue(WellKnownPropertyNames.DependencyPropertyGeneratorUseWindowsUIXaml); + + // Get the 'DependencyProperty' symbol + if (context.Compilation.GetTypeByMetadataName(WellKnownTypeNames.DependencyProperty(useWindowsUIXaml)) is not { } dependencyPropertySymbol) + { + return; + } + + // Check whether the current project is a WinRT component (modern .NET uses CsWinRT, legacy .NET produces .winmd files directly) + bool isWinRTComponent = + context.Options.AnalyzerConfigOptionsProvider.GlobalOptions.GetMSBuildBooleanPropertyValue(WellKnownPropertyNames.CsWinRTComponent) || + context.Compilation.Options.OutputKind is OutputKind.WindowsRuntimeMetadata; + + context.RegisterSymbolAction(context => + { + IPropertySymbol propertySymbol = (IPropertySymbol)context.Symbol; + + // We only care about properties which are of type 'DependencyProperty' + if (!SymbolEqualityComparer.Default.Equals(propertySymbol.Type, dependencyPropertySymbol)) + { + return; + } + + // If the property is an explicit interface implementation, allow it + if (propertySymbol.ExplicitInterfaceImplementations.Length > 0) + { + return; + } + + // Next, make sure this property isn't (implicitly) implementing any interface properties. + // If that's the case, we'll also allow it, as otherwise fixing this would break things. + foreach (INamedTypeSymbol interfaceSymbol in propertySymbol.ContainingType.AllInterfaces) + { + // Go over all properties (we can filter to just those with the same name) in each interface + foreach (IPropertySymbol interfacePropertySymbol in interfaceSymbol.GetMembers(propertySymbol.Name).OfType()) + { + // The property must have the same type to possibly be an interface implementation + if (!SymbolEqualityComparer.Default.Equals(interfacePropertySymbol.Type, propertySymbol.Type)) + { + continue; + } + + // If the property is not implemented at all, ignore it + if (propertySymbol.ContainingType.FindImplementationForInterfaceMember(interfacePropertySymbol) is not IPropertySymbol implementationSymbol) + { + continue; + } + + // If the current property is the one providing the implementation, then we allow it and stop here + if (SymbolEqualityComparer.Default.Equals(implementationSymbol, propertySymbol)) + { + return; + } + } + } + + // Make an exception for WinRT components: in this case declaring properties is valid, as they're needed for WinRT + if (isWinRTComponent && propertySymbol.GetEffectiveAccessibility() is Accessibility.Public) + { + return; + } + + // At this point, we know for sure the property isn't valid, so emit a diagnostic + context.ReportDiagnostic(Diagnostic.Create( + DependencyPropertyFieldDeclaration, + propertySymbol.Locations.First(), + propertySymbol)); + }, SymbolKind.Property); + }); + } +} diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationCorrectlyAnalyzer.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationCorrectlyAnalyzer.cs index d087bf650..1acf9b2da 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationCorrectlyAnalyzer.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseFieldDeclarationCorrectlyAnalyzer.cs @@ -27,7 +27,6 @@ public override void Initialize(AnalysisContext context) context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); - context.RegisterCompilationStartAction(static context => { // Get the XAML mode to use @@ -43,7 +42,7 @@ public override void Initialize(AnalysisContext context) { IFieldSymbol fieldSymbol = (IFieldSymbol)context.Symbol; - // We only care about fields with are of type 'DependencyProperty' + // We only care about fields which are of type 'DependencyProperty' if (!SymbolEqualityComparer.Default.Equals(fieldSymbol.Type, dependencyPropertySymbol)) { return; diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 71093bb77..9d1b5b5bb 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -21,6 +21,11 @@ internal static class DiagnosticDescriptors /// public const string IncorrectDependencyPropertyFieldDeclarationId = "WCTDP0020"; + /// + /// The diagnostic id for . + /// + public const string DependencyPropertyFieldDeclarationId = "WCTDP0021"; + /// /// "The property '{0}' cannot be used to generate a dependency property, as its declaration is not valid (it must be an instance (non static) partial property, with a getter and a setter that is not init-only)". /// @@ -280,4 +285,17 @@ internal static class DiagnosticDescriptors isEnabledByDefault: true, description: "All dependency property fields should be declared as 'public static readonly', and not be nullable.", helpLinkUri: "https://learn.microsoft.com/windows/uwp/xaml-platform/custom-dependency-properties#checklist-for-defining-a-dependency-property"); + + /// + /// "The property '{0}' is a dependency property, which is not the correct declaration type (all dependency properties should be declared as fields, unless implementing interface members or in authored WinRT component types)". + /// + public static readonly DiagnosticDescriptor DependencyPropertyFieldDeclaration = new( + id: DependencyPropertyFieldDeclarationId, + title: "Dependency property declared as a property", + messageFormat: "The property '{0}' is a dependency property, which is not the correct declaration type (all dependency properties should be declared as fields, unless implementing interface members or in authored WinRT component types)", + category: typeof(DependencyPropertyGenerator).FullName, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "All dependency properties should be declared as fields, unless implementing interface members or in authored WinRT component types.", + helpLinkUri: "https://learn.microsoft.com/windows/uwp/xaml-platform/custom-dependency-properties#checklist-for-defining-a-dependency-property"); } diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/ISymbolExtensions.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/ISymbolExtensions.cs index c49e99c1c..df1e8d75b 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/ISymbolExtensions.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/ISymbolExtensions.cs @@ -91,4 +91,42 @@ public static bool TryGetAttributeWithAnyType(this ISymbol symbol, ImmutableArra return false; } + + /// + /// Calculates the effective accessibility for a given symbol. + /// + /// The instance to check. + /// The effective accessibility for . + public static Accessibility GetEffectiveAccessibility(this ISymbol symbol) + { + // Start by assuming it's visible + Accessibility visibility = Accessibility.Public; + + // Handle special cases + switch (symbol.Kind) + { + case SymbolKind.Alias: return Accessibility.Private; + case SymbolKind.Parameter: return GetEffectiveAccessibility(symbol.ContainingSymbol); + case SymbolKind.TypeParameter: return Accessibility.Private; + } + + // Traverse the symbol hierarchy to determine the effective accessibility + while (symbol is not null && symbol.Kind != SymbolKind.Namespace) + { + switch (symbol.DeclaredAccessibility) + { + case Accessibility.NotApplicable: + case Accessibility.Private: + return Accessibility.Private; + case Accessibility.Internal: + case Accessibility.ProtectedAndInternal: + visibility = Accessibility.Internal; + break; + } + + symbol = symbol.ContainingSymbol; + } + + return visibility; + } } diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Helpers/CSharpAnalyzerTest{TAnalyzer}.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Helpers/CSharpAnalyzerTest{TAnalyzer}.cs index 441d35d82..071190a59 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Helpers/CSharpAnalyzerTest{TAnalyzer}.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Helpers/CSharpAnalyzerTest{TAnalyzer}.cs @@ -2,6 +2,9 @@ // 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 System.Text; using System.Threading; using System.Threading.Tasks; using CommunityToolkit.WinUI; @@ -9,6 +12,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis; using Windows.Foundation; using Windows.UI.ViewManagement; @@ -59,4 +63,34 @@ public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVe return test.RunAsync(CancellationToken.None); } + + /// + /// The language version to use to run the test. + public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVersion, (string PropertyName, object PropertyValue)[] editorconfig) + { + CSharpAnalyzerTest test = new(languageVersion) { TestCode = source }; + + test.TestState.ReferenceAssemblies = ReferenceAssemblies.Net.Net80; + test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Point).Assembly.Location)); + test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(ApplicationView).Assembly.Location)); + test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(DependencyProperty).Assembly.Location)); + test.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(GeneratedDependencyPropertyAttribute).Assembly.Location)); + + // Add any editorconfig properties, if present + if (editorconfig.Length > 0) + { + test.SolutionTransforms.Add((solution, projectId) => + solution.AddAnalyzerConfigDocument( + DocumentId.CreateNewId(projectId), + "DependencyPropertyGenerator.editorconfig", + SourceText.From($""" + is_global = true + {string.Join(Environment.NewLine, editorconfig.Select(static p => $"build_property.{p.PropertyName} = {p.PropertyValue}"))} + """, + Encoding.UTF8), + filePath: "/DependencyPropertyGenerator.editorconfig")); + } + + return test.RunAsync(CancellationToken.None); + } } diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs index b69b0ac56..cf03d3f76 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs @@ -1821,7 +1821,7 @@ public class TestAttribute(string P) : Attribute [TestMethod] public async Task UseFieldDeclarationCorrectlyAnalyzer_NotDependencyProperty_DoesNotWarn() { - string source = $$""" + const string source = """ using Windows.UI.Xaml; public class MyObject : DependencyObject @@ -1869,4 +1869,91 @@ public class MyObject : DependencyObject await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); } + + [TestMethod] + public async Task UseFieldDeclarationAnalyzer_NotDependencyProperty_DoesNotWarn() + { + const string source = """ + using Windows.UI.Xaml; + + public class MyObject : DependencyObject + { + public static string TestProperty => "Blah"; + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); + } + + [TestMethod] + public async Task UseFieldDeclarationAnalyzer_ExplicitInterfaceImplementation_DoesNotWarn() + { + const string source = """ + using Windows.UI.Xaml; + + public class MyObject : DependencyObject, IMyObject + { + static DependencyProperty IMyObject.TestProperty => DependencyProperty.Register("Test", typeof(string), typeof(MyObject), null); + } + + public interface IMyObject + { + static abstract DependencyProperty TestProperty { get; } + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); + } + + [TestMethod] + public async Task UseFieldDeclarationAnalyzer_ImplicitInterfaceImplementation_DoesNotWarn() + { + const string source = """ + using Windows.UI.Xaml; + + public class MyObject : DependencyObject, IMyObject + { + public static DependencyProperty TestProperty => DependencyProperty.Register("Test", typeof(string), typeof(MyObject), null); + } + + public interface IMyObject + { + static abstract DependencyProperty TestProperty { get; } + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); + } + + [TestMethod] + public async Task UseFieldDeclarationAnalyzer_WinRTComponent_DoesNotWarn() + { + const string source = """ + using Windows.UI.Xaml; + + public class MyObject : DependencyObject + { + public static DependencyProperty TestProperty => DependencyProperty.Register("Test", typeof(string), typeof(MyObject), null); + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13, editorconfig: [("CsWinRTComponent", true)]); + } + + [TestMethod] + public async Task UseFieldDeclarationAnalyzer_NormalProperty_Warns() + { + const string source = """ + using Windows.UI.Xaml; + + public class MyObject : DependencyObject + { + public static DependencyProperty {|WCTDP0021:Test1Property|} => DependencyProperty.Register("Test1", typeof(string), typeof(MyObject), null); + public static DependencyProperty {|WCTDP0021:Test2Property|} { get; } = DependencyProperty.Register("Test2", typeof(string), typeof(MyObject), null); + public DependencyProperty {|WCTDP0021:Test3Property|} { get; set; } + } + """; + + await CSharpAnalyzerTest.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13); + } } diff --git a/components/DependencyPropertyGenerator/src/CommunityToolkit.WinUI.DependencyPropertyGenerator.targets b/components/DependencyPropertyGenerator/src/CommunityToolkit.WinUI.DependencyPropertyGenerator.targets index 612e2bd54..924a54c6e 100644 --- a/components/DependencyPropertyGenerator/src/CommunityToolkit.WinUI.DependencyPropertyGenerator.targets +++ b/components/DependencyPropertyGenerator/src/CommunityToolkit.WinUI.DependencyPropertyGenerator.targets @@ -13,9 +13,10 @@ false - + +