CA1853: Fix incorrect code fixes

[mpidash]

This PR will fix missing and incorrectly applied code fixes and adds additional tests to cover these cases.
The following has been changed:

1. Negated dictionary.ContainsKey calls now only report a diagnostic (added a wrong fix before). See NegatedCondition_ReportsDiagnostic_CS and NegatedCondition_ReportsDiagnostic_VB.
2. Additional statements in the if block now only report a diagnostic (added a wrong fix before). See AdditionalStatements_ReportsDiagnostic_CS and AdditionalStatements_ReportsDiagnostic_VB.
3. Fixer for single statement ifs in VB (did not work before). See HasElseStatement_OffersFixer_VB.
4. Add missing tests for VB, see TriviaIsPreserved_VB and RemoveIsTheOnlyStatementInABlock_OffersFixer_VB.
5. Remove ConditionalExpressionSyntax check for CS fixer as conditional expressions are not reported by the analyzer.

Fixes #6274

[codecov]

Codecov Report

Merging #6770 (8595b72) into main (45879f0) will increase coverage by 0.00%.
The diff coverage is 99.23%.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #6770    +/-   ##
========================================
  Coverage   96.33%   96.34%            
========================================
  Files        1386     1386            
  Lines      325659   325886   +227     
  Branches    10718    10715     -3     
========================================
+ Hits       313732   313969   +237     
+ Misses       9215     9210     -5     
+ Partials     2712     2707     -5     

[mavasani]

Consider adding a comment why we are skipping the case where the condition is a logical not expression.

[mpidash]

Also added one to the VB fixer.

[mavasani]

I looked at the code for the analyzer and it seems the first additional location for the diagnostic is the conditional operation syntax, which can either be an if statement syntax OR a conditional expression syntax (ternary operator):

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/DoNotGuardDictionaryRemoveByContainsKey.cs

Line 81 in adcbeee

additionalLocation.Add(conditionalOperation.Syntax.GetLocation());

I think your change here to exclude ConditionalExpressionSyntax has identified a test hole. Please add a test for ternary operators to confirm the old behavior, the new behavior and which one is more preferable.

[mpidash]

The analyzer never triggered for conditional expression syntax (even though the C# fixer had that case in mind).
This is due to the following switch always matching the default case for ternary operators:

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/DoNotGuardDictionaryRemoveByContainsKey.cs

Lines 83 to 114 in 20ba369

	switch (conditionalOperation.WhenTrue.Children.First())
	{
	case IInvocationOperation childInvocationOperation:
	if ((childInvocationOperation.TargetMethod.OriginalDefinition.Equals(remove1Param, SymbolEqualityComparer.Default) ||
	childInvocationOperation.TargetMethod.OriginalDefinition.Equals(remove2Param, SymbolEqualityComparer.Default)) &&
	AreInvocationsOnSameInstance(childInvocationOperation, invocationOperation))
	{
	additionalLocation.Add(childInvocationOperation.Syntax.Parent!.GetLocation());
	context.ReportDiagnostic(invocationOperation.CreateDiagnostic(Rule, additionalLocations: additionalLocation.ToImmutable(), null));
	}
	break;
	case IExpressionStatementOperation childStatementOperation:
	/*
	* If the if statement contains a block, only proceed if one of the methods calls Remove.
	* However, a fixer is only offered if there is a single method in the block.
	*/
	var nestedInvocationOperation = childStatementOperation.Children.OfType<IInvocationOperation>()
	.FirstOrDefault(op => op.TargetMethod.OriginalDefinition.Equals(remove1Param, SymbolEqualityComparer.Default) ||
	op.TargetMethod.OriginalDefinition.Equals(remove2Param, SymbolEqualityComparer.Default));
	if (nestedInvocationOperation != null && AreInvocationsOnSameInstance(nestedInvocationOperation, invocationOperation))
	{
	additionalLocation.Add(nestedInvocationOperation.Syntax.Parent!.GetLocation());
	context.ReportDiagnostic(invocationOperation.CreateDiagnostic(Rule, additionalLocations: additionalLocation.ToImmutable(), null));
	}
	break;
	default:
	break;
	}

I am not sure how a conditional expression syntax would look like that we want to fix.
Something like this comes to my mind:

bool removed = MyDictionary.ContainsKey("Key") ? MyDictionary.Remove("Key") : false;

But the above already uses the return value of Dictionary.Remove, so I think it is not likely someone would write it as a ternary expression.

---

I will keep the occurrence of ConditionalExpressionSyntax removed in the C# fixer (as it was not supported anyway) and add a test as you suggested.