diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 15d607d3cf6b0..e534e07be0b7d 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2883,6 +2883,7 @@ private FlowAnalysisAnnotations GetRValueAnnotations(Symbol symbol) MethodSymbol method => method.ReturnTypeFlowAnalysisAnnotations, PropertySymbol property => property.GetOwnOrInheritedGetMethod()?.ReturnTypeFlowAnalysisAnnotations ?? FlowAnalysisAnnotations.None, ParameterSymbol parameter => parameter.FlowAnalysisAnnotations, + FieldSymbol field => field.FlowAnalysisAnnotations, _ => FlowAnalysisAnnotations.None }; @@ -5212,15 +5213,18 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) return FlowAnalysisAnnotations.None; } - var symbol = expr switch + var annotations = expr switch { - BoundPropertyAccess property => property.PropertySymbol, - BoundIndexerAccess indexer => indexer.Indexer, - _ => null + BoundPropertyAccess property => getSetterAnnotations(property.PropertySymbol), + BoundIndexerAccess indexer => getSetterAnnotations(indexer.Indexer), + BoundFieldAccess field => field.FieldSymbol.FlowAnalysisAnnotations, + _ => FlowAnalysisAnnotations.None }; - var annotations = symbol?.GetOwnOrInheritedSetMethod()?.Parameters.Last()?.FlowAnalysisAnnotations ?? FlowAnalysisAnnotations.None; return annotations & (FlowAnalysisAnnotations.DisallowNull | FlowAnalysisAnnotations.AllowNull); + + static FlowAnalysisAnnotations getSetterAnnotations(PropertySymbol property) + => property.GetOwnOrInheritedSetMethod()?.Parameters.Last()?.FlowAnalysisAnnotations ?? FlowAnalysisAnnotations.None; } private static bool UseLegacyWarnings(BoundExpression expr, TypeWithAnnotations exprType) diff --git a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.FieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.FieldSymbol.cs index 746f5f2e0cc71..61426ce47fb2f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.FieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.FieldSymbol.cs @@ -33,6 +33,9 @@ public override string Name get { return GeneratedNames.MakeAnonymousTypeBackingFieldName(_property.Name); } } + public override FlowAnalysisAnnotations FlowAnalysisAnnotations + => FlowAnalysisAnnotations.None; + internal override bool HasSpecialName { get { return false; } diff --git a/src/Compilers/CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/FieldWellKnownAttributeData.cs b/src/Compilers/CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/FieldWellKnownAttributeData.cs new file mode 100644 index 0000000000000..dbd35136c84f9 --- /dev/null +++ b/src/Compilers/CSharp/Portable/Symbols/Attributes/WellKnownAttributeData/FieldWellKnownAttributeData.cs @@ -0,0 +1,90 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.CodeAnalysis.CSharp.Symbols +{ + /// + /// Information decoded from well-known custom attributes applied on a field. + /// + internal sealed class FieldWellKnownAttributeData : CommonFieldWellKnownAttributeData + { + private bool _hasAllowNullAttribute; + public bool HasAllowNullAttribute + { + get + { + VerifySealed(expected: true); + return _hasAllowNullAttribute; + } + set + { + VerifySealed(expected: false); + _hasAllowNullAttribute = value; + SetDataStored(); + } + } + + private bool _hasDisallowNullAttribute; + public bool HasDisallowNullAttribute + { + get + { + VerifySealed(expected: true); + return _hasDisallowNullAttribute; + } + set + { + VerifySealed(expected: false); + _hasDisallowNullAttribute = value; + SetDataStored(); + } + } + + private bool _hasMaybeNullAttribute; + public bool HasMaybeNullAttribute + { + get + { + VerifySealed(expected: true); + return _hasMaybeNullAttribute; + } + set + { + VerifySealed(expected: false); + _hasMaybeNullAttribute = value; + SetDataStored(); + } + } + + private bool? _maybeNullWhenAttribute; + public bool? MaybeNullWhenAttribute + { + get + { + VerifySealed(expected: true); + return _maybeNullWhenAttribute; + } + set + { + VerifySealed(expected: false); + _maybeNullWhenAttribute = value; + SetDataStored(); + } + } + + private bool _hasNotNullAttribute; + public bool HasNotNullAttribute + { + get + { + VerifySealed(expected: true); + return _hasNotNullAttribute; + } + set + { + VerifySealed(expected: false); + _hasNotNullAttribute = value; + SetDataStored(); + } + } + } +} diff --git a/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs index 93d19c6bbd5fd..295d9c3776bae 100644 --- a/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs @@ -56,6 +56,8 @@ public TypeWithAnnotations TypeWithAnnotations } } + public abstract FlowAnalysisAnnotations FlowAnalysisAnnotations { get; } + /// /// Gets the type of this field. /// diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs index 7d78f0e7bd55e..61a73c2a4ca5b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs @@ -20,6 +20,49 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE /// internal sealed class PEFieldSymbol : FieldSymbol { + private struct PackedFlags + { + // Layout: + // |..............................|vvvvv| + // + // f = FlowAnalysisAnnotations. 5 bits (4 value bits + 1 completion bit). + + private const int HasDisallowNullAttribute = 0x1 << 0; + private const int HasAllowNullAttribute = 0x1 << 1; + private const int HasMaybeNullAttribute = 0x1 << 2; + private const int HasNotNullAttribute = 0x1 << 3; + private const int FlowAnalysisAnnotationsCompletionBit = 0x1 << 4; + + private int _bits; + + public bool SetFlowAnalysisAnnotations(FlowAnalysisAnnotations value) + { + Debug.Assert((value & ~(FlowAnalysisAnnotations.DisallowNull | FlowAnalysisAnnotations.AllowNull | FlowAnalysisAnnotations.MaybeNull | FlowAnalysisAnnotations.NotNull)) == 0); + + int bitsToSet = FlowAnalysisAnnotationsCompletionBit; + if ((value & FlowAnalysisAnnotations.DisallowNull) != 0) bitsToSet |= PackedFlags.HasDisallowNullAttribute; + if ((value & FlowAnalysisAnnotations.AllowNull) != 0) bitsToSet |= PackedFlags.HasAllowNullAttribute; + if ((value & FlowAnalysisAnnotations.MaybeNull) != 0) bitsToSet |= PackedFlags.HasMaybeNullAttribute; + if ((value & FlowAnalysisAnnotations.NotNull) != 0) bitsToSet |= PackedFlags.HasNotNullAttribute; + + return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet); + } + + public bool TryGetFlowAnalysisAnnotations(out FlowAnalysisAnnotations value) + { + int theBits = _bits; // Read this.bits once to ensure the consistency of the value and completion flags. + value = FlowAnalysisAnnotations.None; + if ((theBits & PackedFlags.HasDisallowNullAttribute) != 0) value |= FlowAnalysisAnnotations.DisallowNull; + if ((theBits & PackedFlags.HasAllowNullAttribute) != 0) value |= FlowAnalysisAnnotations.AllowNull; + if ((theBits & PackedFlags.HasMaybeNullAttribute) != 0) value |= FlowAnalysisAnnotations.MaybeNull; + if ((theBits & PackedFlags.HasNotNullAttribute) != 0) value |= FlowAnalysisAnnotations.NotNull; + + var result = (theBits & FlowAnalysisAnnotationsCompletionBit) != 0; + Debug.Assert(value == 0 || result); + return result; + } + } + private readonly FieldDefinitionHandle _handle; private readonly string _name; private readonly FieldAttributes _flags; @@ -36,6 +79,7 @@ internal sealed class PEFieldSymbol : FieldSymbol private int _lazyFixedSize; private NamedTypeSymbol _lazyFixedImplementationType; private PEEventSymbol _associatedEventOpt; + private PackedFlags _packedFlags; internal PEFieldSymbol( PEModuleSymbol moduleSymbol, @@ -48,6 +92,7 @@ internal PEFieldSymbol( _handle = fieldDef; _containingType = containingType; + _packedFlags = new PackedFlags(); try { @@ -272,6 +317,30 @@ internal override TypeWithAnnotations GetFieldType(ConsList fieldsB return _lazyType.Value; } + public override FlowAnalysisAnnotations FlowAnalysisAnnotations + { + get + { + FlowAnalysisAnnotations value; + if (!_packedFlags.TryGetFlowAnalysisAnnotations(out value)) + { + value = DecodeFlowAnalysisAttributes(_containingType.ContainingPEModule.Module, _handle); + _packedFlags.SetFlowAnalysisAnnotations(value); + } + return value; + } + } + + private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(PEModule module, FieldDefinitionHandle handle) + { + FlowAnalysisAnnotations annotations = FlowAnalysisAnnotations.None; + if (module.HasAttribute(handle, AttributeDescription.AllowNullAttribute)) annotations |= FlowAnalysisAnnotations.AllowNull; + if (module.HasAttribute(handle, AttributeDescription.DisallowNullAttribute)) annotations |= FlowAnalysisAnnotations.DisallowNull; + if (module.HasAttribute(handle, AttributeDescription.MaybeNullAttribute)) annotations |= FlowAnalysisAnnotations.MaybeNull; + if (module.HasAttribute(handle, AttributeDescription.NotNullAttribute)) annotations |= FlowAnalysisAnnotations.NotNull; + return annotations; + } + public override bool IsFixedSizeBuffer { get diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs index 78c93e5c9ae9e..c708d6a84af1f 100755 --- a/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs @@ -99,7 +99,7 @@ private CustomAttributesBag GetAttributesBag() /// /// Forces binding and decoding of attributes. /// - protected CommonFieldWellKnownAttributeData GetDecodedWellKnownAttributeData() + protected FieldWellKnownAttributeData GetDecodedWellKnownAttributeData() { var attributesBag = _lazyCustomAttributesBag; if (attributesBag == null || !attributesBag.IsDecodedWellKnownAttributeDataComputed) @@ -107,7 +107,7 @@ protected CommonFieldWellKnownAttributeData GetDecodedWellKnownAttributeData() attributesBag = this.GetAttributesBag(); } - return (CommonFieldWellKnownAttributeData)attributesBag.DecodedWellKnownAttributeData; + return (FieldWellKnownAttributeData)attributesBag.DecodedWellKnownAttributeData; } internal sealed override CSharpAttributeData EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments arguments) @@ -163,11 +163,11 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu if (attribute.IsTargetAttribute(this, AttributeDescription.SpecialNameAttribute)) { - arguments.GetOrCreateData().HasSpecialNameAttribute = true; + arguments.GetOrCreateData().HasSpecialNameAttribute = true; } else if (attribute.IsTargetAttribute(this, AttributeDescription.NonSerializedAttribute)) { - arguments.GetOrCreateData().HasNonSerializedAttribute = true; + arguments.GetOrCreateData().HasNonSerializedAttribute = true; } else if (attribute.IsTargetAttribute(this, AttributeDescription.FieldOffsetAttribute)) { @@ -189,12 +189,12 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu // Set field offset even if the attribute specifies an invalid value, so that // post-validation knows that the attribute is applied and reports better errors. - arguments.GetOrCreateData().SetFieldOffset(offset); + arguments.GetOrCreateData().SetFieldOffset(offset); } } else if (attribute.IsTargetAttribute(this, AttributeDescription.MarshalAsAttribute)) { - MarshalAsAttributeDecoder.Decode(ref arguments, AttributeTargets.Field, MessageProvider.Instance); + MarshalAsAttributeDecoder.Decode(ref arguments, AttributeTargets.Field, MessageProvider.Instance); } else if (attribute.IsTargetAttribute(this, AttributeDescription.DynamicAttribute)) { @@ -233,6 +233,38 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu // NullableAttribute should not be set explicitly. arguments.Diagnostics.Add(ErrorCode.ERR_ExplicitNullableAttribute, arguments.AttributeSyntaxOpt.Location); } + else if (attribute.IsTargetAttribute(this, AttributeDescription.AllowNullAttribute)) + { + arguments.GetOrCreateData().HasAllowNullAttribute = true; + } + else if (attribute.IsTargetAttribute(this, AttributeDescription.DisallowNullAttribute)) + { + arguments.GetOrCreateData().HasDisallowNullAttribute = true; + } + else if (attribute.IsTargetAttribute(this, AttributeDescription.MaybeNullAttribute)) + { + arguments.GetOrCreateData().HasMaybeNullAttribute = true; + } + else if (attribute.IsTargetAttribute(this, AttributeDescription.NotNullAttribute)) + { + arguments.GetOrCreateData().HasNotNullAttribute = true; + } + } + + public override FlowAnalysisAnnotations FlowAnalysisAnnotations + => DecodeFlowAnalysisAttributes(GetDecodedWellKnownAttributeData()); + + private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(FieldWellKnownAttributeData attributeData) + { + var annotations = FlowAnalysisAnnotations.None; + if (attributeData != null) + { + if (attributeData.HasAllowNullAttribute) annotations |= FlowAnalysisAnnotations.AllowNull; + if (attributeData.HasDisallowNullAttribute) annotations |= FlowAnalysisAnnotations.DisallowNull; + if (attributeData.HasMaybeNullAttribute) annotations |= FlowAnalysisAnnotations.MaybeNull; + if (attributeData.HasNotNullAttribute) annotations |= FlowAnalysisAnnotations.NotNull; + } + return annotations; } /// @@ -244,7 +276,7 @@ private void VerifyConstantValueMatches(ConstantValue attrValue, ref DecodeWellK { if (!attrValue.IsBad) { - var data = arguments.GetOrCreateData(); + var data = arguments.GetOrCreateData(); ConstantValue constValue; if (this.IsConst) @@ -296,7 +328,7 @@ internal override void PostDecodeWellKnownAttributes(ImmutableArray fieldsBeingBound); + public override FlowAnalysisAnnotations FlowAnalysisAnnotations + => FlowAnalysisAnnotations.None; + public override string Name { get { return _name; } diff --git a/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedFieldSymbol.cs index b6683373f062b..5b5e72fe19c5c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedFieldSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Wrapped/WrappedFieldSymbol.cs @@ -40,6 +40,11 @@ public override bool IsImplicitlyDeclared get { return _underlyingField.IsImplicitlyDeclared; } } + public override FlowAnalysisAnnotations FlowAnalysisAnnotations + { + get { return _underlyingField.FlowAnalysisAnnotations; } + } + public override Accessibility DeclaredAccessibility { get diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 07d9fbc4bfb83..56ebd7e1b761f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -19389,6 +19389,95 @@ public class CClass where TClass : class ); } + [Fact] + public void MaybeNull_Field() + { + // Warn on misused nullability attributes (f2, f3, f4, f5)? https://github.com/dotnet/roslyn/issues/36073 + var lib_cs = +@"using System.Diagnostics.CodeAnalysis; +public class COpen +{ + [MaybeNull]public TOpen f1 = default!; +} +public class CClass where TClass : class +{ + [MaybeNull]public TClass f2 = null!; + [MaybeNull]public TClass? f3 = null; +} +public class CStruct where TStruct : struct +{ + [MaybeNull]public TStruct f4; + [MaybeNull]public TStruct? f5; +} +"; + + var lib = CreateNullableCompilation(new[] { MaybeNullAttributeDefinition, lib_cs }); + + var source = @" +class C +{ + static void M1(T t1) + { + new COpen().f1.ToString(); // 1 + } + static void M2() where T : class + { + new COpen().f1.ToString(); // 2 + new CClass().f2.ToString(); // 3 + new CClass().f3.ToString(); // 4 + } + static void M4(T t4) where T : struct + { + new COpen().f1.ToString(); + new CStruct().f4.ToString(); + new CStruct().f5.Value.ToString(); // 5 + } +}"; + var expected = new[] + { + // (6,9): warning CS8602: Dereference of a possibly null reference. + // new COpen().f1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "new COpen().f1").WithLocation(6, 9), + // (10,9): warning CS8602: Dereference of a possibly null reference. + // new COpen().f1.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "new COpen().f1").WithLocation(10, 9), + // (11,9): warning CS8602: Dereference of a possibly null reference. + // new CClass().f2.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "new CClass().f2").WithLocation(11, 9), + // (12,9): warning CS8602: Dereference of a possibly null reference. + // new CClass().f3.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "new CClass().f3").WithLocation(12, 9), + // (18,9): warning CS8629: Nullable value type may be null. + // new CStruct().f5.Value.ToString(); // 5 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "new CStruct().f5").WithLocation(18, 9) + }; + + var comp = CreateNullableCompilation(source, references: new[] { lib.EmitToImageReference() }); + comp.VerifyDiagnostics(expected); + + var comp2 = CreateNullableCompilation(new[] { source, lib_cs, MaybeNullAttributeDefinition }); + comp2.VerifyDiagnostics(expected); + + CompileAndVerify(comp2, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol m) + { + bool isSource = !(m is PEModuleSymbol); + var copen = (NamedTypeSymbol)m.GlobalNamespace.GetMember("COpen"); + var field = copen.GetMembers().OfType().Single(); + + var fieldAttributes = field.GetAttributes().Select(a => a.ToString()); + if (isSource) + { + AssertEx.Equal(new[] { "System.Diagnostics.CodeAnalysis.MaybeNullAttribute" }, fieldAttributes); + } + else + { + AssertEx.Equal(new[] { "System.Runtime.CompilerServices.NullableAttribute(1)", "System.Diagnostics.CodeAnalysis.MaybeNullAttribute" }, fieldAttributes); + } + } + } + [Fact] public void MaybeNullWhenTrue_OnImplicitReferenceConversion() { @@ -24844,6 +24933,121 @@ static void M5(T? t5) where T : struct comp2.VerifyDiagnostics(expected); } + [Fact] + public void AllowNull_Field() + { + // Warn on misused nullability attributes (f3, f4, f5)? https://github.com/dotnet/roslyn/issues/36073 + var lib_cs = +@"using System.Diagnostics.CodeAnalysis; +public class COpen +{ + [AllowNull]public TOpen f1 = default; +} +public class CClass where TClass : class +{ + [AllowNull]public TClass f2 = null; + [AllowNull]public TClass? f3 = null; +} +public class CStruct where TStruct : struct +{ + [AllowNull]public TStruct f4; + [AllowNull]public TStruct? f5; +} +"; + + var lib = CreateNullableCompilation(new[] { AllowNullAttributeDefinition, lib_cs }); + + var source = @" +class C +{ + static void M1(T t1) + { + new COpen().f1 = t1; + } + static void M2(T t2) where T : class + { + new COpen().f1 = t2; + new CClass().f2 = t2; + new CClass().f3 = t2; + t2 = null; // 1 + new COpen().f1 = t2; + new CClass().f2 = t2; + new CClass().f3 = t2; + } + static void M3(T? t3) where T : class + { + new COpen().f1 = t3; + new CClass().f2 = t3; + new CClass().f3 = t3; + if (t3 == null) return; + new COpen().f1 = t3; + new CClass().f2 = t3; + new CClass().f3 = t3; + } + static void M4(T t4) where T : struct + { + new COpen().f1 = t4; + new CStruct().f4 = t4; + } + static void M5(T? t5) where T : struct + { + new COpen().f1 = t5; + new CStruct().f5 = t5; + if (t5 == null) return; + new COpen().f1 = t5; + new CStruct().f5 = t5; + } +}"; + var expected = new[] + { + // (13,14): warning CS8600: Converting null literal or possible null value to non-nullable type. + // t2 = null; // 1 + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(13, 14) + }; + + var comp = CreateNullableCompilation(source, references: new[] { lib.EmitToImageReference() }); + comp.VerifyDiagnostics(expected); + + var comp2 = CreateNullableCompilation(new[] { source, lib_cs, AllowNullAttributeDefinition }); + comp2.VerifyDiagnostics(expected); + + CompileAndVerify(comp2, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol m) + { + bool isSource = !(m is PEModuleSymbol); + var copen = (NamedTypeSymbol)m.GlobalNamespace.GetMember("COpen"); + var field = copen.GetMembers().OfType().Single(); + + var fieldAttributes = field.GetAttributes().Select(a => a.ToString()); + if (isSource) + { + AssertEx.Equal(new[] { "System.Diagnostics.CodeAnalysis.AllowNullAttribute" }, fieldAttributes); + } + else + { + AssertEx.Equal(new[] { "System.Runtime.CompilerServices.NullableAttribute(1)", "System.Diagnostics.CodeAnalysis.AllowNullAttribute" }, fieldAttributes); + } + } + } + + [Fact] + public void AllowNull_Field_NullInitializer() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [AllowNull]public string field = null; + + string M() => field.ToString(); +} +"; + + var comp = CreateNullableCompilation(new[] { AllowNullAttributeDefinition, source }); + comp.VerifyDiagnostics(); + } + [Fact] public void AllowNull_Operator_CompoundAssignment() { @@ -25743,7 +25947,7 @@ static void M1(string? s, string s2) comp2.VerifyDiagnostics(expected); } - [Fact] + [Fact, WorkItem(36703, "https://github.com/dotnet/roslyn/issues/36703")] public void DisallowNull_RefReturnValue_05() { var source0 = @@ -26020,6 +26224,217 @@ public class C comp.VerifyDiagnostics(); } + [Fact] + public void DisallowNull_Field() + { + // Warn on misused nullability attributes (f2, f3, f4)? https://github.com/dotnet/roslyn/issues/36073 + var lib_cs = +@"using System.Diagnostics.CodeAnalysis; +public class COpen +{ + [DisallowNull]public TOpen f1 = default!; +} +public class CClass where TClass : class +{ + [DisallowNull]public TClass f2 = null!; + [DisallowNull]public TClass? f3 = null!; +} +public class CStruct where TStruct : struct +{ + [DisallowNull]public TStruct f4; + [DisallowNull]public TStruct? f5; +} +"; + + var lib = CreateNullableCompilation(new[] { DisallowNullAttributeDefinition, lib_cs }); + + var source = @" +class C +{ + static void M1(T t1) + { + new COpen().f1 = t1; + } + static void M2(T t2) where T : class + { + new COpen().f1 = t2; + new CClass().f2 = t2; + new CClass().f3 = t2; + t2 = null; // 1 + new COpen().f1 = t2; // 2 + new CClass().f2 = t2; // 3 + new CClass().f3 = t2; // 4 + } + static void M3(T? t3) where T : class + { + new COpen().f1 = t3; // 5 + new CClass().f2 = t3; // 6 + new CClass().f3 = t3; // 7 + if (t3 == null) return; + new COpen().f1 = t3; + new CClass().f2 = t3; + new CClass().f3 = t3; + } + static void M4(T t4) where T : struct + { + new COpen().f1 = t4; + new CStruct().f4 = t4; + } + static void M5(T? t5) where T : struct + { + new COpen().f1 = t5; // 8 + new CStruct().f5 = t5; // 9 + if (t5 == null) return; + new COpen().f1 = t5; + new CStruct().f5 = t5; + } + static void M6([System.Diagnostics.CodeAnalysis.MaybeNull]T t6) where T : class + { + new CClass().f2 = t6; + } + static void M7([System.Diagnostics.CodeAnalysis.NotNull]T? t7) where T : class + { + new CClass().f2 = t7; // 10 + } + static void M8([System.Diagnostics.CodeAnalysis.AllowNull]T t8) where T : class + { + new CClass().f2 = t8; // no warning, we currently don't honor the attribute on t6 within the method body + } +}"; + // we currently don't honor the attribute on t6 within the method body https://github.com/dotnet/roslyn/issues/36039 + + var expected = new[] + { + // (13,14): warning CS8600: Converting null literal or possible null value to non-nullable type. + // t2 = null; // 1 + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(13, 14), + // (14,29): warning CS8601: Possible null reference assignment. + // new COpen().f1 = t2; // 2 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t2").WithLocation(14, 29), + // (15,30): warning CS8601: Possible null reference assignment. + // new CClass().f2 = t2; // 3 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t2").WithLocation(15, 30), + // (16,30): warning CS8601: Possible null reference assignment. + // new CClass().f3 = t2; // 4 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t2").WithLocation(16, 30), + // (20,29): warning CS8601: Possible null reference assignment. + // new COpen().f1 = t3; // 5 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t3").WithLocation(20, 29), + // (21,30): warning CS8601: Possible null reference assignment. + // new CClass().f2 = t3; // 6 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t3").WithLocation(21, 30), + // (22,30): warning CS8601: Possible null reference assignment. + // new CClass().f3 = t3; // 7 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t3").WithLocation(22, 30), + // (35,30): warning CS8607: A possible null value may not be passed to a target marked with the [DisallowNull] attribute + // new COpen().f1 = t5; // 8 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "t5").WithLocation(35, 30), + // (36,31): warning CS8607: A possible null value may not be passed to a target marked with the [DisallowNull] attribute + // new CStruct().f5 = t5; // 9 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "t5").WithLocation(36, 31), + // (47,30): warning CS8601: Possible null reference assignment. + // new CClass().f2 = t7; // 10 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "t7").WithLocation(47, 30) + }; + + var comp = CreateNullableCompilation(new[] { source, AllowNullAttributeDefinition, MaybeNullAttributeDefinition, NotNullAttributeDefinition }, + references: new[] { lib.EmitToImageReference() }); + comp.VerifyDiagnostics(expected); + + var comp2 = CreateNullableCompilation(new[] { source, lib_cs, DisallowNullAttributeDefinition, AllowNullAttributeDefinition, MaybeNullAttributeDefinition, NotNullAttributeDefinition }); + comp2.VerifyDiagnostics(expected); + + CompileAndVerify(comp2, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol m) + { + bool isSource = !(m is PEModuleSymbol); + var copen = (NamedTypeSymbol)m.GlobalNamespace.GetMember("COpen"); + var field = copen.GetMembers().OfType().Single(); + + var fieldAttributes = field.GetAttributes().Select(a => a.ToString()); + if (isSource) + { + AssertEx.Equal(new[] { "System.Diagnostics.CodeAnalysis.DisallowNullAttribute" }, fieldAttributes); + } + else + { + AssertEx.Equal(new[] { "System.Runtime.CompilerServices.NullableAttribute(1)", "System.Diagnostics.CodeAnalysis.DisallowNullAttribute" }, fieldAttributes); + } + } + } + + [Fact] + public void DisallowNull_AndOtherAnnotations_StaticField() + { + var lib_cs = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [DisallowNull]public static string? disallowNull = null!; + [AllowNull]public static string allowNull = null; + [MaybeNull]public static string maybeNull = null!; + [NotNull]public static string? notNull = null; +} +"; + + var lib = CreateNullableCompilation(new[] { DisallowNullAttributeDefinition, AllowNullAttributeDefinition, MaybeNullAttributeDefinition, NotNullAttributeDefinition, lib_cs }); + + var source = @" +class D +{ + static void M() + { + C.disallowNull = null; // 1 + C.allowNull = null; + C.maybeNull.ToString(); // 2 + C.notNull.ToString(); + } +}"; + var expected = new[] + { + // (6,26): warning CS8625: Cannot convert null literal to non-nullable reference type. + // C.disallowNull = null; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 26), + // (8,9): warning CS8602: Dereference of a possibly null reference. + // C.maybeNull.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "C.maybeNull").WithLocation(8, 9) + }; + + var comp = CreateNullableCompilation(source, references: new[] { lib.EmitToImageReference() }); + comp.VerifyDiagnostics(expected); + + var comp2 = CreateNullableCompilation(new[] { source, lib_cs, DisallowNullAttributeDefinition, AllowNullAttributeDefinition, MaybeNullAttributeDefinition, NotNullAttributeDefinition }); + comp2.VerifyDiagnostics(expected); + } + + [Fact] + public void DisallowNull_Field_NullInitializer() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [DisallowNull]public string? field = null; // 1 + + void M() + { + field.ToString(); // 2 + } +} +"; + + var comp = CreateNullableCompilation(new[] { DisallowNullAttributeDefinition, source }); + comp.VerifyDiagnostics( + // (4,42): warning CS8625: Cannot convert null literal to non-nullable reference type. + // [DisallowNull]public string? field = null; // 1 + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(4, 42), + // (8,9): warning CS8602: Dereference of a possibly null reference. + // field.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field").WithLocation(8, 9) + ); + } + [Fact] public void AllowNull_Parameter_NullDefaultValue() { @@ -28750,6 +29165,77 @@ public class CClass2 where TClass : class comp.VerifyDiagnostics(); } + [Fact] + public void NotNull_Field() + { + // Warn on misused nullability attributes (f2, f3, f4, f5)? https://github.com/dotnet/roslyn/issues/36073 + var lib_cs = +@"using System.Diagnostics.CodeAnalysis; +public class COpen +{ + [NotNull]public TOpen f1 = default!; +} +public class CClass where TClass : class +{ + [NotNull]public TClass f2 = null!; + [NotNull]public TClass? f3 = null; +} +public class CStruct where TStruct : struct +{ + [NotNull]public TStruct f4; + [NotNull]public TStruct? f5; +} +"; + + var lib = CreateNullableCompilation(new[] { NotNullAttributeDefinition, lib_cs }); + + var source = @" +class C +{ + static void M1(T t1) + { + new COpen().f1.ToString(); + } + static void M2() where T : class + { + new COpen().f1.ToString(); + new CClass().f2.ToString(); + new CClass().f3.ToString(); + } + static void M4(T t4) where T : struct + { + new COpen().f1.ToString(); + new CStruct().f4.ToString(); + new CStruct().f5.Value.ToString(); + } +}"; + + var comp = CreateNullableCompilation(source, references: new[] { lib.EmitToImageReference() }); + comp.VerifyDiagnostics(); + + var comp2 = CreateNullableCompilation(new[] { source, lib_cs, NotNullAttributeDefinition }); + comp2.VerifyDiagnostics(); + + CompileAndVerify(comp2, sourceSymbolValidator: symbolValidator, symbolValidator: symbolValidator); + + void symbolValidator(ModuleSymbol m) + { + bool isSource = !(m is PEModuleSymbol); + var copen = (NamedTypeSymbol)m.GlobalNamespace.GetMember("COpen"); + var field = copen.GetMembers().OfType().Single(); + + var fieldAttributes = field.GetAttributes().Select(a => a.ToString()); + if (isSource) + { + AssertEx.Equal(new[] { "System.Diagnostics.CodeAnalysis.NotNullAttribute" }, fieldAttributes); + } + else + { + AssertEx.Equal(new[] { "System.Runtime.CompilerServices.NullableAttribute(1)", "System.Diagnostics.CodeAnalysis.NotNullAttribute" }, fieldAttributes); + } + } + } + [Fact, WorkItem(35949, "https://github.com/dotnet/roslyn/issues/35949")] public void NotNull_Complexity() { diff --git a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/DisplayClassVariable.cs b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/DisplayClassVariable.cs index e41c753df4e77..1fee5ac3282e1 100644 --- a/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/DisplayClassVariable.cs +++ b/src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/DisplayClassVariable.cs @@ -155,6 +155,11 @@ public override bool IsVolatile get { return false; } } + public override FlowAnalysisAnnotations FlowAnalysisAnnotations + { + get { return FlowAnalysisAnnotations.None; } + } + public override ImmutableArray Locations { get { throw ExceptionUtilities.Unreachable; }