Fix #1629: Add support for `[module: NullablePublicOnly]`

If this attribute is in use, private/internal members lack nullability annotations.
Previously in such cases, we ended up inheriting the nullability from the `[NullableContext]`, which could cause us to display a misleading nullability for primary methods.
In debug builds, it could also trigger an assertion when trying to apply the "nullable reference type" marking to to value types.

Of note is that properties and events are a special case: they do not explicitly store Accessibility in metadata. For properties computing the accessibility requires decoding the signature (to find overridden base properties). So these two only check the declaring type's accessibility instead; private properties may still carry nullability despite `[NullablePublicOnly]`. However, the property accessors won't store nullability, so we need to read the `returnTypeAttributes` from the property itself.

ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullableRefTypes.cs

@@ -19,6 +19,18 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 		private string[]?[] field_array;
 		private Dictionary<(string, string?), (int, string[]?, string?[])> field_complex;
 
+		public (string A, dynamic? B) PropertyNamedTuple {
+			get {
+				throw new NotImplementedException();
+			}
+		}
+
+		public (string A, dynamic? B) this[(dynamic? C, string D) weirdIndexer] {
+			get {
+				throw new NotImplementedException();
+			}
+		}
+
 		public int GetLength1(string[] arr)
 		{
 			return field_string.Length + arr.Length;

ICSharpCode.Decompiler/TypeSystem/Accessibility.cs

@@ -115,5 +115,18 @@ namespace ICSharpCode.Decompiler.TypeSystem
 				return b;
 			}
 		}
+
+		/// <summary>
+		/// Gets the effective accessibility of the entity.
+		/// For example, a public method in an internal class returns "internal".
+		/// </summary>
+		public static Accessibility EffectiveAccessibility(this IEntity entity)
+		{
+			Accessibility accessibility = entity.Accessibility;
+			for (ITypeDefinition typeDef = entity.DeclaringTypeDefinition; typeDef != null; typeDef = typeDef.DeclaringTypeDefinition) {
+				accessibility = Intersect(accessibility, typeDef.Accessibility);
+			}
+			return accessibility;
+		}
 	}
 }

ICSharpCode.Decompiler/TypeSystem/Implementation/KnownAttributes.cs

@@ -41,6 +41,7 @@ namespace ICSharpCode.Decompiler.TypeSystem
 		TupleElementNames,
 		Nullable,
 		NullableContext,
+		NullablePublicOnly,
 		Conditional,
 		Obsolete,
 		IsReadOnly,
@@ -111,6 +112,7 @@ namespace ICSharpCode.Decompiler.TypeSystem
 			new TopLevelTypeName("System.Runtime.CompilerServices", nameof(TupleElementNamesAttribute)),
 			new TopLevelTypeName("System.Runtime.CompilerServices", "NullableAttribute"),
 			new TopLevelTypeName("System.Runtime.CompilerServices", "NullableContextAttribute"),
+			new TopLevelTypeName("System.Runtime.CompilerServices", "NullablePublicOnlyAttribute"),
 			new TopLevelTypeName("System.Diagnostics", nameof(ConditionalAttribute)),
 			new TopLevelTypeName("System", nameof(ObsoleteAttribute)),
 			new TopLevelTypeName("System.Runtime.CompilerServices", "IsReadOnlyAttribute"),

ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataEvent.cs

@@ -82,7 +82,10 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 				var declaringTypeDef = DeclaringTypeDefinition;
 				var context = new GenericContext(declaringTypeDef?.TypeParameters);
 				var nullableContext = declaringTypeDef?.NullableContext ?? Nullability.Oblivious;
-				returnType = module.ResolveType(ev.Type, context, ev.GetCustomAttributes(), nullableContext);
+				// The event does not have explicit accessibility in metadata, so use its
+				// containing type to determine whether nullability applies to this type.
+				var typeOptions = module.OptionsForEntity(declaringTypeDef);
+				returnType = module.ResolveType(ev.Type, context, typeOptions, ev.GetCustomAttributes(), nullableContext);
 				return LazyInit.GetOrSet(ref this.returnType, returnType);
 			}
 		}

ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataField.cs

