Skip to content

Commit

Permalink
CA1829: Use property instead of Enumerable.Count{).
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulomorgado
paulomorgado committed Aug 10, 2019
1 parent 97498cb commit 97c14d0
Show file tree
Hide file tree
Showing 8 changed files with 657 additions and 342 deletions.
214 changes: 108 additions & 106 deletions src/Microsoft.CodeAnalysis.FxCopAnalyzers/Microsoft.CodeAnalysis.FxCopAnalyzers.md
View file

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions src/Microsoft.CodeAnalysis.FxCopAnalyzers/Microsoft.CodeAnalysis.FxCopAnalyzers.sarif
View file
Expand Up @@ -3661,6 +3661,25 @@
"Telemetry"
]
}
},
"CA5396": {
"id": "CA5396",
"shortDescription": "Set HttpOnly to true for HttpCookie",
"fullDescription": "As a defense in depth measure, ensure security sensitive HTTP cookies are marked as HttpOnly. This indicates web browsers should disallow scripts from accessing the cookies. Injected malicious scripts are a common way of stealing cookies.",
"defaultLevel": "warning",
"properties": {
"category": "Security",
"isEnabledByDefault": false,
"typeName": "SetHttpOnlyForHttpCookie",
"languages": [
"C#",
"Visual Basic"
],
"tags": [
"Dataflow",
"Telemetry"
]
}
}
}
},
Expand Down
148 changes: 108 additions & 40 deletions ...rosoft.NetCore.Analyzers/Core/Performance/UsePropertyInsteadOfCountMethodWhenAvailable.cs
View file
@@ -1,6 +1,7 @@
// 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;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
Expand All @@ -25,6 +26,8 @@ namespace Microsoft.NetCore.Analyzers.Performance
public sealed class UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA1829";
private const string CountPropertyName = "Count";
private const string LengthPropertyName = "Length";
private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.UsePropertyInsteadOfCountMethodWhenAvailableTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.UsePropertyInsteadOfCountMethodWhenAvailableMessage), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.UsePropertyInsteadOfCountMethodWhenAvailableDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));
Expand All @@ -40,8 +43,6 @@ public sealed class UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer : Diagn
helpLinkUri: "https://docs.microsoft.com/visualstudio/code-quality/" + RuleId.ToLowerInvariant());
#pragma warning restore CA1308 // Normalize strings to uppercase

private static readonly ImmutableHashSet<string> propertyNames = ImmutableHashSet.Create("Length", "Count");

/// <summary>
/// Returns a set of descriptors for the diagnostics that this analyzer is capable of producing.
/// </summary>
Expand All @@ -68,42 +69,65 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context)
{
if (WellKnownTypes.Enumerable(context.Compilation) is INamedTypeSymbol enumerableType)
{
var operationActionsContext = new UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsContext(
context.Compilation,
enumerableType,
WellKnownTypes.ICollection(context.Compilation)?.GetMembers(CountPropertyName).OfType<IPropertySymbol>().SingleOrDefault(),
WellKnownTypes.GenericICollection(context.Compilation)?.GetMembers(CountPropertyName).OfType<IPropertySymbol>().SingleOrDefault(),
WellKnownTypes.ImmutableArray(context.Compilation));

var operationActionsHandler = context.Compilation.Language == LanguageNames.CSharp
? (OperationActionsHandler)new CSharpOperationActionsHandler(containingSymbol: enumerableType, rule: s_rule)
: (OperationActionsHandler)new BasicOperationActionsHandler(containingSymbol: enumerableType, rule: s_rule);
? (OperationActionsHandler)new CSharpOperationActionsHandler(operationActionsContext)
: (OperationActionsHandler)new BasicOperationActionsHandler(operationActionsContext);

context.RegisterOperationAction(
operationActionsHandler.AnalyzeInvocationOperation,
OperationKind.Invocation);
}
}

