feat: implement separate diagnostics for variables named '_' and unnecessary typed discards

Documentation/Diagnostics/PH2147.md

@@ -1,4 +1,4 @@
-# PH2147: Avoid variables named exactly '_'
+# PH2147: Avoid variables named '_'
 
 | Property | Value  |
 |--|--|
@@ -12,7 +12,9 @@
 
 ## Introduction
 
-This rule flags variables (local variables, foreach variables, for loop variables, using variables, and out parameters) that are named exactly `_` (single underscore). This rule does not flag proper C# discards (both anonymous discards `_` and typed discards like `string _`).
+This rule flags variables that are named exactly `_` (single underscore). This includes local variables, foreach variables, for loop variables, using variables, and out parameters that are explicitly named `_`.
+
+This rule does not flag proper C# anonymous discards (`_`).
 
 ## Reason
 
@@ -24,7 +26,7 @@ Either use a proper discard (if you don't need the value) or use a more descript
 
 ## Examples
 
-### Incorrect
+### Variables Named '_' (Flagged)
 
 ```csharp
 // Local variable - looks like discard but is actually a variable
@@ -43,36 +45,24 @@ if (int.TryParse(input, out var _))
 }
 ```
 
-### Correct
+### Correct Usage (NOT Flagged)
 
 ```csharp
-// Use a proper discard if you don't need the value
-_ = ExtractData(reader);
+// Use proper anonymous discards - these are NOT flagged
+_ = ExtractData(reader);              // Anonymous discard assignment
+GetValue(out _);                      // Anonymous discard in out parameter
+TryParseHelper("123", out _);         // Anonymous discard in out parameter
 
-// Or use a descriptive name if you do need the value
+// Use descriptive names when you need the value
 byte[] data = ExtractData(reader);
-
-// ForEach with discard
-foreach (var _ in items)
-{
-    // Use discard syntax instead
-}
-
-// Use proper discards - these are NOT flagged by this rule
-if (int.TryParse(input, out _))  // Anonymous discard
+foreach (var item in items)
 {
-    // code
-}
-
-if (int.TryParse(input, out int _))  // Typed discard
-{
-    // code
+    // Use item
 }
 
-// Or use a descriptive name if you need the value
 if (int.TryParse(input, out int result))
 {
-    // code using result
+    // Use result
 }
 ```
 
@@ -83,5 +73,5 @@ This rule is enabled by default with Error severity.
 ## Suppression
 
 ```csharp
-[SuppressMessage("Philips.CodeAnalysis.MaintainabilityAnalyzers", "PH2147:Avoid variables named exactly '_'", Justification = "Reviewed.")]
+[SuppressMessage("Philips.CodeAnalysis.MaintainabilityAnalyzers", "PH2147:Avoid variables named '_'", Justification = "Reviewed.")]
 ```

Documentation/Diagnostics/PH2152.md

@@ -0,0 +1,72 @@
+# PH2152: Avoid unnecessary typed discard
+
+| Property | Value  |
+|--|--|
+| Package | [Philips.CodeAnalysis.MaintainabilityAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MaintainabilityAnalyzers) |
+| Diagnostic ID | PH2152 |
+| Category  | [Naming](../Naming.md) |
+| Analyzer | [AvoidVariableNamedUnderscoreAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Naming/AvoidVariableNamedUnderscoreAnalyzer.cs)
+| CodeFix  | No |
+| Severity | Error |
+| Enabled By Default | Yes |
+
+## Introduction
+
+This rule flags unnecessary typed discards like `out string _` when anonymous discard `out _` would work equally well.
+
+This rule does not flag typed discards that are necessary for overload resolution.
+
+## Reason
+
+Anonymous discards have clearer intent and are more concise than typed discards when the type information is not needed for overload resolution.
+
+## How to fix violations
+
+Replace typed discards with anonymous discards when the type is not needed for overload resolution.
+
+## Examples
+
+### Unnecessary Typed Discards (Flagged)
+
+```csharp
+// These are flagged - unnecessary typed discards when anonymous would work
+GetValue(out string _);           // Should be: GetValue(out _);
+TryParseHelper("123", out int _); // Should be: TryParseHelper("123", out _);
+
+public void GetValue(out string value) { value = "test"; }
+public bool TryParseHelper(string input, out int result) { result = 42; return true; }
+```
+
+### Necessary Typed Discards (NOT Flagged)
+
+```csharp
+// These are NOT flagged - necessary for overload resolution
+Parse("123", out int _);          // Disambiguates from Parse(string, out string)
+Parse("test", out string _);      // Disambiguates from Parse(string, out int)
+
+// Example overloaded methods
+public bool Parse(string input, out int result) { result = 42; return true; }
+public bool Parse(string input, out string result) { result = input; return true; }
+```
+
+### Correct Usage (NOT Flagged)
+
+```csharp
+// Use proper anonymous discards - these are NOT flagged
+GetValue(out _);                      // Anonymous discard in out parameter
+TryParseHelper("123", out _);         // Anonymous discard in out parameter
+
+// Typed discards are allowed when needed for overload resolution
+Parse("123", out int _);       // Needed to select correct overload
+Parse("test", out string _);   // Needed to select correct overload
+```
+
+## Configuration
+
+This rule is enabled by default with Error severity.
+
+## Suppression
+
+```csharp
+[SuppressMessage("Philips.CodeAnalysis.MaintainabilityAnalyzers", "PH2152:Avoid unnecessary typed discard", Justification = "Reviewed.")]
+```

Philips.CodeAnalysis.Common/DiagnosticId.cs

@@ -141,6 +141,7 @@ public enum DiagnosticId
 		AvoidVariableNamedUnderscore = 2147,
 		AvoidProblematicUsingPatterns = 2149,
 		AvoidTodoComments = 2151,
+		AvoidUnnecessaryTypedDiscard = 2152,
 		AvoidUnusedToString = 2153,
 		AvoidUnlicensedPackages = 2155,
 		AvoidPkcsPaddingWithRsaEncryption = 2158,

Philips.CodeAnalysis.MaintainabilityAnalyzers/Naming/AvoidVariableNamedUnderscoreAnalyzer.cs

@@ -10,16 +10,38 @@
 namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Naming
 {
 	[DiagnosticAnalyzer(LanguageNames.CSharp)]
-	public class AvoidVariableNamedUnderscoreAnalyzer : SingleDiagnosticAnalyzer
+	public class AvoidVariableNamedUnderscoreAnalyzer : DiagnosticAnalyzerBase
 	{
-		private const string Title = @"Avoid variables named exactly '_'";
-		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";
-		private const string Description = @"Variables named exactly '_' can be confused with C# discards. Use either a proper discard or a more descriptive variable name.";
-
-		public AvoidVariableNamedUnderscoreAnalyzer()
-			: base(DiagnosticId.AvoidVariableNamedUnderscore, Title, MessageFormat, Description, Categories.Naming)
-		{
-		}
+		private const string VariableNamedUnderscoreTitle = @"Avoid variables named '_'";
+		private const string VariableNamedUnderscoreMessageFormat = @"Avoid variable named '{0}' as it can be confused with discards";
+		private const string VariableNamedUnderscoreDescription = @"Avoid variables named exactly '_' which can be confused with discards.";
+
+		private const string UnnecessaryTypedDiscardTitle = @"Avoid unnecessary typed discard";
+		private const string UnnecessaryTypedDiscardMessageFormat = @"Use anonymous discard '_' instead of typed discard '{0}' when type is not needed";
+		private const string UnnecessaryTypedDiscardDescription = @"Prefer anonymous discards over typed discards when the type is not needed for overload resolution.";
+
+		public static readonly DiagnosticDescriptor VariableNamedUnderscoreRule = new(
+			DiagnosticId.AvoidVariableNamedUnderscore.ToId(),
+			VariableNamedUnderscoreTitle,
+			VariableNamedUnderscoreMessageFormat,
+			Categories.Naming,
+			DiagnosticSeverity.Error,
+			isEnabledByDefault: true,
+			description: VariableNamedUnderscoreDescription,
+			helpLinkUri: DiagnosticId.AvoidVariableNamedUnderscore.ToHelpLinkUrl());
+
+		public static readonly DiagnosticDescriptor UnnecessaryTypedDiscardRule = new(
+			DiagnosticId.AvoidUnnecessaryTypedDiscard.ToId(),
+			UnnecessaryTypedDiscardTitle,
+			UnnecessaryTypedDiscardMessageFormat,
+			Categories.Naming,
+			DiagnosticSeverity.Error,
+			isEnabledByDefault: true,
+			description: UnnecessaryTypedDiscardDescription,
+			helpLinkUri: DiagnosticId.AvoidUnnecessaryTypedDiscard.ToHelpLinkUrl());
+
+		public override System.Collections.Immutable.ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
+			System.Collections.Immutable.ImmutableArray.Create(VariableNamedUnderscoreRule, UnnecessaryTypedDiscardRule);
 
 		protected override void InitializeCompilation(CompilationStartAnalysisContext context)
 		{
@@ -43,7 +65,7 @@ private void AnalyzeForEachStatement(SyntaxNodeAnalysisContext context)
 			}
 
 			Location location = foreachStatement.GetLocation();
-			var diagnostic = Diagnostic.Create(Rule, location, foreachStatement.Identifier.ValueText);
+			var diagnostic = Diagnostic.Create(VariableNamedUnderscoreRule, location, foreachStatement.Identifier.ValueText);
 			context.ReportDiagnostic(diagnostic);
 		}
 
@@ -68,7 +90,7 @@ private void AnalyzeVariableDeclaration(SyntaxNodeAnalysisContext context)
 				}
 
 				Location location = variableDeclaration.GetLocation();
-				var diagnostic = Diagnostic.Create(Rule, location, identifier);
+				var diagnostic = Diagnostic.Create(VariableNamedUnderscoreRule, location, identifier);
 				context.ReportDiagnostic(diagnostic);
 			}
 		}
