Add CA1868: Unnecessary call to Set.Contains(item)

src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpDoNotGuardSetAddOrRemoveByContains.Fixer.cs

@@ -0,0 +1,74 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Composition;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Editing;
+using Microsoft.CodeAnalysis.Formatting;
+using Microsoft.NetCore.Analyzers.Performance;
+
+namespace Microsoft.NetCore.CSharp.Analyzers.Performance
+{
+    [ExportCodeFixProvider(LanguageNames.CSharp), Shared]
+    public sealed class CSharpDoNotGuardSetAddOrRemoveByContainsFixer : DoNotGuardSetAddOrRemoveByContainsFixer
+    {
+        protected override bool SyntaxSupportedByFixer(SyntaxNode conditionalSyntax)
+        {
+            if (conditionalSyntax is IfStatementSyntax ifStatementSyntax)
+            {
+                return ifStatementSyntax.Statement.ChildNodes().Count() == 1;
+            }
+
+            return false;
+        }
+
+        protected override Document ReplaceConditionWithChild(Document document, SyntaxNode root, SyntaxNode conditionalOperationNode, SyntaxNode childOperationNode)
+        {
+            SyntaxNode newRoot;
+
+            if (conditionalOperationNode is ConditionalExpressionSyntax conditionalExpressionSyntax &&
+                conditionalExpressionSyntax.WhenFalse.ChildNodes().Any())
+            {
+                var expression = GetNegatedExpression(document, childOperationNode);
+
+                SyntaxNode newConditionalOperationNode = conditionalExpressionSyntax
+                    .WithCondition((ExpressionSyntax)expression)
+                    .WithWhenTrue(conditionalExpressionSyntax.WhenFalse)
+                    .WithWhenFalse(null!)
+                    .WithAdditionalAnnotations(Formatter.Annotation).WithTriviaFrom(conditionalOperationNode);
+
+                newRoot = root.ReplaceNode(conditionalOperationNode, newConditionalOperationNode);
+            }
+            else if (conditionalOperationNode is IfStatementSyntax ifStatementSyntax && ifStatementSyntax.Else != null)
+            {
+                var expression = GetNegatedExpression(document, childOperationNode);
+
+                SyntaxNode newConditionalOperationNode = ifStatementSyntax
+                    .WithCondition((ExpressionSyntax)expression)
+                    .WithStatement(ifStatementSyntax.Else.Statement)
+                    .WithElse(null)
+                    .WithAdditionalAnnotations(Formatter.Annotation).WithTriviaFrom(conditionalOperationNode);
+
+                newRoot = root.ReplaceNode(conditionalOperationNode, newConditionalOperationNode);
+            }
+            else
+            {
+                SyntaxNode newConditionNode = childOperationNode
+                    .WithAdditionalAnnotations(Formatter.Annotation)
+                    .WithTriviaFrom(conditionalOperationNode);
+
+                newRoot = root.ReplaceNode(conditionalOperationNode, newConditionNode);
+            }
+
+            return document.WithSyntaxRoot(newRoot);
+        }
+
+        private static SyntaxNode GetNegatedExpression(Document document, SyntaxNode newConditionNode)
+        {
+            var generator = SyntaxGenerator.GetGenerator(document);
+            return generator.LogicalNotExpression(((ExpressionStatementSyntax)newConditionNode).Expression.WithoutTrivia());
+        }
+    }
+}

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -4,6 +4,7 @@
 
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
+CA1865 | Performance | Info | DoNotGuardSetAddOrRemoveByContains, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1865)
 CA2261 | Usage | Warning | DoNotUseConfigureAwaitWithSuppressThrowing, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2250)
 CA1510 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1510)
 CA1511 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1511)

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -1509,6 +1509,15 @@
   <data name="DoNotGuardDictionaryRemoveByContainsKeyTitle" xml:space="preserve">
     <value>Unnecessary call to 'Dictionary.ContainsKey(key)'</value>
   </data>
