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

Detection of PLINQ nops analyzer #4126

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
4 changes: 4 additions & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
; Please do not edit this file manually, it should only be updated through code fix application.
### New Rules
Mrnikbobjeff marked this conversation as resolved.
Show resolved Hide resolved
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
CA2250 | Usage | Warning | DetectPLINQNops, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2250)
Original file line number Diff line number Diff line change
Expand Up @@ -1479,4 +1479,13 @@
<data name="PlatformCompatibilityCheckSupportedOsMessage" xml:space="preserve">
<value>'{0}' is supported on '{1}'</value>
</data>
<data name="DetectPLINQNopsDescription" xml:space="preserve">
<value>AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</value>
</data>
<data name="DetectPLINQNopsMessage" xml:space="preserve">
<value>AsParallel() Expression '{0}' has no effect</value>
</data>
<data name="DetectPLINQNopsTitle" xml:space="preserve">
<value>AsParallel() at the end of a LINQ query does not have an effect</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Microsoft.NetCore.Analyzers.Usage
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = DetectPLINQNops.RuleId), Shared]
public sealed class DetectPLINQNopsFixer : CodeFixProvider
Mrnikbobjeff marked this conversation as resolved.
Show resolved Hide resolved
{
private static readonly string[] removableEnds = new string[] { "ToList", "ToArray", "AsParallel" };
Mrnikbobjeff marked this conversation as resolved.
Show resolved Hide resolved
public sealed override ImmutableArray<string> FixableDiagnosticIds
{
get { return ImmutableArray.Create(DetectPLINQNops.RuleId); }
}

public sealed override FixAllProvider GetFixAllProvider()
{
return WellKnownFixAllProviders.BatchFixer;
}

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

foreach (var diagnostic in context.Diagnostics)
{
var diagnosticSpan = diagnostic.Location.SourceSpan;
var declaration = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType<InvocationExpressionSyntax>().Last();

context.RegisterCodeFix(
new AsPArallelCodeAction(
title: MicrosoftNetCoreAnalyzersResources.RemoveRedundantCall,
createChangedSolution: c => RemoveAsParallelCall(context.Document, declaration, c),
equivalenceKey: MicrosoftNetCoreAnalyzersResources.RemoveRedundantCall),
diagnostic);
}
}

private static async Task<Solution> RemoveAsParallelCall(Document document, InvocationExpressionSyntax invocationExpression, CancellationToken cancellationToken)
{
var originalSolution = document.Project.Solution;
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
ExpressionSyntax possibleInvocation = invocationExpression;
do
{
var newExpression = ((possibleInvocation as InvocationExpressionSyntax)!.Expression as MemberAccessExpressionSyntax)!.Expression;
Mrnikbobjeff marked this conversation as resolved.
Show resolved Hide resolved
possibleInvocation = newExpression;
} while (possibleInvocation is InvocationExpressionSyntax nestedInvocation && nestedInvocation.Expression is MemberAccessExpressionSyntax member && removableEnds.Contains(member.Name.Identifier.ValueText));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to have only a check on the method name as a string? It might be better to check for the method symbol.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I was unclear about as it also came up in my other pr, do we have to recheck everything in the fixer that we already did in the analyzer? I.E. if we warn here we already checked the symbol in the actual analyzer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also bit unclear for me how much needs to be rechecked for the fixer.

return originalSolution.WithDocumentSyntaxRoot(document.Id, root.ReplaceNode(invocationExpression, possibleInvocation));
}
private class AsPArallelCodeAction : SolutionChangeAction
Mrnikbobjeff marked this conversation as resolved.
Show resolved Hide resolved
{
public AsPArallelCodeAction(string title, Func<CancellationToken, Task<Solution>> createChangedSolution, string equivalenceKey)
: base(title, createChangedSolution, equivalenceKey)
{
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Linq;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Microsoft.NetCore.Analyzers.Usage
{
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class DetectPLINQNops : DiagnosticAnalyzer
{
internal const string RuleId = "CA2250";
private static readonly string[] s_knownCalls = new string[] { "ToList", "ToArray" };
internal static readonly LocalizableString localizableTitle = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DetectPLINQNopsTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));

private static readonly LocalizableString s_localizableMessageDefault = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DetectPLINQNopsMessage), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DetectPLINQNopsDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));