@@ -88,21 +110,99 @@ private void AnalyzeArgument(SyntaxNodeAnalysisContext context)
 				return;
 			}
 
-			// Check if it's a variable declaration
+			// Check if it's a typed discard (e.g., out int _)
 			if (argument.Expression is DeclarationExpressionSyntax declaration &&
-				declaration.Designation is SingleVariableDesignationSyntax variable)
+				declaration.Designation is DiscardDesignationSyntax discard)
 			{
-				if (variable.Identifier.ValueText != "_")
+				// This is a typed discard (e.g., out int _)
+				// Only flag if the type is not needed for overload resolution
+				if (!IsTypedDiscardNecessaryForOverloadResolution(context, argument))
 				{
-					return;
+					Location location = discard.GetLocation();
+					var diagnostic = Diagnostic.Create(UnnecessaryTypedDiscardRule, location, "_");
+					context.ReportDiagnostic(diagnostic);
 				}
+				return;
+			}
 
+			// Check if it's a variable declaration with identifier "_"
+			if (argument.Expression is DeclarationExpressionSyntax declaration2 &&
+				declaration2.Designation is SingleVariableDesignationSyntax variable &&
+				variable.Identifier.ValueText == "_")
+			{
 				Location location = variable.Identifier.GetLocation();
-				var diagnostic = Diagnostic.Create(Rule, location, variable.Identifier.ValueText);
+				var diagnostic = Diagnostic.Create(VariableNamedUnderscoreRule, location, variable.Identifier.ValueText);
 				context.ReportDiagnostic(diagnostic);
-				// Note: DiscardDesignationSyntax represents proper discards (e.g., "out _" or "out string _")
-				// and should not be flagged, so we don't handle this case.
 			}
