diff --git a/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs b/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs index 937c5f41d3ec708294949177c785fea3472938cc..75415e744376b13aedeafa32cc1cfc5199275a17 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs @@ -101,7 +101,7 @@ public TypeWithAnnotations GetInferredReturnType(ConversionsBase conversions, Nu useSiteDiagnostics.Add(info); } } - if (nullableState == null) + if (nullableState == null || !Binder.Compilation.RunNullableWalker) { return InferredReturnType.TypeWithAnnotations; } diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs index 909f1b9590dddf2ab7325e2e0ae36d2e0c834084..cfae4be9352a7fcd166b59ac6eb36157751ae008 100644 --- a/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs +++ b/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs @@ -120,6 +120,12 @@ internal Conversions Conversions /// private HashSet _lazyCompilationUnitCompletedTrees; + /// + /// Run the nullable walker during the flow analysis passes. True if the project-level nullable + /// context option is set, or if any file file enables nullable or just the nullable warnings. + /// + private ThreeState _lazyRunNullableWalker; + public override string Language { get @@ -171,9 +177,12 @@ internal override CommonAnonymousTypeManager CommonAnonymousTypeManager internal bool FeatureStrictEnabled => Feature("strict") != null; /// - /// True if we should enable nullable analysis in this compilation. + /// True if we should enable nullable semantic analysis in this compilation. /// - internal bool NullableAnalysisEnabled => Feature("run-nullable-analysis") is "true"; + // Note that this is intentionally not conditioned off RunNullableWalker currently. Once we fully enable + // the nullable analysis semantic model feature and feel confident in it, we only run the analysis if + // the feature flag is set. + internal bool NullableSemanticAnalysisEnabled => Feature("run-nullable-analysis") is "true"; /// /// True when the "peverify-compat" feature flag is set or the language version is below C# 7.2. @@ -183,6 +192,34 @@ internal override CommonAnonymousTypeManager CommonAnonymousTypeManager /// internal bool IsPeVerifyCompatEnabled => LanguageVersion < LanguageVersion.CSharp7_2 || Feature("peverify-compat") != null; + internal bool RunNullableWalker + { + get + { + if (!_lazyRunNullableWalker.HasValue()) + { + if (Options.NullableContextOptions != NullableContextOptions.Disable) + { + _lazyRunNullableWalker = ThreeState.True; + return true; + } + + foreach (var syntaxTree in SyntaxTrees) + { + if (((CSharpSyntaxTree)syntaxTree).HasNullableEnables()) + { + _lazyRunNullableWalker = ThreeState.True; + return true; + } + } + + _lazyRunNullableWalker = ThreeState.False; + } + + return _lazyRunNullableWalker.Value(); + } + } + /// /// 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 53297cf2d75c0756a607a3676fe0ef04fbedb515..38871d1802bdf2fa342fd33b3733812313a20c02 100644 --- a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs @@ -1443,8 +1443,8 @@ protected void GuardedAddBoundTreeForStandaloneSyntax(SyntaxNode syntax, BoundNo // cached nodes for the root syntax node, and the existing snapshot manager must be null. Debug.Assert((IsSpeculativeSemanticModel && manager is null) || (!IsSpeculativeSemanticModel && - (manager is null && (!Compilation.NullableAnalysisEnabled || syntax != Root)) || - (manager is object && syntax == Root && Compilation.NullableAnalysisEnabled && _lazySnapshotManager is null))); + (manager is null && (!Compilation.NullableSemanticAnalysisEnabled || syntax != Root)) || + (manager is object && syntax == Root && Compilation.NullableSemanticAnalysisEnabled && _lazySnapshotManager is null))); if (!IsSpeculativeSemanticModel && manager is object) { _lazySnapshotManager = manager; @@ -1846,7 +1846,7 @@ protected void EnsureRootBoundForNullabilityIfNecessary() DiagnosticBag diagnostics = _ignoredDiagnostics; // If we're in DEBUG mode, always enable the analysis, but throw away the results - if (!Compilation.NullableAnalysisEnabled) + if (!Compilation.NullableSemanticAnalysisEnabled) { #if DEBUG diagnostics = new DiagnosticBag(); @@ -1871,7 +1871,7 @@ protected void EnsureRootBoundForNullabilityIfNecessary() // In DEBUG mode, we don't want to increase test run times, so if // nullable analysis isn't enabled and some node has already been bound // we assume we've already done this test binding and just return - || (!Compilation.NullableAnalysisEnabled && _guardedNodeMap.Count > 0) + || (!Compilation.NullableSemanticAnalysisEnabled && _guardedNodeMap.Count > 0) #endif ) { @@ -1914,7 +1914,7 @@ void bindAndRewrite(bool takeSnapshots) boundRoot = RewriteNullableBoundNodesWithSnapshots(boundRoot, binder, diagnostics, takeSnapshots, out var snapshotManager); #if DEBUG // Don't actually cache the results if the nullable analysis is not enabled in debug mode. - if (!Compilation.NullableAnalysisEnabled) return; + if (!Compilation.NullableSemanticAnalysisEnabled) return; #endif GuardedAddBoundTreeForStandaloneSyntax(bindableRoot, boundRoot, snapshotManager); } diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs index 2d6ccfbdc5579a2ce674d2243469aceb9737af54..6c79a58eec2f6f07a4d9c9967617af275c15e332 100644 --- a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs +++ b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs @@ -1651,7 +1651,7 @@ internal static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationSt bodyBinder.ValidateIteratorMethods(diagnostics); BoundNode methodBody = bodyBinder.BindMethodBody(syntaxNode, diagnostics); - if (bodyBinder.Compilation.NullableAnalysisEnabled) + if (bodyBinder.Compilation.NullableSemanticAnalysisEnabled) { // Currently, we're passing an empty DiagnosticBag here because the flow analysis pass later will // also run the nullable walker, and issue duplicate warnings. We should try to only run the pass diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs index 1a31117085c97e4db6ed2d03a063f1d12767c6e4..80bf4ed36e7dd3e7d94f796a6f541a6965363c59 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs @@ -376,7 +376,7 @@ public static void Analyze(CSharpCompilation compilation, MethodSymbol member, B walker.Free(); } - if (compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion()) + if (compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() && compilation.RunNullableWalker) { NullableWalker.Analyze(compilation, member, node, diagnostics); } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index c568a44bf4f1630e5994622aa2b2183fe3072b51..450bc2da756badb349fcbeab7d323703052ed7ea 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -475,7 +475,7 @@ protected override ImmutableArray Scan(ref bool badRegion) #if DEBUG // https://github.com/dotnet/roslyn/issues/34993 Enable for all calls - if (compilation.NullableAnalysisEnabled) + if (compilation.NullableSemanticAnalysisEnabled) { DebugVerifier.Verify(analyzedNullabilitiesMap, snapshotManager, node); } @@ -496,7 +496,7 @@ protected override ImmutableArray Scan(ref bool badRegion) var analyzedNullabilitiesMap = analyzedNullabilities.ToImmutable(); #if DEBUG - if (binder.Compilation.NullableAnalysisEnabled) + if (binder.Compilation.NullableSemanticAnalysisEnabled) { DebugVerifier.Verify(analyzedNullabilitiesMap, snapshotManagerOpt: null, node); } @@ -511,7 +511,7 @@ protected override ImmutableArray Scan(ref bool badRegion) DiagnosticBag diagnostics) { var compilation = binder.Compilation; - if (compilation.LanguageVersion < MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion()) + if (compilation.LanguageVersion < MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() || !compilation.RunNullableWalker) { return; } diff --git a/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.cs b/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.cs index 18c371abd64effc83b109e0a369f54432a08e0a1..119c935a007cfa2b74dfe7f8427a6c36e98e47d3 100644 --- a/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.cs +++ b/src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxTree.cs @@ -619,17 +619,31 @@ internal PragmaWarningState GetPragmaDirectiveWarningState(string id, int positi return _lazyPragmaWarningStateMap.GetWarningState(id, position); } - internal NullableContextState GetNullableContextState(int position) + private void EnsureNullableContextMapInitialized() { if (_lazyNullableContextStateMap == null) { // Create the #nullable directive map on demand. Interlocked.CompareExchange(ref _lazyNullableContextStateMap, NullableContextStateMap.Create(this, IsGeneratedCode()), null); } + } + internal NullableContextState GetNullableContextState(int position) + { + EnsureNullableContextMapInitialized(); return _lazyNullableContextStateMap.GetContextState(position); } + /// + /// Returns true if there are any nullable directives that enable annotations, warnings, or both. + /// This does not include any restore directives. + /// + internal bool HasNullableEnables() + { + EnsureNullableContextMapInitialized(); + return _lazyNullableContextStateMap.HasNullableEnables(); + } + internal bool IsGeneratedCode() { if (_lazyIsGeneratedCode == ThreeState.Unknown) diff --git a/src/Compilers/CSharp/Portable/Syntax/NullableContextStateMap.cs b/src/Compilers/CSharp/Portable/Syntax/NullableContextStateMap.cs index f326576dc2cd2cd82bea9a16ab8cb5105b0e4c5e..5ecd4708f0db61ce551569a74e884760b3987bde 100644 --- a/src/Compilers/CSharp/Portable/Syntax/NullableContextStateMap.cs +++ b/src/Compilers/CSharp/Portable/Syntax/NullableContextStateMap.cs @@ -89,6 +89,23 @@ internal NullableContextState GetContextState(int position) return context; } + /// + /// Returns true if any of the NullableContexts in this map enable annotations, warnings, or both. + /// This does not include any restore directives. + /// + internal bool HasNullableEnables() + { + foreach (var context in _contexts) + { + if (context.AnnotationsState == true || context.WarningsState == true) + { + return true; + } + } + + return false; + } + private static ImmutableArray GetContexts(SyntaxTree tree, bool isGeneratedCode) { var previousContext = GetContextForFileStart(position: 0, isGeneratedCode: isGeneratedCode); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 6cd9afd0771b8955069d10ccfae28c36e22c5eb8..59590489ba8ed57c25d634e6efb5ce86d4266446 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -105747,5 +105747,190 @@ class Program var comp = CreateCompilation(source); comp.VerifyDiagnostics(); } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirective() + { + var source = @" +class C +{ + void M(object? o1) + { + object o2 = o1; + } +}"; + + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics( + // (6,21): warning CS8600: Converting null literal or possible null value to non-nullable type. + // object o2 = o1; + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "o1").WithLocation(6, 21)); + + Assert.True(comp.RunNullableWalker); + } + + + [Theory] + [InlineData("")] + [InlineData("annotations")] + [InlineData("warnings")] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirective_WithNullDisables(string directive) + { + var source = $@" +#nullable disable {directive} +class C +{{ + void M(object? o1) + {{ + object o2 = o1; + }} +}}"; + var comp = CreateCompilation(source, options: WithNonNullTypesTrue()); + comp.VerifyDiagnostics(directive == "warnings" ? + DiagnosticDescription.None : + new[] { + // (5,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o1) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(5, 18) + }); + + Assert.True(comp.RunNullableWalker); + } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_WithNullableEnable() + { + RunNullableWalker_GlobalDirectiveOff_WithNullableDirective("", + // (7,21): warning CS8600: Converting null literal or possible null value to non-nullable type. + // object o2 = o1; + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "o1").WithLocation(7, 21)); + } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_WithNullableEnableAnnotations() + { + RunNullableWalker_GlobalDirectiveOff_WithNullableDirective("annotations"); + } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_WithNullableEnableWarnings() + { + RunNullableWalker_GlobalDirectiveOff_WithNullableDirective("warnings", + // (5,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o1) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(5, 18)); + } + + private void RunNullableWalker_GlobalDirectiveOff_WithNullableDirective(string directive, params DiagnosticDescription[] expectedDiagnostics) + { + var source = $@" +#nullable enable {directive} +class C +{{ + void M(object? o1) + {{ + object o2 = o1; + }} +}}"; + var comp = CreateCompilation(source, options: WithNonNullTypesFalse()); + comp.VerifyDiagnostics(expectedDiagnostics); + + Assert.True(comp.RunNullableWalker); + } + + [Theory] + [InlineData("disable")] + [InlineData("disable annotations")] + [InlineData("disable warnings")] + [InlineData("restore")] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_WithNonInfluencingNullableDirectives(string directive) + { + var source = $@" +#nullable {directive} +class C +{{ + void M(object? o1) + {{ + object o2 = o1; + }} +}}"; + var comp = CreateCompilation(source, options: WithNonNullTypesFalse()); + comp.VerifyDiagnostics( + // (5,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o1) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(5, 18)); + + Assert.False(comp.RunNullableWalker); + } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_MultipleFiles_NullableEnable() + { + RunNullableWalker_GlobalDirectiveOff_MultipleFiles("", + // (4,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o1) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(4, 18), + // (7,21): warning CS8600: Converting null literal or possible null value to non-nullable type. + // object o4 = o3; + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "o3").WithLocation(7, 21)); + } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_MultipleFiles_NullableEnableAnnotations() + { + RunNullableWalker_GlobalDirectiveOff_MultipleFiles("annotations", + // (4,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o1) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(4, 18)); + } + + [Fact] + [WorkItem(36131, "https://github.com/dotnet/roslyn/issues/36131")] + public void RunNullableWalker_GlobalDirectiveOff_MultipleFiles_NullableEnableWarnings() + { + RunNullableWalker_GlobalDirectiveOff_MultipleFiles("warnings", + // (4,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o1) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(4, 18), + // (5,18): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context. + // void M(object? o3) + Diagnostic(ErrorCode.WRN_MissingNonNullTypesContextForAnnotation, "?").WithLocation(5, 18)); + } + + private void RunNullableWalker_GlobalDirectiveOff_MultipleFiles(string directive, params DiagnosticDescription[] expectedDiagnostics) + { + var source1 = @" +class C +{ + void M(object? o1) + { + object o2 = o1; + } +}"; + + var source2 = $@" +#nullable enable {directive} +class D +{{ + void M(object? o3) + {{ + object o4 = o3; + }} +}}"; + + + var comp = CreateCompilation(new[] { source1, source2 }, options: WithNonNullTypesFalse()); + comp.VerifyDiagnostics(expectedDiagnostics); + + Assert.True(comp.RunNullableWalker); + } } }