Skip to content

Commit

Permalink
Enable NetAnalyzers reliability and globalization rules (#22625)
Browse files Browse the repository at this point in the history
The following rules led to code changes:

- CA2000: Dispose objects before losing scope
- CA2012: Use ValueTasks correctly
- CA2008: Do not create tasks without passing a TaskScheduler
- CA2016: Forward the 'CancellationToken' parameter to methods that take one
- CA1310: Specify StringComparison for correctness
- CA1309: Use ordinal stringcomparison
- CA1304: Specify CultureInfo

Part of #18870
  • Loading branch information
roji committed Sep 22, 2020
1 parent a9871fe commit a0a053b
Show file tree
Hide file tree
Showing 55 changed files with 170 additions and 164 deletions.
2 changes: 1 addition & 1 deletion rulesets/FxCop.ruleset
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
<Rule Id="CA1829" Action="None" /> <!-- Use Length/Count property instead of Count() when available -->
<Rule Id="CA2000" Action="None" /> <!-- Dispose objects before losing scope -->
<Rule Id="CA2002" Action="None" /> <!-- Do not lock on objects with weak identity -->
<Rule Id="CA2007" Action="Warning" /> <!-- Consider calling ConfigureAwait on the awaited task -->
<Rule Id="CA2007" Action="None" /> <!-- Consider calling ConfigureAwait on the awaited task -->
<Rule Id="CA2008" Action="None" /> <!-- Do not create tasks without passing a TaskScheduler -->
<Rule Id="CA2009" Action="None" /> <!-- Do not call ToImmutableCollection on an ImmutableCollection value -->
<Rule Id="CA2010" Action="None" /> <!-- Always consume the value returned by methods marked with PreserveSigAttribute -->
Expand Down
68 changes: 68 additions & 0 deletions src/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Code analysis rules and configuration

root = false

# Code files
[*.cs]

# Globalization rules

# CA1303: Do not pass literals as localized parameters
dotnet_diagnostic.CA1303.severity = none

# CA1304: Specify CultureInfo
dotnet_diagnostic.CA1304.severity = error

# CA1305: Specify IFormatProvider
dotnet_diagnostic.CA1305.severity = none

# CA1307: Specify StringComparison for clarity
dotnet_diagnostic.CA1307.severity = none

# CA1308: Normalize strings to uppercase
dotnet_diagnostic.CA1308.severity = none

# CA1309: Use ordinal stringcomparison
dotnet_diagnostic.CA1309.severity = error

# CA1310: Specify StringComparison for correctness
dotnet_diagnostic.CA1310.severity = error

# CA2101: Specify marshaling for P/Invoke string arguments
dotnet_diagnostic.CA2101.severity = error

# Reliability Rules

# CA2000: Dispose objects before losing scope
# Not reliable enough - false positives
dotnet_diagnostic.CA2000.severity = suggestion

# CA2002: Do not lock on objects with weak identity
dotnet_diagnostic.CA2002.severity = error

# CA2007: Consider calling ConfigureAwait on the awaited task
dotnet_diagnostic.CA2007.severity = error

# CA2008: Do not create tasks without passing a TaskScheduler
dotnet_diagnostic.CA2008.severity = error

# CA2009: Do not call ToImmutableCollection on an ImmutableCollection value
dotnet_diagnostic.CA2009.severity = error

# CA2011: Avoid infinite recursion
dotnet_diagnostic.CA2011.severity = error

# CA2012: Use ValueTasks correctly
dotnet_diagnostic.CA2012.severity = error

# CA2013: Do not use ReferenceEquals with value types
dotnet_diagnostic.CA2013.severity = error

# CA2014: Do not use stackalloc in loops
dotnet_diagnostic.CA2014.severity = error

# CA2015: Do not define finalizers for types derived from MemoryManager<T>
dotnet_diagnostic.CA2015.severity = error

# CA2016: Forward the 'CancellationToken' parameter to methods that take one
dotnet_diagnostic.CA2016.severity = error
1 change: 1 addition & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<Import Project="..\Directory.Build.props" />

<PropertyGroup>
<AnalysisLevel>latest</AnalysisLevel>
<IsPackable>True</IsPackable>
<IncludeSymbols>True</IncludeSymbols>
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)..\rulesets\EFCore.ruleset</CodeAnalysisRuleSet>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ private static bool IsInInternalNamespace(ISymbol symbol)
{
if (symbol?.ContainingNamespace?.ToDisplayString() is string ns)
{
var i = ns.IndexOf("EntityFrameworkCore");
var i = ns.IndexOf("EntityFrameworkCore", StringComparison.Ordinal);

return
i != -1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,8 @@ Expression GetPartitionKeyValue(BinaryExpression binaryExpression, IEntityType e

if (valueExpression is ConstantExpression
|| (valueExpression is ParameterExpression valueParameterExpression
&& valueParameterExpression.Name?.StartsWith(QueryCompilationContext.QueryParameterPrefix) == true))
&& valueParameterExpression.Name?
.StartsWith(QueryCompilationContext.QueryParameterPrefix, StringComparison.Ordinal) == true))
{
return valueExpression;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,12 @@ public async ValueTask<bool> MoveNextAsync()

public ValueTask DisposeAsync()
{
_enumerator?.DisposeAsync();
_enumerator = null;

var enumerator = _enumerator;
if (enumerator != null)
{
_enumerator = null;
return enumerator.DisposeAsync();
}
return default;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public override bool Equals(object obj)

private bool Equals(KeyAccessExpression keyAccessExpression)
=> base.Equals(keyAccessExpression)
&& string.Equals(Name, keyAccessExpression.Name)
&& Name == keyAccessExpression.Name
&& AccessExpression.Equals(keyAccessExpression.AccessExpression);

/// <summary>
Expand Down
5 changes: 2 additions & 3 deletions src/EFCore.Cosmos/Query/Internal/ProjectionExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
Check.NotNull(expressionPrinter, nameof(expressionPrinter));

expressionPrinter.Visit(Expression);
if (!string.Equals(string.Empty, Alias)
&& !string.Equals(Alias, Name))
if (Alias != string.Empty && Alias != Name)
{
expressionPrinter.Append(" AS " + Alias);
}
Expand All @@ -127,7 +126,7 @@ public override bool Equals(object obj)
&& Equals(projectionExpression));

private bool Equals(ProjectionExpression projectionExpression)
=> string.Equals(Alias, projectionExpression.Alias)
=> Alias == projectionExpression.Alias
&& Expression.Equals(projectionExpression.Expression);

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public override bool Equals(object obj)
&& Equals(rootReferenceExpression));

private bool Equals(RootReferenceExpression rootReferenceExpression)
=> string.Equals(Alias, rootReferenceExpression.Alias)
=> Alias == rootReferenceExpression.Alias
&& EntityType.Equals(rootReferenceExpression.EntityType);

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Cosmos/Query/Internal/SqlFunctionExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public override bool Equals(object obj)

private bool Equals(SqlFunctionExpression sqlFunctionExpression)
=> base.Equals(sqlFunctionExpression)
&& string.Equals(Name, sqlFunctionExpression.Name)
&& Name == sqlFunctionExpression.Name
&& Arguments.SequenceEqual(sqlFunctionExpression.Arguments);

/// <summary>
Expand Down
3 changes: 1 addition & 2 deletions src/EFCore.Cosmos/Query/Internal/SqlParameterExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ public override bool Equals(object obj)
&& Equals(sqlParameterExpression));