private sealed class OperationActionsContext
{
public OperationActionsContext(Compilation compilation, INamedTypeSymbol enumerableType, IPropertySymbol collectionCountProperty, IPropertySymbol collectionOfTCountProperty, INamedTypeSymbol immutableArrayType)
{
Compilation = compilation;
EnumerableType = enumerableType;
CollectionCountProperty = collectionCountProperty;
CollectionOfTCountProperty = collectionOfTCountProperty;
ImmutableArrayType = immutableArrayType;
}

public Compilation Compilation { get; }
public INamedTypeSymbol EnumerableType { get; }
public IPropertySymbol CollectionCountProperty { get; }
public IPropertySymbol CollectionOfTCountProperty { get; }
public INamedTypeSymbol ImmutableArrayType { get; }
}

/// <summary>
/// Handler for operaction actions.
/// </summary>
private abstract class OperationActionsHandler
{
private readonly INamedTypeSymbol containingSymbol;
private readonly DiagnosticDescriptor rule;

protected OperationActionsHandler(INamedTypeSymbol containingSymbol, DiagnosticDescriptor rule)
protected OperationActionsHandler(OperationActionsContext context)
{
this.containingSymbol = containingSymbol;
this.rule = rule;
Context = context;
}

public OperationActionsContext Context { get; }

internal void AnalyzeInvocationOperation(OperationAnalysisContext context)
{
var invocationOperation = (IInvocationOperation)context.Operation;

if (GetEnumerableCountInvocationTargetType(invocationOperation, this.containingSymbol) is ITypeSymbol invocationTarget &&
if (GetEnumerableCountInvocationTargetType(invocationOperation) is ITypeSymbol invocationTarget &&
GetReplacementProperty(invocationTarget) is string propertyName)
{
var propertiesBuilder = ImmutableDictionary.CreateBuilder<string, string>();
propertiesBuilder.Add("PropertyName", propertyName);

var diagnostic = Diagnostic.Create(
this.rule,
s_rule,
invocationOperation.Syntax.GetLocation(),
propertiesBuilder.ToImmutable(),
propertyName);
Expand All @@ -112,35 +136,79 @@ internal void AnalyzeInvocationOperation(OperationAnalysisContext context)
}
}

protected abstract ITypeSymbol GetEnumerableCountInvocationTargetType(IInvocationOperation invocationOperation, INamedTypeSymbol containingSymbol);
protected abstract ITypeSymbol GetEnumerableCountInvocationTargetType(IInvocationOperation invocationOperation);

private static string GetReplacementProperty(ITypeSymbol invocationTarget)
private string GetReplacementProperty(ITypeSymbol invocationTarget)
{
if (invocationTarget.TypeKind == TypeKind.Array)
{
return nameof(Array.Length);
return LengthPropertyName;
}

foreach (var member in invocationTarget.GetMembers())
if (invocationTarget.OriginalDefinition is ITypeSymbol originalDefinition &&
originalDefinition.MetadataName.ToString().Equals(typeof(ImmutableArray<>).Name, StringComparison.Ordinal) &&
originalDefinition.ContainingNamespace.ToString().Equals(typeof(ImmutableArray<>).Namespace, StringComparison.Ordinal))
{
if (member is IPropertySymbol property && propertyNames.Contains(property.Name))
{
return property.Name;
}
return LengthPropertyName;
}

if (invocationTarget.FindImplementationForInterfaceMember(this.Context.CollectionCountProperty) is IPropertySymbol countProperty &&
!countProperty.ExplicitInterfaceImplementations.Any())
{
return CountPropertyName;
}

foreach (var type in invocationTarget.AllInterfaces)
if (findImplementationForCollectionOfTInterfaceCountProperty(invocationTarget))
{
foreach (var member in type.GetMembers())
return CountPropertyName;
}

return null;

bool findImplementationForCollectionOfTInterfaceCountProperty(ITypeSymbol invocationTarget)
{
if (isCollectionOfTInterface(invocationTarget))
{
if (member is IPropertySymbol property && propertyNames.Contains(property.Name))
return true;
}

if (invocationTarget.TypeKind == TypeKind.Interface)
{
if (invocationTarget.GetMembers(CountPropertyName).OfType<IPropertySymbol>().Any())
{
return property.Name;
return false;
}

foreach (var @interface in invocationTarget.AllInterfaces)
{
if (@interface.OriginalDefinition is INamedTypeSymbol originalInterfaceDefinition &&
isCollectionOfTInterface(originalInterfaceDefinition))
{
return true;
}
}
}
else
{
foreach (var @interface in invocationTarget.AllInterfaces)
{
if (@interface.OriginalDefinition is INamedTypeSymbol originalInterfaceDefinition &&
isCollectionOfTInterface(originalInterfaceDefinition))
{
if (invocationTarget.FindImplementationForInterfaceMember(@interface.GetMembers(CountPropertyName)[0]) is IPropertySymbol propertyImplementation &&
!propertyImplementation.ExplicitInterfaceImplementations.Any())
{
return true;
}
}
}
}
}

return null;
return false;

bool isCollectionOfTInterface(ITypeSymbol type)
=> type.OriginalDefinition?.Equals(this.Context.CollectionOfTCountProperty.ContainingSymbol) ?? false;
}
}
}

