Skip to content

Commit

Permalink
Omsmith/conditional parameters fix (#668)
Browse files Browse the repository at this point in the history
* move dictionary into ImmutabilityContext.Create

* only consider [OnlyIf] parameters as immutable during definition checking

Co-authored-by: Jacob Parker <jacob@solidangle.ca>
  • Loading branch information
omsmith and j3parker committed Dec 1, 2020
1 parent 8baddd9 commit 954ef6d
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ INamedTypeSymbol typeSymbol
return;
}

if( Attributes.Objects.ConditionallyImmutable.IsDefined( typeSymbol ) ) {
immutabilityContext = immutabilityContext.WithConditionalTypeParametersAsImmutable( typeSymbol );
}

ImmutableDefinitionChecker checker = new ImmutableDefinitionChecker(
compilation: ctx.Compilation,
diagnosticSink: ctx.ReportDiagnostic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ internal partial class ImmutabilityContext {
internal static ImmutabilityContext Create( Compilation compilation ) {
ImmutableDictionary<string, IAssemblySymbol> compilationAssmeblies = GetCompilationAssemblies( compilation );

var builder = ImmutableArray.CreateBuilder<ImmutableTypeInfo>( DefaultExtraTypes.Length );
var builder = ImmutableDictionary.CreateBuilder<INamedTypeSymbol, ImmutableTypeInfo>();

foreach( ( string typeName, string qualifiedAssembly ) in DefaultExtraTypes ) {
INamedTypeSymbol type;
Expand All @@ -112,10 +112,13 @@ internal partial class ImmutabilityContext {
type
);

builder.Add( info );
builder.Add( type, info );
}

return new ImmutabilityContext( builder.ToImmutable() );
return new ImmutabilityContext(
extraImmutableTypes: builder.ToImmutable(),
conditionalTypeParamemters: ImmutableHashSet<ITypeParameterSymbol>.Empty
);
}

private static ImmutableDictionary<string, IAssemblySymbol> GetCompilationAssemblies( Compilation compilation ) {
Expand Down
43 changes: 33 additions & 10 deletions src/D2L.CodeStyle.Analyzers/Immutability/ImmutabilityContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace D2L.CodeStyle.Analyzers.Immutability {
/// </summary>
internal sealed partial class ImmutabilityContext {
private readonly ImmutableDictionary<INamedTypeSymbol, ImmutableTypeInfo> m_extraImmutableTypes;
private readonly ImmutableHashSet<ITypeParameterSymbol> m_conditionalTypeParameters;

// Hard code this to avoid looking up the ITypeSymbol to include it in m_extraImmutableTypes
private static readonly ImmutableHashSet<SpecialType> m_totallyImmutableSpecialTypes = ImmutableHashSet.Create(
Expand All @@ -40,12 +41,34 @@ internal sealed partial class ImmutabilityContext {
SpecialType.System_ValueType
);

private ImmutabilityContext( IEnumerable<ImmutableTypeInfo> extraImmutableTypes ) {
m_extraImmutableTypes = extraImmutableTypes
.ToImmutableDictionary(
info => info.Type,
info => info
);
private ImmutabilityContext(
ImmutableDictionary<INamedTypeSymbol, ImmutableTypeInfo> extraImmutableTypes,
ImmutableHashSet<ITypeParameterSymbol> conditionalTypeParamemters
) {
m_extraImmutableTypes = extraImmutableTypes;
m_conditionalTypeParameters = conditionalTypeParamemters;
}

/// <summary>
/// This function is intended to provide a new immutability context that considers the conditional type parameters
/// of the ConditionallyImmutable type being checked as immutable. It is important this is only used in those cases,
/// and not for instance while validating type arguments to an [Immutable] type parameter.
/// </summary>
/// <param name="type">The ConditionallyImmutable type definition being checked</param>
/// <returns></returns>
public ImmutabilityContext WithConditionalTypeParametersAsImmutable(
INamedTypeSymbol type
) {
if( !Attributes.Objects.ConditionallyImmutable.IsDefined( type ) ) {
throw new InvalidOperationException( $"{nameof( WithConditionalTypeParametersAsImmutable )} should only be called on ConditionallyImmutable types" );
}

var conditionalTypeParameters = type.TypeParameters.Where( p => Attributes.Objects.OnlyIf.IsDefined( p ) );

return new ImmutabilityContext(
extraImmutableTypes: m_extraImmutableTypes,
conditionalTypeParamemters: m_conditionalTypeParameters.Union( conditionalTypeParameters )
);
}

/// <summary>
Expand Down Expand Up @@ -138,6 +161,10 @@ out diagnostic
return true;
}

if( type is ITypeParameterSymbol tp && m_conditionalTypeParameters.Contains( tp ) ) {
return true;
}

diagnostic = Diagnostic.Create(
Diagnostics.TypeParameterIsNotKnownToBeImmutable,
getLocation(),
Expand Down Expand Up @@ -218,10 +245,6 @@ ITypeSymbol type
return ImmutableTypeKind.Total;
}

if( Attributes.Objects.OnlyIf.IsDefined( type ) ) {
return ImmutableTypeKind.Total;
}

if ( Attributes.Objects.ImmutableBaseClass.IsDefined( type ) ) {
return ImmutableTypeKind.Instance;
}
Expand Down
Loading

0 comments on commit 954ef6d

Please sign in to comment.