Skip to content

Commit 81cb8c5

Browse files
sungam3rsungam3r
authored
Handle Nullable<T> gracefully (#465)
1 parent 0288483 commit 81cb8c5

File tree

8 files changed

+85
-31
lines changed

8 files changed

+85
-31
lines changed

‎src/PublicApiGenerator/ApiGenerator.cs‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -633,12 +633,6 @@ private static void PopulateMethodParameters(IMethodSignature member,
633633

634634
var type = parameterType.CreateCodeTypeReference(parameter);
635635

636-
if (isExtension)
637-
{
638-
type = type.MakeThis();
639-
isExtension = false;
640-
}
641-
642636
// special case of ref is in
643637
// TODO: Move CustomAttributes.Any(a => a.AttributeType.FullName == "System.Runtime.CompilerServices.IsReadOnlyAttribute") to extension method once other PR is merged
644638
if (parameter.CustomAttributes.Any(a =>
@@ -651,12 +645,15 @@ private static void PopulateMethodParameters(IMethodSignature member,
651645
var name = parameter.HasConstant
652646
? string.Format(CultureInfo.InvariantCulture, "{0} = {1}", parameter.Name, FormatParameterConstant(parameter))
653647
: parameter.Name;
654-
var expression = new CodeParameterDeclarationExpression(type, name)
648+
var expression = new CodeParameterDeclarationExpressionEx(type, name)
655649
{
656650
Direction = direction,
657-
CustomAttributes = CreateCustomAttributes(parameter, attributeFilter)
651+
CustomAttributes = CreateCustomAttributes(parameter, attributeFilter),
652+
This = isExtension,
658653
};
659654

655+
isExtension = false;
656+
660657
if (parameter.IsNativeInteger(parameter.ParameterType.FullName))
661658
expression.Type.MakeNativeInteger();
662659
parameters.Add(expression);
@@ -792,17 +789,16 @@ private static void AddFieldToTypeDeclaration(CodeTypeDeclaration typeDeclaratio
792789

793790
// TODO: Values for readonly fields are set in the ctor
794791
var codeTypeReference = memberInfo.FieldType.CreateCodeTypeReference(memberInfo);
795-
if (memberInfo.IsInitOnly)
796-
codeTypeReference = codeTypeReference.MakeReadonly();
797792
if (memberInfo.FieldType.IsUnsafeSignatureType())
798793
codeTypeReference = codeTypeReference.MakeUnsafe();
799794
if (memberInfo.FieldType.IsVolatile())
800795
codeTypeReference = codeTypeReference.MakeVolatile();
801796

802-
var field = new CodeMemberField(codeTypeReference, memberInfo.Name)
797+
var field = new CodeMemberFieldEx(codeTypeReference, memberInfo.Name)
803798
{
804799
Attributes = attributes,
805-
CustomAttributes = CreateCustomAttributes(memberInfo, attributeFilter)
800+
CustomAttributes = CreateCustomAttributes(memberInfo, attributeFilter),
801+
IsReadonly = memberInfo.IsInitOnly
806802
};
807803

808804
if (memberInfo.HasConstant)

‎src/PublicApiGenerator/CecilEx.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,14 +203,10 @@ private static IEnumerable<TypeDefinition> GetBaseTypes(TypeDefinition type)
203203
}
204204
}
205205

206-
public static CodeTypeReference MakeReadonly(this CodeTypeReference typeReference) => ModifyCodeTypeReference(typeReference, "readonly");
207-
208206
public static CodeTypeReference MakeUnsafe(this CodeTypeReference typeReference) => ModifyCodeTypeReference(typeReference, "unsafe");
209207

210208
public static CodeTypeReference MakeVolatile(this CodeTypeReference typeReference) => ModifyCodeTypeReference(typeReference, "volatile");
211209

212-
public static CodeTypeReference MakeThis(this CodeTypeReference typeReference) => ModifyCodeTypeReference(typeReference, "this");
213-
214210
public static CodeTypeReference MakeReturn(this CodeTypeReference typeReference) => ModifyCodeTypeReference(typeReference, "return:");
215211

216212
public static CodeTypeReference MakeGet(this CodeTypeReference typeReference) => ModifyCodeTypeReference(typeReference, "get:");

‎src/PublicApiGenerator/CodeTypeReferenceBuilder.cs‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,7 @@ internal static CodeTypeReference CreateCodeTypeReference(this TypeReference typ
1515
private static CodeTypeReference CreateCodeTypeReferenceWithNullabilityMap(TypeReference type, IEnumerator<bool?> nullabilityMap, NullableMode mode, bool disableNested)
1616
{
1717
var typeName = GetTypeName(type, nullabilityMap, mode, disableNested);
18-
if (type.IsValueType && type.Name == "Nullable`1" && type.Namespace == "System")
19-
{
20-
// unwrap System.Nullable<Type> into Type? for readability
21-
var genericArgs = type is IGenericInstance instance
22-
? instance.GenericArguments
23-
: type.HasGenericParameters ? type.GenericParameters.Cast<TypeReference>() : throw new NotSupportedException(type.ToString());
24-
return CreateCodeTypeReferenceWithNullabilityMap(genericArgs.Single(), nullabilityMap, NullableMode.Force, disableNested);
25-
}
26-
else
27-
{
28-
return new CodeTypeReference(typeName, CreateGenericArguments(type, nullabilityMap));
29-
}
18+
return new CodeTypeReference(typeName, CreateGenericArguments(type, nullabilityMap));
3019
}
3120

3221
private static CodeTypeReference[] CreateGenericArguments(TypeReference type, IEnumerator<bool?> nullabilityMap)
@@ -86,7 +75,7 @@ private static CodeTypeReference[] CreateGenericArguments(TypeReference type, IE
8675

8776
// The compiler optimizes the size of metadata bypassing a sequence of bytes for value types.
8877
// Thus, it can even delete the entire NullableAttribute if the whole signature consists only of value types,
89-
// for example KeyValuePair<int, int?>, thus we can call IsNullable() only by looking first deep into the signature
78+
// for example KeyValuePair<int, int?>, thus we can call IsNullable() only by looking first deep into the signature.
9079
private static bool HasAnyReferenceType(TypeReference type)
9180
{
9281
if (!type.IsValueType)
@@ -107,7 +96,11 @@ private static bool HasAnyReferenceType(TypeReference type)
10796

10897
private static string GetTypeName(TypeReference type, IEnumerator<bool?>? nullabilityMap, NullableMode mode, bool disableNested)
10998
{
110-
bool nullable = mode != NullableMode.Disable && (mode == NullableMode.Force || HasAnyReferenceType(type) && IsNullable());
99+
bool nullable =
100+
mode != NullableMode.Disable &&
101+
!(type.Namespace == "System" && type.Name == "Nullable`1") &&
102+
HasAnyReferenceType(type) &&
103+
IsNullable(); // we should not touch nullabilityMap enumerator in the above cases
111104

112105
var typeName = GetTypeNameCore(type, nullabilityMap, nullable, disableNested);
113106

‎src/PublicApiGenerator/Declarations/CodeMemberFieldEx.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
using System.CodeDom;
2+
3+
namespace PublicApiGenerator;
4+
5+
internal class CodeMemberFieldEx : CodeMemberField
6+
{
7+
public CodeMemberFieldEx(CodeTypeReference type, string name)
8+
: base(type, name)
9+
{
10+
}
11+
12+
public bool IsReadonly { get; set; }
13+
}

‎src/PublicApiGenerator/Declarations/CodeParameterDeclarationExpressionEx.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
using System.CodeDom;
2+
3+
namespace PublicApiGenerator;
4+
5+
internal class CodeParameterDeclarationExpressionEx : CodeParameterDeclarationExpression
6+
{
7+
public CodeParameterDeclarationExpressionEx(CodeTypeReference type, string name) : base(type, name)
8+
{
9+
}
10+
11+
public bool This { get; set; }
12+
}

‎src/PublicApiGenerator/NullableMode.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,5 @@ namespace PublicApiGenerator;
33
internal enum NullableMode
44
{
55
Default,
6-
Force,
76
Disable
87
}

‎src/PublicApiGenerator/System.CodeDom/CSharpCodeGenerator.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,6 +1295,10 @@ private void GenerateField(CodeMemberField e)
12951295
OutputMemberAccessModifier(e.Attributes);
12961296
OutputVTableModifier(e.Attributes);
12971297
OutputFieldScopeModifier(e.Attributes);
1298+
if (e is CodeMemberFieldEx { IsReadonly: true })
1299+
{
1300+
Output.Write("readonly ");
1301+
}
12981302
OutputTypeNamePair(e.Type, e.Name);
12991303
if (e.InitExpression != null)
13001304
{
@@ -1317,6 +1321,10 @@ private void GenerateParameterDeclarationExpression(CodeParameterDeclarationExpr
13171321
}
13181322

13191323
OutputDirection(e.Direction);
1324+
if (e is CodeParameterDeclarationExpressionEx { This: true })
1325+
{
1326+
Output.Write("this ");
1327+
}
13201328
OutputTypeNamePair(e.Type, e.Name, new DynamicContext(e.CustomAttributes));
13211329
}
13221330

@@ -2917,6 +2925,13 @@ private string GetBaseTypeOutput(CodeTypeReference typeRef, bool preferBuiltInTy
29172925
}
29182926
}
29192927

2928+
// special handling for Nullable<T> to output T? instead
2929+
if (typeRef.BaseType == "System.Nullable`1" && typeRef.TypeArguments.Count == 1)
2930+
{
2931+
dynamicContext?.Move();
2932+
return GetTypeOutput(typeRef.TypeArguments[0], dynamicContext) + "?";
2933+
}
2934+
29202935
// replace + with . for nested classes.
29212936
//
29222937
var sb = new StringBuilder(s.Length + 10);

‎src/PublicApiGeneratorTests/Issue459.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
namespace PublicApiGeneratorTests;
2+
3+
public sealed class Issue459 : ApiGeneratorTestsBase
4+
{
5+
[Fact]
6+
public void Issue459_Should_Work()
7+
{
8+
AssertPublicApi(typeof(Issue459Extensions), """
9+
namespace PublicApiGeneratorTests
10+
{
11+
public static class Issue459Extensions
12+
{
13+
public static T[] WhereNotNull1<T>(this T?[] source)
14+
where T : struct { }
15+
public static T?[] WhereNotNull2<T>(this T[] source)
16+
where T : struct { }
17+
}
18+
}
19+
""", opt => opt.IncludeAssemblyAttributes = false);
20+
}
21+
}
22+
23+
public static class Issue459Extensions
24+
{
25+
public static T[] WhereNotNull1<T>(this T?[] source)
26+
where T : struct => throw null;
27+
28+
public static T?[] WhereNotNull2<T>(this T[] source)
29+
where T : struct => throw null;
30+
}

0 commit comments

Comments
 (0)