private bool Equals(SqlParameterExpression sqlParameterExpression)
=> base.Equals(sqlParameterExpression)
&& string.Equals(Name, sqlParameterExpression.Name);
=> base.Equals(sqlParameterExpression) && Name != sqlParameterExpression.Name;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,8 @@ private static JObject JObjectFromReadItemResponseMessage(ResponseMessage respon
responseMessage.EnsureSuccessStatusCode();

var responseStream = responseMessage.Content;
var reader = new StreamReader(responseStream);
var jsonReader = new JsonTextReader(reader);
using var reader = new StreamReader(responseStream);
using var jsonReader = new JsonTextReader(reader);

var jObject = Serializer.Deserialize<JObject>(jsonReader);

Expand Down
64 changes: 0 additions & 64 deletions src/EFCore.Relational/Extensions/RelationalTaskExtensions.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -942,14 +942,12 @@ private static bool EntityTypePathEquals(IEntityType source, IEntityType target,
return true;
}

if (!string.Equals(source.Name, target.Name))
if (source.Name != target.Name)
{
return false;
}

if (!string.Equals(
GetDefiningNavigationName(source),
GetDefiningNavigationName(target)))
if (GetDefiningNavigationName(source) != GetDefiningNavigationName(target))
{
return false;
}
Expand Down
5 changes: 2 additions & 3 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,8 @@ protected override Expression VisitProjection(ProjectionExpression projectionExp

Visit(projectionExpression.Expression);

if (!string.Equals(string.Empty, projectionExpression.Alias)
&& !(projectionExpression.Expression is ColumnExpression column
&& string.Equals(column.Name, projectionExpression.Alias)))
if (projectionExpression.Alias != string.Empty
&& !(projectionExpression.Expression is ColumnExpression column && column.Name == projectionExpression.Alias))
{
_relationalCommandBuilder.Append(AliasSeparator + _sqlGenerationHelper.DelimitIdentifier(projectionExpression.Alias));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
fromSqlQueryRootExpression.EntityType,
new FromSqlExpression(
fromSqlQueryRootExpression.EntityType.GetDefaultMappings().SingleOrDefault().Table.Name.Substring(0, 1)
.ToLower(),
.ToLowerInvariant(),
fromSqlQueryRootExpression.Sql,
fromSqlQueryRootExpression.Argument)));

Expand Down Expand Up @@ -1461,7 +1461,7 @@ private static IDictionary<IProperty, ColumnExpression> GetPropertyExpressionFro
tableExpression = selectExpression.Tables
.Select(t => (t as InnerJoinExpression)?.Table ?? (t as LeftJoinExpression)?.Table ?? t)
.Cast<TableExpression>()
.First(t => string.Equals(t.Name, table.Name) && string.Equals(t.Schema, table.Schema));
.First(t => t.Name == table.Name && t.Schema == table.Schema);
}

