From 5ceb38a8c735131ab85f5968f46c4dfc123010a9 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Sat, 12 Feb 2022 16:01:47 -0500 Subject: [PATCH] enforce immutable type params during immutability analysis Decide a type is not immutable if any [Immutable] type parameters to do satisfy that constraint during immutable type analysis. Previously we would assume a different section of the analyzer caught these, as we look at arguments to these type parameters specifically even outside of actual immutable type analysis. Checking during actual immutable type analysis means we don't miss it when it really matters, in case we did otherwise. --- .../Immutability/ImmutabilityAnalyzer.cs | 2 +- .../Immutability/ImmutabilityContext.cs | 1 + .../Immutability/ImmutabilityQuery.cs | 18 +++------- .../ImmutableDefinitionChecker.cs | 4 +-- .../Immutability/ImmutableTypeInfo.cs | 36 +++++++++++-------- 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityAnalyzer.cs b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityAnalyzer.cs index e9c06404..50388cf9 100644 --- a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityAnalyzer.cs +++ b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityAnalyzer.cs @@ -294,7 +294,7 @@ Func getArgumentLocation new ImmutabilityQuery( ImmutableTypeKind.Total, argument - ), + ) { EnforceImmutableTypeParams = false }, getLocation: () => getArgumentLocation( position ), out Diagnostic diagnostic ) ) { diff --git a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityContext.cs b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityContext.cs index ec82a917..e0459deb 100644 --- a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityContext.cs +++ b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityContext.cs @@ -123,6 +123,7 @@ out Diagnostic diagnostic return info.IsImmutableDefinition( context: this, definition: namedType, + enforceImmutableTypeParams: query.EnforceImmutableTypeParams, getLocation: getLocation, out diagnostic ); diff --git a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityQuery.cs b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityQuery.cs index ee3ca016..65ae22f0 100644 --- a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityQuery.cs +++ b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityQuery.cs @@ -1,18 +1,10 @@ -#nullable disable - using Microsoft.CodeAnalysis; namespace D2L.CodeStyle.Analyzers.Immutability { - internal readonly struct ImmutabilityQuery { - public ImmutabilityQuery( - ImmutableTypeKind kind, - ITypeSymbol type - ) { - Kind = kind; - Type = type; - } - - public ImmutableTypeKind Kind { get; } - public ITypeSymbol Type { get; } + internal readonly record struct ImmutabilityQuery( + ImmutableTypeKind Kind, + ITypeSymbol Type + ) { + public bool EnforceImmutableTypeParams { get; init; } = true; } } diff --git a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableDefinitionChecker.cs b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableDefinitionChecker.cs index 371c7da2..e6587167 100644 --- a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableDefinitionChecker.cs +++ b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableDefinitionChecker.cs @@ -523,13 +523,13 @@ .Symbol is not IMethodSymbol methodSymbol // being anything other than an instance of T. query = new ImmutabilityQuery( ImmutableTypeKind.Instance, - type: assignment.AssignedType + Type: assignment.AssignedType ); } else { // In general we need to handle subtypes. query = new ImmutabilityQuery( ImmutableTypeKind.Total, - type: assignment.AssignedType + Type: assignment.AssignedType ); } diff --git a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableTypeInfo.cs b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableTypeInfo.cs index 813fee6b..559050a1 100644 --- a/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableTypeInfo.cs +++ b/src/D2L.CodeStyle.Analyzers/Immutability/ImmutableTypeInfo.cs @@ -11,29 +11,28 @@ namespace D2L.CodeStyle.Analyzers.Immutability { /// internal readonly struct ImmutableTypeInfo { - // a mapping of which type parameters considered necessarily immutable for the - // conditionally immutable type to be immutable - private readonly ImmutableArray m_conditionalTypeParameters; + private readonly ImmutableArray<(bool RequiresImmutability, bool IsImmutableCondition)> m_typeParameterInfo; private ImmutableTypeInfo( ImmutableTypeKind kind, INamedTypeSymbol type, - ImmutableArray conditionalTypeParameters + ImmutableArray<(bool RequiresImmutability, bool IsImmutableCondition)> typeParameterInfo ) { Kind = kind; Type = type; - m_conditionalTypeParameters = conditionalTypeParameters; + m_typeParameterInfo = typeParameterInfo; } public ImmutableTypeKind Kind { get; } public INamedTypeSymbol Type { get; } - public bool IsConditional => m_conditionalTypeParameters.Length > 0; + public bool IsConditional => m_typeParameterInfo.Any( p => p.IsImmutableCondition ); public bool IsImmutableDefinition( ImmutabilityContext context, INamedTypeSymbol definition, + bool enforceImmutableTypeParams, Func getLocation, out Diagnostic diagnostic ) { @@ -45,9 +44,10 @@ out Diagnostic diagnostic var argRelevance = definition .TypeArguments - .Zip( m_conditionalTypeParameters, ( a, relevant ) => (a, relevant) ); - foreach( (ITypeSymbol argument, bool isRelevant) in argRelevance ) { - if( !isRelevant ) { + .Zip( m_typeParameterInfo, ( a, info ) => (a, info) ); + foreach( (ITypeSymbol argument, (bool requiresImmutability, bool isImmutableCondition)) in argRelevance ) { + bool relevant = isImmutableCondition || ( requiresImmutability && enforceImmutableTypeParams ); + if( !relevant ) { continue; } @@ -72,15 +72,18 @@ public static ImmutableTypeInfo Create( ImmutableTypeKind kind, INamedTypeSymbol type ) { - ImmutableArray immutableTypeParameters = type + ImmutableArray<(bool RequiresImmutability, bool IsImmutableCondition)> typeParameterInfo = type .TypeParameters - .Select( p => annotationsContext.Objects.OnlyIf.IsDefined( p ) ) + .Select( p => ( + annotationsContext.Objects.Immutable.IsDefined( p ), + annotationsContext.Objects.OnlyIf.IsDefined( p ) + ) ) .ToImmutableArray(); return new ImmutableTypeInfo( kind: kind, type: type, - conditionalTypeParameters: immutableTypeParameters + typeParameterInfo: typeParameterInfo ); } @@ -88,15 +91,18 @@ public static ImmutableTypeInfo CreateWithAllConditionalTypeParameters( ImmutableTypeKind kind, INamedTypeSymbol type ) { - ImmutableArray immutableTypeParameters = type + ImmutableArray<(bool RequiresImmutability, bool IsImmutableCondition)> typeParameterInfo = type .TypeParameters - .Select( p => true ) + .Select( p => ( + false, + true + ) ) .ToImmutableArray(); return new ImmutableTypeInfo( kind: kind, type: type, - conditionalTypeParameters: immutableTypeParameters + typeParameterInfo: typeParameterInfo ); } }