Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 46 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
d5e2607
Add analyzer and fixer for CA1865
mpidash Jul 11, 2023
4d10f23
Merge branch 'dotnet:main' into issue-85490-analyzer-unnecessary-set-…
mpidash Jul 13, 2023
5c6bcdb
Add CA1865 to Microsoft.CodeAnalysis.NetAnalyzers.md
mpidash Jul 13, 2023
9547162
Add CA1865 to RulesMissingDocumentation.md
mpidash Jul 13, 2023
52e7f91
Add changes from msbuild pack
mpidash Jul 13, 2023
fea7a0a
Update DoNotGuardSetAddOrRemoveByContainsDescription
mpidash Jul 13, 2023
02fc236
Add ExportCodeFixProviderAttribute
mpidash Jul 14, 2023
68c210f
Remove call to First with call to indexer
mpidash Jul 14, 2023
5a0113f
Change equivalenceKey of CodeAction
mpidash Jul 14, 2023
5e841a6
Check for interface implementation instead of signature match
mpidash Jul 14, 2023
07943c4
Revert "Remove call to First with call to indexer"
mpidash Jul 14, 2023
9b43e00
Remove call to First with call to indexer
mpidash Jul 14, 2023
a8e4875
Add additional tests
mpidash Jul 14, 2023
29a4630
Fix ifs with additional statements
mpidash Jul 14, 2023
468274e
Fix single line ifs in VB
mpidash Jul 14, 2023
326481c
Add fixer case for single line ifs in VB
mpidash Jul 14, 2023
533cd78
Apply suggestions from code review
mpidash Jul 19, 2023
cf49578
CA1865: Add tests for other set types
mpidash Jul 18, 2023
d063408
CA1865: Check descendants instead of switch
mpidash Jul 18, 2023
d7c7c54
CA1865: Do not fire when arguments are different
mpidash Jul 18, 2023
4c71d4c
CA1865: Change fixer title
mpidash Jul 19, 2023
ed09398
CA1865: Update resources
mpidash Jul 19, 2023
4f62beb
WIP: Try to handle IImmutableSet Add and Remove
mpidash Jul 19, 2023
315e2c5
Apply suggestions from code review
mpidash Jul 19, 2023
a0f8d26
CA1865: Use original definitions to filter methods
mpidash Jul 20, 2023
f430939
Merge branch 'main' into issue-85490-analyzer-unnecessary-set-call
mpidash Jul 20, 2023
f22e2dc
Check parameter length before using them
mpidash Jul 22, 2023
efe2993
Only check child operations instead of descendants
mpidash Jul 22, 2023
7c68257
Fix diagnostic message
mpidash Jul 22, 2023
860f217
Also flag when interface types are used
mpidash Jul 22, 2023
8b7a007
Use helper class for required symbols
mpidash Jul 22, 2023
1facbcf
Merge branch 'dotnet:main' into issue-85490-analyzer-unnecessary-set-…
mpidash Jul 22, 2023
ba59f85
Add changes from msbuild pack
mpidash Jul 22, 2023
9513d9d
Only compare first argument value
mpidash Jul 25, 2023
8a3f6ef
Make symbol display format static
mpidash Jul 25, 2023
be9d96e
Typo
mpidash Jul 25, 2023
5fef8d0
Update DoNotGuardSetAddOrRemoveByContainsTitle
mpidash Jul 25, 2023
1d220c6
Change condition to pattern matching
mpidash Jul 25, 2023
c5376f3
Add changes from msbuild pack
mpidash Jul 25, 2023
7a9fa60
Add tests for ternary operators
mpidash Jul 26, 2023
6f69703
Use raw strings and inline class and usings
mpidash Jul 26, 2023
ad82982
Support Add or Remove in else block
mpidash Jul 26, 2023
f723ac5
Support ternary operator (diagnostic only)
mpidash Jul 26, 2023
915a4ad
Move from CA1865 to CA1868
mpidash Jul 26, 2023
8866f5d
Merge branch 'main' into issue-85490-analyzer-unnecessary-set-call
mpidash Jul 27, 2023
953a052
Update src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Perform…
buyaa-n Jul 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// 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, SyntaxNode childStatementSyntax)
{
if (childStatementSyntax is not ExpressionStatementSyntax)
{
return false;
}

if (conditionalSyntax is IfStatementSyntax ifStatementSyntax)
{
var addOrRemoveInElse = childStatementSyntax.Parent is ElseClauseSyntax || childStatementSyntax.Parent?.Parent is ElseClauseSyntax;

return addOrRemoveInElse
? ifStatementSyntax.Else?.Statement.ChildNodes().Count() == 1
: ifStatementSyntax.Statement.ChildNodes().Count() == 1;
}

return false;
}

protected override Document ReplaceConditionWithChild(Document document, SyntaxNode root, SyntaxNode conditionalOperationNode, SyntaxNode childOperationNode)
{
SyntaxNode newRoot;

if (conditionalOperationNode is IfStatementSyntax { Else: not null } ifStatementSyntax)
{
var expression = GetNegatedExpression(document, childOperationNode);
var addOrRemoveInElse = childOperationNode.Parent is ElseClauseSyntax || childOperationNode.Parent?.Parent is ElseClauseSyntax;

SyntaxNode newConditionalOperationNode = ifStatementSyntax
.WithCondition((ExpressionSyntax)expression)
.WithStatement(addOrRemoveInElse ? ifStatementSyntax.Statement : 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());
}
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Rule ID | Category | Severity | Notes
CA1865 | Performance | Info | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1865)
CA1866 | Performance | Info | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)
CA1867 | Performance | Disabled | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1867)
CA1868 | 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 'Add(item)' or 'Remove(item)' with 'Contains(item)' for the set. 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 '{0}' with '{1}'</value>
</data>
<data name="DoNotGuardSetAddOrRemoveByContainsTitle" xml:space="preserve">
<value>Unnecessary call to 'Contains(item)'</value>
</data>
<data name="RemoveRedundantGuardCallCodeFixTitle" xml:space="preserve">
<value>Remove unnecessary call</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// 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;
}

if (!SyntaxSupportedByFixer(conditionalSyntax, childStatementSyntax))
{
return;
}

var codeAction = CodeAction.Create(MicrosoftNetCoreAnalyzersResources.RemoveRedundantGuardCallCodeFixTitle,
ct => Task.FromResult(ReplaceConditionWithChild(context.Document, root, conditionalSyntax, childStatementSyntax)),
nameof(MicrosoftNetCoreAnalyzersResources.DoNotGuardSetAddOrRemoveByContainsTitle));

context.RegisterCodeFix(codeAction, diagnostic);
}

protected abstract bool SyntaxSupportedByFixer(SyntaxNode conditionalSyntax, SyntaxNode childStatementSyntax);

protected abstract Document ReplaceConditionWithChild(Document document, SyntaxNode root,
SyntaxNode conditionalOperationNode,
SyntaxNode childOperationNode);
}
}
Loading