Skip to content

Commit fab7718

Browse files
bcollamorebcollamore
authored
Merge branch 'main' into copilot/fix-848
2 parents 7c39d32 + 9bfd6f3 commit fab7718

9 files changed

Lines changed: 436 additions & 404 deletions

File tree

‎Documentation/Diagnostics/PH2035.md‎

Lines changed: 0 additions & 61 deletions
This file was deleted.

‎Documentation/Diagnostics/PH2147.md‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# PH2147: Avoid variables named exactly '_'
2+
3+
| Property | Value |
4+
|--|--|
5+
| Package | [Philips.CodeAnalysis.MaintainabilityAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MaintainabilityAnalyzers) |
6+
| Diagnostic ID | PH2147 |
7+
| Category | [Naming](../Naming.md) |
8+
| Analyzer | [AvoidVariableNamedUnderscoreAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Naming/AvoidVariableNamedUnderscoreAnalyzer.cs)
9+
| CodeFix | No |
10+
| Severity | Error |
11+
| Enabled By Default | Yes |
12+
13+
## Introduction
14+
15+
This rule flags variables (local variables, foreach variables, for loop variables, using variables, and out parameters) that are named exactly `_` (single underscore).
16+
17+
## Reason
18+
19+
Variables named exactly `_` can be confused with C# discards. This can lead to unintended behavior where developers think they are using a discard but are actually creating a variable that captures the value.
20+
21+
## How to fix violations
22+
23+
Either use a proper discard (if you don't need the value) or use a more descriptive variable name (if you do need the value).
24+
25+
## Examples
26+
27+
### Incorrect
28+
29+
```csharp
30+
// Local variable - looks like discard but is actually a variable
31+
byte[] _ = ExtractData(reader);
32+
33+
// ForEach variable - confusing
34+
foreach (var _ in items)
35+
{
36+
// code
37+
}
38+
39+
// Out parameter - looks like discard
40+
if (int.TryParse(input, out int _))
41+
{
42+
// code
43+
}
44+
```
45+
46+
### Correct
47+
48+
```csharp
49+
// Use a proper discard if you don't need the value
50+
_ = ExtractData(reader);
51+
52+
// Or use a descriptive name if you do need the value
53+
byte[] data = ExtractData(reader);
54+
55+
// ForEach with discard
56+
foreach (var _ in items)
57+
{
58+
// Use discard syntax instead
59+
}
60+
61+
// Out parameter with discard
62+
if (int.TryParse(input, out _))
63+
{
64+
// code
65+
}
66+
67+
// Or with a descriptive name
68+
if (int.TryParse(input, out int result))
69+
{
70+
// code using result
71+
}
72+
```
73+
74+
## Configuration
75+
76+
This rule is enabled by default with Error severity.
77+
78+
## Suppression
79+
80+
```csharp
81+
[SuppressMessage("Philips.CodeAnalysis.MaintainabilityAnalyzers", "PH2147:Avoid variables named exactly '_'", Justification = "Reviewed.")]
82+
```

‎Philips.CodeAnalysis.Common/DiagnosticId.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ public enum DiagnosticId
3434
AvoidTryParseWithoutCulture = 2031,
3535
AvoidEmptyTypeInitializer = 2032,
3636
TestMethodsMustBeInTestClass = 2034,
37-
TestMethodsMustHaveTheCorrectNumberOfArguments = 2035,
3837
TestMethodsMustBePublic = 2036,
3938
TestMethodsMustHaveUniqueNames = 2037,
4039
TestClassesMustBePublic = 2038,
@@ -139,5 +138,6 @@ public enum DiagnosticId
139138
AvoidIncorrectForLoopCondition = 2144,
140139
AvoidStringFormatInInterpolatedString = 2145,
141140
AvoidToStringOnString = 2146,
141+
AvoidVariableNamedUnderscore = 2147,
142142
}
143143
}

