Skip to content

Commit 2ae6c2a

Browse files
committed
Fix handling of additional locations for code fixer
1 parent e8b0b1b commit 2ae6c2a

File tree

2 files changed

+97
-8
lines changed

2 files changed

+97
-8
lines changed

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

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.Collections.Generic;
67
using System.Collections.Immutable;
78
using System.Composition;
@@ -464,7 +465,7 @@ private static void RemoveLeftoverLeadingEndOfLines(IReadOnlyCollection<FieldDec
464465

465466
MemberDeclarationSyntax nextMember = fieldParentTypeDeclaration.Members[fieldDeclarationIndex + 1];
466467

467-
// It's especially important to skip members that have been rmeoved. This would otherwise fail when computing
468+
// It's especially important to skip members that have been removed. This would otherwise fail when computing
468469
// the final document. We only care about fixing trivia for members that will still be present after all edits.
469470
if (fieldDeclarations.Contains(nextMember))
470471
{
@@ -515,9 +516,10 @@ private static bool TryGetAdditionalLocations(
515516
[NotNullWhen(true)] out Location? propertyTypeExpressionLocation,
516517
[NotNullWhen(true)] out Location? defaultValueExpressionLocation)
517518
{
518-
// We always expect 3 additional locations, as per contract with the analyzer.
519-
// Do a sanity check just in case, as we've seen sporadic issues with this.
520-
if (diagnostic.AdditionalLocations is not [{ } location1, { } location2, { } location3])
519+
// Ensure we have the additional location kind, and parse it
520+
if (!Enum.TryParse(
521+
diagnostic.Properties[UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.AdditionalLocationKindPropertyName],
522+
out AdditionalLocationKind additionalLocationKind))
521523
{
522524
fieldLocation = null;
523525
propertyTypeExpressionLocation = null;
@@ -526,9 +528,60 @@ private static bool TryGetAdditionalLocations(
526528
return false;
527529
}
528530

529-
fieldLocation = location1;
530-
propertyTypeExpressionLocation = location2;
531-
defaultValueExpressionLocation = location3;
531+
int currentLocationIndex = 0;
532+
533+
// Helper to extract an additional location with a specified kind
534+
bool TryExtractAdditionalLocation(AdditionalLocationKind currentLocationKind, [NotNullWhen(true)] out Location? fieldLocation)
535+
{
536+
// Ensure the current kind is present and that we can extract an additional location
537+
if (!additionalLocationKind.HasFlag(currentLocationKind))
538+
{
539+
// Special case for the unit test runner. If we have 3 diagnostics (see details on the contract in the analyzer),
540+
// it means we're running in the test runner, which does not remove 'None' diagnostics. In this case, we need to
541+
// increment the current index, or otherwise we'll read incorrect locations after this one.
542+
if (diagnostic.AdditionalLocations.Count == 3)
543+
{
544+
currentLocationIndex++;
545+
}
546+
547+
fieldLocation = null;
548+
549+
return false;
550+
}
551+
552+
// Parse the additional location
553+
fieldLocation = diagnostic.AdditionalLocations[currentLocationIndex++];
554+
555+
// Special case: if the location is 'None', we should ignore it. This is because while the location is
556+
// available when running tests, it will be removed when running in the IDE, because the serialization
557+
// logic that Roslyn uses will filter out all 'None' locations. This step is needed to match that logic.
558+
if (fieldLocation == Location.None)
559+
{
560+
fieldLocation = null;
561+
562+
return false;
563+
}
564+
565+
return true;
566+
}
567+
568+
// We always expect to have the field location
569+
if (!TryExtractAdditionalLocation(AdditionalLocationKind.FieldLocation, out fieldLocation))
570+
{
571+
fieldLocation = null;
572+
propertyTypeExpressionLocation = null;
573+
defaultValueExpressionLocation = null;
574+
575+
return false;
576+
}
577+
578+
// Try to extract all optional additional locations
579+
_ = TryExtractAdditionalLocation(AdditionalLocationKind.PropertyTypeExpressionLocation, out propertyTypeExpressionLocation);
580+
_ = TryExtractAdditionalLocation(AdditionalLocationKind.DefaultValueExpressionLocation, out defaultValueExpressionLocation);
581+
582+
// None of the additional locations should ever be 'null'
583+
propertyTypeExpressionLocation ??= Location.None;
584+
defaultValueExpressionLocation ??= Location.None;
532585

533586
return true;
534587
}

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.Collections.Generic;
67
using System.Collections.Immutable;
78
using System.Diagnostics.CodeAnalysis;
@@ -62,6 +63,11 @@ public sealed class UseGeneratedDependencyPropertyOnManualPropertyAnalyzer : Dia
6263
/// </summary>
6364
public const string DefaultValueTypeReferenceIdPropertyName = "DefaultValueTypeReferenceId";
6465

66+
/// <summary>
67+
/// The property name for the serialized <see cref="AdditionalLocationKind"/> value.
68+
/// </summary>
69+
public const string AdditionalLocationKindPropertyName = "AdditionalLocationKind";
70+
6571
/// <inheritdoc/>
6672
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = [UseGeneratedDependencyPropertyForManualProperty];
6773

@@ -598,13 +604,21 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla
598604
fieldFlags.DefaultValueExpressionLocation ?? Location.None
599605
];
600606

607+
// Track the available locations, so we can extract them back. We cannot rely on the length
608+
// of the supplied array, because Roslyn will remove all 'None' locations from the array.
609+
AdditionalLocationKind additionalLocationKind =
610+
AdditionalLocationKind.FieldLocation |
611+
(additionalLocations[1] != Location.None ? AdditionalLocationKind.PropertyTypeExpressionLocation : 0) |
612+
(additionalLocations[2] != Location.None ? AdditionalLocationKind.DefaultValueExpressionLocation : 0);
613+
601614
context.ReportDiagnostic(Diagnostic.Create(
602615
UseGeneratedDependencyPropertyForManualProperty,
603616
pair.Key.Locations.FirstOrDefault(),
604617
additionalLocations,
605618
ImmutableDictionary.Create<string, string?>()
606619
.Add(DefaultValuePropertyName, fieldFlags.DefaultValue?.ToString())
607-
.Add(DefaultValueTypeReferenceIdPropertyName, fieldFlags.DefaultValueTypeReferenceId),
620+
.Add(DefaultValueTypeReferenceIdPropertyName, fieldFlags.DefaultValueTypeReferenceId)
621+
.Add(AdditionalLocationKindPropertyName, additionalLocationKind.ToString()),
608622
pair.Key));
609623
}
610624
}
@@ -723,3 +737,25 @@ private sealed class FieldFlags
723737
public Location? FieldLocation;
724738
}
725739
}
740+
741+
/// <summary>
742+
/// An enum indicating the type of additional locations that are produced by the analyzer.
743+
/// </summary>
744+
[Flags]
745+
public enum AdditionalLocationKind
746+
{
747+
/// <summary>
748+
/// The location of the field to remove, always present.
749+
/// </summary>
750+
FieldLocation = 1 << 0,
751+
752+
/// <summary>
753+
/// The location of the property type expression, if present.
754+
/// </summary>
755+
PropertyTypeExpressionLocation = 1 << 1,
756+
757+
/// <summary>
758+
/// The location of the default value expression, if present.
759+
/// </summary>
760+
DefaultValueExpressionLocation = 1 << 2
761+
}

0 commit comments

Comments
 (0)