Skip to content

Commit

Permalink
Merge pull request #2736 from paulomorgado/features/UsePropertyInstea…
Browse files Browse the repository at this point in the history
…dOfCountMethodWhenAvailable

Use property instead of count method when available
  • Loading branch information
mavasani committed Sep 12, 2019
2 parents 9c43dfb + 8e28777 commit 6c0fad0
Show file tree
Hide file tree
Showing 32 changed files with 1,741 additions and 212 deletions.

Large diffs are not rendered by default.

Expand Up @@ -1489,6 +1489,20 @@
"Visual Basic"
]
}
},
"CA2246": {
"id": "CA2246",
"shortDescription": "Assigning symbol and its member in the same statement.",
"fullDescription": "Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.",
"defaultLevel": "warning",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "AssigningSymbolAndItsMemberInSameStatement",
"languages": [
"C#"
]
}
}
}
},
Expand Down Expand Up @@ -3871,6 +3885,21 @@
]
}
},
"CA1829": {
"id": "CA1829",
"shortDescription": "Use Length/Count property instead of Count() when available",
"fullDescription": "Enumerable.Count() potentially enumerates the sequence while a Length/Count property is a direct access.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/visualstudio/code-quality/ca1829",
"properties": {
"category": "Performance",
"isEnabledByDefault": true,
"typeName": "CSharpUsePropertyInsteadOfCountMethodWhenAvailableAnalyzer",
"languages": [
"C#"
]
}
},
"CA2010": {
"id": "CA2010",
"shortDescription": "Always consume the value returned by methods marked with PreserveSigAttribute",
Expand Down Expand Up @@ -4006,6 +4035,21 @@
]
}
},
"CA1829": {
"id": "CA1829",
"shortDescription": "Use Length/Count property instead of Count() when available",
"fullDescription": "Enumerable.Count() potentially enumerates the sequence while a Length/Count property is a direct access.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/visualstudio/code-quality/ca1829",
"properties": {
"category": "Performance",
"isEnabledByDefault": true,
"typeName": "BasicUsePropertyInsteadOfCountMethodWhenAvailableAnalyzer",
"languages": [
"Visual Basic"
]
}
},
"CA2010": {
"id": "CA2010",
"shortDescription": "Always consume the value returned by methods marked with PreserveSigAttribute",
Expand Down
Expand Up @@ -87,3 +87,4 @@ Sr. No. | Rule ID | Title | Category | Enabled | CodeFix | Description |
84 | [CA2234](https://docs.microsoft.com/visualstudio/code-quality/ca2234-pass-system-uri-objects-instead-of-strings) | Pass system uri objects instead of strings | Usage | True | False | A call is made to a method that has a string parameter whose name contains "uri", "URI", "urn", "URN", "url", or "URL". The declaring type of the method contains a corresponding method overload that has a System.Uri parameter. |
85 | CA2244 | Do not duplicate indexed element initializations | Usage | True | False | Indexed elements in objects initializers must initialize unique elements. A duplicate index might overwrite a previous element initialization. |
86 | CA2245 | Do not assign a property to itself. | Usage | True | False | The property {0} should not be assigned to itself. |
87 | CA2246 | Assigning symbol and its member in the same statement. | Usage | True | False | Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements. |
Expand Up @@ -1474,6 +1474,20 @@
"Visual Basic"
]
}
},
"CA2246": {
"id": "CA2246",
"shortDescription": "Assigning symbol and its member in the same statement.",
"fullDescription": "Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.",
"defaultLevel": "warning",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "AssigningSymbolAndItsMemberInSameStatement",
"languages": [
"C#"
]
}
}
}
},
Expand Down
@@ -0,0 +1,50 @@
// 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.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.NetCore.Analyzers.Performance;

