Issue 33798: Prefer Dictionary<K, V>.TryGetValue() over guarded indexer access

[CollinAlpert]

Added analyzer and code fix for guarded dictionary indexer accesses.
The analyzer looks for instances of a dictionary indexer access and checks if it is guarded by a ContainsKey check. If so, a diagnostic is raised and a code fix is offered. When applied, the ContainsKey check is replaced with a TryGetValue invocation.
Fixes issue #33798.

[dnfadmin]

All CLA requirements met.

[Youssef1313]

Same as above, do we need getInnermostNodeForTie here? If so, needs a test that demonstrates that.

[CollinAlpert]

We do not, as I still need to access the InvocationExpressionSyntax later on.

[buyaa-n]

if we do not need InvocationExpressionSyntax set, should we remove this?

[Youssef1313]

You may want to use the CreateDiagnostic extension method.

https://github.com/dotnet/roslyn-analyzers/blob/0919dbcf9bb03874aec10ec190c2f7f2397c10fb/src/Utilities/Compiler/Extensions/DiagnosticExtensions.cs

[CollinAlpert]

Why? What advantages do I gain?

[buyaa-n]

The analyzer looks for instances of a dictionary indexer access and checks if it is guarded by a ContainsKey check. If so, a diagnostic is raised and a code fix is offered. When applied, the ContainsKey check is replaced with a TryGetValue invocation.

@CollinAlpert I think it might be better to start with ContainsKey conditional operation because the conditional operation should have much less occurrence than property references, another reason for doing that is because of the below comment

Fixes issue #33798.

Thanks, i see dotnet/runtime#33799 is also assigned to you, you must have already noticed that they are very closely related and preferably one analyzer (could have a different ID if needed). What are you planning for that analyzer? i think to cover both it's better lookup ContainsKey conditional operation first and then check if there is indexer or Add method used, what you think? There is actually 3rd similar analyzer which was better to be handled together but seems somebody else claimed for that, anyway these analyzers at least should not fight with each other.

[buyaa-n]

Seems need to run dotnet msbuild -t:pack again build failures caused by generated files

[CollinAlpert]

@buyaa-n I just merged the main branch and am now getting an exception when running dotnet msbuild -t:pack:

Unhandled exception. System.Threading.Tasks.TaskCanceledException: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing.
   ---> System.TimeoutException: A task was canceled.
   ---> System.Threading.Tasks.TaskCanceledException: A task was canceled.
     at System.Threading.Tasks.TaskCompletionSourceWithCancellation`1.WaitWithCancellationAsync(CancellationToken cancellationToken)
     at System.Net.Http.HttpConnectionPool.GetHttp11ConnectionAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
     at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
     at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
     at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
     --- End of inner exception stack trace ---
     --- End of inner exception stack trace ---
     at System.Net.Http.HttpClient.HandleFailure(Exception e, Boolean telemetryStarted, HttpResponseMessage response, CancellationTokenSource cts, CancellationToken cancellationToken, CancellationTokenSource pendingRequestsCts)
     at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
     at GenerateDocumentationAndConfigFiles.Program.<>c__DisplayClass1_0.<<Main>g__checkHelpLinkAsync|16>d.MoveNext() in /Users/collin/Programmieren/dotNET/roslyn-analyzers/src/Tools/GenerateDocumentationAndConfigFiles/Program.cs:line 650
  --- End of stack trace from previous location ---
     at GenerateDocumentationAndConfigFiles.Program.<>c__DisplayClass1_0.<<Main>g__createAnalyzerRulesMissingDocumentationFileAsync|10>d.MoveNext() in /Users/collin/Programmieren/dotNET/roslyn-analyzers/src/Tools/GenerateDocumentationAndConfigFiles/Program.cs:line 593
  --- End of stack trace from previous location ---
     at GenerateDocumentationAndConfigFiles.Program.Main(String[] args) in /Users/collin/Programmieren/dotNET/roslyn-analyzers/src/Tools/GenerateDocumentationAndConfigFiles/Program.cs:line 199
     at GenerateDocumentationAndConfigFiles.Program.<Main>(String[] args)

[buyaa-n]

I just merged the main branch and am now getting an exception when running dotnet msbuild -t:pack

Probably caused by that missing quota, if that not fixes the issue try reverting all changes in all generated files:

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md
src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif
src/NetAnalyzers/RulesMissingDocumentation.md

And run the dotnet msbuild -t:pack

[buyaa-n]

Looks good to me, thank you for your contribution @CollinAlpert