Skip to content

Commit 28ab171

Browse files
committed
Preserve named constants in code fixer
1 parent b8a7824 commit 28ab171

File tree

3 files changed

+154
-19
lines changed

3 files changed

+154
-19
lines changed

components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.CodeFixers/UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
4646
Diagnostic diagnostic = context.Diagnostics[0];
4747
TextSpan diagnosticSpan = context.Span;
4848

49-
// We can only possibly fix diagnostics with an additional location
50-
if (diagnostic.AdditionalLocations is not [{ } fieldLocation])
49+
// We always expect the field location to be the first additional location, this must be present
50+
if (diagnostic.AdditionalLocations is not [{ } fieldLocation, ..])
5151
{
5252
return;
5353
}
@@ -62,6 +62,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
6262
string? defaultValue = diagnostic.Properties[UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.DefaultValuePropertyName];
6363
string? defaultValueTypeReferenceId = diagnostic.Properties[UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.DefaultValueTypeReferenceIdPropertyName];
6464

65+
// Get any additional locations, if available
66+
Location? defaultValueExpressionLocation = diagnostic.AdditionalLocations.ElementAtOrDefault(1);
67+
6568
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
6669

6770
// Get the property declaration and the field declaration from the target diagnostic
@@ -82,7 +85,8 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
8285
propertyDeclaration,
8386
fieldDeclaration,
8487
defaultValue,
85-
defaultValueTypeReferenceId),
88+
defaultValueTypeReferenceId,
89+
defaultValueExpressionLocation),
8690
equivalenceKey: "Use a partial property"),
8791
diagnostic);
8892
}
@@ -123,21 +127,38 @@ private static bool TryGetGeneratedDependencyPropertyAttributeList(
123127
/// </summary>
124128
/// <param name="document">The original document being fixed.</param>
125129
/// <param name="semanticModel">The <see cref="SemanticModel"/> instance for the current compilation.</param>
130+
/// <param name="root">The original tree root belonging to the current document.</param>
126131
/// <param name="defaultValueExpression">The expression for the default value of the property, if present</param>
127-
/// <param name="defaultValueTypeFullyQualifiedMetadataName">The fully qualified metadata name of the default value, if present.</param>
132+
/// <param name="defaultValueTypeReferenceId">The documentation comment reference id for type of the default value, if present.</param>
133+
/// <param name="defaultValueExpressionLocation">The location for the default value, if available.</param>
128134
/// <returns>The updated attribute syntax.</returns>
129135
private static AttributeListSyntax UpdateGeneratedDependencyPropertyAttributeList(
130136
Document document,
131137
SemanticModel semanticModel,
138+
SyntaxNode root,
132139
AttributeListSyntax generatedDependencyPropertyAttributeList,
133140
string? defaultValueExpression,
134-
string? defaultValueTypeReferenceId)
141+
string? defaultValueTypeReferenceId,
142+
Location? defaultValueExpressionLocation)
135143
{
136144
// If we do have a default value expression, set it in the attribute.
137145
// We extract the generated attribute so we can add the new argument.
138146
// It's important to reuse it, as it has the "add usings" annotation.
139147
if (defaultValueExpression is not null)
140148
{
149+
SyntaxGenerator syntaxGenerator = SyntaxGenerator.GetGenerator(document);
150+
151+
// Special case if we have a location for the original expression, and we can resolve the node.
152+
// In this case, we want to just carry that over with no changes (this is used for named constants).
153+
// See notes below for how this method is constructing the new attribute argument to insert.
154+
if (defaultValueExpressionLocation is not null &&
155+
root.FindNode(defaultValueExpressionLocation.SourceSpan) is ArgumentSyntax { Expression: { } defaultValueOriginalExpression })
156+
{
157+
return (AttributeListSyntax)syntaxGenerator.AddAttributeArguments(
158+
generatedDependencyPropertyAttributeList,
159+
[syntaxGenerator.AttributeArgument("DefaultValue", defaultValueOriginalExpression)]);
160+
}
161+
141162
ExpressionSyntax parsedExpression = ParseExpression(defaultValueExpression);
142163

143164
// Special case values which are simple enum member accesses, like 'global::Windows.UI.Xaml.Visibility.Collapsed'.
@@ -155,8 +176,6 @@ private static AttributeListSyntax UpdateGeneratedDependencyPropertyAttributeLis
155176
Simplifier.AddImportsAnnotation,
156177
new SyntaxAnnotation("SymbolId", defaultValueTypeReferenceId));
157178

158-
SyntaxGenerator syntaxGenerator = SyntaxGenerator.GetGenerator(document);
159-
160179
// Create the attribute argument to insert
161180
SyntaxNode attributeArgumentSyntax = syntaxGenerator.AttributeArgument("DefaultValue", parsedExpression);
162181

@@ -178,8 +197,6 @@ private static AttributeListSyntax UpdateGeneratedDependencyPropertyAttributeLis
178197
// That is, they will be the same if the type is not nested, and not generic, which is what we expect.
179198
if (semanticModel.Compilation.GetTypeByMetadataName(fullyQualifiedMetadataName) is INamedTypeSymbol enumTypeSymbol)
180199
{
181-
SyntaxGenerator syntaxGenerator = SyntaxGenerator.GetGenerator(document);
182-
183200
// Create the identifier syntax for the enum type, with the right annotations
184201
SyntaxNode enumTypeSyntax = syntaxGenerator.TypeExpression(enumTypeSymbol).WithAdditionalAnnotations(Simplifier.AddImportsAnnotation);
185202

@@ -215,7 +232,8 @@ private static AttributeListSyntax UpdateGeneratedDependencyPropertyAttributeLis
215232
/// <param name="propertyDeclaration">The <see cref="PropertyDeclarationSyntax"/> for the property being updated.</param>
216233
/// <param name="fieldDeclaration">The <see cref="FieldDeclarationSyntax"/> for the declared property to remove.</param>
217234
/// <param name="defaultValueExpression">The expression for the default value of the property, if present</param>
218-
/// <param name="defaultValueTypeFullyQualifiedMetadataName">The fully qualified metadata name of the default value, if present.</param>
235+
/// <param name="defaultValueTypeReferenceId">The documentation comment reference id for type of the default value, if present.</param>
236+
/// <param name="defaultValueExpressionLocation">The location for the default value, if available.</param>
219237
/// <returns>An updated document with the applied code fix, and <paramref name="propertyDeclaration"/> being replaced with a partial property.</returns>
220238
private static async Task<Document> ConvertToPartialProperty(
221239
Document document,
@@ -224,7 +242,8 @@ private static async Task<Document> ConvertToPartialProperty(
224242
PropertyDeclarationSyntax propertyDeclaration,
225243
FieldDeclarationSyntax fieldDeclaration,
226244
string? defaultValueExpression,
227-
string? defaultValueTypeReferenceId)
245+
string? defaultValueTypeReferenceId,
246+
Location? defaultValueExpressionLocation)
228247
{
229248
await Task.CompletedTask;
230249

@@ -240,12 +259,14 @@ private static async Task<Document> ConvertToPartialProperty(
240259
ConvertToPartialProperty(
241260
document,
242261
semanticModel,
262+
root,
243263
propertyDeclaration,
244264
fieldDeclaration,
245265
generatedDependencyPropertyAttributeList,
246266
syntaxEditor,
247267
defaultValueExpression,
248-
defaultValueTypeReferenceId);
268+
defaultValueTypeReferenceId,
269+
defaultValueExpressionLocation);
249270

250271
RemoveLeftoverLeadingEndOfLines([fieldDeclaration], syntaxEditor);
251272

@@ -258,22 +279,26 @@ private static async Task<Document> ConvertToPartialProperty(
258279
/// </summary>
259280
/// <param name="document">The original document being fixed.</param>
260281
/// <param name="semanticModel">The <see cref="SemanticModel"/> instance for the current compilation.</param>
282+
/// <param name="root">The original tree root belonging to the current document.</param>
261283
/// <param name="propertyDeclaration">The <see cref="PropertyDeclarationSyntax"/> for the property being updated.</param>
262284
/// <param name="fieldDeclaration">The <see cref="FieldDeclarationSyntax"/> for the declared property to remove.</param>
263285
/// <param name="generatedDependencyPropertyAttributeList">The <see cref="AttributeListSyntax"/> with the attribute to add.</param>
264286
/// <param name="syntaxEditor">The <see cref="SyntaxEditor"/> instance to use.</param>
265287
/// <param name="defaultValueExpression">The expression for the default value of the property, if present</param>
266-
/// <param name="defaultValueTypeFullyQualifiedMetadataName">The fully qualified metadata name of the default value, if present.</param>
288+
/// <param name="defaultValueTypeReferenceId">The documentation comment reference id for type of the default value, if present.</param>
289+
/// <param name="defaultValueExpressionLocation">The location for the default value, if available.</param>
267290
/// <returns>An updated document with the applied code fix, and <paramref name="propertyDeclaration"/> being replaced with a partial property.</returns>
268291
private static void ConvertToPartialProperty(
269292
Document document,
270293
SemanticModel semanticModel,
294+
SyntaxNode root,
271295
PropertyDeclarationSyntax propertyDeclaration,
272296
FieldDeclarationSyntax fieldDeclaration,
273297
AttributeListSyntax generatedDependencyPropertyAttributeList,
274298
SyntaxEditor syntaxEditor,
275299
string? defaultValueExpression,
276-
string? defaultValueTypeReferenceId)
300+
string? defaultValueTypeReferenceId,
301+
Location? defaultValueExpressionLocation)
277302
{
278303
// Replace the property with the partial property using the attribute. Note that it's important to use the
279304
// lambda 'ReplaceNode' overload here, rather than creating a modifier property declaration syntax node and
@@ -287,9 +312,11 @@ private static void ConvertToPartialProperty(
287312
generatedDependencyPropertyAttributeList = UpdateGeneratedDependencyPropertyAttributeList(
288313
document,
289314
semanticModel,
315+
root,
290316
generatedDependencyPropertyAttributeList,
291317
defaultValueExpression,
292-
defaultValueTypeReferenceId);
318+
defaultValueTypeReferenceId,
319+
defaultValueExpressionLocation);
293320

294321
// Start setting up the updated attribute lists
295322
SyntaxList<AttributeListSyntax> attributeLists = propertyDeclaration.AttributeLists;
@@ -482,26 +509,30 @@ private sealed class FixAllProvider : DocumentBasedFixAllProvider
482509
}
483510

484511
// Also check that we can find the target field to remove
485-
if (diagnostic.AdditionalLocations is not [{ } fieldLocation] ||
512+
if (diagnostic.AdditionalLocations is not [{ } fieldLocation, ..] ||
486513
root.FindNode(fieldLocation.SourceSpan) is not FieldDeclarationSyntax fieldDeclaration)
487514
{
488515
continue;
489516
}
490517

491518
// Retrieve the properties passed by the analyzer
492519
string? defaultValue = diagnostic.Properties[UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.DefaultValuePropertyName];
493-
string? defaultValueTypeFullyQualifiedMetadataName = diagnostic.Properties[UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.DefaultValueTypeReferenceIdPropertyName];
520+
string? defaultValueTypeReferenceId = diagnostic.Properties[UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.DefaultValueTypeReferenceIdPropertyName];
494521

522+
// Get any additional locations, if available
523+
Location? defaultValueExpressionLocation = diagnostic.AdditionalLocations.ElementAtOrDefault(1);
495524

496525
ConvertToPartialProperty(
497526
document,
498527
semanticModel,
528+
root,
499529
propertyDeclaration,
500530
fieldDeclaration,
501531
generatedDependencyPropertyAttributeList,
502532
syntaxEditor,
503533
defaultValue,
504-
defaultValueTypeFullyQualifiedMetadataName);
534+
defaultValueTypeReferenceId,
535+
defaultValueExpressionLocation);
505536

506537
fieldDeclarations.Add(fieldDeclaration);
507538
}

components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,14 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
486486
fieldFlags.DefaultValueTypeReferenceId = DocumentationCommentId.CreateReferenceId(operandType);
487487
}
488488
}
489+
490+
// Special case for named constants: in this case we want to carry the whole expression over, rather
491+
// than serializing the constant value itself. This preserves the actual fields being referenced.
492+
// We skip enum fields, as those are not named constants. Those will be handled by the logic above.
493+
if (conversionOperation.Operand is IFieldReferenceOperation { Field: { IsConst: true, ContainingType.TypeKind: not TypeKind.Enum } })
494+
{
495+
fieldFlags.DefaultValueExpressionLocation = conversionOperation.Operand.Syntax.GetLocation();
496+
}
489497
}
490498
else if (conversionOperation.Operand is IFieldReferenceOperation { Field: { ContainingType.SpecialType: SpecialType.System_String, Name: "Empty" } })
491499
{
@@ -573,7 +581,7 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
573581
context.ReportDiagnostic(Diagnostic.Create(
574582
UseGeneratedDependencyPropertyForManualProperty,
575583
pair.Key.Locations.FirstOrDefault(),
576-
[fieldLocation],
584+
((Location?[])[fieldLocation, fieldFlags.DefaultValueExpressionLocation]).OfType<Location>(),
577585
ImmutableDictionary.Create<string, string?>()
578586
.Add(DefaultValuePropertyName, fieldFlags.DefaultValue?.ToString())
579587
.Add(DefaultValueTypeReferenceIdPropertyName, fieldFlags.DefaultValueTypeReferenceId),
@@ -598,6 +606,7 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
598606
fieldFlags.PropertyType = null;
599607
fieldFlags.DefaultValue = null;
600608
fieldFlags.DefaultValueTypeReferenceId = null;
609+
fieldFlags.DefaultValueExpressionLocation = null;
601610
fieldFlags.FieldLocation = null;
602611

603612
fieldFlagsStack.Push(fieldFlags);
@@ -677,6 +686,11 @@ private sealed class FieldFlags
677686
/// </summary>
678687
public string? DefaultValueTypeReferenceId;
679688

689+
/// <summary>
690+
/// The location for the default value, if available.
691+
/// </summary>
692+
public Location? DefaultValueExpressionLocation;
693+
680694
/// <summary>
681695
/// The location of the target field being initialized.
682696
/// </summary>

components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,96 @@ public partial class MyControl : Control
544544
await test.RunAsync();
545545
}
546546

547+
[TestMethod]
548+
[DataRow("int", "MyContainer1.X")]
549+
[DataRow("float", "MyContainer1.Y")]
550+
[DataRow("string", "MyContainer1.S")]
551+
[DataRow("MyContainer1.MyContainer2.NestedEnum", "MyContainer1.E1")]
552+
[DataRow("MyContainer1.MyEnum1", "MyContainer1.E2")]
553+
[DataRow("MyEnum3", "MyContainer1.E3")]
554+
[DataRow("int", "MyContainer1.MyContainer2.X")]
555+
[DataRow("float", "MyContainer1.MyContainer2.Y")]
556+
[DataRow("string", "MyContainer1.MyContainer2.S")]
557+
[DataRow("MyContainer1.MyContainer2.NestedEnum", "MyContainer1.MyContainer2.E1")]
558+
[DataRow("MyContainer1.MyEnum1", "MyContainer1.MyContainer2.E2")]
559+
[DataRow("MyEnum3", "MyContainer1.MyContainer2.E3")]
560+
public async Task SimpleProperty_WithExplicitValue_NamedConstant(string propertyType, string defaultValue)
561+
{
562+
const string types = """
563+
public class MyContainer1
564+
{
565+
public const int X = 42;
566+
public const float Y = 3.14f;
567+
public const string S = "Test";
568+
public const MyContainer2.NestedEnum E1 = MyContainer2.NestedEnum.B;
569+
public const MyEnum1 E2 = MyEnum1.B;
570+
public const MyEnum3 E3 = MyEnum3.B;
571+
572+
public enum MyEnum1 { A, B };
573+
574+
public struct MyContainer2
575+
{
576+
public const int X = 42;
577+
public const float Y = 3.14f;
578+
public const string S = "Test";
579+
public const NestedEnum E1 = NestedEnum.B;
580+
public const MyEnum1 E2 = MyEnum1.B;
581+
public const MyEnum3 E3 = MyEnum3.B;
582+
583+
public enum NestedEnum { A, B };
584+
}
585+
}
586+
587+
public enum MyEnum3 { A, B }
588+
""";
589+
590+
string original = $$"""
591+
using Windows.UI.Xaml;
592+
593+
namespace MyApp;
594+
595+
public class MyObject : DependencyObject
596+
{
597+
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
598+
name: "Name",
599+
propertyType: typeof({{propertyType}}),
600+
ownerType: typeof(MyObject),
601+
typeMetadata: new PropertyMetadata({{defaultValue}}));
602+
603+
public {{propertyType}} [|Name|]
604+
{
605+
get => ({{propertyType}})GetValue(NameProperty);
606+
set => SetValue(NameProperty, value);
607+
}
608+
}
609+
610+
{{types}}
611+
""";
612+
613+
string @fixed = $$"""
614+
using CommunityToolkit.WinUI;
615+
using Windows.UI.Xaml;
616+
617+
namespace MyApp;
618+
619+
public partial class MyObject : DependencyObject
620+
{
621+
[GeneratedDependencyProperty(DefaultValue = {{defaultValue}})]
622+
public partial {{propertyType}} {|CS9248:Name|} { get; set; }
623+
}
624+
625+
{{types}}
626+
""";
627+
628+
CSharpCodeFixTest test = new(LanguageVersion.Preview)
629+
{
630+
TestCode = original,
631+
FixedCode = @fixed
632+
};
633+
634+
await test.RunAsync();
635+
}
636+
547637
[TestMethod]
548638
[DataRow("[A]", "[static: A]")]
549639
[DataRow("""[Test(42, "Hello")]""", """[static: Test(42, "Hello")]""")]

0 commit comments

Comments
 (0)