internal static readonly DiagnosticDescriptor DefaultRule = DiagnosticDescriptorHelper.Create(RuleId,
localizableTitle,
s_localizableMessageDefault,
DiagnosticCategory.Usage,
RuleLevel.BuildWarning,
description: s_localizableDescription,
isPortedFxCopRule: false,
isDataflowRule: false,
isEnabledByDefaultInFxCopAnalyzers: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(DefaultRule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(AnalyzeSymbol, SyntaxKind.InvocationExpression);
}

private static void AnalyzeSymbol(SyntaxNodeAnalysisContext context)
{
var invocation = (InvocationExpressionSyntax)context.Node;

if (invocation.Expression is MemberAccessExpressionSyntax memberAccess)
{
if (!memberAccess.Name.Identifier.ValueText.Equals("AsParallel", StringComparison.Ordinal))
{
if (!(s_knownCalls.Contains(memberAccess.Name.Identifier.ValueText)
&& memberAccess.Expression is InvocationExpressionSyntax nestedInvocation
&& nestedInvocation.Expression is MemberAccessExpressionSyntax possibleAsParallelCall
&& possibleAsParallelCall.Name.Identifier.ValueText.Equals("AsParallel", StringComparison.Ordinal)))
{
return;
}
}//true when it is the last statement or second last
}
if (invocation.Parent is ForEachStatementSyntax parentForEach)
{
if (parentForEach.Expression.IsEquivalentTo(invocation) || //Last call is AsParallel
parentForEach.Expression is MemberAccessExpressionSyntax mem && s_knownCalls.Contains(mem.Name.Identifier.ValueText) //OrToList and ToValue
)
{
var diagnostic = Diagnostic.Create(DefaultRule, invocation.GetLocation(), invocation);
context.ReportDiagnostic(diagnostic);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,21 @@
<target state="translated">Nepoužívat zastaralé hodnoty SslProtocols</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsDescription">
<source>AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</source>
<target state="new">AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsMessage">
<source>AsParallel() Expression '{0}' has no effect</source>
<target state="new">AsParallel() Expression '{0}' has no effect</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsTitle">
<source>AsParallel() at the end of a LINQ query does not have an effect</source>
<target state="new">AsParallel() at the end of a LINQ query does not have an effect</target>
<note />
</trans-unit>
<trans-unit id="DisposableFieldsShouldBeDisposedDescription">
<source>A type that implements System.IDisposable declares fields that are of types that also implement IDisposable. The Dispose method of the field is not called by the Dispose method of the declaring type. To fix a violation of this rule, call Dispose on fields that are of types that implement IDisposable if you are responsible for allocating and releasing the unmanaged resources held by the field.</source>
<target state="translated">Typ, který implementuje System.IDisposable, deklaruje pole, která mají typ, který IDisposable implementuje taky. Metoda Dispose deklarujícího typu nevolá metodu Dispose daného pole. Pokud chcete porušení tohoto pravidla opravit a je vaší odpovědností přidělovat a uvolňovat nespravované prostředky, které pole uchovává, zavolejte Dispose pro pole s typy, které implementují IDisposable.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,21 @@
<target state="translated">Keine veralteten SslProtocols-Werte verwenden</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsDescription">
<source>AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</source>
<target state="new">AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsMessage">
<source>AsParallel() Expression '{0}' has no effect</source>
<target state="new">AsParallel() Expression '{0}' has no effect</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsTitle">
<source>AsParallel() at the end of a LINQ query does not have an effect</source>
<target state="new">AsParallel() at the end of a LINQ query does not have an effect</target>
<note />
</trans-unit>
<trans-unit id="DisposableFieldsShouldBeDisposedDescription">
<source>A type that implements System.IDisposable declares fields that are of types that also implement IDisposable. The Dispose method of the field is not called by the Dispose method of the declaring type. To fix a violation of this rule, call Dispose on fields that are of types that implement IDisposable if you are responsible for allocating and releasing the unmanaged resources held by the field.</source>
<target state="translated">Ein Typ, der System.IDisposable implementiert, deklariert Felder, die Typen aufweisen, die ebenfalls IDisposable implementieren. Die Dispose-Methode des Felds wird nicht von der Dispose-Methode des deklarierenden Typs aufgerufen. Wenn Sie für das Zuordnen und Freigeben nicht verwalteter Ressourcen zuständig sind, die von diesem Feld belegt werden, rufen Sie zum Beheben eines Verstoßes gegen diese Regel Dispose für Felder auf, die Typen aufweisen, welche IDisposable implementieren.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,21 @@
<target state="translated">No usar valores de SslProtocols en desuso</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsDescription">
<source>AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</source>
<target state="new">AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsMessage">
<source>AsParallel() Expression '{0}' has no effect</source>
<target state="new">AsParallel() Expression '{0}' has no effect</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsTitle">
<source>AsParallel() at the end of a LINQ query does not have an effect</source>
<target state="new">AsParallel() at the end of a LINQ query does not have an effect</target>
<note />
</trans-unit>
<trans-unit id="DisposableFieldsShouldBeDisposedDescription">
<source>A type that implements System.IDisposable declares fields that are of types that also implement IDisposable. The Dispose method of the field is not called by the Dispose method of the declaring type. To fix a violation of this rule, call Dispose on fields that are of types that implement IDisposable if you are responsible for allocating and releasing the unmanaged resources held by the field.</source>
<target state="translated">Un tipo que implementa System.IDisposable declara campos de tipos que también implementan IDisposable. El método Dispose del tipo declarativo no llama al método Dispose del campo. Para corregir una infracción de esta regla, llame a Dispose en campos que sean de tipos que implementan IDisposable si usted es el responsable de asignar y liberar los recursos no administrados que contiene el campo.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,21 @@
<target state="translated">N'utilisez pas de valeurs dépréciées pour SslProtocols</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsDescription">
<source>AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</source>
<target state="new">AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsMessage">
<source>AsParallel() Expression '{0}' has no effect</source>
<target state="new">AsParallel() Expression '{0}' has no effect</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsTitle">
<source>AsParallel() at the end of a LINQ query does not have an effect</source>
<target state="new">AsParallel() at the end of a LINQ query does not have an effect</target>
<note />
</trans-unit>
<trans-unit id="DisposableFieldsShouldBeDisposedDescription">
<source>A type that implements System.IDisposable declares fields that are of types that also implement IDisposable. The Dispose method of the field is not called by the Dispose method of the declaring type. To fix a violation of this rule, call Dispose on fields that are of types that implement IDisposable if you are responsible for allocating and releasing the unmanaged resources held by the field.</source>
<target state="translated">Un type qui implémente System.IDisposable déclare des champs dont les types implémentent également IDisposable. La méthode Dispose du champ n'est pas appelée par la méthode Dispose du type déclarant. Pour corriger toute violation de cette règle, appelez Dispose sur les champs dont les types implémentent IDisposable si vous êtes responsable de l'allocation et de la libération des ressources non managées détenues par le champ.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,21 @@
<target state="translated">Non usare valori SslProtocols deprecati</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsDescription">
<source>AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</source>
<target state="new">AsParallel() does not have an effect if it is the last Expression or the only remaining Expressions are ToList() or ToArray().</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsMessage">
<source>AsParallel() Expression '{0}' has no effect</source>
<target state="new">AsParallel() Expression '{0}' has no effect</target>
<note />
</trans-unit>
<trans-unit id="DetectPLINQNopsTitle">
<source>AsParallel() at the end of a LINQ query does not have an effect</source>
<target state="new">AsParallel() at the end of a LINQ query does not have an effect</target>
<note />
</trans-unit>
<trans-unit id="DisposableFieldsShouldBeDisposedDescription">
<source>A type that implements System.IDisposable declares fields that are of types that also implement IDisposable. The Dispose method of the field is not called by the Dispose method of the declaring type. To fix a violation of this rule, call Dispose on fields that are of types that implement IDisposable if you are responsible for allocating and releasing the unmanaged resources held by the field.</source>
<target state="translated">Un tipo che implementa System.IDisposable dichiara i campi di tipi che implementano anch'essi IDisposable. Il metodo Dispose del campo non viene chiamato dal metodo Dispose del tipo dichiarante. Per correggere una violazione di questa regola, chiamare Dispose su campi di tipi che implementano IDisposable se si è responsabile dell'allocazione e del rilascio delle risorse non gestite utilizzate dal campo.</target>
Expand Down
Loading