Merge pull request #159 from postsharp/topic/2024.2/35356-observable

gfraiteur · web-flow · commit 8780c5176ca0 · 2024-08-27T08:52:45.000+02:00
Topic/2024.2/35356 observable
diff --git a/.idea/.idea.Metalama.Patterns/.idea/projectSettingsUpdater.xml b/.idea/.idea.Metalama.Patterns/.idea/projectSettingsUpdater.xml
diff --git a/.teamcity/settings.kts b/.teamcity/settings.kts
@@ -27,7 +27,7 @@ object DebugBuild : BuildType({
 
     name = "Build [Debug]"
 
-    artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n"
+    artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n+:artifacts/logs/**/*=>logs\n"
 
     params {
         text("BuildArguments", "", label = "Build Arguments", description = "Arguments to append to the 'Build' build step.", allowEmpty = true)
@@ -157,7 +157,7 @@ object ReleaseBuild : BuildType({
 
     name = "Build [Release]"
 
-    artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n"
+    artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n+:artifacts/logs/**/*=>logs\n"
 
     params {
         text("BuildArguments", "", label = "Build Arguments", description = "Arguments to append to the 'Build' build step.", allowEmpty = true)
@@ -278,7 +278,7 @@ object PublicBuild : BuildType({
 
     name = "Build [Public]"
 
-    artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n"
+    artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n+:artifacts/logs/**/*=>logs\n"
 
     params {
         text("BuildArguments", "", label = "Build Arguments", description = "Arguments to append to the 'Build' build step.", allowEmpty = true)
@@ -511,7 +511,7 @@ object PublicDeployment : BuildType({
 
             artifacts {
                 cleanDestination = true
-                artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults"
+                artifactRules = "+:artifacts/publish/public/**/*=>artifacts/publish/public\n+:artifacts/publish/private/**/*=>artifacts/publish/private\n+:artifacts/testResults/**/*=>artifacts/testResults\n+:artifacts/logs/**/*=>logs"
             }
         }
 
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -5,7 +5,8 @@
   </PropertyGroup>
   <!-- Set versions of dependencies that need special treatment or consist of multiple packages of the same version. -->
   <PropertyGroup>
-    <PostSharpEngineeringVersion Condition="'$(PostSharpEngineeringVersion)'==''">2023.2.111</PostSharpEngineeringVersion>
+    <PostSharpEngineeringVersion Condition="'$(PostSharpEngineeringVersion)'==''">2023.2.117</PostSharpEngineeringVersion>
+    <StyleCopVersion>1.2.0-beta.556</StyleCopVersion>
   </PropertyGroup>
   <ItemGroup>
     <PackageVersion Include="Azure.Identity" Version="1.11.4" />
@@ -32,6 +33,6 @@
     <PackageVersion Include="System.Threading.Tasks.Extensions" Version="4.5.4" />
     <PackageVersion Include="System.Threading.Thread" Version="4.3.0" />
     <PackageVersion Include="System.Threading.ThreadPool" Version="4.3.0" />
-    <PackageVersion Include="CommunityToolkit.Mvvm"  Version="8.1.0"/>
+    <PackageVersion Include="CommunityToolkit.Mvvm" Version="8.1.0" />
   </ItemGroup>
 </Project>
diff --git a/global.json b/global.json
@@ -4,6 +4,6 @@
     "rollForward": "patch"
   },
   "msbuild-sdks": {
-    "PostSharp.Engineering.Sdk": "2023.2.111"
+    "PostSharp.Engineering.Sdk": "2023.2.117"
   }
 }
diff --git a/src/Metalama.Patterns.Caching.Aspects/InvalidateCacheAttribute.cs b/src/Metalama.Patterns.Caching.Aspects/InvalidateCacheAttribute.cs
@@ -391,7 +391,7 @@ private static bool ValidateAndFindInvalidatedMethods(
 
                 // Match parameter by name.
                 var invalidatingMethodParameter =
-                    invalidatingMethodParameters.FirstOrDefault( p => p.Name == invalidatedMethodParameter.Name );
+                    invalidatingMethodParameters.OfName( invalidatedMethodParameter.Name );
 
                 if ( invalidatingMethodParameter == null )
                 {
diff --git a/src/Metalama.Patterns.Contracts/NotEmptyAttribute.cs b/src/Metalama.Patterns.Contracts/NotEmptyAttribute.cs
@@ -136,7 +136,7 @@ private static bool TryGetCompatibleTargetInterface(
 
         if ( targetType.Is( typeOfICollection ) )
         {
-            var countProperty = typeOfICollection.Properties.Single( p => p.Name == nameof(ICollection.Count) );
+            var countProperty = typeOfICollection.Properties.OfName( nameof(ICollection.Count) ).Single();
             targetType.TryFindImplementationForInterfaceMember( countProperty, out var countPropertyImpl );
 
             interfaceType = typeOfICollection;
@@ -169,7 +169,7 @@ private static bool TryGetCompatibleTargetInterface(
 
             if ( foundInterface != null )
             {
-                var countProperty = foundInterface.Properties.Single( p => p.Name == "Count" );
+                var countProperty = foundInterface.Properties.OfName( "Count" ).Single();
                 targetType.TryFindImplementationForInterfaceMember( countProperty, out var countPropertyImpl );
 
                 interfaceType = foundInterface;
diff --git a/src/Metalama.Patterns.Observability/Implementation/Assets.cs b/src/Metalama.Patterns.Observability/Implementation/Assets.cs
@@ -42,5 +42,5 @@ public Assets()
     /// Gets the <see cref="IProperty"/> for property <c>EqualityComparer<paramref name="type"/>>.Default</c>.
     /// </summary>
     public IProperty GetDefaultEqualityComparerForType( IType type )
-        => this.EqualityComparerOfT.WithTypeArguments( type ).Properties.Single( p => p.Name == "Default" );
+        => this.EqualityComparerOfT.WithTypeArguments( type ).Properties.OfName( "Default" ).Single();
 }
diff --git a/src/Metalama.Patterns.Observability/Implementation/ClassicStrategy/ClassicDependencyGraphBuilder.cs b/src/Metalama.Patterns.Observability/Implementation/ClassicStrategy/ClassicDependencyGraphBuilder.cs
@@ -13,7 +13,7 @@ internal sealed class ClassicDependencyGraphBuilder : DependencyGraphBuilder
 
     public INamedType CurrentType { get; }
 
-    public ClassicDependencyGraphBuilder( ClassicGraphBuildingContext context, INamedType currentType )
+    public ClassicDependencyGraphBuilder( ClassicGraphBuildingContext context, INamedType currentType ) : base( context.Assets )
     {
         this.Context = context;
         this.CurrentType = currentType;
diff --git a/src/Metalama.Patterns.Observability/Implementation/ClassicStrategy/ClassicGraphBuildingContext.cs b/src/Metalama.Patterns.Observability/Implementation/ClassicStrategy/ClassicGraphBuildingContext.cs
@@ -15,7 +15,7 @@ internal sealed class ClassicGraphBuildingContext : GraphBuildingContext
 {
     private readonly ClassicObservabilityStrategyImpl _strategy;
 
-    public ClassicGraphBuildingContext( ClassicObservabilityStrategyImpl strategy ) : base( strategy.CurrentType.Compilation )
+    public ClassicGraphBuildingContext( ClassicObservabilityStrategyImpl strategy ) : base( strategy.CurrentType.Compilation, strategy.Assets )
     {
         this._strategy = strategy;
     }
diff --git a/src/Metalama.Patterns.Observability/Implementation/ClassicStrategy/ClassicObservabilityStrategyImpl.cs b/src/Metalama.Patterns.Observability/Implementation/ClassicStrategy/ClassicObservabilityStrategyImpl.cs
@@ -27,9 +27,10 @@ internal sealed class ClassicObservabilityStrategyImpl : IObservabilityStrategy
     private readonly IMethod? _baseOnPropertyChangedOverridableMethod;
     private readonly IMethod? _baseOnChildPropertyChangedMethod;
     private readonly IMethod? _baseOnObservablePropertyChangedMethod;
-    private readonly Assets _assets;
     private readonly bool _targetImplementsInpc;
 
+    public Assets Assets { get; }
+
     // Useful to see when debugging:
     // ReSharper disable once PrivateFieldCanBeConvertedToLocalVariable
     private readonly bool _baseImplementsInpc;
@@ -52,23 +53,23 @@ public ClassicObservabilityStrategyImpl( IAspectBuilder<INamedType> aspectBuilde
         var target = aspectBuilder.Target;
 
         this.AspectBuilder = aspectBuilder;
-        this._assets = target.Compilation.Cache.GetOrAdd( _ => new Assets() );
-        this.InpcInstrumentationKindLookup = new InpcInstrumentationKindLookup( this.CurrentType, this._assets );
+        this.Assets = target.Compilation.Cache.GetOrAdd( _ => new Assets() );
+        this.InpcInstrumentationKindLookup = new InpcInstrumentationKindLookup( this.CurrentType, this.Assets );
         this._commonOptions = this.CurrentType.Enhancements().GetOptions<ObservabilityOptions>();
         this._classicOptions = this.CurrentType.Enhancements().GetOptions<ClassicObservabilityStrategyOptions>();
 
         // TODO: Consider using BaseType.Definition where possible for better performance.
 
         this._baseImplementsInpc =
             target.BaseType != null && (
-                target.BaseType.Is( this._assets.INotifyPropertyChanged )
+                target.BaseType.Is( this.Assets.INotifyPropertyChanged )
                 || (target.BaseType is { BelongsToCurrentProject: true }
                     && target.BaseType.Definition.Enhancements().HasAspect( typeof(ObservableAttribute) )));
 
-        this._targetImplementsInpc = this._baseImplementsInpc || target.Is( this._assets.INotifyPropertyChanged );
+        this._targetImplementsInpc = this._baseImplementsInpc || target.Is( this.Assets.INotifyPropertyChanged );
         (this._baseOnPropertyChangedInvocableMethod, this._baseOnPropertyChangedOverridableMethod) = GetOnPropertyChangedMethods( target );
         this._baseOnChildPropertyChangedMethod = GetOnChildPropertyChangedMethod( target );
-        this._baseOnObservablePropertyChangedMethod = GetOnObservablePropertyChangedMethod( target, this._assets );
+        this._baseOnObservablePropertyChangedMethod = GetOnObservablePropertyChangedMethod( target, this.Assets );
 
         var useOnObservablePropertyChangedMethod =
             this._classicOptions.EnableOnObservablePropertyChangedMethod == true &&
@@ -128,7 +129,7 @@ public void BuildAspect( IAspectBuilder<INamedType> builder )
             this._commonOptions,
             this._classicOptions,
             this.CurrentType,
-            this._assets,
+            this.Assets,
             this.InpcInstrumentationKindLookup,
             this.ObservableTypeInfo,
             this._onObservablePropertyChangedMethod?.Value,
@@ -154,7 +155,7 @@ private bool ValidateRootAutoProperties()
                 .Where(
                     p =>
                         p is { IsStatic: false, IsAutoPropertyOrField: true }
-                        && !p.Attributes.Any( this._assets.NotObservableAttribute ) );
+                        && !p.Attributes.Any( this.Assets.NotObservableAttribute ) );
 
         var allValid = true;
 
@@ -316,7 +317,7 @@ private bool TryIntroduceOnChildPropertyChangedMethod()
                     {
                         b.AddAttribute(
                             AttributeConstruction.Create(
-                                this._assets.InvokedForAttribute,
+                                this.Assets.InvokedForAttribute,
                                 this._propertyPathsForOnChildPropertyChangedMethod.OrderBy( s => s ).ToArray() ) );
                     }
 
@@ -383,7 +384,7 @@ private bool TryIntroduceOnObservablePropertyChanged()
                     {
                         b.AddAttribute(
                             AttributeConstruction.Create(
-                                this._assets.InvokedForAttribute,
+                                this.Assets.InvokedForAttribute,
                                 this._propertyNamesForOnObservablePropertyChangedMethod.OrderBy( s => s ).ToArray() ) );
                     }
 
@@ -447,7 +448,7 @@ private void IntroduceInterfaceIfRequired()
         if ( !this._targetImplementsInpc )
         {
             this.AspectBuilder.Advice.WithTemplateProvider( Templates.Provider )
-                .ImplementInterface( this.CurrentType, this._assets.INotifyPropertyChanged );
+                .ImplementInterface( this.CurrentType, this.Assets.INotifyPropertyChanged );
         }
     }
 
@@ -539,7 +540,7 @@ private void ProcessAutoPropertiesAndReferencedFields()
                     target.Fields
                         .Where( f => f is { IsStatic: false, IsImplicitlyDeclared: false } )
                         .Select( p => (ClassicObservablePropertyInfo) this.ObservableTypeInfo.GetOrAddProperty( p ) ) )
-                .Where( node => !node.FieldOrProperty.Attributes.Any( this._assets.NotObservableAttribute ) )
+                .Where( node => !node.FieldOrProperty.Attributes.Any( this.Assets.NotObservableAttribute ) )
                 .ToList();
 
         foreach ( var propertyInfo in properties )
@@ -645,7 +646,7 @@ private void ProcessAutoPropertiesAndReferencedFields()
     public bool HasInheritedOnChildPropertyChangedPropertyPath( string parentPropertyPath )
     {
         this._inheritedOnChildPropertyChangedPropertyPaths ??=
-            BuildPropertyPathLookup( GetPropertyPaths( this._assets.InvokedForAttribute, this._baseOnChildPropertyChangedMethod ) );
+            BuildPropertyPathLookup( GetPropertyPaths( this.Assets.InvokedForAttribute, this._baseOnChildPropertyChangedMethod ) );
 
         return this._inheritedOnChildPropertyChangedPropertyPaths.Contains( parentPropertyPath );
     }
@@ -657,7 +658,7 @@ public bool HasInheritedOnObservablePropertyChangedProperty( string propertyName
         this._inheritedOnObservablePropertyChangedPropertyNames ??=
             BuildPropertyPathLookup(
                 GetPropertyPaths(
-                    this._assets.InvokedForAttribute,
+                    this.Assets.InvokedForAttribute,
                     this._baseOnObservablePropertyChangedMethod ) );
 
         return this._inheritedOnObservablePropertyChangedPropertyNames.Contains( propertyName );
@@ -721,7 +722,7 @@ private IField GetOrCreateHandlerField( ClassicObservableExpression node )
                 .IntroduceField(
                     this.CurrentType,
                     handlerFieldName,
-                    this._assets.NullablePropertyChangedEventHandler,
+                    this.Assets.NullablePropertyChangedEventHandler,
                     IntroductionScope.Instance,
                     OverrideStrategy.Fail,
                     b => b.Accessibility = Accessibility.Private );
diff --git a/src/Metalama.Patterns.Observability/Implementation/DependencyAnalysis/DependencyGraphBuilder.cs b/src/Metalama.Patterns.Observability/Implementation/DependencyAnalysis/DependencyGraphBuilder.cs
@@ -14,7 +14,14 @@ namespace Metalama.Patterns.Observability.Implementation.DependencyAnalysis;
 internal partial class DependencyGraphBuilder
 {
     private readonly Dictionary<INamedType, ObservableTypeInfo> _types = new();
-    
+
+    private readonly Assets _assets;
+
+    public DependencyGraphBuilder( Assets assets )
+    {
+        this._assets = assets;
+    }
+
     protected virtual ObservableTypeInfo CreateTypeInfo( INamedType type ) => new( this, type );
 
     public virtual ObservablePropertyInfo CreatePropertyInfo( IFieldOrProperty fieldOrProperty, ObservableTypeInfo parent ) => new( fieldOrProperty, parent );
@@ -50,10 +57,16 @@ public ObservableTypeInfo GetDependencyGraph(
 
         foreach ( var p in type.Properties )
         {
+            if ( p.IsStatic || p.IsImplicitlyDeclared || p.Attributes.Any( this._assets.NotObservableAttribute ) )
+            {
+                continue;
+            }
+
             var propertySymbol = p.GetSymbol();
 
             if ( propertySymbol == null )
             {
+                // Only source-defined properties are supported.
                 continue;
             }
 
diff --git a/src/Metalama.Patterns.Observability/Implementation/DependencyAnalysis/GraphBuildingContext.cs b/src/Metalama.Patterns.Observability/Implementation/DependencyAnalysis/GraphBuildingContext.cs
@@ -14,10 +14,13 @@ namespace Metalama.Patterns.Observability.Implementation.DependencyAnalysis;
 [CompileTime]
 internal abstract class GraphBuildingContext
 {
+    public Assets Assets { get; }
+
     private readonly ICompilation _compilation;
 
-    protected GraphBuildingContext( ICompilation compilation )
+    protected GraphBuildingContext( ICompilation compilation, Assets assets )
     {
+        this.Assets = assets;
         this._compilation = compilation;
     }
 
diff --git a/src/tests/Metalama.Patterns.Observability.AspectTests/NotObservableVirtualProperty.cs b/src/tests/Metalama.Patterns.Observability.AspectTests/NotObservableVirtualProperty.cs
@@ -0,0 +1,12 @@
+﻿// Copyright (c) SharpCrafters s.r.o. See the LICENSE.md file in the root directory of this repository root for details.
+
+namespace Metalama.Patterns.Observability.AspectTests.NotObservableVirtualProperty
+{
+    // <target>
+    [Observable]
+    public class TestClass
+    {
+        [NotObservable]
+        public virtual int Foo { get; set; }
+    }
+}
diff --git a/src/tests/Metalama.Patterns.Observability.AspectTests/NotObservableVirtualProperty.t.cs b/src/tests/Metalama.Patterns.Observability.AspectTests/NotObservableVirtualProperty.t.cs
@@ -0,0 +1,11 @@
+  [Observable]
+  public class TestClass : INotifyPropertyChanged
+  {
+    [NotObservable]
+    public virtual int Foo { get; set; }
+    protected virtual void OnPropertyChanged(string propertyName)
+    {
+      PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+    }
+    public event PropertyChangedEventHandler? PropertyChanged;
+}
diff --git a/src/tests/Metalama.Patterns.Observability.CompileTimeTests/GetDependencyGraphTests.cs b/src/tests/Metalama.Patterns.Observability.CompileTimeTests/GetDependencyGraphTests.cs
diff --git a/src/tests/Metalama.Patterns.Observability.CompileTimeTests/TestGraphBuildingContext.cs b/src/tests/Metalama.Patterns.Observability.CompileTimeTests/TestGraphBuildingContext.cs

Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,6 @@`
`4`	`4`	`"rollForward": "patch"`
`5`	`5`	`},`
`6`	`6`	`"msbuild-sdks": {`
`7`		`- "PostSharp.Engineering.Sdk": "2023.2.111"`
	`7`	`+ "PostSharp.Engineering.Sdk": "2023.2.117"`
`8`	`8`	`}`
`9`	`9`	`}`
Original file line number	Diff line number	Diff line change
`@@ -391,7 +391,7 @@ private static bool ValidateAndFindInvalidatedMethods(`
`391`	`391`
`392`	`392`	`// Match parameter by name.`
`393`	`393`	`var invalidatingMethodParameter =`
`394`		`- invalidatingMethodParameters.FirstOrDefault( p => p.Name == invalidatedMethodParameter.Name );`
	`394`	`+ invalidatingMethodParameters.OfName( invalidatedMethodParameter.Name );`
`395`	`395`
`396`	`396`	`if ( invalidatingMethodParameter == null )`
`397`	`397`	`{`
Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ private static bool TryGetCompatibleTargetInterface(`
`136`	`136`
`137`	`137`	`if ( targetType.Is( typeOfICollection ) )`
`138`	`138`	`{`
`139`		`- var countProperty = typeOfICollection.Properties.Single( p => p.Name == nameof(ICollection.Count) );`
	`139`	`+ var countProperty = typeOfICollection.Properties.OfName( nameof(ICollection.Count) ).Single();`
`140`	`140`	`targetType.TryFindImplementationForInterfaceMember( countProperty, out var countPropertyImpl );`
`141`	`141`
`142`	`142`	`interfaceType = typeOfICollection;`
`@@ -169,7 +169,7 @@ private static bool TryGetCompatibleTargetInterface(`
`169`	`169`
`170`	`170`	`if ( foundInterface != null )`
`171`	`171`	`{`
`172`		`- var countProperty = foundInterface.Properties.Single( p => p.Name == "Count" );`
	`172`	`+ var countProperty = foundInterface.Properties.OfName( "Count" ).Single();`
`173`	`173`	`targetType.TryFindImplementationForInterfaceMember( countProperty, out var countPropertyImpl );`
`174`	`174`
`175`	`175`	`interfaceType = foundInterface;`
Original file line number	Diff line number	Diff line change
`@@ -42,5 +42,5 @@ public Assets()`
`42`	`42`	`/// Gets the <see cref="IProperty"/> for property <c>EqualityComparer<paramref name="type"/>>.Default</c>.`
`43`	`43`	`/// </summary>`
`44`	`44`	`public IProperty GetDefaultEqualityComparerForType( IType type )`
`45`		`- => this.EqualityComparerOfT.WithTypeArguments( type ).Properties.Single( p => p.Name == "Default" );`
	`45`	`+ => this.EqualityComparerOfT.WithTypeArguments( type ).Properties.OfName( "Default" ).Single();`
`46`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ internal sealed class ClassicDependencyGraphBuilder : DependencyGraphBuilder`
`13`	`13`
`14`	`14`	`public INamedType CurrentType { get; }`
`15`	`15`
`16`		`- public ClassicDependencyGraphBuilder( ClassicGraphBuildingContext context, INamedType currentType )`
	`16`	`+ public ClassicDependencyGraphBuilder( ClassicGraphBuildingContext context, INamedType currentType ) : base( context.Assets )`
`17`	`17`	`{`
`18`	`18`	`this.Context = context;`
`19`	`19`	`this.CurrentType = currentType;`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ internal sealed class ClassicGraphBuildingContext : GraphBuildingContext`
`15`	`15`	`{`
`16`	`16`	`private readonly ClassicObservabilityStrategyImpl _strategy;`
`17`	`17`
`18`		`- public ClassicGraphBuildingContext( ClassicObservabilityStrategyImpl strategy ) : base( strategy.CurrentType.Compilation )`
	`18`	`+ public ClassicGraphBuildingContext( ClassicObservabilityStrategyImpl strategy ) : base( strategy.CurrentType.Compilation, strategy.Assets )`
`19`	`19`	`{`
`20`	`20`	`this._strategy = strategy;`
`21`	`21`	`}`