diff --git a/src/Meziantou.Analyzer/Internals/SymbolExtensions.cs b/src/Meziantou.Analyzer/Internals/SymbolExtensions.cs index 73c7717b..4a08e9e9 100644 --- a/src/Meziantou.Analyzer/Internals/SymbolExtensions.cs +++ b/src/Meziantou.Analyzer/Internals/SymbolExtensions.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Threading; using Microsoft.CodeAnalysis; @@ -85,6 +85,24 @@ public static IEnumerable GetAllMembers(this ITypeSymbol? symbol) } } + public static IEnumerable GetAllMembers(this INamespaceOrTypeSymbol? symbol) + { + while (symbol is not null) + { + foreach (var member in symbol.GetMembers()) + yield return member; + + if (symbol is ITypeSymbol typeSymbol) + { + symbol = typeSymbol.BaseType; + } + else + { + yield break; + } + } + } + public static IEnumerable GetAllMembers(this ITypeSymbol? symbol, string name) { while (symbol is not null) @@ -96,6 +114,24 @@ public static IEnumerable GetAllMembers(this ITypeSymbol? symbol, strin } } + public static IEnumerable GetAllMembers(this INamespaceOrTypeSymbol? symbol, string name) + { + while (symbol is not null) + { + foreach (var member in symbol.GetMembers(name)) + yield return member; + + if (symbol is ITypeSymbol typeSymbol) + { + symbol = typeSymbol.BaseType; + } + else + { + yield break; + } + } + } + public static bool IsTopLevelStatement(this ISymbol symbol, CancellationToken cancellationToken) { if (symbol.DeclaringSyntaxReferences.Length == 0) @@ -142,6 +178,7 @@ public static bool IsTopLevelStatementsEntryPointType([NotNullWhen(true)] this I IFieldSymbol field => field.Type, IPropertySymbol { GetMethod: not null } property => property.Type, ILocalSymbol local => local.Type, + IMethodSymbol method => method.ReturnType, _ => null, }; } diff --git a/src/Meziantou.Analyzer/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzer.cs b/src/Meziantou.Analyzer/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzer.cs index 558d181c..8cbef95f 100644 --- a/src/Meziantou.Analyzer/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzer.cs @@ -1,9 +1,15 @@ -using System; +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.SymbolStore; +using System.Globalization; +using System.Linq; +using System.Linq.Expressions; using Meziantou.Analyzer.Internals; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; namespace Meziantou.Analyzer.Rules; @@ -11,7 +17,26 @@ namespace Meziantou.Analyzer.Rules; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzer : DiagnosticAnalyzer { - private static readonly char[] MemberSeparators = [',', '(', '.', '[', ' ']; + private static readonly Dictionary CSharpKeywordToTypeName = new(StringComparer.Ordinal) + { + ["bool"] = SpecialType.System_Boolean, + ["byte"] = SpecialType.System_Byte, + ["sbyte"] = SpecialType.System_SByte, + ["char"] = SpecialType.System_Char, + ["decimal"] = SpecialType.System_Decimal, + ["double"] = SpecialType.System_Double, + ["float"] = SpecialType.System_Single, + ["int"] = SpecialType.System_Int32, + ["uint"] = SpecialType.System_UInt32, + ["nint"] = SpecialType.System_IntPtr, + ["nuint"] = SpecialType.System_UIntPtr, + ["long"] = SpecialType.System_Int64, + ["ulong"] = SpecialType.System_UInt64, + ["short"] = SpecialType.System_Int16, + ["ushort"] = SpecialType.System_UInt16, + ["object"] = SpecialType.System_Object, + ["string"] = SpecialType.System_String, + }; private static readonly DiagnosticDescriptor Rule = new( RuleIdentifiers.DebuggerDisplayAttributeShouldContainValidExpressions, @@ -63,94 +88,223 @@ private static void AnalyzeNamedType(SymbolAnalysisContext context, INamedTypeSy } } } + } - static bool MemberExists(INamedTypeSymbol? symbol, string name) + private static void ValidateValue(SymbolAnalysisContext context, INamedTypeSymbol symbol, AttributeData attribute, string value) + { + var expressions = ExtractExpressions(value.AsSpan()); + if (expressions is null) + return; + + foreach (var expression in expressions) { - while (symbol is not null) - { - if (!symbol.GetMembers(name).IsEmpty) - return true; + if (string.IsNullOrWhiteSpace(expression)) + continue; - symbol = symbol.BaseType; + var expressionSyntax = SyntaxFactory.ParseExpression(expression); + foreach (var memberPath in ExtractMemberPaths(expressionSyntax)) + { + if (!IsValid(context.Compilation, symbol, memberPath, out var invalidMember)) + { + context.ReportDiagnostic(Rule, attribute, invalidMember); + break; + } } - - return false; } + } + + private static List> ExtractMemberPaths(ExpressionSyntax expressionSyntax) + { + var paths = new List>(); + ExtractMemberPaths(paths, expressionSyntax); + return paths; - static List? ParseMembers(ReadOnlySpan value) + static void ExtractMemberPaths(List> results, ExpressionSyntax expressionSyntax) { - List? result = null; + var path = new List(); - static int IndexOf(ReadOnlySpan value, char c) + while (expressionSyntax is not null) { - var skipped = 0; - while (!value.IsEmpty) + switch (expressionSyntax) { - var index = value.IndexOfAny(c, '\\'); - if (index < 0) - return -1; - - if (value[index] == c) - return index + skipped; - - if (index + 1 < value.Length) - { - skipped += index + 2; - value = value[(index + 2)..]; - } - else - { - return -1; - } - } + case ParenthesizedExpressionSyntax parenthesizedExpression: + // Check if parentheses are necessary + // (Instance).Member => Instance.Member + // ((Instance + value)).Member => stop evaluating + if (path.Count is 0) + { + expressionSyntax = parenthesizedExpression.Expression; + } + else + { + if (parenthesizedExpression.Expression is MemberAccessExpressionSyntax or IdentifierNameSyntax or ParenthesizedExpressionSyntax) + { + expressionSyntax = parenthesizedExpression.Expression; + } + else + { + return; + } + } + + break; + + case IdentifierNameSyntax identifierName: + path.Insert(0, identifierName.Identifier.ValueText); + results.Add(path); + return; - return -1; + case MemberAccessExpressionSyntax memberAccessExpression: + path.Insert(0, memberAccessExpression.Name.Identifier.ValueText); + expressionSyntax = memberAccessExpression.Expression; + break; + + case InvocationExpressionSyntax invocationExpression: + foreach (var argument in invocationExpression.ArgumentList.Arguments) + { + ExtractMemberPaths(results, argument.Expression); + } + + path.Clear(); // Clear the path because we don't know the return type + expressionSyntax = invocationExpression.Expression; + break; + + case BinaryExpressionSyntax binaryExpression: + ExtractMemberPaths(results, binaryExpression.Left); + ExtractMemberPaths(results, binaryExpression.Right); + return; + + case PrefixUnaryExpressionSyntax unaryExpression: + ExtractMemberPaths(results, unaryExpression.Operand); + return; + + case PostfixUnaryExpressionSyntax unaryExpression: + ExtractMemberPaths(results, unaryExpression.Operand); + return; + + case ElementAccessExpressionSyntax elementAccess: + foreach (var argument in elementAccess.ArgumentList.Arguments) + { + ExtractMemberPaths(results, argument.Expression); + } + + path.Clear(); // Clear the path because we don't know the return type + expressionSyntax = elementAccess.Expression; + break; + + default: + return; + } } - while (!value.IsEmpty) - { - var startIndex = IndexOf(value, '{'); - if (startIndex < 0) - break; + results.Add(path); + } + } - value = value[(startIndex + 1)..]; - var endIndex = IndexOf(value, '}'); - if (endIndex < 0) - break; + private static bool IsValid(Compilation compilation, ISymbol rootSymbol, List syntax, out string? invalidMember) + { + if (syntax.Count is 0) + { + invalidMember = null; + return true; + } - var member = value[..endIndex]; - result ??= []; - result.Add(GetMemberName(member)); + var firstMember = syntax.First(); + var current = FindSymbol(rootSymbol, firstMember) ?? FindGlobalSymbol(compilation, firstMember); + if (current is null) + { + invalidMember = firstMember; + return false; + } - value = value[(endIndex + 1)..]; + foreach (var member in syntax.Skip(1)) + { + var next = FindSymbol(current, member); + if (next is null) + { + invalidMember = member; + return false; } - return result; + current = next; + } + + invalidMember = null; + return true; - static string GetMemberName(ReadOnlySpan member) + static ISymbol? FindSymbol(ISymbol parent, string name) + { + if (parent is INamespaceOrTypeSymbol typeSymbol) { - var index = member.IndexOfAny(MemberSeparators); - if (index < 0) - return member.ToString(); + if (typeSymbol.GetAllMembers(name).FirstOrDefault() is { } member) + { + if (member is INamespaceOrTypeSymbol) + return member; - return member[..index].ToString(); + return member.GetSymbolType(); + } } + + return null; } - static void ValidateValue(SymbolAnalysisContext context, INamedTypeSymbol symbol, AttributeData attribute, string value) + static ISymbol? FindGlobalSymbol(Compilation compilation, string name) { - var members = ParseMembers(value.AsSpan()); - if (members is not null) + if (CSharpKeywordToTypeName.TryGetValue(name, out var specialType)) + return compilation.GetSpecialType(specialType); + + return compilation.GlobalNamespace.GetMembers(name).FirstOrDefault(); + } + } + + private static List? ExtractExpressions(ReadOnlySpan value) + { + List? result = null; + + while (!value.IsEmpty) + { + var startIndex = IndexOf(value, '{'); + if (startIndex < 0) + break; + + value = value[(startIndex + 1)..]; + var endIndex = IndexOf(value, '}'); + if (endIndex < 0) + break; + + var expression = value[..endIndex]; + result ??= []; + result.Add(expression.ToString()); + + value = value[(endIndex + 1)..]; + } + + return result; + + static int IndexOf(ReadOnlySpan value, char c) + { + var skipped = 0; + while (!value.IsEmpty) { - foreach (var member in members) + var index = value.IndexOfAny(c, '\\'); + if (index < 0) + return -1; + + if (value[index] == c) + return index + skipped; + + if (index + 1 < value.Length) { - if (!MemberExists(symbol, member)) - { - context.ReportDiagnostic(Rule, attribute, member); - return; - } + skipped += index + 2; + value = value[(index + 2)..]; + } + else + { + return -1; } } + + return -1; } } } diff --git a/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasCancellationTokenAnalyzer.cs b/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasCancellationTokenAnalyzer.cs index bf35c3cf..f6604a13 100644 --- a/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasCancellationTokenAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasCancellationTokenAnalyzer.cs @@ -313,6 +313,9 @@ private string[] FindCancellationTokens(IOperation operation, CancellationToken foreach (var symbol in operation.LookupAvailableSymbols(cancellationToken)) { + if (symbol is IMethodSymbol) + continue; + var symbolType = symbol.GetSymbolType(); if (symbolType is null) continue; diff --git a/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasTimeProviderAnalyzer.cs b/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasTimeProviderAnalyzer.cs index b3f1b57f..b79caa08 100644 --- a/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasTimeProviderAnalyzer.cs +++ b/src/Meziantou.Analyzer/Rules/UseAnOverloadThatHasTimeProviderAnalyzer.cs @@ -215,6 +215,9 @@ private string[] FindTimeProviders(IOperation operation, CancellationToken cance foreach (var symbol in operation.LookupAvailableSymbols(cancellationToken)) { + if (symbol is IMethodSymbol) + continue; + var symbolType = symbol.GetSymbolType(); if (symbolType is null) continue; diff --git a/tests/Meziantou.Analyzer.Test/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzerTests.cs b/tests/Meziantou.Analyzer.Test/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzerTests.cs index dfbe3bac..e4428064 100755 --- a/tests/Meziantou.Analyzer.Test/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzerTests.cs +++ b/tests/Meziantou.Analyzer.Test/Rules/DebuggerDisplayAttributeShouldContainValidExpressionsAnalyzerTests.cs @@ -1,8 +1,6 @@ -using System.Threading.Tasks; using Meziantou.Analyzer.Rules; using Meziantou.Analyzer.Test.Helpers; using TestHelper; -using Xunit; namespace Meziantou.Analyzer.Test.Rules; @@ -20,6 +18,7 @@ private static ProjectBuilder CreateProjectBuilder() [InlineData("Invalid,np")] [InlineData("Invalid()")] [InlineData("Invalid.Length")] + [InlineData("System.IO.Path.DirectorySeparatorChar.Unknown()")] public async Task UnknownMember(string memberName) { var sourceCode = $$""" @@ -60,6 +59,7 @@ await CreateProjectBuilder() [InlineData("Invalid,np")] [InlineData("Invalid()")] [InlineData("Invalid.Length")] + [InlineData("Display.UnknownProperty")] public async Task UnknownMember_Type(string memberName) { var sourceCode = $$""" @@ -75,19 +75,21 @@ await CreateProjectBuilder() .ValidateAsync(); } - [Fact] - public async Task Valid() + [Theory] + [InlineData("Display")] + [InlineData("System.IO.Path.DirectorySeparatorChar.ToString()")] + public async Task Valid(string value) { - const string SourceCode = """ + var sourceCode = $$""" using System.Diagnostics; - [DebuggerDisplay("{Display}")] + [DebuggerDisplay("{{{value}}}")] public class Dummy { public string Display { get; } } """; await CreateProjectBuilder() - .WithSourceCode(SourceCode) + .WithSourceCode(sourceCode) .ValidateAsync(); } @@ -171,19 +173,22 @@ await CreateProjectBuilder() .ValidateAsync(); } - [Fact] - public async Task Valid_Method() + [Theory] + [InlineData("Display()")] + [InlineData("Display().Length")] + [InlineData("Display().Invalid")] // Invalid is ignored because we cannot determine the return type of Display() + public async Task Valid_Method(string value) { - const string SourceCode = """ + var sourceCode = $$""" using System.Diagnostics; - [DebuggerDisplay("{Display()}")] + [DebuggerDisplay("{{{value}}}")] public class Dummy { private string Display() => ""; } """; await CreateProjectBuilder() - .WithSourceCode(SourceCode) + .WithSourceCode(sourceCode) .ValidateAsync(); } @@ -278,14 +283,143 @@ await CreateProjectBuilder() .ValidateAsync(); } + [Theory] + [InlineData("Value + 1")] + [InlineData("Value - 1")] + [InlineData("Value < 10")] + [InlineData("Value <= 10")] + [InlineData("Value > 10")] + [InlineData("Value >= 10")] + [InlineData("Value == 10")] + [InlineData("Value != 10")] + [InlineData("Demo.Display(Value > 1)")] + [InlineData("Demo.Display(Value > 1, Value)")] + public async Task Valid_BinaryOperator(string value) + { + var sourceCode = $$""" + using System.Diagnostics; + + [DebuggerDisplay(@"{{{value}}}")] + public record Person(int Value); + + public class Demo + { + public static string Display(bool a) => throw null; + } + """; + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + + [Theory] + [InlineData("(Unknown + 1)")] + [InlineData("(Unknown + (1))")] + [InlineData("((Unknown) + (1))")] + [InlineData("Unknown + 1")] + [InlineData("Unknown - 1")] + [InlineData("Unknown < 10")] + [InlineData("Unknown <= 10")] + [InlineData("Unknown > 10")] + [InlineData("Unknown >= 10")] + [InlineData("Unknown == 10")] + [InlineData("Unknown != 10")] + [InlineData("Unknown != \\\"abc\\\"")] + [InlineData("Unknown != 'a'")] + [InlineData("Demo.Display(Unknown > 1)")] + [InlineData("Demo.Display(Unknown > 1.0)")] + [InlineData("Demo.Display(Unknown > 1u)")] + [InlineData("Demo.Display(Unknown > 1uL)")] + [InlineData("Demo.Display(Unknown > 1L)")] + [InlineData("Demo.Display(Unknown > 1.0f)")] + [InlineData("Demo.Display(Unknown > 1.0d)")] + [InlineData("Demo.Display(Unknown > 1.0m)")] + [InlineData("Demo.Display(Unknown > 1e+3)")] + [InlineData("Demo.Display(Value > 1, Unknown)")] + public async Task Invalid_BinaryOperator(string value) + { + var sourceCode = $$""" + using System.Diagnostics; + + [[|DebuggerDisplay("{{{value}}}")|]] + public record Person(int Value); + + public class Demo + { + public static string Display(bool a) => throw null; + } + """; + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + + [Theory] + [InlineData("!Value")] + public async Task Valid_UnaryOperator(string value) + { + var sourceCode = $$""" + using System.Diagnostics; + + [DebuggerDisplay(@"{{{value}}}")] + public record Person(bool Value); + """; + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + + [Theory] + [InlineData("!Unknown")] + public async Task Invalid_UnaryOperator(string value) + { + var sourceCode = $$""" + using System.Diagnostics; + + [[|DebuggerDisplay(@"{{{value}}}")|]] + public record Person(bool Value); + """; + await CreateProjectBuilder() + .WithSourceCode(sourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task CallStaticMethodOnAnotherType() + { + const string SourceCode = """ + using System.Diagnostics; + + [DebuggerDisplay(@"{System.Linq.Enumerable.Count(Test)}")] + public record Person(string[] Test); + """; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + [Fact] - public async Task ExpressionLessThan() + public async Task CallStaticMethodOnAnotherUsingKeyword() { const string SourceCode = """ using System.Diagnostics; - [DebuggerDisplay(@"{Value < 10}")] - public record Person(int Value); + [DebuggerDisplay(@"{char.IsAscii(Test)}")] + public record Person(char Test); + """; + await CreateProjectBuilder() + .WithSourceCode(SourceCode) + .ValidateAsync(); + } + + [Fact] + public async Task CallStaticMethodOnAnotherType_InvalidMethodName() + { + const string SourceCode = """ + using System.Diagnostics; + + [[|DebuggerDisplay(@"{System.Linq.Enumerable.InvalidMethod(Test)}")|]] + public record Person(string[] Test); """; await CreateProjectBuilder() .WithSourceCode(SourceCode)