I've completed two files so far. It's going to take me a long time to get through the rest of this because there are so many unrelated changes.

Can you please clarify? Removing Opt suffix absolutely falls within the scope of this change as those suffixes were present due to files not being nullable enabled. Changing x = x ?? new X() to x ??= new X() are extremely trivial changes also related to null-ness and do not harm the review ability for the PR. Forcing these into separate PRs leads to multiple review cycles for similar cleanup changes, and lot of times also get dropped. I would prefer to keep all these changes as a single "null-ness enable/cleanup" PR.

        Debug.Assert(_lazyGeneratedCodeAttribute != null);

How do we know this?
The initialization _lazyGeneratedCodeAttribute = analyzerExecutor.Compilation?.GetTypeByMetadataName("System.CodeDom.Compiler.GeneratedCodeAttribute"); could leave this variable as null #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:1153 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

            if (_lazyInitializeTask == null)

nit: not related to this PR, but I didn't follow how _lazyInitializeTask could still be null here #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:417 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

        Debug.Assert(WhenInitializedTask != null);

Consider keeping the assertion Never mind, it's in the property now :-) #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:520 in b819e2f. [](commit_id = b819e2f, deletion_comment = True)

        analyzerDriver.AttachQueueAndStartProcessingEvents(newCompilation.EventQueue!, analysisScope, cancellationToken: cancellationToken);

This probably should be an assertion (I don't see a related check in this method) #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:854 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

            if (tree != null &&

Consider inlining this with a pattern, so the suppression would not be needed. #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:1926 in 21a46d1. [](commit_id = 21a46d1, deletion_comment = False)

Debug.Assert(_lazyGeneratedCodeAttribute != null);

How do we know this?
The initialization _lazyGeneratedCodeAttribute = analyzerExecutor.Compilation?.GetTypeByMetadataName("System.CodeDom.Compiler.GeneratedCodeAttribute"); could leave this variable as null

The only place where the method was being called had a null check for _lazyGeneratedCodeAttribute. I have made this method now be a local function for the method with that callsite, so this is much more explicit.

Debug.Assert(location.SourceTree != null);

Consider leaving a comment on Location.IsInSource. We should annotate it with MemberNotNullWhen. Referencing #41964

Ah, I filed #46729. Feel free to resolve that as a dupe of #41964 if that is preferable.

    internal (Compilation? compilation, AnalyzerOptions? analyzerOptions) TryGetCompilationAndAnalyzerOptions() => (_compilation, _analyzerOptions);

Should we prefer a bool returning method with outs, and NotNullWhens on the outs here? #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerExecutor.cs:228 in 21a46d1. [](commit_id = 21a46d1, deletion_comment = False)

    internal ImmutableDictionary<DiagnosticAnalyzer, TimeSpan> AnalyzerExecutionTimes => _analyzerExecutionTimeMap!.ToImmutableDictionary(pair => pair.Key, pair => TimeSpan.FromTicks(pair.Value.Value));

Why is this safe? I would prefer an assert. #Closed

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerExecutor.cs:250 in 21a46d1. [](commit_id = 21a46d1, deletion_comment = False)

analyzerDriver.AttachQueueAndStartProcessingEvents(newCompilation.EventQueue!, analysisScope, cancellationToken: cancellationToken);

This probably should be an assertion (I don't see a related check in this method)

We create newCompilation explicitly with an event queue just a couple of lines above:

Debug.Assert(tree != null);

We could keep the assertion

Same as previous comment - this is in a private method with all callers in this file.

Done review pass (commit 2). In future, I'd prefer if nullable enable PRs were a bit smaller: when I review a file that has been #nullable enabled for the first time, it involves reviewing the entire file. There were multiple 2k+ line files in this PR, which makes for a very slow review. #Closed

 onAnalyzerException ??= (ex, analyzer, diagnostic) => { };

Did we have a bug? If yes, can we cover with a test?

Thanks, let me add a test.

Thanks everyone for the reviews! I think I have responded to and/or addressed all feedback, please let me know if I missed anything.

            if (tree != null &&

I was actually thinking diagnostic is { Location: { SourceTree: { } tree } }, but this works too.

In reply to: 672296393 [](ancestors = 672296393)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:1926 in 21a46d1. [](commit_id = 21a46d1, deletion_comment = False)

            if (_lazyInitializeTask == null)

That doesn't really matter for flow analysis though. We assume exceptions can occur at any time, for any reason. The cancellation token could have already been cancelled, for example, or the source for the cancellation token have been disposed.

In reply to: 672244161 [](ancestors = 672244161,672240283,671516788)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:417 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

            if (_lazyInitializeTask == null)

My question is not about flow analysis, but whether we can actually get here with a null value during execution. I'm not seeing how it's possible. Task.Run won't return null if the token was cancelled, from what I see in the docs.

Maybe the answer is "we're want to be defensive to future code changes" (ie. just to be safe), and that's fine.
But assumptions made by flow analysis don't help me see whether it's actually possible or not.

In reply to: 673013938 [](ancestors = 673013938,672244161,672240283,671516788)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:417 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

            if (_lazyInitializeTask == null)

If the token is cancelled, it throws. See https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.run?view=netcore-3.1#System_Threading_Tasks_Task_Run_System_Action_System_Threading_CancellationToken_.

In reply to: 673016668 [](ancestors = 673016668,673013938,672244161,672240283,671516788)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:417 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

            if (_lazyInitializeTask == null)

Thanks for clarifying. That's what I was curious about.

In reply to: 673059936 [](ancestors = 673059936,673016668,673013938,672244161,672240283,671516788)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:417 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

        analyzerDriver.AttachQueueAndStartProcessingEvents(newCompilation.EventQueue!, analysisScope, cancellationToken: cancellationToken);

@mavasani I didn't see a resolution

In reply to: 671530244 [](ancestors = 671530244)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:854 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)

        Debug.Assert(tree != null);

What was the resolution?

In reply to: 671548175 [](ancestors = 671548175)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:2101 in b819e2f. [](commit_id = b819e2f, deletion_comment = True)

        analyzerDriver.AttachQueueAndStartProcessingEvents(newCompilation.EventQueue!, analysisScope, cancellationToken: cancellationToken);

newCompilation = compilation.WithEventQueue(new AsyncQueue<CompilationEvent>());

In reply to: 673108299 [](ancestors = 673108299,671530244)

Refers to: src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs:854 in b819e2f. [](commit_id = b819e2f, deletion_comment = False)