From 4bdc40d37fda334591e7099c9379a537f198ba81 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Sat, 5 May 2018 12:06:34 -0700 Subject: [PATCH] Make IsNullOrEmpty well-known --- .../Portable/FlowAnalysis/NullableWalker.cs | 45 ++++++++ .../Semantic/Semantics/StaticNullChecking.cs | 100 ++++++++++++++++++ .../Core/Portable/WellKnownMember.cs | 1 + .../Core/Portable/WellKnownMembers.cs | 10 ++ src/Compilers/Core/Portable/WellKnownTypes.cs | 2 + 5 files changed, 158 insertions(+) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index f75c50eaab5d6..d50bd3fe4074a 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -1579,6 +1579,13 @@ public override BoundNode VisitCall(BoundCall node) VisitTrueWhenExits(condition: arguments[0]); } + if (MethodReturnsTrueWhenNotNull(method) && arguments.Length > 0) + { + VisitTrueWhenNotNull(arguments[0], method.ReturnType.TypeSymbol); + _result = method.ReturnType; + return null; + } + Debug.Assert(!IsConditionalState); //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability? { @@ -1588,6 +1595,44 @@ public override BoundNode VisitCall(BoundCall node) return null; } + private bool MethodReturnsTrueWhenNotNull(MethodSymbol method) + { + if (method.ReturnType.SpecialType != SpecialType.System_Boolean) + { + return false; + } + + return method.Equals(compilation.GetWellKnownTypeMember(WellKnownMember.System_String_IsNullOrEmpty)); + } + + private void VisitTrueWhenNotNull(BoundExpression operand, TypeSymbol boolType) + { + if (!operand.Type.IsReferenceType) + { + return; + } + + // operand == null || someUnknownBoolean + BoundExpression equivalent = LogicalOr( + new BoundBinaryOperator(operand.Syntax, BinaryOperatorKind.Equal, operand, + new BoundLiteral(operand.Syntax, ConstantValue.Null, operand.Type), + constantValueOpt: null, methodOpt: null, LookupResultKind.Viable, boolType), + new BoundLiteral(operand.Syntax, constantValueOpt: null, boolType)); + + // We only need to update the null-state, so we'll do the flow analysis without diagnostics + bool savedDisableDiagnostics = _disableDiagnostics; + _disableDiagnostics = true; + + Visit(equivalent); + + _disableDiagnostics = savedDisableDiagnostics; + } + + internal static BoundBinaryOperator LogicalOr(BoundExpression left, BoundExpression right) + { + return new BoundBinaryOperator(left.Syntax, BinaryOperatorKind.LogicalBoolOr, left, right, constantValueOpt: null, methodOpt: null, LookupResultKind.Viable, left.Type); + } + private bool MethodEnsuresTrueWhenExits(MethodSymbol method) { return method.Equals(compilation.GetWellKnownTypeMember(WellKnownMember.System_Diagnostics_Debug_Assert1)) || diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs index f6b0d8b0373ce..810912095f4a9 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/StaticNullChecking.cs @@ -5615,6 +5615,106 @@ void Main(C? c) ); } + [Fact] + public void Wellknown_String_IsNullOrEmpty() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Main(string? s) + { + if (string.IsNullOrEmpty(s)) + { + s.ToString(); // warn + } + else + { + s.ToString(); // ok + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (8,13): warning CS8602: Possible dereference of a null reference. + // s.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(8, 13) + ); + } + + [Fact] + public void Wellknown_String_NotIsNullOrEmpty() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Main(string? s) + { + if (!string.IsNullOrEmpty(s)) + { + s.ToString(); // ok + } + else + { + s.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (12,13): warning CS8602: Possible dereference of a null reference. + // s.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(12, 13) + ); + } + + [Fact] + public void Wellknown_String_NotIsNullOrEmpty_NoDuplicateWarnings() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void M() + { + if (!string.IsNullOrEmpty(M2(null))) + { + } + } + string? M2(string s) => throw null; +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (6,38): warning CS8625: Cannot convert null literal to non-nullable reference or unconstrained type parameter. + // if (!string.IsNullOrEmpty(M2(null))) + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(6, 38) + ); + } + + [Fact] + public void Wellknown_String_NotIsNullOrEmpty_NotAString() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void M() + { + if (!string.IsNullOrEmpty(M2(null))) + { + } + } + void M2(string s) => throw null; +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (6,35): error CS1503: Argument 1: cannot convert from 'void' to 'string' + // if (!string.IsNullOrEmpty(M2(null))) + Diagnostic(ErrorCode.ERR_BadArgType, "M2(null)").WithArguments("1", "void", "string").WithLocation(6, 35) + ); + } + [Fact] public void ConditionalBranching_01() { diff --git a/src/Compilers/Core/Portable/WellKnownMember.cs b/src/Compilers/Core/Portable/WellKnownMember.cs index 003a947844bda..95f375f195cc5 100644 --- a/src/Compilers/Core/Portable/WellKnownMember.cs +++ b/src/Compilers/Core/Portable/WellKnownMember.cs @@ -439,6 +439,7 @@ internal enum WellKnownMember System_Diagnostics_Debug_Assert2, System_Diagnostics_Debug_Assert3, System_Diagnostics_Debug_Assert4, + System_String_IsNullOrEmpty, Count } diff --git a/src/Compilers/Core/Portable/WellKnownMembers.cs b/src/Compilers/Core/Portable/WellKnownMembers.cs index a2dd186e2a505..f0c5a524e8d1f 100644 --- a/src/Compilers/Core/Portable/WellKnownMembers.cs +++ b/src/Compilers/Core/Portable/WellKnownMembers.cs @@ -3028,6 +3028,15 @@ static WellKnownMembers() (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_String, (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_String, (byte)SignatureTypeCode.SZArray, (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_Object, + + // System_String_IsNullOrEmpty + (byte)(MemberFlags.Method | MemberFlags.Static), // Flags + (byte)WellKnownType.ExtSentinel, (byte)(WellKnownType.System_String - WellKnownType.ExtSentinel), // DeclaringTypeId + 0, // Arity + 1, // Method Signature + (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_Boolean, + (byte)SignatureTypeCode.TypeHandle, (byte)SpecialType.System_String, + }; string[] allNames = new string[(int)WellKnownMember.Count] @@ -3405,6 +3414,7 @@ static WellKnownMembers() "Assert", // System_Diagnostics_Debug_Assert2 "Assert", // System_Diagnostics_Debug_Assert3 "Assert", // System_Diagnostics_Debug_Assert4 + "IsNullOrEmpty", // System_IsNullOrEmpty }; s_descriptors = MemberDescriptor.InitializeFromStream(new System.IO.MemoryStream(initializationBytes, writable: false), allNames); diff --git a/src/Compilers/Core/Portable/WellKnownTypes.cs b/src/Compilers/Core/Portable/WellKnownTypes.cs index c8b9e58deab35..72a959955ade0 100644 --- a/src/Compilers/Core/Portable/WellKnownTypes.cs +++ b/src/Compilers/Core/Portable/WellKnownTypes.cs @@ -276,6 +276,7 @@ internal enum WellKnownType System_Runtime_CompilerServices_IsUnmanagedAttribute, System_Diagnostics_Debug, + System_String, NextAvailable, } @@ -547,6 +548,7 @@ internal static class WellKnownTypes "System.Runtime.CompilerServices.IsUnmanagedAttribute", "System.Diagnostics.Debug", + "System.String", }; private readonly static Dictionary s_nameToTypeIdMap = new Dictionary((int)Count);