From 92c6bee67eedaf0605f6d8be30f21bda6064c9da Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Wed, 9 Dec 2020 13:02:01 -0800 Subject: [PATCH] Test SemanticModel --- .../Portable/Compilation/CSharpCompilation.cs | 4 ++ .../Compilation/MemberSemanticModel.cs | 10 +++- .../Portable/FlowAnalysis/NullableWalker.cs | 13 +---- .../Semantics/NullableContextTests.cs | 55 +++++++++++++++++++ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 87b8ddaa7e16a..8e598a014c5f6 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -246,6 +246,10 @@ ThreeState calculateResult() } } +#if DEBUG + internal bool ShouldRunNullableWalkerInDebug => Feature("nullableAnalysis") != "false"; +#endif + /// /// The language version that was used to parse the syntax trees of this compilation. /// diff --git a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs index bb935344424ef..5da82cd7e47b8 100644 --- a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs @@ -1730,7 +1730,10 @@ private BoundNode GetBoundLambdaOrQuery(CSharpSyntaxNode lambdaOrQuery) // https://github.com/dotnet/roslyn/issues/35038: We need to do a rewrite here, and create a test that can hit this. #if DEBUG - AnalyzeBoundNodeNullability(boundOuterExpression, incrementalBinder, diagnostics: new DiagnosticBag(), createSnapshots: false); + if (Compilation.ShouldRunNullableWalkerInDebug) + { + AnalyzeBoundNodeNullability(boundOuterExpression, incrementalBinder, diagnostics: new DiagnosticBag(), createSnapshots: false); + } #endif nodes = GuardedAddBoundTreeAndGetBoundNodeFromMap(lambdaOrQuery, boundOuterExpression); @@ -2003,7 +2006,10 @@ void rewriteAndCache(DiagnosticBag diagnosticBag) #if DEBUG if (!Compilation.NullableSemanticAnalysisEnabled) { - AnalyzeBoundNodeNullability(boundRoot, binder, diagnosticBag, createSnapshots: true); + if (Compilation.ShouldRunNullableWalkerInDebug) + { + AnalyzeBoundNodeNullability(boundRoot, binder, diagnosticBag, createSnapshots: true); + } return; } #endif diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 0e25e441cd270..362ecfd486e4c 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -1075,7 +1075,7 @@ internal static void AnalyzeIfNeeded( if (compilation.LanguageVersion < MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() || !compilation.ShouldRunNullableWalker) { #if DEBUG - if (ShouldRunAnalysisInDebug(compilation)) + if (compilation.ShouldRunNullableWalkerInDebug) { // Always run analysis in debug builds so that we can more reliably catch // nullable regressions e.g. https://github.com/dotnet/roslyn/issues/40136 @@ -1092,13 +1092,6 @@ internal static void AnalyzeIfNeeded( Analyze(compilation, method, node, diagnostics, useConstructorExitWarnings, initialNullableState, getFinalNullableState, out finalNullableState); } -#if DEBUG - private static bool ShouldRunAnalysisInDebug(CSharpCompilation compilation) - { - return compilation.Feature("nullableAnalysis") != "false"; - } -#endif - internal static void Analyze( CSharpCompilation compilation, MethodSymbol method, @@ -1305,7 +1298,7 @@ internal static bool NeedsAnalysis(CSharpCompilation compilation) #if DEBUG // Always run analysis in debug builds so that we can more reliably catch // nullable regressions e.g. https://github.com/dotnet/roslyn/issues/40136 - return ShouldRunAnalysisInDebug(compilation); + return compilation.ShouldRunNullableWalkerInDebug; #else var canSkipAnalysis = compilation.LanguageVersion < MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() || !compilation.ShouldRunNullableWalker; return !canSkipAnalysis; @@ -1322,7 +1315,7 @@ internal static void AnalyzeIfNeeded( if (compilation.LanguageVersion < MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() || !compilation.ShouldRunNullableWalker) { #if DEBUG - if (ShouldRunAnalysisInDebug(compilation)) + if (compilation.ShouldRunNullableWalkerInDebug) { // Always run analysis in debug builds so that we can more reliably catch // nullable regressions e.g. https://github.com/dotnet/roslyn/issues/40136 diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableContextTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableContextTests.cs index 3a33dfdda89d3..386bad0a58f28 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableContextTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableContextTests.cs @@ -305,6 +305,61 @@ void verify(CSharpParseOptions parseOptions, params string[] expectedAnalyzedKey } } + [Fact] + public void NullableAnalysisFlags_04() + { + var source = +@"#nullable enable +class Program +{ + static object F(object? obj) + { + if (obj == null) return null; // 1 + return obj; + } +}"; + + // https://github.com/dotnet/roslyn/issues/49746: Currently, if we analyze any members, we analyze all. + var expectedAnalyzedKeysAll = new[] { ".cctor", ".ctor", "F" }; + + verify(parseOptions: TestOptions.Regular, expectedFlowState: true, expectedAnalyzedKeysAll); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", null), expectedFlowState: true, expectedAnalyzedKeysAll); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", "true"), expectedFlowState: true, expectedAnalyzedKeysAll); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", "false"), expectedFlowState: false); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", "false").WithFeature("run-nullable-analysis", "false"), expectedFlowState: false); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", "false").WithFeature("run-nullable-analysis", "true"), expectedFlowState: true, "F"); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", "true").WithFeature("run-nullable-analysis", "false"), expectedFlowState: false, expectedAnalyzedKeysAll); + verify(parseOptions: TestOptions.Regular.WithFeature("nullableAnalysis", "true").WithFeature("run-nullable-analysis", "true"), expectedFlowState: true, expectedAnalyzedKeysAll); + + void verify(CSharpParseOptions parseOptions, bool expectedFlowState, params string[] expectedAnalyzedKeys) + { + var comp = CreateCompilation(source, parseOptions: parseOptions); + comp.NullableAnalysisData = new ConcurrentDictionary(); + if (expectedAnalyzedKeys.Length > 0) + { + comp.VerifyDiagnostics( + // (6,33): warning CS8603: Possible null reference return. + // if (obj == null) return null; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReturn, "null").WithLocation(6, 33)); + } + else + { + comp.VerifyDiagnostics(); + } + + var syntaxTree = comp.SyntaxTrees[0]; + var model = comp.GetSemanticModel(syntaxTree); + var statement = syntaxTree.GetRoot().DescendantNodes().OfType().Skip(1).Single(); + Assert.Equal("return obj;", statement.ToString()); + var typeInfo = model.GetTypeInfo(statement.Expression); + var expectedNullability = expectedFlowState ? Microsoft.CodeAnalysis.NullableFlowState.NotNull : Microsoft.CodeAnalysis.NullableFlowState.None; + Assert.Equal(expectedNullability, typeInfo.Nullability.FlowState); + + var actualAnalyzedKeys = GetNullableDataKeysAsStrings(comp.NullableAnalysisData); + AssertEx.Equal(expectedAnalyzedKeys, actualAnalyzedKeys); + } + } + private static string[] GetNullableDataKeysAsStrings(ConcurrentDictionary nullableData) => nullableData.Keys.Select(key => GetNullableDataKeyAsString(key)).OrderBy(key => key).ToArray();