Omsmith/destructured assignments

src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityAnalyzer.cs

@@ -31,7 +31,7 @@ public sealed class ImmutabilityAnalyzer : DiagnosticAnalyzer {
 		);
 
 		public override void Initialize( AnalysisContext context ) {
-			context.EnableConcurrentExecution();
+			//context.EnableConcurrentExecution();
 			context.ConfigureGeneratedCodeAnalysis( GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics );
 			context.RegisterCompilationStartAction( CompilationStart );
 		}

src/D2L.CodeStyle.Analyzers/Immutability/ImmutableDefinitionChecker.cs

@@ -333,6 +333,18 @@ ExpressionSyntax expression
 					assignedType: assignedType
 				);
 			}
+
+			public static AssignmentInfo Create(
+				bool isInitializer,
+				ExpressionSyntax expression,
+				ITypeSymbol assignedType
+			) {
+				return new AssignmentInfo(
+					isInitializer: isInitializer,
+					expression: expression,
+					assignedType: assignedType
+				);
+			}
 		}
 
 
@@ -357,13 +369,9 @@ ExpressionSyntax initializer
 				.Select( sr => sr.GetSyntax() )
 				.SelectMany( constructorSyntax => constructorSyntax.DescendantNodes() )
 				.OfType<AssignmentExpressionSyntax>()