var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Query/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ public virtual SelectExpression Select(IEntityType entityType, string sql, Expre
Check.NotNull(sql, nameof(sql));

var tableExpression = new FromSqlExpression(
entityType.GetDefaultMappings().SingleOrDefault().Table.Name.Substring(0, 1).ToLower(), sql, sqlArguments);
entityType.GetDefaultMappings().SingleOrDefault().Table.Name.Substring(0, 1).ToLowerInvariant(), sql, sqlArguments);
var selectExpression = new SelectExpression(entityType, tableExpression);
AddConditions(selectExpression, entityType);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public override bool Equals(object obj)

private bool Equals(ColumnExpression columnExpression)
=> base.Equals(columnExpression)
&& string.Equals(Name, columnExpression.Name)
&& Name == columnExpression.Name
&& Table.Equals(columnExpression.Table)
&& IsNullable == columnExpression.IsNullable;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public override bool Equals(object obj)

private bool Equals(FromSqlExpression fromSqlExpression)
=> base.Equals(fromSqlExpression)
&& string.Equals(Sql, fromSqlExpression.Sql)
&& Sql == fromSqlExpression.Sql
&& ExpressionEqualityComparer.Instance.Equals(Arguments, fromSqlExpression.Arguments);

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
Check.NotNull(expressionPrinter, nameof(expressionPrinter));

expressionPrinter.Visit(Expression);
if (!string.Equals(string.Empty, Alias)
if (Alias != string.Empty
&& !(Expression is ColumnExpression column
&& string.Equals(column.Name, Alias)))
&& column.Name == Alias))
{
expressionPrinter.Append(" AS " + Alias);
}
Expand All @@ -92,8 +92,7 @@ public override bool Equals(object obj)
&& Equals(projectionExpression));

private bool Equals(ProjectionExpression projectionExpression)
=> string.Equals(Alias, projectionExpression.Alias)
&& Expression.Equals(projectionExpression.Expression);
=> Alias == projectionExpression.Alias && Expression.Equals(projectionExpression.Expression);

/// <inheritdoc />
public override int GetHashCode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ public override bool Equals(object obj)

private bool Equals(SqlFragmentExpression sqlFragmentExpression)
=> base.Equals(sqlFragmentExpression)
&& string.Equals(Sql, sqlFragmentExpression.Sql)
&& !string.Equals(Sql, "*"); // We make star projection different because it could be coming from different table.
&& Sql == sqlFragmentExpression.Sql
&& Sql != "*"; // We make star projection different because it could be coming from different table.

/// <inheritdoc />
public override int GetHashCode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ public override bool Equals(object obj)

private bool Equals(SqlFunctionExpression sqlFunctionExpression)
=> base.Equals(sqlFunctionExpression)
&& string.Equals(Name, sqlFunctionExpression.Name)
&& string.Equals(Schema, sqlFunctionExpression.Schema)
&& Name == sqlFunctionExpression.Name
&& Schema == sqlFunctionExpression.Schema
&& ((Instance == null && sqlFunctionExpression.Instance == null)
|| (Instance != null && Instance.Equals(sqlFunctionExpression.Instance)))
&& Arguments.SequenceEqual(sqlFunctionExpression.Arguments);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public override bool Equals(object obj)

private bool Equals(SqlParameterExpression sqlParameterExpression)
=> base.Equals(sqlParameterExpression)
&& string.Equals(Name, sqlParameterExpression.Name);
&& Name == sqlParameterExpression.Name;

/// <inheritdoc />
public override int GetHashCode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions
public sealed class TableExpression : TableExpressionBase
{
internal TableExpression([NotNull] ITableBase table)
: base(table.Name.Substring(0, 1).ToLower())
: base(table.Name.Substring(0, 1).ToLowerInvariant())
{
Name = table.Name;
Schema = table.Schema;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public override bool Equals(object obj)
&& Equals(tableExpressionBase));

private bool Equals(TableExpressionBase tableExpressionBase)
=> string.Equals(Alias, tableExpressionBase.Alias);
=> Alias == tableExpressionBase.Alias;

/// <inheritdoc />
public override int GetHashCode()
Expand Down
Loading

0 comments on commit a0a053b

Please sign in to comment.