+
+			// Note: We don't flag IdentifierNameSyntax("_") because that represents
+			// anonymous discards like "out _" which are the preferred form
+		}
+
+		private static bool IsTypedDiscardNecessaryForOverloadResolution(SyntaxNodeAnalysisContext context, ArgumentSyntax argument)
+		{
+			// Find the method call containing this argument
+			InvocationExpressionSyntax invocation = argument.Ancestors()
+				.OfType<InvocationExpressionSyntax>()
+				.FirstOrDefault();
+			if (invocation is null)
+			{
+				return false; // Can't find method call, allow the flag
+			}
+
+			// Early check: if the invocation doesn't look like it could have overloads, 
+			// we can avoid semantic model access entirely
+			if (invocation.Expression is not (MemberAccessExpressionSyntax or IdentifierNameSyntax))
+			{
+				return false; // Complex expression, allow the flag
+			}
+
+			// Get semantic information about the method - moved lower for performance
+			SymbolInfo symbolInfo = context.SemanticModel.GetSymbolInfo(invocation);
+			if (symbolInfo.Symbol is not IMethodSymbol method)
+			{
+				return false; // Can't resolve method, allow the flag
+			}
+
+			// Get the argument position - handle named arguments
+			var argumentIndex = GetArgumentIndex(invocation.ArgumentList.Arguments, argument, method);
+			if (argumentIndex < 0 || argumentIndex >= method.Parameters.Length)
+			{
+				return false; // Invalid position, allow the flag
+			}
+
+			// Simple heuristic: check if there are method overloads with different out parameter types at this position
+			INamedTypeSymbol containingType = method.ContainingType;
+			System.Collections.Generic.IEnumerable<IMethodSymbol> methodsWithSameName = containingType.GetMembers(method.Name).OfType<IMethodSymbol>();
+
+			// Count how many overloads have an out parameter at this position
+			var overloadCount = methodsWithSameName
+				.Where(m => argumentIndex < m.Parameters.Length)
+				.Count(m => m.Parameters[argumentIndex].RefKind == RefKind.Out);
+
+			// If there are multiple overloads with out parameters at this position,
+			// then the typed discard might be necessary for overload resolution
+			return overloadCount > 1;
+		}
+
+		private static int GetArgumentIndex(SeparatedSyntaxList<ArgumentSyntax> arguments, ArgumentSyntax targetArgument, IMethodSymbol method)
+		{
+			// Handle named arguments by checking if the target argument has a name
+			if (targetArgument.NameColon != null)
+			{
+				var parameterName = targetArgument.NameColon.Name.Identifier.ValueText;
+				for (var i = 0; i < method.Parameters.Length; i++)
+				{
+					if (method.Parameters[i].Name == parameterName)
+					{
+						return i;
+					}
+				}
+				return -1; // Parameter name not found
+			}
+
+			// For positional arguments, just get the index
+			return arguments.IndexOf(targetArgument);
 		}
 	}
 }