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.

Sign up for GitHub

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

Fix CA1801 false positive with positional records primary ctor #4499

Merged
merged 7 commits into from
Dec 7, 2020
Merged

Fix CA1801 false positive with positional records primary ctor #4499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
79bd89a
Update Versions.props
jmarolf Dec 3, 2020
0ebe82d
Fix CA1801 false positive with inline records.
Evangelink Dec 4, 2020
e232ffa
Add tests for implicit record
Evangelink Dec 1, 2020
dd34f6a
Address review comments
Evangelink Dec 4, 2020
123f064
Remove unused directives
Evangelink Dec 4, 2020
03ffbef
Update eng/Versions.props
Evangelink Dec 7, 2020
a32cdc2
Update sarif files
Evangelink Dec 7, 2020
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
61 changes: 40 additions & 21 deletions src/Microsoft.CodeAnalysis.FxCopAnalyzers/Microsoft.CodeAnalysis.FxCopAnalyzers.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1370,27 +1370,6 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "ReviewUnusedParametersAnalyzer",
"languages": [
"C#",
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1802": {
"id": "CA1802",
"shortDescription": "Use literals where appropriate",
Expand Down Expand Up @@ -2052,6 +2031,26 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "CSharpReviewUnusedParametersAnalyzer",
"languages": [
"C#"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1812": {
"id": "CA1812",
"shortDescription": "Avoid uninstantiated internal classes",
Expand Down Expand Up @@ -2220,6 +2219,26 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "BasicReviewUnusedParametersAnalyzer",
"languages": [
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1812": {
"id": "CA1812",
"shortDescription": "Avoid uninstantiated internal classes",
Expand Down
61 changes: 40 additions & 21 deletions src/Microsoft.CodeQuality.Analyzers/Microsoft.CodeQuality.Analyzers.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1355,27 +1355,6 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "ReviewUnusedParametersAnalyzer",
"languages": [
"C#",
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1802": {
"id": "CA1802",
"shortDescription": "Use literals where appropriate",
Expand Down Expand Up @@ -2037,6 +2016,26 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "CSharpReviewUnusedParametersAnalyzer",
"languages": [
"C#"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1812": {
"id": "CA1812",
"shortDescription": "Avoid uninstantiated internal classes",
Expand Down Expand Up @@ -2205,6 +2204,26 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": true,
"typeName": "BasicReviewUnusedParametersAnalyzer",
"languages": [
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1812": {
"id": "CA1812",
"shortDescription": "Avoid uninstantiated internal classes",
Expand Down
35 changes: 35 additions & 0 deletions ...p/Microsoft.CodeQuality.Analyzers/Maintainability/CSharpReviewUnusedParametersAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// 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 Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeQuality.Analyzers.Maintainability;

namespace Microsoft.CodeQuality.CSharp.Analyzers.Maintainability
{
/// <summary>
/// CA1801: Review unused parameters
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpReviewUnusedParametersAnalyzer : ReviewUnusedParametersAnalyzer
{
private const SyntaxKind RecordDeclaration = (SyntaxKind)9063;
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

protected override bool IsPositionalRecordPrimaryConstructor(IMethodSymbol methodSymbol)
{
if (methodSymbol.MethodKind != MethodKind.Constructor)
{
return false;
}

if (methodSymbol.DeclaringSyntaxReferences.Length == 0)
{
return false;
}

return methodSymbol.DeclaringSyntaxReferences[0]
.GetSyntax()
.IsKind(RecordDeclaration);
}
}
}
17 changes: 12 additions & 5 deletions ...tAnalyzers/Core/Microsoft.CodeQuality.Analyzers/Maintainability/ReviewUnusedParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ namespace Microsoft.CodeQuality.Analyzers.Maintainability
/// <summary>
/// CA1801: Review unused parameters
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class ReviewUnusedParametersAnalyzer : DiagnosticAnalyzer
public abstract class ReviewUnusedParametersAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA1801";

Expand All @@ -34,9 +33,9 @@ public sealed class ReviewUnusedParametersAnalyzer : DiagnosticAnalyzer
isPortedFxCopRule: true,
isDataflowRule: false);

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

public override void Initialize(AnalysisContext context)
public sealed override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();

Expand Down Expand Up @@ -163,7 +162,7 @@ private static void ReportUnusedParameters(
}

#pragma warning disable RS1012 // Start action has no registered actions.
private static bool ShouldAnalyzeMethod(
private bool ShouldAnalyzeMethod(
IMethodSymbol method,
OperationBlockStartAnalysisContext startOperationBlockContext,
INamedTypeSymbol? eventsArgSymbol,
Expand Down Expand Up @@ -196,6 +195,12 @@ private static bool ShouldAnalyzeMethod(
return false;
}

// Ignore primary constructor (body-less) of positional records.
if (IsPositionalRecordPrimaryConstructor(method))
{
return false;
}

// Ignore serialization special methods
if (method.IsSerializationConstructor(serializationInfoType, streamingContextType) ||
method.IsGetObjectData(serializationInfoType, streamingContextType))
Expand Down Expand Up @@ -241,5 +246,7 @@ private static bool ShouldAnalyzeMethod(

return true;
}

protected abstract bool IsPositionalRecordPrimaryConstructor(IMethodSymbol methodSymbol);
}
}
61 changes: 40 additions & 21 deletions src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,26 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": false,
"typeName": "CSharpReviewUnusedParametersAnalyzer",
"languages": [
"C#"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1812": {
"id": "CA1812",
"shortDescription": "Avoid uninstantiated internal classes",
Expand Down Expand Up @@ -1797,27 +1817,6 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": false,
"typeName": "ReviewUnusedParametersAnalyzer",
"languages": [
"C#",
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1802": {
"id": "CA1802",
"shortDescription": "Use literals where appropriate",
Expand Down Expand Up @@ -5210,6 +5209,26 @@
]
}
},
"CA1801": {
"id": "CA1801",
"shortDescription": "Review unused parameters",
"fullDescription": "Avoid unused paramereters in your code. If the parameter cannot be removed, then change its name so it starts with an underscore and is optionally followed by an integer, such as '_', '_1', '_2', etc. These are treated as special discard symbol names.",
"defaultLevel": "warning",
"helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1801",
"properties": {
"category": "Usage",
"isEnabledByDefault": false,
"typeName": "BasicReviewUnusedParametersAnalyzer",
"languages": [
"Visual Basic"
],
"tags": [
"PortedFromFxCop",
"Telemetry",
"EnabledRuleInAggressiveMode"
]
}
},
"CA1812": {
"id": "CA1812",
"shortDescription": "Avoid uninstantiated internal classes",
Expand Down
4 changes: 2 additions & 2 deletions ...ests/Microsoft.CodeQuality.Analyzers/Maintainability/ReviewUnusedParametersTests.Fixer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
using Microsoft.CodeAnalysis.Testing;
using Xunit;
using VerifyCS = Test.Utilities.CSharpCodeFixVerifier<
Microsoft.CodeQuality.Analyzers.Maintainability.ReviewUnusedParametersAnalyzer,
Microsoft.CodeQuality.CSharp.Analyzers.Maintainability.CSharpReviewUnusedParametersAnalyzer,
Microsoft.CodeQuality.CSharp.Analyzers.Maintainability.CSharpReviewUnusedParametersFixer>;
using VerifyVB = Test.Utilities.VisualBasicCodeFixVerifier<
Microsoft.CodeQuality.Analyzers.Maintainability.ReviewUnusedParametersAnalyzer,
Microsoft.CodeQuality.VisualBasic.Analyzers.Maintainability.BasicReviewUnusedParametersAnalyzer,
Microsoft.CodeQuality.VisualBasic.Analyzers.Maintainability.BasicReviewUnusedParametersFixer>;

namespace Microsoft.CodeQuality.Analyzers.Maintainability.UnitTests
Expand Down
Loading