namespace Microsoft.NetCore.CSharp.Analyzers.Performance
{
/// <summary>
/// CA1829: C# implementation of use property instead of <see cref="System.Linq.Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})"/>, when available.
/// Implements the <see cref="CodeFixProvider" />
/// </summary>
/// <seealso cref="UsePropertyInsteadOfCountMethodWhenAvailableFixer"/>
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class CSharpUsePropertyInsteadOfCountMethodWhenAvailableFixer : UsePropertyInsteadOfCountMethodWhenAvailableFixer
{
/// <summary>
/// Gets the expression from the specified <paramref name="invocationNode" /> where to replace the invocation of the
/// <see cref="System.Linq.Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})" /> method with a property invocation.
/// </summary>
/// <param name="invocationNode">The invocation node to get a fixer for.</param>
/// <param name="memberAccessNode">The member access node for the invocation node.</param>
/// <param name="nameNode">The name node for the invocation node.</param>
/// <returns><see langword="true" /> if a <paramref name="memberAccessNode" /> and <paramref name="nameNode" /> were found;
/// <see langword="false" /> otherwise.</returns>
protected override bool TryGetExpression(SyntaxNode invocationNode, out SyntaxNode memberAccessNode, out SyntaxNode nameNode)
{
if (invocationNode is InvocationExpressionSyntax invocationExpression)
{
switch (invocationExpression.Expression)
{
case MemberAccessExpressionSyntax memberAccessExpression:
memberAccessNode = invocationExpression.Expression;
nameNode = memberAccessExpression.Name;
return true;
case MemberBindingExpressionSyntax memberBindingExpression:
memberAccessNode = invocationExpression.Expression;
nameNode = memberBindingExpression.Name;
return true;
}
}

memberAccessNode = default;
nameNode = default;
return false;
}
}
}
@@ -0,0 +1,62 @@
// 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.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.NetCore.Analyzers.Performance;

namespace Microsoft.NetCore.CSharp.Analyzers.Performance
{
/// <summary>
/// CA1829: C# implementation of use property instead of <see cref="Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})"/>, when available.
/// Implements the <see cref="UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer" />
/// </summary>
/// <remarks>
/// Flags the use of <see cref="Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})"/> on types that are know to have a property with the same semantics:
/// <c>Length</c>, <c>Count</c>.
/// </remarks>
/// <seealso cref="UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer"/>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpUsePropertyInsteadOfCountMethodWhenAvailableAnalyzer
: UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer
{
/// <summary>
/// Creates the operation actions handler.
/// </summary>
/// <param name="context">The context.</param>
/// <returns>The operation actions handler.</returns>
protected override OperationActionsHandler CreateOperationActionsHandler(OperationActionsContext context)
=> new CSharpOperationActionsHandler(context);

/// <summary>
/// Handler for operaction actions for C#. This class cannot be inherited.
/// Implements the <see cref="Microsoft.NetCore.Analyzers.Performance.UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsHandler" />
/// </summary>
/// <seealso cref="Microsoft.NetCore.Analyzers.Performance.UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsHandler" />
private sealed class CSharpOperationActionsHandler : OperationActionsHandler
{
internal CSharpOperationActionsHandler(OperationActionsContext context)
: base(context)
{
}

protected override ITypeSymbol GetEnumerableCountInvocationTargetType(IInvocationOperation invocationOperation)
{
var method = invocationOperation.TargetMethod;

if (invocationOperation.Arguments.Length == 1 &&
method.Name.Equals(nameof(Enumerable.Count), StringComparison.Ordinal) &&
this.Context.IsEnumerableType(method.ContainingSymbol))
{
return invocationOperation.Arguments[0].Value is IConversionOperation convertionOperation
? convertionOperation.Operand.Type
: invocationOperation.Arguments[0].Value.Type;
}

return null;
}
}
}
}
Expand Up @@ -1134,6 +1134,15 @@
<data name="DoNotUseCountAsyncWhenAnyAsyncCanBeUsedTitle" xml:space="preserve">
<value>Do not use CountAsync() or LongCountAsync() when AnyAsync() can be used</value>
</data>
<data name="UsePropertyInsteadOfCountMethodWhenAvailableDescription" xml:space="preserve">
<value>Enumerable.Count() potentially enumerates the sequence while a Length/Count property is a direct access.</value>
</data>
<data name="UsePropertyInsteadOfCountMethodWhenAvailableMessage" xml:space="preserve">
<value>Use the "{0}" property instead of Enumerable.Count().</value>
</data>
<data name="UsePropertyInsteadOfCountMethodWhenAvailableTitle" xml:space="preserve">
<value>Use Length/Count property instead of Count() when available</value>
</data>
<data name="SetHttpOnlyForHttpCookie" xml:space="preserve">
<value>Set HttpOnly to true for HttpCookie</value>
</data>
Expand Down
@@ -0,0 +1,123 @@
// 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.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;