‎Philips.CodeAnalysis.MaintainabilityAnalyzers/Naming/AvoidVariableNamedUnderscoreAnalyzer.cs‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
// © 2019 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using System.Linq;
4+
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.CSharp;
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
using Microsoft.CodeAnalysis.Diagnostics;
8+
using Philips.CodeAnalysis.Common;
9+
10+
namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Naming
11+
{
12+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
13+
public class AvoidVariableNamedUnderscoreAnalyzer : SingleDiagnosticAnalyzer
14+
{
15+
private const string Title = @"Avoid variables named exactly '_'";
16+
private const string MessageFormat = @"Variable '{0}' is named '_' which can be confused with a discard. Consider using a discard or a more descriptive variable name";
17+
private const string Description = @"Variables named exactly '_' can be confused with C# discards. Use either a proper discard or a more descriptive variable name.";
18+
19+
public AvoidVariableNamedUnderscoreAnalyzer()
20+
: base(DiagnosticId.AvoidVariableNamedUnderscore, Title, MessageFormat, Description, Categories.Naming)
21+
{
22+
}
23+
24+
protected override void InitializeCompilation(CompilationStartAnalysisContext context)
25+
{
26+
context.RegisterSyntaxNodeAction(AnalyzeVariableDeclaration, SyntaxKind.VariableDeclaration);
27+
context.RegisterSyntaxNodeAction(AnalyzeForEachStatement, SyntaxKind.ForEachStatement);
28+
context.RegisterSyntaxNodeAction(AnalyzeArgument, SyntaxKind.Argument);
29+
}
30+
31+
private void AnalyzeForEachStatement(SyntaxNodeAnalysisContext context)
32+
{
33+
if (Helper.ForGeneratedCode.IsGeneratedCode(context))
34+
{
35+
return;
36+
}
37+
38+
var foreachStatement = (ForEachStatementSyntax)context.Node;
39+
40+
if (foreachStatement.Identifier.ValueText != "_")
41+
{
42+
return;
43+
}
44+
45+
Location location = foreachStatement.GetLocation();
46+
var diagnostic = Diagnostic.Create(Rule, location, foreachStatement.Identifier.ValueText);
47+
context.ReportDiagnostic(diagnostic);
48+
}
49+
50+
private void AnalyzeVariableDeclaration(SyntaxNodeAnalysisContext context)
51+
{
52+
if (Helper.ForGeneratedCode.IsGeneratedCode(context))
53+
{
54+
return;
55+
}
56+
57+
var variableDeclaration = (VariableDeclarationSyntax)context.Node;
58+
if (variableDeclaration.Parent is not (ForStatementSyntax or UsingStatementSyntax or LocalDeclarationStatementSyntax))
59+
{
60+
return;
61+
}
62+
63+
foreach (SyntaxToken identifier in variableDeclaration.Variables.Select(variable => variable.Identifier))
64+
{
65+
if (identifier.ValueText != "_")
66+
{
67+
continue;
68+
}
69+
70+
Location location = variableDeclaration.GetLocation();
71+
var diagnostic = Diagnostic.Create(Rule, location, identifier.ValueText);
72+
context.ReportDiagnostic(diagnostic);
73+
}
74+
}
75+
76+
private void AnalyzeArgument(SyntaxNodeAnalysisContext context)
77+
{
78+
if (Helper.ForGeneratedCode.IsGeneratedCode(context))
79+
{
80+
return;
81+
}
82+
83+
var argument = (ArgumentSyntax)context.Node;
84+
85+
// Only check out arguments
86+
if (!argument.RefKindKeyword.IsKind(SyntaxKind.OutKeyword))
87+
{
88+
return;
89+
}
90+
91+
// Check if it's a variable declaration
92+
if (argument.Expression is DeclarationExpressionSyntax declaration)
93+
{
94+
if (declaration.Designation is SingleVariableDesignationSyntax variable)
95+
{
96+
if (variable.Identifier.ValueText != "_")
97+
{
98+
return;
99+
}
100+
101+
Location location = variable.Identifier.GetLocation();
102+
var diagnostic = Diagnostic.Create(Rule, location, variable.Identifier.ValueText);
103+
context.ReportDiagnostic(diagnostic);
104+
}
105+
else if (declaration.Designation is DiscardDesignationSyntax discard)
106+
{
107+
Location location = discard.UnderscoreToken.GetLocation();
108+
var diagnostic = Diagnostic.Create(Rule, location, discard.UnderscoreToken.ValueText);
109+
context.ReportDiagnostic(diagnostic);
110+
}
111+
}
112+
}
113+
}
114+
}

‎Philips.CodeAnalysis.MsTestAnalyzers/Philips.CodeAnalysis.MsTestAnalyzers.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
| [PH2017](../Documentation/Diagnostics/PH2017.md) | Avoid ClassInitialize attribute | This attribute is unnecessary in the C# programming language. They circumvent TestTimeouts, allowing for slower Test Runs. They can contribute to non-deterministic test execution order, resulting complications while debugging. |
1717
| [PH2018](../Documentation/Diagnostics/PH2018.md) | Avoid ClassCleanup attribute | This attribute is unnecessary in the C# programming language. They circumvent TestTimeouts, allowing for slower Test Runs. They can contribute to non-deterministic test execution order, resulting complications while debugging. |
1818
| [PH2019](../Documentation/Diagnostics/PH2019.md) | Avoid TestCleanup attribute | This attribute is unnecessary in the C# programming language. They circumvent TestTimeouts, allowing for slower Test Runs. They can contribute to non-deterministic test execution order, resulting complications while debugging. |
19-
| [PH2034](../Documentation/Diagnostics/PH2034.md) | TestMethod requires TestClass | TestMethods not inside a TestClass are not executed. They are dead code. |
20-
| [PH2035](../Documentation/Diagnostics/PH2035.md) | DataTestMethod requires correct parameter count | DataTestMethods must have the same number of parameters of the DataRows, TestMethods should have no arguments. |
19+
| [PH2034](../Documentation/Diagnostics/PH2034.md) | TestMethod requires TestClass | TestMethods not inside a TestClass are not executed. They are dead code. |
2120
| [PH2036](../Documentation/Diagnostics/PH2036.md) | TestMethod is public | TestMethods must be public. |
2221
| [PH2037](../Documentation/Diagnostics/PH2037.md) | TestMethods and DataTestMethods require unique names | TestMethods and DataTestMethods require unique names. |
2322
| [PH2038](../Documentation/Diagnostics/PH2038.md) | TestClass is public | TestClass must be public. |

‎Philips.CodeAnalysis.MsTestAnalyzers/TestHasTimeoutAnalyzer.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ protected override void OnTestMethod(SyntaxNodeAnalysisContext context, MethodDe
100100
return;
101101
}
102102

103-
if (!TryExtractAttributeArgument(context, argumentSyntax, out var timeoutString, out int _))
103+
if (!TryExtractAttributeArgument<int>(context, argumentSyntax, out var timeoutString, out _))
104104
{
105105
return;
106106
}

0 commit comments

Comments
 (0)