Skip to content

Commit

Permalink
Use ConstructedFrom for diagnostics with Ignored annotations in symbols
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Dec 4, 2020
1 parent 322f89a commit dbfb16b
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 22 deletions.
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/NullableAnnotation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ internal enum NullableAnnotation : byte

/// <summary>
/// Used for indexed type parameters and used locally in override/implementation checks.
/// When substituting a type parameter with Ignored annotation into some original type parameter
/// with some other annotation, the result is the annotation from the original symbol.
///
/// T annotated + (T -> U ignored) = U annotated
/// T oblivious + (T -> U ignored) = U oblivious
/// T not-annotated + (T -> U ignored) = U not-annotated
/// </summary>
Ignored,
}
Expand Down
10 changes: 6 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/NullableAnnotationExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,10 @@ internal static CodeAnalysis.NullableAnnotation ToPublicAnnotation(this TypeWith
internal static ImmutableArray<CodeAnalysis.NullableAnnotation> ToPublicAnnotations(this ImmutableArray<TypeWithAnnotations> types) =>
types.SelectAsArray(t => t.ToPublicAnnotation());

internal static CodeAnalysis.NullableAnnotation ToPublicAnnotation(TypeSymbol type, NullableAnnotation annotation) =>
annotation switch
internal static CodeAnalysis.NullableAnnotation ToPublicAnnotation(TypeSymbol type, NullableAnnotation annotation)
{
Debug.Assert(annotation != NullableAnnotation.Ignored);
return annotation switch
{
NullableAnnotation.Annotated => CodeAnalysis.NullableAnnotation.Annotated,
NullableAnnotation.NotAnnotated => CodeAnalysis.NullableAnnotation.NotAnnotated,
Expand All @@ -136,11 +138,11 @@ internal static CodeAnalysis.NullableAnnotation ToPublicAnnotation(TypeSymbol ty
NullableAnnotation.Oblivious when type.IsValueType => CodeAnalysis.NullableAnnotation.NotAnnotated,
NullableAnnotation.Oblivious => CodeAnalysis.NullableAnnotation.None,

// May occur as type argument in symbols in diagnostic
NullableAnnotation.Ignored when type.IsTypeParameter() => CodeAnalysis.NullableAnnotation.None,
NullableAnnotation.Ignored => CodeAnalysis.NullableAnnotation.None,

_ => throw ExceptionUtilities.UnexpectedValue(annotation)
};
}

internal static CSharp.NullableAnnotation ToInternalAnnotation(this CodeAnalysis.NullableAnnotation annotation) =>
annotation switch
Expand Down
14 changes: 8 additions & 6 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1690,19 +1690,21 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
{
if (arg.isExplicit)
{
// We use ConstructedFrom symbols here and below to not leak methods with Ignored annotations in type arguments
// into diagnostics
diagnostics.Add(topLevel ?
ErrorCode.WRN_TopLevelNullabilityMismatchInReturnTypeOnExplicitImplementation :
ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation,
implementingMethod.Locations[0], new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
implementingMethod.Locations[0], new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
else
{
diagnostics.Add(topLevel ?
ErrorCode.WRN_TopLevelNullabilityMismatchInReturnTypeOnImplicitImplementation :
ErrorCode.WRN_NullabilityMismatchInReturnTypeOnImplicitImplementation,
GetImplicitImplementationDiagnosticLocation(implementedMethod, arg.implementingType, implementingMethod),
new FormattedSymbol(implementingMethod, SymbolDisplayFormat.MinimallyQualifiedFormat),
new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
new FormattedSymbol(implementingMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat),
new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
};

Expand All @@ -1716,7 +1718,7 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
ErrorCode.WRN_NullabilityMismatchInParameterTypeOnExplicitImplementation,
implementingMethod.Locations[0],
new FormattedSymbol(implementingParameter, SymbolDisplayFormat.ShortFormat),
new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
else
{
Expand All @@ -1725,8 +1727,8 @@ internal static void CheckNullableReferenceTypeMismatchOnImplementingMember(Type
ErrorCode.WRN_NullabilityMismatchInParameterTypeOnImplicitImplementation,
GetImplicitImplementationDiagnosticLocation(implementedMethod, arg.implementingType, implementingMethod),
new FormattedSymbol(implementingParameter, SymbolDisplayFormat.ShortFormat),
new FormattedSymbol(implementingMethod, SymbolDisplayFormat.MinimallyQualifiedFormat),
new FormattedSymbol(implementedMethod, SymbolDisplayFormat.MinimallyQualifiedFormat));
new FormattedSymbol(implementingMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat),
new FormattedSymbol(implementedMethod.ConstructedFrom, SymbolDisplayFormat.MinimallyQualifiedFormat));
}
};

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ internal static TypeWithAnnotations Create(TypeSymbol typeSymbol, NullableAnnota
return default;
}

Debug.Assert(nullableAnnotation != NullableAnnotation.Ignored || typeSymbol.IsTypeParameter());
switch (nullableAnnotation)
{
case NullableAnnotation.Oblivious:
Expand Down
24 changes: 12 additions & 12 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13235,9 +13235,9 @@ class B : IA
var compilation = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());

compilation.VerifyDiagnostics(
// (23,17): warning CS8613: Nullability of reference types in return type of 'S?[] B.M2<S>()' doesn't match implicitly implemented member 'S[] IA.M2<S>()'.
// (23,17): warning CS8613: Nullability of reference types in return type of 'S?[] B.M2<S>()' doesn't match implicitly implemented member 'T[] IA.M2<T>()'.
// public S?[] M2<S>() where S : class
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnImplicitImplementation, "M2").WithArguments("S?[] B.M2<S>()", "S[] IA.M2<S>()").WithLocation(23, 17),
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnImplicitImplementation, "M2").WithArguments("S?[] B.M2<S>()", "T[] IA.M2<T>()").WithLocation(23, 17),
// (18,22): warning CS8613: Nullability of reference types in return type of 'string?[] B.M1()' doesn't match implicitly implemented member 'string[] IA.M1()'.
// public string?[] M1()
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnImplicitImplementation, "M1").WithArguments("string?[] B.M1()", "string[] IA.M1()").WithLocation(18, 22)
Expand Down Expand Up @@ -13433,22 +13433,22 @@ class B : IA
";
var compilation = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());
compilation.VerifyDiagnostics(
// (17,6): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' context.
// (17,6): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
// S?[] IA.M2<S>() where S : class
Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(17, 6),
// (17,13): warning CS8616: Nullability of reference types in return type doesn't match implemented member 'S[] IA.M2<S>()'.
// (17,13): warning CS8616: Nullability of reference types in return type doesn't match implemented member 'T[] IA.M2<T>()'.
// S?[] IA.M2<S>() where S : class
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation, "M2").WithArguments("S[] IA.M2<S>()").WithLocation(17, 13),
// (11,11): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' context.
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation, "M2").WithArguments("T[] IA.M2<T>()").WithLocation(17, 13),
// (11,11): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
// string?[] IA.M1()
Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(11, 11),
// (11,18): warning CS8616: Nullability of reference types in return type doesn't match implemented member 'string[] IA.M1()'.
// string?[] IA.M1()
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation, "M1").WithArguments("string[] IA.M1()").WithLocation(11, 18),
// (13,26): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' context.
// (13,26): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
// return new string?[] {};
Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(13, 26),
// (19,21): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' context.
// (19,21): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
// return new S?[] {};
Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(19, 21)
);
Expand Down Expand Up @@ -13519,9 +13519,9 @@ void IA.M2<S>(S[] x)
// (12,13): warning CS8617: Nullability of reference types in type of parameter 'x' doesn't match implemented member 'void IA.M1(string?[] x)'.
// void IA.M1(string[] x) => throw null!;
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOnExplicitImplementation, "M1").WithArguments("x", "void IA.M1(string?[] x)").WithLocation(12, 13),
// (13,13): warning CS8617: Nullability of reference types in type of parameter 'x' doesn't match implemented member 'void IA.M2<S>(S?[] x)'.
// (13,13): warning CS8617: Nullability of reference types in type of parameter 'x' doesn't match implemented member 'void IA.M2<T>(T?[] x)'.
// void IA.M2<S>(S[] x)
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOnExplicitImplementation, "M2").WithArguments("x", "void IA.M2<S>(S?[] x)").WithLocation(13, 13)
Diagnostic(ErrorCode.WRN_NullabilityMismatchInParameterTypeOnExplicitImplementation, "M2").WithArguments("x", "void IA.M2<T>(T?[] x)").WithLocation(13, 13)
);
}

Expand Down Expand Up @@ -17679,9 +17679,9 @@ class B : IA
var compilation = CreateCompilation(new[] { source }, options: WithNonNullTypesTrue());

compilation.VerifyDiagnostics(
// (23,13): warning CS8616: Nullability of reference types in return type doesn't match implemented member 'S[] IA.M2<S>()'.
// (23,13): warning CS8616: Nullability of reference types in return type doesn't match implemented member 'T[] IA.M2<T>()'.
// S?[] IA.M2<S>() where S : class
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation, "M2").WithArguments("S[] IA.M2<S>()").WithLocation(23, 13),
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation, "M2").WithArguments("T[] IA.M2<T>()").WithLocation(23, 13),
// (18,18): warning CS8616: Nullability of reference types in return type doesn't match implemented member 'string[] IA.M1()'.
// string?[] IA.M1()
Diagnostic(ErrorCode.WRN_NullabilityMismatchInReturnTypeOnExplicitImplementation, "M1").WithArguments("string[] IA.M1()").WithLocation(18, 18)
Expand Down

0 comments on commit dbfb16b

Please sign in to comment.