-				.Where( assignmentSyntax => IsAnAssignmentTo( assignmentSyntax, memberSymbol ) )
-				.Select( assignmentSyntax => assignmentSyntax.Right )
-				.Select( expr => AssignmentInfo.Create(
-					model: m_compilation.GetSemanticModel( expr.SyntaxTree ),
-					isInitializer: false,
-					expression: expr
-				) );
+				.Select( assignmentSyntax => GetAssignmentInfoIfToSymbolOrNull( assignmentSyntax, memberSymbol ) )
+				.Where( info => info.HasValue )
+				.Select( info => info.Value );
 
 			if ( initializer != null ) {
 				assignmentExpressions = assignmentExpressions.Append(
@@ -378,19 +386,112 @@ ExpressionSyntax initializer
 			return assignmentExpressions;
 		}
 
-		private bool IsAnAssignmentTo(
+		private AssignmentInfo? GetAssignmentInfoIfToSymbolOrNull(
 			AssignmentExpressionSyntax assignmentSyntax,
 			ISymbol memberSymbol
 		) {
 			var semanticModel = m_compilation.GetSemanticModel( assignmentSyntax.SyntaxTree );
 
-			var leftSideSymbol = semanticModel.GetSymbolInfo( assignmentSyntax.Left )
-				.Symbol;
+			var lhsExpressions = assignmentSyntax.Left switch {
+				TupleExpressionSyntax tuple => tuple.Arguments.Select( arg => arg.Expression ).ToImmutableArray(),
+				_ => ImmutableArray.Create( assignmentSyntax.Left )
+			};
 
-			return SymbolEqualityComparer.Default.Equals(
-				memberSymbol,
-				leftSideSymbol
-			);
+			for( var i = 0; i < lhsExpressions.Length; ++i ) {
+				var lhs = lhsExpressions[ i ];
+
+				var leftSideSymbol = semanticModel.GetSymbolInfo( lhs )
+					.Symbol;
+
+				if( !SymbolEqualityComparer.Default.Equals(
+					memberSymbol,
+					leftSideSymbol
+				) ) {
+					continue;
+				}
+
+				// LHS is just a single variable, no deconstruction happening
+				if( lhsExpressions.Length == 1 ) {
+					return AssignmentInfo.Create(
+						model: semanticModel,
+						isInitializer: false,
+						expression: assignmentSyntax.Right
+					);
+				}
+
+				// Value tuples are a common RHS assignment for deconstruction, but it's built-in and there's
+				// no deconstruction method. Handle value tuples specifically if it is one.
+				if( semanticModel.GetTypeInfo( assignmentSyntax.Right ).Type is INamedTypeSymbol assignedType
+					&& assignedType.IsTupleType
+				) {
+					if( assignedType.TypeArguments.Length != lhsExpressions.Length ) {
+						return AssignmentInfo.Create(
+							isInitializer: false,
+							expression: assignmentSyntax.Right,
+							assignedType: null
+						);
+					}
+
+					// If this is a value tuple literal then we can narrow in to the exact expression
+					// responsible for the tuple item. This allows for additional specificity prior to conversions
+					// or things like "known immutable method returns" (such as Array.Empty())
+					ExpressionSyntax rhs = assignmentSyntax.Right switch {
+						TupleExpressionSyntax tuple => tuple.Arguments[ i ].Expression,
+						_ => assignmentSyntax.Right
+					};
+
+					return AssignmentInfo.Create(
+						isInitializer: false,
+						expression: rhs,
+						assignedType: assignedType.TypeArguments[ i ]
+					);
+				}
+
+				// Other types can participate in deconstruction by specifying a deconstruction method
+				// public void Deconstruct( out T1 a, out T2 b ... )
+				// These methods must have a unique number of arguments. e.g. the following are ambiguous
+				//   public void Deconstruct( out string a, out string b )
+				//   public void Deconstruct( out int a, out int b )
+				// The overall deconstruction can be "nested" if a type's deconstruction includes a type which
+				// is itself deconstructed as well
+				DeconstructionInfo deconstruction = semanticModel.GetDeconstructionInfo( assignmentSyntax );
+
+				// If there was no method, then:
+				//   * This might've been something like a value tuple, where the deconstruction is implicit
+				//   * This isn't a valid assignment (and thus should be compiler error)
+				if( deconstruction.Method == null ) {
+					return AssignmentInfo.Create(
+						isInitializer: false,
+						expression: assignmentSyntax.Right,
+						assignedType: null
+					);
+				}
+
+				// DeconstructionInfo is a tree. Each non-terminal node has a method which produces the values
+				// Some child nodes may themselves have methods as well, producing more values (referred to as
+				// "nested" deconstruction).
+				//
+				// Only handling the simple case of non-nested deconstruction, meaning the tree has a depth of 2
+				// with a single method. In this case there should be a parameter for every variable being assigned to
+				if( deconstruction.Method.Parameters.Length != lhsExpressions.Length ) {
+					return AssignmentInfo.Create(
+						isInitializer: false,
+						expression: assignmentSyntax.Right,
+
+						// While we weren't able to determine the pre-conversion type being assigned (with this simple logic)
+						// just consider the variable's type so this isn't always a complete error
+						assignedType: semanticModel.GetTypeInfo( lhsExpressions[ i ] ).Type
+					);
+				}
+
+				return AssignmentInfo.Create(
+					isInitializer: false,
+					expression: assignmentSyntax.Right,
+					assignedType: deconstruction.Method.Parameters[ i ].Type
+				);
+			}
+
+			return null;
 		}
 
 		private enum AssignmentQueryKind {
@@ -474,6 +575,17 @@ .Symbol is not IMethodSymbol methodSymbol
 					break;
 			}
 
+			if( assignment.AssignedType == null
+				|| assignment.AssignedType.TypeKind == TypeKind.Error
+			) {
+				diagnostic = Diagnostic.Create(
+					Diagnostics.NonImmutableTypeHeldByImmutable,
+					assignment.Expression.GetLocation(),
+					"blarg", "blarg", "blarg"
+				);
+				return AssignmentQueryKind.Hopeless;
+			}
+
 			// If nothing above was caught, then fallback to querying.
 
 			if( assignment.Expression is BaseObjectCreationExpressionSyntax _ ) {

tests/D2L.CodeStyle.Analyzers.Test/Specs/ImmutabilityAnalyzer.cs

@@ -116,10 +116,110 @@ public SomeClassWithConstructor7( U you ) {
 
 		[Immutable]
 		public sealed class SomeClassWithConstructor8 {
-			public readonly RegularInterface m_interface = new Good();
+			public readonly RegularInterface m_interface;
 
 			public SomeClassWithConstructor8() {
-				(m_interface) = /* NonImmutableTypeHeldByImmutable(class, SpecTests.Types.Bad,  (or [ImmutableBaseClass])) */ new Bad() /**/;
+				( m_interface ) = /* NonImmutableTypeHeldByImmutable(class, SpecTests.Types.Bad,  (or [ImmutableBaseClass])) */ new Bad() /**/;
+			}
+		}
+
+		[Immutable]
+		public sealed class SomeClassWithConstructor9 {
+			public readonly RegularInterface m_interface;
+			public readonly RegularInterface m_interface2;
+
+			public SomeClassWithConstructor9() {
+				(m_interface, m_interface2) = (
+					/* NonImmutableTypeHeldByImmutable(class, SpecTests.Types.Bad,  (or [ImmutableBaseClass])) */ new Bad() /**/,
+					new Good();
+				);
+			}
+		}
+
+		[Immutable]
+		public sealed class SomeClassWithConstructor10 {
+			public readonly RegularInterface m_interface;
+			public readonly RegularInterface m_interface2;
+
+			public SomeClassWithConstructor10() {
+				(m_interface, m_interface2) = /* NonImmutableTypeHeldByImmutable(class, SpecTests.Types.Bad, ) */ GetValues() /**/;
+			}
+
+			public static (Bad, Good) GetValues() => default;
+		}
+
+		[Immutable]
+		public sealed class SomeClassWithConstructor11 {
+			public readonly RegularInterface m_interface;
+			public readonly RegularInterface m_interface2;
+
+			public SomeClassWithConstructor11() {
+				(m_interface, m_interface2) = /* NonImmutableTypeHeldByImmutable(class, SpecTests.Types.Bad, ) */ GetValues() /**/;
+			}
+
+			public static DeconstructingClass GetValues() => default;
+
+			public sealed class DeconstructingClass {
+				public void Deconstruct( out Bad bad, out Good good ) => (bad, good) = (default, default);
+			}
+		}
+
+		[Immutable]
+		public sealed class SomeClassWithConstructor12 {
+			public readonly RegularInterface m_interface;
+			public readonly RegularInterface m_interface2;
+
+			public SomeClassWithConstructor12() {
+				(m_interface, m_interface2) = /* NonImmutableTypeHeldByImmutable(blarg, blarg, blarg) | NonImmutableTypeHeldByImmutable(blarg, blarg, blarg) */ GetValues() /**/;
+			}
+
+			public static DeconstructingClass GetValues() => default;
+
+			public sealed class DeconstructingClass {
+				public void Deconstruct( out Good A, out Good B, out Good C ) => (A, B, C) = (default, default, default);
+			}
+		}
+
+		[Immutable]
+		public sealed class SomeClassWithConstructor13 {
+			public readonly RegularInterface m_interface;
+			public readonly RegularInterface m_interface2;
+			public readonly RegularInterface m_interface3;
+
+			public SomeClassWithConstructor13() {
+				(m_interface, (m_interface2, m_interface3)) =
+					/*NonImmutableTypeHeldByImmutable(interface, SpecTests.Types.RegularInterface, ) | NonImmutableTypeHeldByImmutable(interface, SpecTests.Types.RegularInterface, ) | NonImmutableTypeHeldByImmutable(interface, SpecTests.Types.RegularInterface, ) */ GetValues() /**/;
+			}
+
+			public static DeconstructingClass GetValues() => default;
+
+			public sealed class DeconstructingClass {
+				public void Deconstruct( out Good A, out DeconstructingClass2 DC ) => (A, DC) = (default, default);
+			}
+
+			private sealed class DeconstructingClass2 {
+				public void Deconstruct( out Good B, out Good C ) => (B, C) = (default, default);
+			}
+		}
+
+		[Immutable]
+		public sealed class SomeClassWithConstructor14 {
+			public readonly Good m_interface;
+			public readonly Good m_interface2;
+			public readonly Good m_interface3;
+
+			public SomeClassWithConstructor14() {
+				(m_interface, (m_interface2, m_interface3)) = GetValues();
+			}
+
+			public static DeconstructingClass GetValues() => default;
+
+			public sealed class DeconstructingClass {
+				public void Deconstruct( out Good A, out DeconstructingClass2 DC ) => (A, DC) = (default, default);
+			}
+
+			public sealed class DeconstructingClass2 {
+				public void Deconstruct( out Good B, out Good C ) => (B, C) = (default, default);
 			}
 		}
 		#endregion
@@ -1199,10 +1299,10 @@ public record SomeRecord {
 
 		int /* MemberIsNotReadOnly(Property, Y, SomeRecord) */ Y /**/ { get; set; }
 
-		/* NonImmutableTypeHeldByImmutable(class, object, ) */ object /**/ Z { get; }
+		object Z { get; }
 
 		public SomeRecord( SomeRecord v, int w, Types.SomeImmutableClass x, int y, object z )
-			=> (V, W, X, Y, Z) = (v, w, x, y, z);
+			=> (V, W, X, Y, Z) = (v, w, x, y, /* NonImmutableTypeHeldByImmutable(class, object, ) */ z /**/);
     }
 
 	record NonImmutableBaseRecord(object x);