+  <data name="DoNotGuardSetAddOrRemoveByContainsDescription" xml:space="preserve">
+    <value>Do not guard 'Set.Add(item)' or 'Set.Remove(item)' with 'Set.Contains(item)'. The former two already check whether the item exists and will return if it was added or removed.</value>
+  </data>
+  <data name="DoNotGuardSetAddOrRemoveByContainsMessage" xml:space="preserve">
+    <value>Do not guard 'Set.Add(item)' or 'Set.Remove(item)' with 'Set.Contains(item)'</value>
+  </data>
+  <data name="DoNotGuardSetAddOrRemoveByContainsTitle" xml:space="preserve">
+    <value>Unnecessary call to 'Set.Contains(item)'</value>
+  </data>
   <data name="RemoveRedundantGuardCallCodeFixTitle" xml:space="preserve">
     <value>Remove unnecessary call</value>
   </data>

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/DoNotGuardSetAddOrRemoveByContains.Fixer.cs

@@ -0,0 +1,61 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Collections.Immutable;
+using System.Threading.Tasks;
+using Analyzer.Utilities;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+
+namespace Microsoft.NetCore.Analyzers.Performance
+{
+    public abstract class DoNotGuardSetAddOrRemoveByContainsFixer : CodeFixProvider
+    {
+        public override ImmutableArray<string> FixableDiagnosticIds { get; } =
+            ImmutableArray.Create(DoNotGuardSetAddOrRemoveByContains.RuleId);
+
+        public sealed override FixAllProvider GetFixAllProvider()
+        {
+            return WellKnownFixAllProviders.BatchFixer;
+        }
+
+        public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            var root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+            var node = root.FindNode(context.Span, getInnermostNodeForTie: true);
+
+            if (node is null)
+            {
+                return;
+            }
+
+            var diagnostic = context.Diagnostics[0];
+            var conditionalLocation = diagnostic.AdditionalLocations[0];
+            var childLocation = diagnostic.AdditionalLocations[1];
+
+            if (root.FindNode(conditionalLocation.SourceSpan) is not SyntaxNode conditionalSyntax ||
+                root.FindNode(childLocation.SourceSpan) is not SyntaxNode childStatementSyntax)
+            {
+                return;
+            }
+
+            // We only offer a fixer if the conditonal true branch has a single statement, either 'Add' or 'Delete'
+            if (!SyntaxSupportedByFixer(conditionalSyntax))
+            {
+                return;
+            }
+
+            var codeAction = CodeAction.Create(MicrosoftNetCoreAnalyzersResources.DoNotGuardSetAddOrRemoveByContainsTitle,
+                ct => Task.FromResult(ReplaceConditionWithChild(context.Document, root, conditionalSyntax, childStatementSyntax)),
+                nameof(MicrosoftNetCoreAnalyzersResources.DoNotGuardSetAddOrRemoveByContainsTitle));
+
+            context.RegisterCodeFix(codeAction, diagnostic);
+        }
+
+        protected abstract bool SyntaxSupportedByFixer(SyntaxNode conditionalSyntax);
+
+        protected abstract Document ReplaceConditionWithChild(Document document, SyntaxNode root,
+                                                              SyntaxNode conditionalOperationNode,
+                                                              SyntaxNode childOperationNode);
+    }
+}

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/DoNotGuardSetAddOrRemoveByContains.cs