Expand All @@ -151,23 +219,23 @@ private static string GetReplacementProperty(ITypeSymbol invocationTarget)
/// <seealso cref="Microsoft.NetCore.Analyzers.Performance.UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsHandler" />
private sealed class CSharpOperationActionsHandler : OperationActionsHandler
{
internal CSharpOperationActionsHandler(INamedTypeSymbol containingSymbol, DiagnosticDescriptor rule) : base(containingSymbol, rule)
internal CSharpOperationActionsHandler(UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsContext context)
: base(context)
{
}

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

if (invocationOperation.Arguments.Length == 1 &&
method.Name.Equals(nameof(Enumerable.Count), StringComparison.Ordinal) &&
method.ContainingSymbol.Equals(containingSymbol))
method.ContainingSymbol.Equals(this.Context.EnumerableType) &&
((INamedTypeSymbol)(method.Parameters[0].Type)).TypeArguments[0] is ITypeSymbol methodSourceItemType)
{
var targetType = invocationOperation.Arguments[0].Value is IConversionOperation convertionOperation
return invocationOperation.Arguments[0].Value is IConversionOperation convertionOperation
? convertionOperation.Operand.Type
: invocationOperation.Arguments[0].Value.Type;

return targetType as ITypeSymbol;
}

return null;
Expand All @@ -181,23 +249,23 @@ protected override ITypeSymbol GetEnumerableCountInvocationTargetType(IInvocatio
/// <seealso cref="Microsoft.NetCore.Analyzers.Performance.UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsHandler" />
private sealed class BasicOperationActionsHandler : OperationActionsHandler
{
internal BasicOperationActionsHandler(INamedTypeSymbol containingSymbol, DiagnosticDescriptor rule) : base(containingSymbol, rule)
internal BasicOperationActionsHandler(UsePropertyInsteadOfCountMethodWhenAvailableAnalyzer.OperationActionsContext context)
: base(context)
{
}

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

if (invocationOperation.Arguments.Length == 0 &&
method.Name.Equals(nameof(Enumerable.Count), StringComparison.Ordinal) &&
method.ContainingSymbol.Equals(containingSymbol))
method.ContainingSymbol.Equals(this.Context.EnumerableType) &&
((INamedTypeSymbol)(invocationOperation.Instance.Type)).TypeArguments[0] is ITypeSymbol methodSourceItemType)
{
var targetType = invocationOperation.Instance is IConversionOperation convertionOperation
? convertionOperation.Operand.Type
: invocationOperation.Instance.Type;

return targetType as ITypeSymbol;
return invocationOperation.Instance is IConversionOperation convertionOperation
? convertionOperation.Operand.Type
: invocationOperation.Instance.Type;
}

return null;
Expand Down

0 comments on commit 97c14d0

Please sign in to comment.