@@ -186,7 +186,7 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 					ty = mod.ElementType;
 				}
 				ty = ApplyAttributeTypeVisitor.ApplyAttributesToType(ty, Compilation,
-					fieldDef.GetCustomAttributes(), metadata, module.TypeSystemOptions,
+					fieldDef.GetCustomAttributes(), metadata, module.OptionsForEntity(this),
 					DeclaringTypeDefinition?.NullableContext ?? Nullability.Oblivious);
 			} catch (BadImageFormatException) {
 				ty = SpecialType.UnknownType;

ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataMethod.cs

@@ -166,7 +166,7 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 			try {
 				var nullableContext = methodDef.GetCustomAttributes().GetNullableContext(module.metadata) ?? DeclaringTypeDefinition.NullableContext;
 				var signature = methodDef.DecodeSignature(module.TypeProvider, genericContext);
-				(returnType, parameters) = DecodeSignature(module, this, signature, methodDef.GetParameters(), nullableContext);
+				(returnType, parameters) = DecodeSignature(module, this, signature, methodDef.GetParameters(), nullableContext, module.OptionsForEntity(this));
 			} catch (BadImageFormatException) {
 				returnType = SpecialType.UnknownType;
 				parameters = Empty<IParameter>.Array;
@@ -175,11 +175,13 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 			LazyInit.GetOrSet(ref this.parameters, parameters);
 		}
 
-		internal static (IType, IParameter[]) DecodeSignature(MetadataModule module, IParameterizedMember owner, MethodSignature<IType> signature, ParameterHandleCollection? parameterHandles, Nullability nullableContext)
+		internal static (IType, IParameter[]) DecodeSignature(MetadataModule module, IParameterizedMember owner,
+			MethodSignature<IType> signature, ParameterHandleCollection? parameterHandles,
+			Nullability nullableContext, TypeSystemOptions typeSystemOptions,
+			CustomAttributeHandleCollection? returnTypeAttributes = null)
 		{
 			var metadata = module.metadata;
 			int i = 0;
-			CustomAttributeHandleCollection? returnTypeAttributes = null;
 			IParameter[] parameters = new IParameter[signature.RequiredParameterCount
 				+ (signature.Header.CallingConvention == SignatureCallingConvention.VarArgs ? 1 : 0)];
 			IType parameterType;
@@ -187,8 +189,14 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 				foreach (var parameterHandle in parameterHandles) {
 					var par = metadata.GetParameter(parameterHandle);
 					if (par.SequenceNumber == 0) {
-						// "parameter" holds return type attributes
-						returnTypeAttributes = par.GetCustomAttributes();
+						// "parameter" holds return type attributes.
+						// Note: for properties, the attributes normally stored on a method's return type
+						// are instead stored as normal attributes on the property.
+						// So MetadataProperty provides a non-null value for returnTypeAttributes,
+						// which then should be preferred over the attributes on the accessor's parameters.
+						if (returnTypeAttributes == null) {
+							returnTypeAttributes = par.GetCustomAttributes();
+						}
 					} else if (par.SequenceNumber > 0 && i < signature.RequiredParameterCount) {
 						// "Successive rows of the Param table that are owned by the same method shall be
 						// ordered by increasing Sequence value - although gaps in the sequence are allowed"
@@ -196,14 +204,14 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 						// Fill gaps in the sequence with non-metadata parameters:
 						while (i < par.SequenceNumber - 1) {
 							parameterType = ApplyAttributeTypeVisitor.ApplyAttributesToType(
-								signature.ParameterTypes[i], module.Compilation, null, metadata, module.TypeSystemOptions, nullableContext);
+								signature.ParameterTypes[i], module.Compilation, null, metadata, typeSystemOptions, nullableContext);
 							parameters[i] = new DefaultParameter(parameterType, name: string.Empty, owner,
 								referenceKind: parameterType.Kind == TypeKind.ByReference ? ReferenceKind.Ref : ReferenceKind.None);
 							i++;
 						}
 						parameterType = ApplyAttributeTypeVisitor.ApplyAttributesToType(
 							signature.ParameterTypes[i], module.Compilation,
-							par.GetCustomAttributes(), metadata, module.TypeSystemOptions, nullableContext);
+							par.GetCustomAttributes(), metadata, typeSystemOptions, nullableContext);
 						parameters[i] = new MetadataParameter(module, owner, parameterType, parameterHandle);
 						i++;
 					}
@@ -211,7 +219,7 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 			}
 			while (i < signature.RequiredParameterCount) {
 				parameterType = ApplyAttributeTypeVisitor.ApplyAttributesToType(
-					signature.ParameterTypes[i], module.Compilation, null, metadata, module.TypeSystemOptions, nullableContext);
+					signature.ParameterTypes[i], module.Compilation, null, metadata, typeSystemOptions, nullableContext);
 				parameters[i] = new DefaultParameter(parameterType, name: string.Empty, owner,
 					referenceKind: parameterType.Kind == TypeKind.ByReference ? ReferenceKind.Ref : ReferenceKind.None);
 				i++;