@@ -0,0 +1,197 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using Analyzer.Utilities;
+using Analyzer.Utilities.Extensions;
+using Analyzer.Utilities.PooledObjects;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.NetCore.Analyzers.Performance
+{
+    using static MicrosoftNetCoreAnalyzersResources;
+
+    /// <summary>
+    /// CA1865: <inheritdoc cref="@DoNotGuardSetAddOrRemoveByContainsTitle"/>
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+    public sealed class DoNotGuardSetAddOrRemoveByContains : DiagnosticAnalyzer
+    {
+        internal const string RuleId = "CA1865";
+
+        private const string Contains = nameof(Contains);
+        private const string Add = nameof(Add);
+        private const string Remove = nameof(Remove);
+
+        internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
+            RuleId,
+            CreateLocalizableResourceString(nameof(DoNotGuardSetAddOrRemoveByContainsTitle)),
+            CreateLocalizableResourceString(nameof(DoNotGuardSetAddOrRemoveByContainsMessage)),
+            DiagnosticCategory.Performance,
+            RuleLevel.IdeSuggestion,
+            CreateLocalizableResourceString(nameof(DoNotGuardSetAddOrRemoveByContainsDescription)),
+            isPortedFxCopRule: false,
+            isDataflowRule: false);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.EnableConcurrentExecution();
+            context.RegisterCompilationStartAction(OnCompilationStart);
+        }
+
+        private void OnCompilationStart(CompilationStartAnalysisContext context)
+        {
+            if (!TryGetRequiredMethods(context.Compilation, out var containsMethod, out var addMethod, out var removeMethod))
+            {
+                return;
+            }
+
+            context.RegisterOperationAction(context => OnConditional(context, containsMethod, addMethod, removeMethod), OperationKind.Conditional);
+        }
+
+        private static void OnConditional(OperationAnalysisContext context, IMethodSymbol containsMethod, IMethodSymbol addMethod, IMethodSymbol removeMethod)
+        {
+            var conditional = (IConditionalOperation)context.Operation;
+
+            if (!TryExtractContainsInvocation(conditional.Condition, containsMethod, out var containsInvocation, out var containsNegated))
+            {
+                return;
+            }
+
+            if (!TryExtractAddOrRemoveInvocation(conditional.WhenTrue.Children, addMethod, removeMethod, containsNegated, out var addOrRemoveInvocation))
+            {
+                return;
+            }
+
+            if (!AreInvocationsOnSameInstance(containsInvocation, addOrRemoveInvocation))
+            {
+                return;
+            }
+
+            using var locations = ArrayBuilder<Location>.GetInstance(2);
+            locations.Add(conditional.Syntax.GetLocation());
+            locations.Add(addOrRemoveInvocation.Syntax.Parent!.GetLocation());
+
+            context.ReportDiagnostic(containsInvocation.CreateDiagnostic(Rule, additionalLocations: locations.ToImmutable(), null));
+        }
+
+        private static bool TryGetRequiredMethods(
+            Compilation compilation,
+            [NotNullWhen(true)] out IMethodSymbol? containsMethod,
+            [NotNullWhen(true)] out IMethodSymbol? addMethod,
+            [NotNullWhen(true)] out IMethodSymbol? removeMethod)
+        {
+            var iSetType = WellKnownTypeProvider.GetOrCreate(compilation).GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericISet1);
+            var iCollectionType = WellKnownTypeProvider.GetOrCreate(compilation).GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemCollectionsGenericICollection1);
+
+            if (iSetType is null || iCollectionType is null)
+            {
+                containsMethod = null;
+                addMethod = null;
+                removeMethod = null;
+
+                return false;
+            }
+
+            addMethod = iSetType.GetMembers(Add).OfType<IMethodSymbol>().FirstOrDefault();
+            containsMethod = iCollectionType.GetMembers(Contains).OfType<IMethodSymbol>().FirstOrDefault();
+            removeMethod = iCollectionType.GetMembers(Remove).OfType<IMethodSymbol>().FirstOrDefault();
+
+            return containsMethod is not null && addMethod is not null && removeMethod is not null;
+        }
+
+        private static bool TryExtractContainsInvocation(
+            IOperation condition,
+            IMethodSymbol containsMethod,
+            [NotNullWhen(true)] out IInvocationOperation? containsInvocation,
+            out bool containsNegated)
+        {
+            containsNegated = false;
+            containsInvocation = null;
+
+            switch (condition.WalkDownParentheses())
+            {
+                case IInvocationOperation invocation:
+                    containsInvocation = invocation;
+                    break;
+                case IUnaryOperation unaryOperation when unaryOperation.OperatorKind == UnaryOperatorKind.Not && unaryOperation.Operand is IInvocationOperation operand:
+                    containsNegated = true;
+                    containsInvocation = operand;
+                    break;
+                default:
+                    return false;
+            }
+
+            return DoesImplementInterfaceMethod(containsInvocation.TargetMethod, containsMethod);
+        }
+
+        private static bool TryExtractAddOrRemoveInvocation(
+            IEnumerable<IOperation> operations,
+            IMethodSymbol addMethod,
+            IMethodSymbol removeMethod,
+            bool containsNegated,
+            [NotNullWhen(true)] out IInvocationOperation? addOrRemoveInvocation)
+        {
+            addOrRemoveInvocation = null;
+            var firstOperation = operations.FirstOrDefault();
+
+            if (firstOperation is null)
+            {
+                return false;
+            }
+
+            switch (firstOperation)
+            {
+                case IInvocationOperation invocation:
+                    if ((containsNegated && DoesImplementInterfaceMethod(invocation.TargetMethod, addMethod)) ||
+                        (!containsNegated && DoesImplementInterfaceMethod(invocation.TargetMethod, removeMethod)))
+                    {
+                        addOrRemoveInvocation = invocation;
+                        return true;
+                    }
+
+                    break;
+                case IExpressionStatementOperation expressionStatement:
+                    var nestedAddOrRemove = expressionStatement.Children
+                        .OfType<IInvocationOperation>()
+                        .FirstOrDefault(i => containsNegated ?
+                            DoesImplementInterfaceMethod(i.TargetMethod, addMethod) :
+                            DoesImplementInterfaceMethod(i.TargetMethod, removeMethod));
+
+                    if (nestedAddOrRemove != null)
+                    {
+                        addOrRemoveInvocation = nestedAddOrRemove;
+                        return true;
+                    }
+
+                    break;
+            }
+
+            return false;
+        }
+
+        private static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2)
+        {
+            return (invocation1.Instance, invocation2.Instance) switch
+            {
+                (IFieldReferenceOperation fieldRef1, IFieldReferenceOperation fieldRef2) => fieldRef1.Member == fieldRef2.Member,
+                (IPropertyReferenceOperation propRef1, IPropertyReferenceOperation propRef2) => propRef1.Member == propRef2.Member,
+                (IParameterReferenceOperation paramRef1, IParameterReferenceOperation paramRef2) => paramRef1.Parameter == paramRef2.Parameter,
+                (ILocalReferenceOperation localRef1, ILocalReferenceOperation localRef2) => localRef1.Local == localRef2.Local,
+                _ => false,
+            };
+        }
+
+        private static bool DoesImplementInterfaceMethod(IMethodSymbol method, IMethodSymbol interfaceMethod)
+        {
+            return method.IsImplementationOfInterfaceMethod(method.Parameters[0].Type, interfaceMethod.ContainingType, interfaceMethod.Name);
+        }
+    }
+}

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf

@@ -763,6 +763,21 @@ Obecné přetypování (IL unbox.any) používané sekvencí vrácenou metodou E
         <target state="translated">Nepotřebné volání Dictionary.ContainsKey(key)</target>
         <note />
       </trans-unit>
+      <trans-unit id="DoNotGuardSetAddOrRemoveByContainsDescription">
+        <source>Do not guard 'Set.Add(item)' or 'Set.Remove(item)' with 'Set.Contains(item)'. The former two already check whether the item exists and will return if it was added or removed.</source>
+        <target state="new">Do not guard 'Set.Add(item)' or 'Set.Remove(item)' with 'Set.Contains(item)'. The former two already check whether the item exists and will return if it was added or removed.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="DoNotGuardSetAddOrRemoveByContainsMessage">
+        <source>Do not guard 'Set.Add(item)' or 'Set.Remove(item)' with 'Set.Contains(item)'</source>
+        <target state="new">Do not guard 'Set.Add(item)' or 'Set.Remove(item)' with 'Set.Contains(item)'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="DoNotGuardSetAddOrRemoveByContainsTitle">
+        <source>Unnecessary call to 'Set.Contains(item)'</source>
+        <target state="new">Unnecessary call to 'Set.Contains(item)'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="DoNotHardCodeCertificate">
         <source>Do not hard-code certificate</source>
         <target state="translated">Nepoužívejte pevně zakódovaný certifikát</target>