Add switch to skip nullable analysis

src/Compilers/CSharp/Portable/Compilation/AttributeSemanticModel.cs

@@ -114,12 +114,10 @@ protected override BoundNode RewriteNullableBoundNodesWithSnapshots(
             return NullableWalker.AnalyzeAndRewrite(Compilation, symbol: null, boundRoot, binder, initialState: null, diagnostics, createSnapshots, out snapshotManager, ref remappedSymbols);
         }
 
-#if DEBUG
         protected override void AnalyzeBoundNodeNullability(BoundNode boundRoot, Binder binder, DiagnosticBag diagnostics, bool createSnapshots)
         {
             NullableWalker.AnalyzeWithoutRewrite(Compilation, symbol: null, boundRoot, binder, diagnostics, createSnapshots);
         }
-#endif
 
         internal override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticModel parentModel, int position, ConstructorInitializerSyntax constructorInitializer, out SemanticModel speculativeModel)
         {

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

@@ -133,7 +133,15 @@ internal Conversions Conversions
         /// Run the nullable walker during the flow analysis passes. True if the project-level nullable
         /// context option is set, or if any file enables nullable or just the nullable warnings.
         /// </summary>
-        private ThreeState _lazyShouldRunNullableWalker;
+        private ThreeState _lazyShouldRunNullableAnalysis;
+
+        /// <summary>
+        /// Nullable analysis data for methods, parameter default values, and attributes.
+        /// The key is a symbol for methods or parameters, and syntax for attributes,
+        /// and the value is the number of entries tracked during analysis of that key.
+        /// The data is collected during testing only.
+        /// </summary>
+        internal ConcurrentDictionary<object, int>? NullableAnalysisData;
 
         public override string Language
         {
@@ -185,23 +193,6 @@ internal override CommonAnonymousTypeManager CommonAnonymousTypeManager
         /// </summary>
         internal bool FeatureStrictEnabled => Feature("strict") != null;
 
-        /// <summary>
-        /// True if we should enable nullable semantic analysis in this compilation.
-        /// </summary>
-        internal bool NullableSemanticAnalysisEnabled
-        {
-            get
-            {
-                var nullableAnalysisFlag = Feature("run-nullable-analysis");
-                if (nullableAnalysisFlag == "false")
-                {
-                    return false;
-                }
-
-                return ShouldRunNullableWalker || nullableAnalysisFlag == "true";
-            }
-        }
-
         /// <summary>
         /// True when the "peverify-compat" feature flag is set or the language version is below C# 7.2.
         /// With this flag we will avoid certain patterns known not be compatible with PEVerify.
@@ -210,34 +201,78 @@ internal bool NullableSemanticAnalysisEnabled
         /// </summary>
         internal bool IsPeVerifyCompatEnabled => LanguageVersion < LanguageVersion.CSharp7_2 || Feature("peverify-compat") != null;
 
-        internal bool ShouldRunNullableWalker
+        /// <summary>
+        /// Returns true if nullable analysis is enabled in this compilation.
+        /// </summary>
+        /// <remarks>
+        /// Returns false if analysis is explicitly disabled for all methods; otherwise
+        /// returns true if the nullable context is enabled in the compilation options
+        /// or if there are any nullable-enabled contexts in the compilation.
+        /// </remarks>
+        internal bool IsNullableAnalysisEnabled
         {
             get
             {
-                if (!_lazyShouldRunNullableWalker.HasValue())
+                if (!_lazyShouldRunNullableAnalysis.HasValue())
                 {
-                    if (Options.NullableContextOptions != NullableContextOptions.Disable)
+                    _lazyShouldRunNullableAnalysis = isEnabled().ToThreeState();
+                }
+                return _lazyShouldRunNullableAnalysis.Value();
+
+                bool isEnabled()
+                {
+                    if (GetNullableAnalysisValue() == false)
                     {
-                        _lazyShouldRunNullableWalker = ThreeState.True;
-                        return true;
+                        return false;
                     }
-
-                    foreach (var syntaxTree in SyntaxTrees)
+                    if (Options.NullableContextOptions != NullableContextOptions.Disable)
                     {
-                        if (((CSharpSyntaxTree)syntaxTree).HasNullableEnables())
-                        {
-                            _lazyShouldRunNullableWalker = ThreeState.True;
-                            return true;
-                        }
+                        return true;
                     }
-
-                    _lazyShouldRunNullableWalker = ThreeState.False;
+                    return SyntaxTrees.Any(tree => ((CSharpSyntaxTree)tree).HasNullableEnables());
                 }
+            }
+        }
 
-                return _lazyShouldRunNullableWalker.Value();
+        /// <summary>
+        /// Returns true if nullable analysis is enabled for all methods regardless
+        /// of the actual nullable context.
+        /// If this property returns true but IsNullableAnalysisEnabled returns false,
+        /// any nullable analysis should be enabled but results should be ignored.
+        /// </summary>
+        /// <remarks>
+        /// For DEBUG builds, we treat nullable analysis as enabled for all methods
+        /// unless explicitly disabled, so that analysis is run, even though results may
+        /// be ignored, to increase the chance of catching nullable regressions
+        /// (e.g. https://github.com/dotnet/roslyn/issues/40136).
+        /// </remarks>
+        internal bool IsNullableAnalysisEnabledAlways
+        {
+            get
+            {
+                var value = GetNullableAnalysisValue();
+#if DEBUG
+                return value != false;
+#else
+                return value == true;
+#endif
             }
         }
 
+        /// <summary>
+        /// Returns Feature("run-nullable-analysis") as a bool? value:
+        /// true for "always"; false for "never"; and null otherwise.
+        /// </summary>
+        private bool? GetNullableAnalysisValue()
+        {
+            return Feature("run-nullable-analysis") switch
+            {
+                "always" => true,
+                "never" => false,
+                _ => null,
+            };
+        }
+
         /// <summary>
         /// The language version that was used to parse the syntax trees of this compilation.
         /// </summary>

src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs

@@ -1777,7 +1777,7 @@ private void AppendSymbolsWithNameAndArity(
 
         private Symbol RemapSymbolIfNecessary(Symbol symbol)
         {
-            if (!Compilation.NullableSemanticAnalysisEnabled) return symbol;
+            if (!Compilation.IsNullableAnalysisEnabled) return symbol;
 
             switch (symbol)
             {

src/Compilers/CSharp/Portable/Compilation/InitializerSemanticModel.cs

@@ -272,11 +272,9 @@ protected override BoundNode RewriteNullableBoundNodesWithSnapshots(
             return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol, boundRoot, binder, initialState: null, diagnostics, createSnapshots, out snapshotManager, ref remappedSymbols);
         }
 
-#if DEBUG
         protected override void AnalyzeBoundNodeNullability(BoundNode boundRoot, Binder binder, DiagnosticBag diagnostics, bool createSnapshots)
         {
             NullableWalker.AnalyzeWithoutRewrite(Compilation, MemberSymbol, boundRoot, binder, diagnostics, createSnapshots);
         }
-#endif
     }
 }

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.SpeculativeMemberSemanticModel.cs

@@ -46,12 +46,10 @@ protected override BoundNode RewriteNullableBoundNodesWithSnapshots(
                 return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol as MethodSymbol, boundRoot, binder, initialState: null, diagnostics, createSnapshots: false, out snapshotManager, ref remappedSymbols);
             }
 
-#if DEBUG
             protected override void AnalyzeBoundNodeNullability(BoundNode boundRoot, Binder binder, DiagnosticBag diagnostics, bool createSnapshots)
             {
                 NullableWalker.AnalyzeWithoutRewrite(Compilation, MemberSymbol as MethodSymbol, boundRoot, binder, diagnostics, createSnapshots);
             }
-#endif
 
             internal override bool TryGetSpeculativeSemanticModelCore(SyntaxTreeSemanticModel parentModel, int position, ConstructorInitializerSyntax constructorInitializer, out SemanticModel speculativeModel)
             {

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

@@ -157,14 +157,14 @@ internal override MemberSemanticModel GetMemberModel(SyntaxNode node)
         protected virtual NullableWalker.SnapshotManager GetSnapshotManager()
         {
             EnsureNullabilityAnalysisPerformedIfNecessary();
-            Debug.Assert(_lazySnapshotManager is object || !Compilation.NullableSemanticAnalysisEnabled);
+            Debug.Assert(_lazySnapshotManager is object || !Compilation.IsNullableAnalysisEnabled);
             return _lazySnapshotManager;
         }
 
         internal ImmutableDictionary<Symbol, Symbol> GetRemappedSymbols()
         {
             EnsureNullabilityAnalysisPerformedIfNecessary();
-            Debug.Assert(_lazyRemappedSymbols is object || this is AttributeSemanticModel || !Compilation.NullableSemanticAnalysisEnabled);
+            Debug.Assert(_lazyRemappedSymbols is object || this is AttributeSemanticModel || !Compilation.IsNullableAnalysisEnabled);
             return _lazyRemappedSymbols;
         }
 
@@ -197,7 +197,7 @@ internal override BoundExpression GetSpeculativelyBoundExpression(int position,
                 throw new ArgumentNullException(nameof(expression));
             }
 
-            if (!Compilation.NullableSemanticAnalysisEnabled || bindingOption != SpeculativeBindingOption.BindAsExpression)
+            if (!Compilation.IsNullableAnalysisEnabled || bindingOption != SpeculativeBindingOption.BindAsExpression)
             {
                 return GetSpeculativelyBoundExpressionWithoutNullability(position, expression, bindingOption, out binder, out crefSymbols);
             }
@@ -701,7 +701,7 @@ private LocalFunctionSymbol GetDeclaredLocalFunction(LocalFunctionStatementSynta
 
         private T GetRemappedSymbol<T>(T originalSymbol) where T : Symbol
         {
-            if (!Compilation.NullableSemanticAnalysisEnabled) return originalSymbol;
+            if (!Compilation.IsNullableAnalysisEnabled) return originalSymbol;
 
             EnsureNullabilityAnalysisPerformedIfNecessary();
             if (_lazyRemappedSymbols is null) return originalSymbol;
@@ -1512,11 +1512,11 @@ protected void GuardedAddBoundTreeForStandaloneSyntax(SyntaxNode syntax, BoundNo
                     NodeMapBuilder.AddToMap(bound, _guardedNodeMap, SyntaxTree, syntax);
                 }
 
-                Debug.Assert((manager is null && (!Compilation.NullableSemanticAnalysisEnabled || syntax != Root || syntax is TypeSyntax ||
+                Debug.Assert((manager is null && (!Compilation.IsNullableAnalysisEnabled || syntax != Root || syntax is TypeSyntax ||
                                                   // Supporting attributes is tracked by
                                                   // https://github.com/dotnet/roslyn/issues/36066
                                                   this is AttributeSemanticModel)) ||
-                             (manager is object && remappedSymbols is object && syntax == Root && Compilation.NullableSemanticAnalysisEnabled && _lazySnapshotManager is null));
+                             (manager is object && remappedSymbols is object && syntax == Root && Compilation.IsNullableAnalysisEnabled && _lazySnapshotManager is null));
                 if (manager is object)
                 {
                     _lazySnapshotManager = manager;
@@ -1729,9 +1729,10 @@ private BoundNode GetBoundLambdaOrQuery(CSharpSyntaxNode lambdaOrQuery)
                 BoundNode boundOuterExpression = this.Bind(incrementalBinder, lambdaOrQuery, _ignoredDiagnostics);
 
                 // 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);
-#endif
+                if (!Compilation.IsNullableAnalysisEnabled && Compilation.IsNullableAnalysisEnabledAlways)
+                {
+                    AnalyzeBoundNodeNullability(boundOuterExpression, incrementalBinder, diagnostics: new DiagnosticBag(), createSnapshots: false);
+                }
 
                 nodes = GuardedAddBoundTreeAndGetBoundNodeFromMap(lambdaOrQuery, boundOuterExpression);
             }
@@ -1914,13 +1915,10 @@ private static Binder GetLambdaEnclosingBinder(int position, CSharpSyntaxNode st
         /// </summary>
         protected void EnsureNullabilityAnalysisPerformedIfNecessary()
         {
-            // If we're in DEBUG mode, always enable the analysis, but throw away the results
-#if !DEBUG
-            if (!Compilation.NullableSemanticAnalysisEnabled)
+            if (!Compilation.IsNullableAnalysisEnabled && !Compilation.IsNullableAnalysisEnabledAlways)
             {
                 return;
             }
-#endif
 
             // If we have a snapshot manager, then we've already done
             // all the work necessary and we should avoid taking an
@@ -1941,7 +1939,7 @@ protected void EnsureNullabilityAnalysisPerformedIfNecessary()
             // first BoundNode corresponds to the underlying EqualsValueSyntax of the initializer)
             if (_guardedNodeMap.Count > 0)
             {
-                Debug.Assert(!Compilation.NullableSemanticAnalysisEnabled ||
+                Debug.Assert(!Compilation.IsNullableAnalysisEnabled ||
                              _guardedNodeMap.ContainsKey(bindableRoot) ||
                              _guardedNodeMap.ContainsKey(bind(bindableRoot, getDiagnosticBag(), out _).Syntax));
                 return;
@@ -2000,34 +1998,30 @@ BoundNode bind(CSharpSyntaxNode root, DiagnosticBag diagnosticBag, out Binder bi
 
             void rewriteAndCache(DiagnosticBag diagnosticBag)
             {
-#if DEBUG
-                if (!Compilation.NullableSemanticAnalysisEnabled)
+                if (!Compilation.IsNullableAnalysisEnabled && Compilation.IsNullableAnalysisEnabledAlways)
                 {
                     AnalyzeBoundNodeNullability(boundRoot, binder, diagnosticBag, createSnapshots: true);
                     return;
                 }
-#endif
 
                 boundRoot = RewriteNullableBoundNodesWithSnapshots(boundRoot, binder, diagnosticBag, createSnapshots: true, out snapshotManager, ref remappedSymbols);
                 cache(bindableRoot, boundRoot, snapshotManager, remappedSymbols);
             }
 
             void cache(CSharpSyntaxNode bindableRoot, BoundNode boundRoot, NullableWalker.SnapshotManager snapshotManager, ImmutableDictionary<Symbol, Symbol> remappedSymbols)
             {
-                Debug.Assert(Compilation.NullableSemanticAnalysisEnabled);
+                Debug.Assert(Compilation.IsNullableAnalysisEnabled);
                 GuardedAddBoundTreeForStandaloneSyntax(bindableRoot, boundRoot, snapshotManager, remappedSymbols);
             }
 
             DiagnosticBag getDiagnosticBag()
             {
-                // In DEBUG without nullable analysis enabled, we want to use a temp diagnosticbag
+                // If nullable analysis is not enabled, we want to use a temp diagnosticbag
                 // that can't produce any observable side effects
-#if DEBUG
-                if (!Compilation.NullableSemanticAnalysisEnabled)
+                if (!Compilation.IsNullableAnalysisEnabled)
                 {
                     return new DiagnosticBag();
                 }
-#endif
                 return _ignoredDiagnostics;
             }
         }
@@ -2044,13 +2038,13 @@ protected abstract BoundNode RewriteNullableBoundNodesWithSnapshots(
             out NullableWalker.SnapshotManager snapshotManager,
             ref ImmutableDictionary<Symbol, Symbol>? remappedSymbols);
 
-#if DEBUG
         /// <summary>
-        /// Performs just the analysis step of getting nullability information for a semantic model. This is only used during DEBUG builds where nullable
-        /// is turned off on a compilation level, giving some extra debug verification of the nullable flow analysis. 
+        /// Performs the analysis step of getting nullability information for a semantic model but
+        /// does not actually use the results. This gives us extra verification of nullable flow analysis.
+        /// It is only used in contexts where nullable analysis is disabled in the compilation but requested
+        /// through "run-nullable-analysis=always" or when the compiler is running in DEBUG.
         /// </summary>
         protected abstract void AnalyzeBoundNodeNullability(BoundNode boundRoot, Binder binder, DiagnosticBag diagnostics, bool createSnapshots);
-#endif
 #nullable disable
 
         /// <summary>
@@ -2323,7 +2317,7 @@ internal override Symbol RemapSymbolIfNecessaryCore(Symbol symbol)
             Debug.Assert(symbol is LocalSymbol ||
                          symbol is ParameterSymbol ||
                          symbol is MethodSymbol { MethodKind: MethodKind.LambdaMethod });
-            Debug.Assert(Compilation.NullableSemanticAnalysisEnabled);
+            Debug.Assert(Compilation.IsNullableAnalysisEnabled);
             EnsureNullabilityAnalysisPerformedIfNecessary();
 
             if (_lazyRemappedSymbols is null)

src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs

@@ -304,11 +304,9 @@ protected override BoundNode RewriteNullableBoundNodesWithSnapshots(
             return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol, boundRoot, binder, afterInitializersState, diagnostics, createSnapshots, out snapshotManager, ref remappedSymbols);
         }
 
-#if DEBUG
         protected override void AnalyzeBoundNodeNullability(BoundNode boundRoot, Binder binder, DiagnosticBag diagnostics, bool createSnapshots)
         {
             NullableWalker.AnalyzeWithoutRewrite(Compilation, MemberSymbol, boundRoot, binder, diagnostics, createSnapshots);
         }
-#endif
     }
 }

src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs

@@ -773,7 +773,7 @@ internal override BoundExpression GetSpeculativelyBoundExpression(int position,
 
         internal AttributeSemanticModel CreateSpeculativeAttributeSemanticModel(int position, AttributeSyntax attribute, Binder binder, AliasSymbol aliasOpt, NamedTypeSymbol attributeType)
         {
-            var memberModel = Compilation.NullableSemanticAnalysisEnabled ? GetMemberModel(position) : null;
+            var memberModel = Compilation.IsNullableAnalysisEnabled ? GetMemberModel(position) : null;
             return AttributeSemanticModel.CreateSpeculative(this, attribute, attributeType, aliasOpt, binder, memberModel?.GetRemappedSymbols(), position);
         }
 

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -1689,7 +1689,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
                     ImmutableDictionary<Symbol, Symbol> remappedSymbols = null;
                     var compilation = bodyBinder.Compilation;
                     var isSufficientLangVersion = compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion();
-                    if (compilation.NullableSemanticAnalysisEnabled)
+                    if (compilation.IsNullableAnalysisEnabled)
                     {
                         methodBodyForSemanticModel = NullableWalker.AnalyzeAndRewrite(
                             compilation,