Skip to content

Commit

Permalink
Add 'UseFieldDeclarationAnalyzer' and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergio0694 committed Dec 28, 2024
1 parent 59e4306 commit 618e6bb
Show file tree
Hide file tree
Showing 9 changed files with 290 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ WCTDP0017 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenera
WCTDP0018 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Error |
WCTDP0019 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Error |
WCTDP0020 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Warning |
WCTDP0021 | CommunityToolkit.GeneratedDependencyPropertyDependencyPropertyGenerator | Warning |
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,9 @@ internal static class WellKnownPropertyNames
/// The MSBuild property to control the XAML mode.
/// </summary>
public const string DependencyPropertyGeneratorUseWindowsUIXaml = nameof(DependencyPropertyGeneratorUseWindowsUIXaml);

/// <summary>
/// The MSBuild property to control whether the project is a WinRT component.
/// </summary>
public const string CsWinRTComponent = nameof(CsWinRTComponent);
}
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// A diagnostic analyzer that generates a warning whenever a dependency property is declared as a property.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UseFieldDeclarationAnalyzer : DiagnosticAnalyzer
{
/// <inheritdoc/>
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [DependencyPropertyFieldDeclaration];

/// <inheritdoc/>
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<IPropertySymbol>())
{
// 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);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ internal static class DiagnosticDescriptors
/// </summary>
public const string IncorrectDependencyPropertyFieldDeclarationId = "WCTDP0020";

/// <summary>
/// The diagnostic id for <see cref="DependencyPropertyFieldDeclaration"/>.
/// </summary>
public const string DependencyPropertyFieldDeclarationId = "WCTDP0021";

/// <summary>
/// <c>"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)"</c>.
/// </summary>
Expand Down Expand Up @@ -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");

/// <summary>
/// <c>"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)"</c>.
/// </summary>
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");
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,42 @@ public static bool TryGetAttributeWithAnyType(this ISymbol symbol, ImmutableArra

return false;
}

/// <summary>
/// Calculates the effective accessibility for a given symbol.
/// </summary>
/// <param name="symbol">The <see cref="ISymbol"/> instance to check.</param>
/// <returns>The effective accessibility for <paramref name="symbol"/>.</returns>
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
// 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;
using Microsoft.CodeAnalysis.CSharp.Testing;
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;
Expand Down Expand Up @@ -59,4 +63,34 @@ public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVe

return test.RunAsync(CancellationToken.None);
}

/// <inheritdoc cref="AnalyzerVerifier{TAnalyzer, TTest, TVerifier}.VerifyAnalyzerAsync"/>
/// <param name="languageVersion">The language version to use to run the test.</param>
public static Task VerifyAnalyzerAsync(string source, LanguageVersion languageVersion, (string PropertyName, object PropertyValue)[] editorconfig)
{
CSharpAnalyzerTest<TAnalyzer> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1869,4 +1869,91 @@ public class MyObject : DependencyObject

await CSharpAnalyzerTest<UseFieldDeclarationCorrectlyAnalyzer>.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<UseFieldDeclarationAnalyzer>.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<UseFieldDeclarationAnalyzer>.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<UseFieldDeclarationAnalyzer>.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<UseFieldDeclarationAnalyzer>.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<UseFieldDeclarationAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
<EnableGeneratedDependencyPropertyEmbeddedMode Condition="'$(EnableGeneratedDependencyPropertyEmbeddedMode)' == ''">false</EnableGeneratedDependencyPropertyEmbeddedMode>
</PropertyGroup>

<!-- Allow the source generators to detect the selected XAML mode -->
<!-- Mark all the MSBuild properties that the generators/analyzers might need -->
<ItemGroup>
<CompilerVisibleProperty Include="DependencyPropertyGeneratorUseWindowsUIXaml" />
<CompilerVisibleProperty Include="CsWinRTComponent" />
</ItemGroup>

<!-- Define the build constants depending on the current configuration -->
Expand Down

0 comments on commit 618e6bb

Please sign in to comment.