namespace Microsoft.NetCore.Analyzers.Performance
{
/// <summary>
/// CA1829: Use property instead of <see cref="Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})"/>, when available.
/// Implements the <see cref="CodeFixProvider" />
/// </summary>
public abstract class UsePropertyInsteadOfCountMethodWhenAvailableFixer : CodeFixProvider
{
/// <summary>
/// A list of diagnostic IDs that this provider can provider fixes for.
/// </summary>
/// <value>The fixable diagnostic ids.</value>
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.RuleId);


/// <summary>
/// Gets an optional <see cref="FixAllProvider" /> that can fix all/multiple occurrences of diagnostics fixed by this code fix provider.
/// Return null if the provider doesn't support fix all/multiple occurrences.
/// Otherwise, you can return any of the well known fix all providers from <see cref="WellKnownFixAllProviders" /> or implement your own fix all provider.
/// </summary>
/// <returns>FixAllProvider.</returns>
public sealed override FixAllProvider GetFixAllProvider()
{
// See https://github.com/dotnet/roslyn/blob/master/docs/analyzers/FixAllProvider.md for more information on Fix All Providers
return WellKnownFixAllProviders.BatchFixer;
}

/// <summary>
/// Computes one or more fixes for the specified <see cref="CodeFixContext" />.
/// </summary>
/// <param name="context">A <see cref="CodeFixContext" /> containing context information about the diagnostics to fix.
/// The context must only contain diagnostics with a <see cref="Diagnostic.Id" /> included in the <see cref="CodeFixProvider.FixableDiagnosticIds" />
/// for the current provider.</param>
/// <returns>A <see cref="Task" /> that represents the asynchronous operation.</returns>
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var node = root.FindNode(context.Span);

if (node is object &&
context.Diagnostics[0].Properties.TryGetValue(UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.PropertyNameKey, out var propertyName) &&
propertyName is object &&
TryGetExpression(node, out var expressionNode, out var nameNode))
{
context.RegisterCodeFix(
new UsePropertyInsteadOfCountMethodWhenAvailableCodeAction(context.Document, node, expressionNode, nameNode, propertyName),
context.Diagnostics);
}
}

/// <summary>
/// Gets the expression from the specified <paramref name="invocationNode" /> where to replace the invocation of the
/// <see cref="Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})" /> method with a property invocation.
/// </summary>
/// <param name="invocationNode">The invocation node to get a fixer for.</param>
/// <param name="memberAccessNode">The member access node for the invocation node.</param>
/// <param name="nameNode">The name node for the invocation node.</param>
/// <returns><see langword="true"/> if a <paramref name="memberAccessNode" /> and <paramref name="nameNode"/> were found;
/// <see langword="false" /> otherwise.</returns>
protected abstract bool TryGetExpression(SyntaxNode invocationNode, out SyntaxNode memberAccessNode, out SyntaxNode nameNode);

/// <summary>
/// Implements the <see cref="CodeAction"/> for replacing the use of <see cref="System.Linq.Enumerable.Count{TSource}(System.Collections.Generic.IEnumerable{TSource})"/>
/// for the use of a property of the receiving type.
/// This class cannot be inherited.
/// </summary>
/// <seealso cref="Microsoft.CodeAnalysis.CodeActions.CodeAction" />
private sealed class UsePropertyInsteadOfCountMethodWhenAvailableCodeAction : CodeAction
{
private readonly Document _document;
private readonly SyntaxNode _invocationNode;
private readonly SyntaxNode _memberAccessNode;
private readonly SyntaxNode _nameNode;
private readonly string _propertyName;

public UsePropertyInsteadOfCountMethodWhenAvailableCodeAction(
Document document,
SyntaxNode invocationNode,
SyntaxNode memberAccessNode,
SyntaxNode nameNode,
string propertyName)
{
this._document = document;
this._invocationNode = invocationNode;
this._memberAccessNode = memberAccessNode;
this._nameNode = nameNode;
this._propertyName = propertyName;
}

/// <inheritdoc/>
public override string Title { get; } = MicrosoftNetCoreAnalyzersResources.UsePropertyInsteadOfCountMethodWhenAvailableTitle;

/// <inheritdoc/>
public override string EquivalenceKey { get; } = MicrosoftNetCoreAnalyzersResources.UsePropertyInsteadOfCountMethodWhenAvailableTitle;

/// <inheritdoc/>
protected sealed override async Task<Document> GetChangedDocumentAsync(CancellationToken cancellationToken)
{
var editor = await DocumentEditor.CreateAsync(this._document, cancellationToken).ConfigureAwait(false);
var generator = editor.Generator;
var replacementSyntax = generator.ReplaceNode(this._memberAccessNode, this._nameNode, generator.IdentifierName(_propertyName))
.WithAdditionalAnnotations(Formatter.Annotation)
.WithTriviaFrom(this._invocationNode);

editor.ReplaceNode(this._invocationNode, replacementSyntax);

return editor.GetChangedDocument();
}
}
}
}

0 comments on commit 6c0fad0

Please sign in to comment.