@@ -222,7 +230,7 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 			}
 			Debug.Assert(i == parameters.Length);
 			var returnType = ApplyAttributeTypeVisitor.ApplyAttributesToType(signature.ReturnType,
-				module.Compilation, returnTypeAttributes, metadata, module.TypeSystemOptions, nullableContext);
+				module.Compilation, returnTypeAttributes, metadata, typeSystemOptions, nullableContext);
 			return (returnType, parameters);
 		}
 		#endregion

ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataProperty.cs

@@ -129,23 +129,33 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 			try {
 				var signature = propertyDef.DecodeSignature(module.TypeProvider, genericContext);
 				var accessors = propertyDef.GetAccessors();
+				var declTypeDef = this.DeclaringTypeDefinition;
 				ParameterHandleCollection? parameterHandles;
 				Nullability nullableContext;
 				if (!accessors.Getter.IsNil) {
 					var getter = module.metadata.GetMethodDefinition(accessors.Getter);
 					parameterHandles = getter.GetParameters();
 					nullableContext = getter.GetCustomAttributes().GetNullableContext(module.metadata)
-						?? DeclaringTypeDefinition?.NullableContext ?? Nullability.Oblivious;
+						?? declTypeDef?.NullableContext ?? Nullability.Oblivious;
 				} else if (!accessors.Setter.IsNil) {
 					var setter = module.metadata.GetMethodDefinition(accessors.Setter);
 					parameterHandles = setter.GetParameters();
 					nullableContext = setter.GetCustomAttributes().GetNullableContext(module.metadata)
-						?? DeclaringTypeDefinition?.NullableContext ?? Nullability.Oblivious;
+						?? declTypeDef?.NullableContext ?? Nullability.Oblivious;
 				} else {
 					parameterHandles = null;
-					nullableContext = DeclaringTypeDefinition?.NullableContext ?? Nullability.Oblivious;
+					nullableContext = declTypeDef?.NullableContext ?? Nullability.Oblivious;
 				}
-				(returnType, parameters) = MetadataMethod.DecodeSignature(module, this, signature, parameterHandles, nullableContext);
+				// We call OptionsForEntity() for the declaring type, not the property itself,
+				// because the property's accessibilty isn't stored in metadata but computed.
+				// Otherwise we'd get infinite recursion, because computing the accessibility
+				// requires decoding the signature for the GetBaseMembers() call.
+				// Roslyn uses the same workaround (see the NullableTypeDecoder.TransformType
+				// call in PEPropertySymbol).
+				var typeOptions = module.OptionsForEntity(declTypeDef);
+				(returnType, parameters) = MetadataMethod.DecodeSignature(module, this, signature,
+					parameterHandles, nullableContext, typeOptions,
+					returnTypeAttributes: propertyDef.GetCustomAttributes());
 			} catch (BadImageFormatException) {
 				returnType = SpecialType.UnknownType;
 				parameters = Empty<IParameter>.Array;

ICSharpCode.Decompiler/TypeSystem/Implementation/MetadataTypeParameter.cs

@@ -145,7 +145,7 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation
 
 		Nullability LoadNullabilityConstraint()
 		{
-			if ((module.TypeSystemOptions & TypeSystemOptions.NullabilityAnnotations) == 0)
+			if (!module.ShouldDecodeNullableAttributes(Owner))
 				return Nullability.Oblivious;
 
 			var metadata = module.metadata;

ICSharpCode.Decompiler/TypeSystem/MetadataModule.cs

@@ -67,7 +67,9 @@ namespace ICSharpCode.Decompiler.TypeSystem
 				this.AssemblyName = metadata.GetString(moddef.Name);
 				this.FullAssemblyName = this.AssemblyName;
 			}
-			this.NullableContext = metadata.GetModuleDefinition().GetCustomAttributes().GetNullableContext(metadata) ?? Nullability.Oblivious;
+			var customAttrs = metadata.GetModuleDefinition().GetCustomAttributes();
+			this.NullableContext = customAttrs.GetNullableContext(metadata) ?? Nullability.Oblivious;
+			this.minAccessibilityForNRT = FindMinimumAccessibilityForNRT(metadata, customAttrs);
 			this.rootNamespace = new MetadataNamespace(this, null, string.Empty, metadata.GetNamespaceDefinitionRoot());
 
 			if (!options.HasFlag(TypeSystemOptions.Uncached)) {
@@ -739,5 +741,51 @@ namespace ICSharpCode.Decompiler.TypeSystem
 				|| att == MethodAttributes.FamORAssem;
 		}
 		#endregion
+
+		#region Nullability Reference Type Support
+		readonly Accessibility minAccessibilityForNRT;
+
+		static Accessibility FindMinimumAccessibilityForNRT(MetadataReader metadata, CustomAttributeHandleCollection customAttributes)
+		{
+			// Determine the minimum effective accessibility an entity must have, so that the metadata stores the nullability for its type.
+			foreach (var handle in customAttributes) {
+				var customAttribute = metadata.GetCustomAttribute(handle);
+				if (customAttribute.IsKnownAttribute(metadata, KnownAttribute.NullablePublicOnly)) {
+					CustomAttributeValue<IType> value;
+					try {
+						value = customAttribute.DecodeValue(Metadata.MetadataExtensions.MinimalAttributeTypeProvider);
+					} catch (BadImageFormatException) {
+						continue;
+					} catch (EnumUnderlyingTypeResolveException) {
+						continue;
+					}
+					if (value.FixedArguments.Length == 1 && value.FixedArguments[0].Value is bool includesInternals) {
+						return includesInternals ? Accessibility.ProtectedAndInternal : Accessibility.Protected;
+					}
+				}
+			}
+			return Accessibility.None;
+		}
+
+		internal bool ShouldDecodeNullableAttributes(IEntity entity)
+		{
+			if ((options & TypeSystemOptions.NullabilityAnnotations) == 0)
+				return false;
+			if (minAccessibilityForNRT == Accessibility.None || entity == null)
+				return true;
+			return minAccessibilityForNRT.LessThanOrEqual(entity.EffectiveAccessibility());
+		}
+
+		internal TypeSystemOptions OptionsForEntity(IEntity entity)
+		{
+			var opt = this.options;
+			if ((opt & TypeSystemOptions.NullabilityAnnotations) != 0) {
+				if (!ShouldDecodeNullableAttributes(entity)) {
+					opt &= ~TypeSystemOptions.NullabilityAnnotations;
+				}
+			}
+			return opt;
+		}
+		#endregion
 	}
 }