From c500383cb775fc42182f27ca781123f167b57c0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=90=BD=E7=AC=94?= <46271592+Skymly@users.noreply.github.com> Date: Thu, 2 Jul 2026 23:54:01 +0800 Subject: [PATCH 1/2] Replace Location with value-equatable LocationInfo for incremental cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Location uses reference equality, which breaks model value equality when Compilation.Clone() creates new Location objects. This forced AssertCacheHit to use the same compilation (not Clone), weakening the cache test. Fix: introduce LocationInfo, a value-equatable struct storing FilePath, TextSpan, and LineSpan. Replace Location with LocationInfo in all 9 record model types and DiagnosticInfo. Convert Location to LocationInfo in transform methods, and LocationInfo to Location in diagnostic reporting. Also fix List return types in KeyedRegistration and HandlerRegistration transforms to EquatableArray — List uses reference equality, which broke caching even with LocationInfo. EquatableArray provides value equality for correct incremental caching. AssertCacheHit now uses Compilation.Clone() for the second run, verifying that LocationInfo value equality holds across cloned compilations. All 7 cache tests pass with Clone, plus a new LocationInfo_Equals_AfterClone test. Closes #215 --- .../Generators/CompositePartGenerator.cs | 16 ++--- .../Generators/DecoratorGenerator.cs | 16 ++--- .../Generators/GenerateSingletonGenerator.cs | 4 +- .../Generators/HandlerOrderGenerator.cs | 24 +++---- .../Generators/LocationInfo.cs | 71 +++++++++++++++++++ .../RegisterEventHandlerGenerator.cs | 8 +-- .../Generators/RegistrationGeneratorHelper.cs | 28 ++++---- .../Generators/Result.cs | 11 ++- .../StateTransition/StateTransitionModels.cs | 10 +-- .../StateTransitionTransform.cs | 6 +- .../StateTransitionValidator.cs | 44 ++++++------ .../Generators/ActionChainComposerTests.cs | 20 +++--- .../Generators/IncrementalCacheTests.cs | 30 +++++++- .../SourceGeneratorTestContext.cs | 17 ++--- 14 files changed, 201 insertions(+), 104 deletions(-) create mode 100644 DesignPatterns.SourceGenerators/Generators/LocationInfo.cs diff --git a/DesignPatterns.SourceGenerators/Generators/CompositePartGenerator.cs b/DesignPatterns.SourceGenerators/Generators/CompositePartGenerator.cs index 5a0846a..1036688 100644 --- a/DesignPatterns.SourceGenerators/Generators/CompositePartGenerator.cs +++ b/DesignPatterns.SourceGenerators/Generators/CompositePartGenerator.cs @@ -142,7 +142,7 @@ private static Result Transform(GeneratorAttributeSyntaxC ImplementsContract(implementation, contract), HasPublicParameterlessConstructor(implementation), ImplementsBuildable(implementation, contract), - context.TargetNode.GetLocation())); + new LocationInfo(context.TargetNode.GetLocation()))); } return Result.Empty; @@ -202,7 +202,7 @@ private static void ReportDuplicateKeys(SourceProductionContext context, List Transform(GeneratorAttributeSyntaxC ImplementsAsyncDecoratorInterface(decoratorType, serviceType), HasValidAsyncSignature(decoratorType, serviceType), HasPublicParameterlessConstructor(decoratorType), - context.TargetNode.GetLocation())); + new LocationInfo(context.TargetNode.GetLocation()))); } return Result.Empty; @@ -186,7 +186,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DuplicateOrderDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.Order, serviceName)); } @@ -196,7 +196,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( ContractMismatchDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.DecoratorName, registration.Service.FullyQualifiedName)); } @@ -205,7 +205,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( MissingDecoratorInterfaceDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.DecoratorName, registration.Service.FullyQualifiedName)); } @@ -214,7 +214,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( AsyncSignatureMismatchDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.DecoratorName, registration.Service.FullyQualifiedName)); } @@ -225,14 +225,14 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DiNotResolvableDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.DecoratorName)); } else { context.ReportDiagnostic(Diagnostic.Create( MissingParameterlessConstructorDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.DecoratorName)); } } @@ -398,5 +398,5 @@ private sealed record DecoratorRegistration( bool ImplementsAsyncDecoratorInterface, bool HasValidAsyncSignature, bool HasPublicParameterlessConstructor, - Location Location); + LocationInfo Location); } diff --git a/DesignPatterns.SourceGenerators/Generators/GenerateSingletonGenerator.cs b/DesignPatterns.SourceGenerators/Generators/GenerateSingletonGenerator.cs index 4b68138..a70ca88 100644 --- a/DesignPatterns.SourceGenerators/Generators/GenerateSingletonGenerator.cs +++ b/DesignPatterns.SourceGenerators/Generators/GenerateSingletonGenerator.cs @@ -63,7 +63,7 @@ private static Result GetTargetInfo(GeneratorAttributeSynta } } - var location = classDeclaration.GetLocation(); + var location = new LocationInfo(classDeclaration.GetLocation()); var className = symbol.Name; var isStatic = symbol.IsStatic; var typeKind = symbol.TypeKind; @@ -110,7 +110,7 @@ private static void Execute(SourceProductionContext context, Result Transform(GeneratorAttributeSyntaxContext context, bool isGenericAttribute) + private static EquatableArray Transform(GeneratorAttributeSyntaxContext context, bool isGenericAttribute) { var result = new List(); if (context.TargetSymbol is not INamedTypeSymbol handlerType) { - return result; + return new EquatableArray(result.ToArray()); } if (context.Attributes.IsDefaultOrEmpty) { - return result; + return new EquatableArray(result.ToArray()); } - var location = context.TargetNode.GetLocation(); + var location = new LocationInfo(context.TargetNode.GetLocation()); var handlerName = handlerType.Name; var handlerFullyQualifiedDisplayString = handlerType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); @@ -147,7 +147,7 @@ private static List Transform(GeneratorAttributeSyntaxConte location)); } - return result; + return new EquatableArray(result.ToArray()); } private static void Execute( @@ -218,7 +218,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DuplicateOrderDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.Order, contextName)); } @@ -228,7 +228,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( HandlerContractMismatchDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.HandlerName, registration.Context.FullyQualifiedName)); } @@ -237,7 +237,7 @@ private static void ReportValidationDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( MissingParameterlessConstructorDescriptor, - registration.Location, + registration.Location.ToLocation(), registration.HandlerName)); } @@ -262,7 +262,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.HandlerOrderGuardMethodNotFound, - registration.Location, + registration.Location.ToLocation(), guard.Name, registration.HandlerName, contextTypeDisplay)); @@ -273,7 +273,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.HandlerOrderGuardMethodNotStatic, - registration.Location, + registration.Location.ToLocation(), guard.Name, registration.HandlerName)); continue; @@ -283,7 +283,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.HandlerOrderGuardMethodWrongSignature, - registration.Location, + registration.Location.ToLocation(), guard.Name, registration.HandlerName, contextTypeDisplay)); @@ -346,5 +346,5 @@ private sealed record HandlerRegistration( bool ImplementsHandler, bool HasPublicParameterlessConstructor, GuardResolution Guard, - Location Location); + LocationInfo Location); } diff --git a/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs b/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs new file mode 100644 index 0000000..da2bb49 --- /dev/null +++ b/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs @@ -0,0 +1,71 @@ +using System; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; + +namespace DesignPatterns.SourceGenerators.Generators; + +/// +/// A value-equatable representation of a . Used in +/// incremental generator models to ensure correct caching by the Roslyn +/// incremental pipeline — uses reference equality, +/// which breaks model value equality when +/// creates new objects. +/// +internal readonly struct LocationInfo : IEquatable +{ + public string? FilePath { get; } + public TextSpan TextSpan { get; } + public LinePositionSpan LineSpan { get; } + + public LocationInfo(Location? location) + { + if (location is null) + { + FilePath = null; + TextSpan = default; + LineSpan = default; + return; + } + + FilePath = location.SourceTree?.FilePath ?? location.GetLineSpan().Path; + TextSpan = location.SourceSpan; + LineSpan = location.GetLineSpan().Span; + } + + /// + /// Reconstructs a suitable for diagnostic reporting. + /// The returned location is an that does + /// not reference a , which is sufficient for + /// . + /// + public Location? ToLocation() + { + if (FilePath is null) + { + return null; + } + + return Location.Create(FilePath, TextSpan, LineSpan); + } + + public bool Equals(LocationInfo other) + { + return string.Equals(FilePath, other.FilePath, StringComparison.Ordinal) + && TextSpan == other.TextSpan + && LineSpan == other.LineSpan; + } + + public override bool Equals(object? obj) => obj is LocationInfo other && Equals(other); + + public override int GetHashCode() + { + unchecked + { + return ((FilePath?.GetHashCode() ?? 0) * 397) ^ TextSpan.GetHashCode() ^ LineSpan.GetHashCode(); + } + } + + public static bool operator ==(LocationInfo left, LocationInfo right) => left.Equals(right); + + public static bool operator !=(LocationInfo left, LocationInfo right) => !left.Equals(right); +} diff --git a/DesignPatterns.SourceGenerators/Generators/RegisterEventHandlerGenerator.cs b/DesignPatterns.SourceGenerators/Generators/RegisterEventHandlerGenerator.cs index 1f81bee..b74501b 100644 --- a/DesignPatterns.SourceGenerators/Generators/RegisterEventHandlerGenerator.cs +++ b/DesignPatterns.SourceGenerators/Generators/RegisterEventHandlerGenerator.cs @@ -33,7 +33,7 @@ internal sealed record EventHandlerRegistration( string HandlerFullyQualifiedDisplayString, bool ImplementsHandlerInterface, bool HasPublicParameterlessConstructor, - Location Location); + LocationInfo Location); /// /// Generates {Event}EventHandlerRegistry static classes for @@ -122,7 +122,7 @@ private static List Transform( : eventType.ContainingNamespace.ToDisplayString(), eventType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)); - var location = context.TargetNode.GetLocation(); + var location = new LocationInfo(context.TargetNode.GetLocation()); result.Add(new EventHandlerRegistration( eventInfo, handler.Name, @@ -152,7 +152,7 @@ private static void Execute( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.RegisterEventHandlerContractMismatch, - registration.Location, + registration.Location.ToLocation(), registration.HandlerName, registration.Event.FullyQualifiedName)); } @@ -169,7 +169,7 @@ private static void Execute( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.RegisterEventHandlerDuplicateOnSameClass, - registration.Location, + registration.Location.ToLocation(), registration.HandlerName, registration.Event.FullyQualifiedName)); } diff --git a/DesignPatterns.SourceGenerators/Generators/RegistrationGeneratorHelper.cs b/DesignPatterns.SourceGenerators/Generators/RegistrationGeneratorHelper.cs index 86672d8..27ad369 100644 --- a/DesignPatterns.SourceGenerators/Generators/RegistrationGeneratorHelper.cs +++ b/DesignPatterns.SourceGenerators/Generators/RegistrationGeneratorHelper.cs @@ -33,11 +33,11 @@ internal sealed record KeyedRegistration( bool IsAsync, bool ImplementsAsyncFactory, int PoolSize, - Location Location); + LocationInfo Location); internal static class RegistrationGeneratorHelper { - internal static List Transform( + internal static EquatableArray Transform( GeneratorAttributeSyntaxContext context, bool isGenericAttribute) { @@ -45,7 +45,7 @@ internal static List Transform( if (context.TargetSymbol is not INamedTypeSymbol implementation) { - return result; + return new EquatableArray(result.ToArray()); } foreach (var attribute in context.Attributes) @@ -85,7 +85,7 @@ internal static List Transform( : contract.ContainingNamespace.ToDisplayString(), contract.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)); - var location = context.TargetNode.GetLocation(); + var location = new LocationInfo(context.TargetNode.GetLocation()); // Resolve optional Guard property from the attribute. string? guardName = null; @@ -134,7 +134,7 @@ internal static List Transform( location)); } - return result; + return new EquatableArray(result.ToArray()); } internal static void Execute( @@ -210,7 +210,7 @@ private static void ReportDuplicateKeys( { context.ReportDiagnostic(Diagnostic.Create( descriptor, - registration.Location, + registration.Location.ToLocation(), registration.Key, contractName)); } @@ -226,7 +226,7 @@ private static void ReportContractMismatches( { context.ReportDiagnostic(Diagnostic.Create( descriptor, - registration.Location, + registration.Location.ToLocation(), registration.ImplementationName, registration.Contract.FullyQualifiedName)); } @@ -241,7 +241,7 @@ private static void ReportMissingConstructors( { context.ReportDiagnostic(Diagnostic.Create( descriptor, - registration.Location, + registration.Location.ToLocation(), registration.ImplementationName)); } } @@ -264,7 +264,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StrategyGuardMethodNotFound, - registration.Location, + registration.Location.ToLocation(), guard.Name, registration.ImplementationName, keyTypeDisplay)); @@ -275,7 +275,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StrategyGuardMethodNotStatic, - registration.Location, + registration.Location.ToLocation(), guard.Name, registration.ImplementationName)); continue; @@ -285,7 +285,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StrategyGuardMethodWrongSignature, - registration.Location, + registration.Location.ToLocation(), guard.Name, registration.ImplementationName, keyTypeDisplay)); @@ -306,7 +306,7 @@ private static void ReportFactoryAsyncDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.FactoryAsyncSignatureMismatch, - registration.Location, + registration.Location.ToLocation(), registration.ImplementationName, registration.Contract.FullyQualifiedName)); } @@ -316,7 +316,7 @@ private static void ReportFactoryAsyncDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.FactoryPoolSizeInvalid, - registration.Location, + registration.Location.ToLocation(), registration.ImplementationName, registration.PoolSize)); } @@ -326,7 +326,7 @@ private static void ReportFactoryAsyncDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.FactoryPoolSizeTooLarge, - registration.Location, + registration.Location.ToLocation(), registration.ImplementationName, registration.PoolSize)); } diff --git a/DesignPatterns.SourceGenerators/Generators/Result.cs b/DesignPatterns.SourceGenerators/Generators/Result.cs index 8ee7564..f164528 100644 --- a/DesignPatterns.SourceGenerators/Generators/Result.cs +++ b/DesignPatterns.SourceGenerators/Generators/Result.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Runtime.CompilerServices; using Microsoft.CodeAnalysis; namespace DesignPatterns.SourceGenerators.Generators; @@ -17,7 +16,7 @@ namespace DesignPatterns.SourceGenerators.Generators; public DiagnosticDescriptor Descriptor { get; } /// Location where the diagnostic should be reported (may be null). - public Location? Location { get; } + public LocationInfo? Location { get; } /// Message arguments passed to Diagnostic.Create. public object?[] MessageArgs { get; } @@ -26,7 +25,7 @@ namespace DesignPatterns.SourceGenerators.Generators; /// Creates a from a descriptor, optional /// location, and message arguments. /// - public DiagnosticInfo(DiagnosticDescriptor descriptor, Location? location, params object?[] messageArgs) + public DiagnosticInfo(DiagnosticDescriptor descriptor, LocationInfo? location, params object?[] messageArgs) { Descriptor = descriptor; Location = location; @@ -34,12 +33,12 @@ public DiagnosticInfo(DiagnosticDescriptor descriptor, Location? location, param } /// Reconstructs the for reporting. - public Diagnostic ToDiagnostic() => Diagnostic.Create(Descriptor, Location, MessageArgs)!; + public Diagnostic ToDiagnostic() => Diagnostic.Create(Descriptor, Location?.ToLocation(), MessageArgs)!; /// public bool Equals(DiagnosticInfo other) => string.Equals(Descriptor.Id, other.Descriptor.Id, StringComparison.Ordinal) - && ReferenceEquals(Location, other.Location) + && Location == other.Location && MessageArgsEqual(other.MessageArgs); /// @@ -49,7 +48,7 @@ public bool Equals(DiagnosticInfo other) => public override int GetHashCode() { var hash = Descriptor.Id?.GetHashCode() ?? 0; - hash = (hash * 31) + (Location is null ? 0 : RuntimeHelpers.GetHashCode(Location)); + hash = (hash * 31) + (Location?.GetHashCode() ?? 0); foreach (var messageArg in MessageArgs ?? Array.Empty()) { hash = (hash * 31) + EqualityComparer.Default.GetHashCode(messageArg); diff --git a/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionModels.cs b/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionModels.cs index 406368e..468823d 100644 --- a/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionModels.cs +++ b/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionModels.cs @@ -115,7 +115,7 @@ internal sealed record TransitionModel( TransitionArg From, TransitionArg Trigger, TransitionArg To, - Location Location, + LocationInfo Location, GuardResolution Guard, ActionResolution OnEnter, ActionResolution OnExit); @@ -127,7 +127,7 @@ internal sealed record TransitionModel( internal sealed record StateParentModel( TransitionArg Child, TransitionArg Parent, - Location Location); + LocationInfo Location); /// /// The full state machine model extracted from a class marked with @@ -141,7 +141,7 @@ internal sealed record StateMachineModel( EquatableArray Transitions, EquatableArray StateParents, bool IsHierarchical, - Location Location, + LocationInfo Location, bool IsValidHolder); /// @@ -213,7 +213,7 @@ public ResolvedTransition( string fromMember, string triggerMember, string toMember, - Location location, + LocationInfo location, string fromExpression, string triggerExpression, string toExpression, @@ -243,7 +243,7 @@ public ResolvedTransition( public string ToMember { get; } - public Location Location { get; } + public LocationInfo Location { get; } public string FromExpression { get; } diff --git a/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionTransform.cs b/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionTransform.cs index 9ff45aa..5fa0fa6 100644 --- a/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionTransform.cs +++ b/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionTransform.cs @@ -39,7 +39,7 @@ attribute.AttributeClass is not null && return Result.Empty; } - var location = context.TargetNode.GetLocation(); + var location = new LocationInfo(context.TargetNode.GetLocation()); var classDeclaration = (ClassDeclarationSyntax)context.TargetNode; var isValidHolder = classDeclaration.Modifiers.Any(SyntaxKind.StaticKeyword) && classDeclaration.Modifiers.Any(SyntaxKind.PartialKeyword); @@ -105,7 +105,7 @@ attribute.AttributeClass is not null && } var transitionLocation = attribute.ApplicationSyntaxReference is { } syntaxReference - ? syntaxReference.SyntaxTree!.GetLocation(syntaxReference.Span) + ? new LocationInfo(syntaxReference.SyntaxTree!.GetLocation(syntaxReference.Span)) : location; var from = ResolveTransitionArg(attribute.ConstructorArguments[0], stateType, stateTypeInfo); @@ -167,7 +167,7 @@ attribute.AttributeClass is not null && } var parentLocation = attribute.ApplicationSyntaxReference is { } syntaxRef - ? syntaxRef.SyntaxTree!.GetLocation(syntaxRef.Span) + ? new LocationInfo(syntaxRef.SyntaxTree!.GetLocation(syntaxRef.Span)) : location; var child = ResolveTransitionArg(attribute.ConstructorArguments[0], stateType, stateTypeInfo); diff --git a/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionValidator.cs b/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionValidator.cs index e4fb43a..053df4e 100644 --- a/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionValidator.cs +++ b/DesignPatterns.SourceGenerators/Generators/StateTransition/StateTransitionValidator.cs @@ -30,7 +30,7 @@ internal static class StateTransitionValidator { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateMachineHolderInvalid, - model.Location, + model.Location.ToLocation(), model.Holder.Name)); return null; } @@ -47,7 +47,7 @@ internal static class StateTransitionValidator { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionInvalidInitialState, - model.Location, + model.Location.ToLocation(), model.Initial.DisplayValue, stateType.FullyQualifiedName)); return null; @@ -116,7 +116,7 @@ private static List ResolveValidTransitions( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionInvalidStateMember, - transition.Location, + transition.Location.ToLocation(), transition.From.DisplayValue, stateType.FullyQualifiedName)); continue; @@ -126,7 +126,7 @@ private static List ResolveValidTransitions( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionInvalidStateMember, - transition.Location, + transition.Location.ToLocation(), transition.To.DisplayValue, stateType.FullyQualifiedName)); continue; @@ -136,7 +136,7 @@ private static List ResolveValidTransitions( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionInvalidTriggerMember, - transition.Location, + transition.Location.ToLocation(), transition.Trigger.DisplayValue, triggerType.FullyQualifiedName)); continue; @@ -204,7 +204,7 @@ private static void ReportDuplicateEdges( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionDuplicateEdge, - transition.Location, + transition.Location.ToLocation(), transition.FromMember, transition.TriggerMember)); } @@ -216,7 +216,7 @@ private static void ReportIsolatedStates( EnumTypeInfo stateType, string initialMember, IReadOnlyCollection transitions, - Location location) + LocationInfo location) { var fromStates = new HashSet( transitions.Select(static transition => transition.FromMember), @@ -233,7 +233,7 @@ private static void ReportIsolatedStates( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionIsolatedState, - location, + location.ToLocation(), member.Name, stateType.FullyQualifiedName)); } @@ -253,7 +253,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionGuardMethodNotFound, - transition.Location, + transition.Location.ToLocation(), guard.Name, holderName, stateType.FullyQualifiedName, @@ -265,7 +265,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionGuardMethodNotStatic, - transition.Location, + transition.Location.ToLocation(), guard.Name, holderName)); return; @@ -275,7 +275,7 @@ private static void ReportGuardDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionGuardMethodWrongSignature, - transition.Location, + transition.Location.ToLocation(), guard.Name, holderName, stateType.FullyQualifiedName, @@ -285,7 +285,7 @@ private static void ReportGuardDiagnostics( private static void ReportActionDiagnostics( SourceProductionContext context, - Location location, + LocationInfo location, ActionResolution action, string holderName, EnumTypeInfo stateType, @@ -295,7 +295,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionActionMethodNotFound, - location, + location.ToLocation(), action.Name, holderName, stateType.FullyQualifiedName, @@ -307,7 +307,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionActionMethodNotStatic, - location, + location.ToLocation(), action.Name, holderName)); return; @@ -317,7 +317,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateTransitionActionMethodWrongSignature, - location, + location.ToLocation(), action.Name, holderName, stateType.FullyQualifiedName, @@ -352,7 +352,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateParentInvalidMember, - sp.Location, + sp.Location.ToLocation(), sp.Child.DisplayValue, stateType.FullyQualifiedName)); hasFatalError = true; @@ -363,7 +363,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateParentInvalidMember, - sp.Location, + sp.Location.ToLocation(), sp.Parent.DisplayValue, stateType.FullyQualifiedName)); hasFatalError = true; @@ -378,7 +378,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateParentSelfReference, - sp.Location, + sp.Location.ToLocation(), childName)); hasFatalError = true; continue; @@ -390,7 +390,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateParentInvalidMember, - sp.Location, + sp.Location.ToLocation(), $"{childName} already has parent '{existingParent}'", stateType.FullyQualifiedName)); hasFatalError = true; @@ -408,7 +408,7 @@ private static void ReportActionDiagnostics( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateHierarchyCycle, - model.Location, + model.Location.ToLocation(), cycle)); hasFatalError = true; } @@ -470,7 +470,7 @@ private static void ReportOrphanParents( EnumTypeInfo stateType, Dictionary parentMap, IReadOnlyCollection transitions, - Location location) + LocationInfo location) { var parentStates = new HashSet( parentMap.Values, @@ -486,7 +486,7 @@ private static void ReportOrphanParents( { context.ReportDiagnostic(Diagnostic.Create( DesignPatternsDiagnosticDescriptors.StateParentOrphanParent, - location, + location.ToLocation(), parentState)); } } diff --git a/tests/DesignPatterns.SourceGenerators.Tests/Generators/ActionChainComposerTests.cs b/tests/DesignPatterns.SourceGenerators.Tests/Generators/ActionChainComposerTests.cs index aea576b..4f8411a 100644 --- a/tests/DesignPatterns.SourceGenerators.Tests/Generators/ActionChainComposerTests.cs +++ b/tests/DesignPatterns.SourceGenerators.Tests/Generators/ActionChainComposerTests.cs @@ -210,12 +210,12 @@ public void BuildStateActionMap_CollectsExitActionsFromFromState() { var transitions = new List { - new("Draft", "Submit", "Submitted", location: null!, + new("Draft", "Submit", "Submitted", location: default, "S.Draft", "T.Submit", "S.Submitted", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitDraft", onEnterAsyncReference: null, onExitAsyncReference: null), - new("Submitted", "Pay", "Paid", location: null!, + new("Submitted", "Pay", "Paid", location: default, "S.Submitted", "T.Pay", "S.Paid", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitSubmitted", @@ -233,7 +233,7 @@ public void BuildStateActionMap_CollectsEnterActionsFromToState() { var transitions = new List { - new("Draft", "Submit", "Submitted", location: null!, + new("Draft", "Submit", "Submitted", location: default, "S.Draft", "T.Submit", "S.Submitted", guardMethodReference: null, onEnterSyncReference: "Holder.OnEnterSubmitted", onExitSyncReference: null, @@ -250,12 +250,12 @@ public void BuildStateActionMap_FirstWins_WhenMultipleExitActionsForSameState() { var transitions = new List { - new("Draft", "Submit", "Submitted", location: null!, + new("Draft", "Submit", "Submitted", location: default, "S.Draft", "T.Submit", "S.Submitted", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitDraft1", onEnterAsyncReference: null, onExitAsyncReference: null), - new("Draft", "Cancel", "Cancelled", location: null!, + new("Draft", "Cancel", "Cancelled", location: default, "S.Draft", "T.Cancel", "S.Cancelled", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitDraft2", @@ -281,18 +281,18 @@ public void Compose_CreatesCompositeDelegate_WhenChainHasMultipleActions() var transitions = new List { - new("Submitted", "Pay", "Paid", location: null!, + new("Submitted", "Pay", "Paid", location: default, "S.Submitted", "T.Pay", "S.Paid", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitSubmitted", onEnterAsyncReference: null, onExitAsyncReference: null), - new("Active", "Cancel", "Cancelled", location: null!, + new("Active", "Cancel", "Cancelled", location: default, "S.Active", "T.Cancel", "S.Cancelled", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitActive", onEnterAsyncReference: null, onExitAsyncReference: null), // Flattened inherited edge - new("Submitted", "Cancel", "Cancelled", location: null!, + new("Submitted", "Cancel", "Cancelled", location: default, "S.Submitted", "T.Cancel", "S.Cancelled", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitActive", @@ -327,7 +327,7 @@ public void Compose_NoOverride_WhenChainHasSingleAction() var transitions = new List { - new("Active", "Cancel", "Cancelled", location: null!, + new("Active", "Cancel", "Cancelled", location: default, "S.Active", "T.Cancel", "S.Cancelled", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: "Holder.OnExitActive", @@ -352,7 +352,7 @@ public void Compose_NoOverride_WhenChainIsEmpty() var transitions = new List { - new("Draft", "Submit", "Submitted", location: null!, + new("Draft", "Submit", "Submitted", location: default, "S.Draft", "T.Submit", "S.Submitted", guardMethodReference: null, onEnterSyncReference: null, onExitSyncReference: null, diff --git a/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs b/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs index 1a03b5e..cfab1ec 100644 --- a/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs +++ b/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs @@ -1,15 +1,41 @@ using DesignPatterns.SourceGenerators.Generators; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; namespace DesignPatterns.SourceGenerators.Tests.Generators; /// /// Verifies that incremental generator pipeline stages return Cached or Unchanged -/// on the second run when the input compilation is identical. This validates the -/// P0 refactoring (value-equatable models) and P2 (WithTrackingName instrumentation). +/// on the second run when the input compilation is cloned. This validates the +/// P0 refactoring (value-equatable models) and P1-#5 (LocationInfo value equality). /// public sealed class IncrementalCacheTests { + [Fact] + public void LocationInfo_Equals_AfterClone() + { + var parseOptions = new CSharpParseOptions(LanguageVersion.Latest); + var tree = CSharpSyntaxTree.ParseText("class A {}", parseOptions); + var compilation = CSharpCompilation.Create( + "Test", + new[] { tree }, + new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) }); + + var cloned = compilation.Clone(); + var clonedTree = cloned.SyntaxTrees.First(); + + var origNode = tree.GetRoot().DescendantNodes().First(); + var clonedNode = clonedTree.GetRoot().DescendantNodes().First(); + + var info1 = new LocationInfo(origNode.GetLocation()); + var info2 = new LocationInfo(clonedNode.GetLocation()); + + Assert.Equal(info1, info2); + Assert.Equal(info1.GetHashCode(), info2.GetHashCode()); + } + [Fact] public void GenerateSingletonCachesOnSecondRun() { diff --git a/tests/DesignPatterns.SourceGenerators.Tests/SourceGeneratorTestContext.cs b/tests/DesignPatterns.SourceGenerators.Tests/SourceGeneratorTestContext.cs index c6a433a..28a8296 100644 --- a/tests/DesignPatterns.SourceGenerators.Tests/SourceGeneratorTestContext.cs +++ b/tests/DesignPatterns.SourceGenerators.Tests/SourceGeneratorTestContext.cs @@ -250,14 +250,15 @@ internal static void AssertCacheHit(params (string Path, string Sour // First run — populates the cache. driver = driver.RunGeneratorsAndUpdateCompilation(compilation, out _, out _); - // Second run — same compilation; our tagged stages should be cached. - // We use the same compilation (not Clone) because Location uses reference - // equality: Clone forces the transform to re-execute, producing new - // Location objects that break model value equality even though the - // semantic content is identical. Using the same compilation verifies - // that the transform outputs are cached when inputs are unchanged, - // which is the guarantee the P0 refactoring restored. - var secondResult = driver.RunGenerators(compilation).GetRunResult(); + // Second run — cloned compilation; our tagged stages should be cached. + // We use Clone (not the same compilation) to verify that LocationInfo + // value equality holds across cloned compilations. Previously, Location + // used reference equality, so Clone forced the transform to re-execute + // and broke model value equality. With LocationInfo (value-equatable), + // the transform outputs are cached even when Clone creates new syntax + // trees and Location objects. + var clonedCompilation = compilation.Clone(); + var secondResult = driver.RunGenerators(clonedCompilation).GetRunResult(); // Collect all tracking names defined in TrackingNames so we only check // our own stages, not Roslyn-internal ones (Compilation, etc.). From f5250e581aaa09e4af01256f33a332477cd0a501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=90=BD=E7=AC=94?= <46271592+Skymly@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:28:28 +0800 Subject: [PATCH 2/2] Fix review issues: ordinal GetHashCode, ToLocation round-trip tests - LocationInfo.GetHashCode now uses StringComparer.Ordinal for FilePath, matching Equals which uses StringComparison.Ordinal. The previous FilePath?.GetHashCode() could return culture-sensitive hashes on netstandard2.0, violating the hash/equals contract. - Cache GetLineSpan() result in constructor (was called twice). - Add LocationInfo_ToLocation_RoundTripsCoordinates test verifying FilePath, SourceSpan, and LineSpan survive the ToLocation round-trip. - Add LocationInfo_NullLocation_ToLocationReturnsNull test for the null-location edge case. --- .../Generators/LocationInfo.cs | 9 ++-- .../Generators/IncrementalCacheTests.cs | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs b/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs index da2bb49..32180e4 100644 --- a/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs +++ b/DesignPatterns.SourceGenerators/Generators/LocationInfo.cs @@ -27,9 +27,10 @@ public LocationInfo(Location? location) return; } - FilePath = location.SourceTree?.FilePath ?? location.GetLineSpan().Path; + var lineSpan = location.GetLineSpan(); + FilePath = location.SourceTree?.FilePath ?? lineSpan.Path; TextSpan = location.SourceSpan; - LineSpan = location.GetLineSpan().Span; + LineSpan = lineSpan.Span; } /// @@ -61,7 +62,9 @@ public override int GetHashCode() { unchecked { - return ((FilePath?.GetHashCode() ?? 0) * 397) ^ TextSpan.GetHashCode() ^ LineSpan.GetHashCode(); + return (StringComparer.Ordinal.GetHashCode(FilePath ?? string.Empty) * 397) + ^ TextSpan.GetHashCode() + ^ LineSpan.GetHashCode(); } } diff --git a/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs b/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs index cfab1ec..9ce6d92 100644 --- a/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs +++ b/tests/DesignPatterns.SourceGenerators.Tests/Generators/IncrementalCacheTests.cs @@ -36,6 +36,54 @@ public void LocationInfo_Equals_AfterClone() Assert.Equal(info1.GetHashCode(), info2.GetHashCode()); } + /// + /// Verifies that reconstructs a + /// with the correct file path, source span, and + /// line span. The reconstructed location is an ExternalFileLocation (not + /// syntax-tree-backed), which is sufficient for Diagnostic.Create. + /// + [Fact] + public void LocationInfo_ToLocation_RoundTripsCoordinates() + { + var parseOptions = new CSharpParseOptions(LanguageVersion.Latest); + // Use a multi-line source so the line span is non-trivial. + var tree = CSharpSyntaxTree.ParseText( + """ + class A + { + void M() { } + } + """, + parseOptions, + path: "Test.cs"); + + var methodNode = tree.GetRoot().DescendantNodes().First(n => n is MethodDeclarationSyntax); + var originalLocation = methodNode.GetLocation(); + var info = new LocationInfo(originalLocation); + + var reconstructed = info.ToLocation(); + + Assert.NotNull(reconstructed); + Assert.Equal("Test.cs", reconstructed!.SourceTree?.FilePath ?? reconstructed.GetLineSpan().Path); + Assert.Equal(originalLocation.SourceSpan, reconstructed.SourceSpan); + + var expectedLineSpan = originalLocation.GetLineSpan().Span; + var actualLineSpan = reconstructed.GetLineSpan().Span; + Assert.Equal(expectedLineSpan, actualLineSpan); + } + + /// + /// Verifies that a null Location produces a default LocationInfo whose + /// ToLocation returns null (no crash). + /// + [Fact] + public void LocationInfo_NullLocation_ToLocationReturnsNull() + { + var info = new LocationInfo(null); + Assert.Null(info.ToLocation()); + Assert.Null(info.FilePath); + } + [Fact] public void GenerateSingletonCachesOnSecondRun() {