diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1141CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1141CodeFixProvider.cs index b53676c8a..15134c55a 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1141CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1141CodeFixProvider.cs @@ -77,7 +77,7 @@ private static async Task GetTransformedDocumentAsync(Document documen break; } - var newSyntaxRoot = syntaxRoot.ReplaceNode(node, newNode).WithAdditionalAnnotations(Simplifier.Annotation); + var newSyntaxRoot = syntaxRoot.ReplaceNode(node, newNode.WithAdditionalAnnotations(Simplifier.Annotation)); return document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting()); } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1141CSharp7UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1141CSharp7UnitTests.cs index e5849baf3..47e4a266a 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1141CSharp7UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1141CSharp7UnitTests.cs @@ -182,6 +182,49 @@ public void TestMethod() await VerifyCSharpFixAsync(testCode, expectedDiagnostics, fixedCode, CancellationToken.None).ConfigureAwait(false); } + /// + /// Validates that the usage of within LINQ expression trees will produce no + /// diagnostics. + /// + /// A representing the asynchronous unit test. + [Fact] + [WorkItem(3305, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3305")] + public async Task ValidateValueTupleUsageInExpressionTreeAsync() + { + var testCode = @"using System; +using System.Linq.Expressions; + +public class TestClass +{ + Expression> expression1 = () => ValueTuple.Create(10, 20); + Expression> expression2 = () => new ValueTuple(10, 20); + Expression> expression3 = () => new ValueTuple, int>(ValueTuple.Create(10, 10), 20); + Expression> expression4 = () => new System.ValueTuple(10, 20); + Expression> expression5 = arg => new System.ValueTuple(arg.Item1, arg.Item2); + Expression> expression6 = (arg) => new System.ValueTuple(arg.Item1, arg.Item2); + + Expression|], [|ValueTuple|]>> expression7 = ([|ValueTuple|] arg) => ValueTuple.Create(arg.Item1, arg.Item2); +} +"; + var fixedCode = @"using System; +using System.Linq.Expressions; + +public class TestClass +{ + Expression> expression1 = () => ValueTuple.Create(10, 20); + Expression> expression2 = () => new ValueTuple(10, 20); + Expression> expression3 = () => new ValueTuple, int>(ValueTuple.Create(10, 10), 20); + Expression> expression4 = () => new System.ValueTuple(10, 20); + Expression> expression5 = arg => new System.ValueTuple(arg.Item1, arg.Item2); + Expression> expression6 = (arg) => new System.ValueTuple(arg.Item1, arg.Item2); + + Expression> expression7 = ((int, int) arg) => ValueTuple.Create(arg.Item1, arg.Item2); +} +"; + + await VerifyCSharpFixAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + /// /// Validates that the usage of within pattern matching will produce no diagnostics. /// diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1141UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1141UnitTests.cs index 11dc95b04..3e3092ffb 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1141UnitTests.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1141UnitTests.cs @@ -3,6 +3,7 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules { + using System; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; @@ -20,7 +21,7 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules public class SA1141UnitTests { /// - /// Verifies that usage of the ValueTuple type will not produce a diagnostic. + /// Verifies that usage of will not produce a diagnostic. /// /// A representing the asynchronous operation. [Fact] diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxNodeExtensions.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxNodeExtensions.cs new file mode 100644 index 000000000..6ae097a67 --- /dev/null +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxNodeExtensions.cs @@ -0,0 +1,92 @@ +// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +#nullable enable + +namespace StyleCop.Analyzers.Helpers +{ + using System.Threading; + using Microsoft.CodeAnalysis; + using Microsoft.CodeAnalysis.CSharp; + using Microsoft.CodeAnalysis.CSharp.Syntax; + + internal static class SyntaxNodeExtensions + { + public static bool IsInExpressionTree( + this SyntaxNode? node, + SemanticModel semanticModel, + INamedTypeSymbol? expressionType, + CancellationToken cancellationToken) + { + if (expressionType != null) + { + for (var current = node; current != null; current = current.Parent) + { + if (current.IsAnyLambda()) + { + var typeInfo = semanticModel.GetTypeInfo(current, cancellationToken); + if (expressionType.Equals(typeInfo.ConvertedType?.OriginalDefinition)) + { + return true; + } + } + else if (current is SelectOrGroupClauseSyntax or OrderingSyntax) + { + var info = semanticModel.GetSymbolInfo(current, cancellationToken); + if (AnyTakesExpressionTree(info, expressionType)) + { + return true; + } + } + else if (current is QueryClauseSyntax queryClause) + { + var info = semanticModel.GetQueryClauseInfo(queryClause, cancellationToken); + if (AnyTakesExpressionTree(info.CastInfo, expressionType) + || AnyTakesExpressionTree(info.OperationInfo, expressionType)) + { + return true; + } + } + } + } + + return false; + + static bool AnyTakesExpressionTree(SymbolInfo info, INamedTypeSymbol expressionType) + { + if (TakesExpressionTree(info.Symbol, expressionType)) + { + return true; + } + + foreach (var symbol in info.CandidateSymbols) + { + if (TakesExpressionTree(symbol, expressionType)) + { + return true; + } + } + + return false; + } + + static bool TakesExpressionTree(ISymbol symbol, INamedTypeSymbol expressionType) + { + if (symbol is IMethodSymbol method + && method.Parameters.Length > 0 + && expressionType.Equals(method.Parameters[0].Type?.OriginalDefinition)) + { + return true; + } + + return false; + } + } + + public static bool IsAnyLambda(this SyntaxNode? node) + { + return node.IsKind(SyntaxKind.ParenthesizedLambdaExpression) + || node.IsKind(SyntaxKind.SimpleLambdaExpression); + } + } +} diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1141UseTupleSyntax.cs b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1141UseTupleSyntax.cs index e2e01d475..de3d41dd4 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1141UseTupleSyntax.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1141UseTupleSyntax.cs @@ -25,15 +25,13 @@ internal class SA1141UseTupleSyntax : DiagnosticAnalyzer private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(ReadabilityResources.SA1141MessageFormat), ReadabilityResources.ResourceManager, typeof(ReadabilityResources)); private static readonly LocalizableString Description = new LocalizableResourceString(nameof(ReadabilityResources.SA1141Description), ReadabilityResources.ResourceManager, typeof(ReadabilityResources)); + private static readonly Action CompilationStartAction = HandleCompilationStart; private static readonly Action MethodDeclarationAction = HandleMethodDeclaration; private static readonly Action ConversionOperatorAction = HandleConversionOperator; private static readonly Action PropertyDeclarationAction = HandleBasePropertyDeclaration; private static readonly Action IndexerDeclarationAction = HandleBasePropertyDeclaration; - private static readonly Action ObjectCreationExpressionAction = HandleObjectCreationExpression; - private static readonly Action InvocationExpressionAction = HandleInvocationExpression; - private static readonly Action DefaultExpressionAction = HandleDefaultExpression; + private static readonly Action FieldDeclarationAction = HandleFieldDeclaration; private static readonly Action DelegateDeclarationAction = HandleDelegateDeclaration; - private static readonly Action CastExpressionAction = HandleCastExpression; private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink); @@ -48,19 +46,28 @@ public override void Initialize(AnalysisContext context) context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(CompilationStartAction); + context.RegisterSyntaxNodeAction(MethodDeclarationAction, SyntaxKind.MethodDeclaration); context.RegisterSyntaxNodeAction(ConversionOperatorAction, SyntaxKind.ConversionOperatorDeclaration); context.RegisterSyntaxNodeAction(PropertyDeclarationAction, SyntaxKind.PropertyDeclaration); context.RegisterSyntaxNodeAction(IndexerDeclarationAction, SyntaxKind.IndexerDeclaration); + context.RegisterSyntaxNodeAction(FieldDeclarationAction, SyntaxKind.FieldDeclaration); - context.RegisterSyntaxNodeAction(ObjectCreationExpressionAction, SyntaxKind.ObjectCreationExpression); - context.RegisterSyntaxNodeAction(InvocationExpressionAction, SyntaxKind.InvocationExpression); + context.RegisterSyntaxNodeAction(DelegateDeclarationAction, SyntaxKind.DelegateDeclaration); + } - context.RegisterSyntaxNodeAction(DefaultExpressionAction, SyntaxKind.DefaultExpression); + private static void HandleCompilationStart(CompilationStartAnalysisContext context) + { + var expressionType = context.Compilation.GetTypeByMetadataName("System.Linq.Expressions.Expression`1"); - context.RegisterSyntaxNodeAction(DelegateDeclarationAction, SyntaxKind.DelegateDeclaration); + context.RegisterSyntaxNodeAction(context => HandleLambdaExpression(context, expressionType), SyntaxKinds.LambdaExpression); + context.RegisterSyntaxNodeAction(context => HandleObjectCreationExpression(context, expressionType), SyntaxKind.ObjectCreationExpression); + context.RegisterSyntaxNodeAction(context => HandleInvocationExpression(context, expressionType), SyntaxKind.InvocationExpression); + + context.RegisterSyntaxNodeAction(context => HandleDefaultExpression(context, expressionType), SyntaxKind.DefaultExpression); - context.RegisterSyntaxNodeAction(CastExpressionAction, SyntaxKind.CastExpression); + context.RegisterSyntaxNodeAction(context => HandleCastExpression(context, expressionType), SyntaxKind.CastExpression); } private static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context) @@ -72,7 +79,7 @@ private static void HandleMethodDeclaration(SyntaxNodeAnalysisContext context) var methodDeclaration = (MethodDeclarationSyntax)context.Node; - CheckType(context, methodDeclaration.ReturnType); + CheckType(context, expressionType: null, methodDeclaration.ReturnType); CheckParameterList(context, methodDeclaration.ParameterList); } @@ -85,7 +92,7 @@ private static void HandleConversionOperator(SyntaxNodeAnalysisContext context) var conversionOperatorDeclaration = (ConversionOperatorDeclarationSyntax)context.Node; - CheckType(context, conversionOperatorDeclaration.Type); + CheckType(context, expressionType: null, conversionOperatorDeclaration.Type); CheckParameterList(context, conversionOperatorDeclaration.ParameterList); } @@ -97,10 +104,35 @@ private static void HandleBasePropertyDeclaration(SyntaxNodeAnalysisContext cont } var propertyDeclaration = (BasePropertyDeclarationSyntax)context.Node; - CheckType(context, propertyDeclaration.Type); + CheckType(context, expressionType: null, propertyDeclaration.Type); + } + + private static void HandleFieldDeclaration(SyntaxNodeAnalysisContext context) + { + if (!context.SupportsTuples()) + { + return; + } + + var fieldDeclaration = (BaseFieldDeclarationSyntax)context.Node; + CheckType(context, expressionType: null, fieldDeclaration.Declaration.Type); } - private static void HandleObjectCreationExpression(SyntaxNodeAnalysisContext context) + private static void HandleLambdaExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType) + { + if (!context.SupportsTuples()) + { + return; + } + + var lambdaExpression = (LambdaExpressionSyntax)context.Node; + if (lambdaExpression is ParenthesizedLambdaExpressionSyntax parenthesizedLambdaExpression) + { + CheckParameterList(context, parenthesizedLambdaExpression.ParameterList); + } + } + + private static void HandleObjectCreationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType) { if (!context.SupportsTuples()) { @@ -108,10 +140,10 @@ private static void HandleObjectCreationExpression(SyntaxNodeAnalysisContext con } var objectCreationExpression = (ObjectCreationExpressionSyntax)context.Node; - CheckType(context, objectCreationExpression.Type, objectCreationExpression.GetLocation()); + CheckType(context, expressionType, objectCreationExpression.Type, objectCreationExpression.GetLocation()); } - private static void HandleInvocationExpression(SyntaxNodeAnalysisContext context) + private static void HandleInvocationExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType) { if (!context.SupportsTuples()) { @@ -133,8 +165,9 @@ private static void HandleInvocationExpression(SyntaxNodeAnalysisContext context var memberAccessExpression = (MemberAccessExpressionSyntax)invocationExpression.Expression; var symbolInfo = context.SemanticModel.GetSymbolInfo(memberAccessExpression.Expression); - if ((symbolInfo.Symbol is INamedTypeSymbol namedTypeSymbol) - && (namedTypeSymbol.ToDisplayString(DisplayFormat) == "System.ValueTuple")) + if (symbolInfo.Symbol is INamedTypeSymbol namedTypeSymbol + && namedTypeSymbol.ToDisplayString(DisplayFormat) == "System.ValueTuple" + && !context.Node.IsInExpressionTree(context.SemanticModel, expressionType, context.CancellationToken)) { if (memberAccessExpression.Name.Identifier.ValueText == "Create") { @@ -143,7 +176,7 @@ private static void HandleInvocationExpression(SyntaxNodeAnalysisContext context } } - private static void HandleDefaultExpression(SyntaxNodeAnalysisContext context) + private static void HandleDefaultExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType) { if (!context.SupportsTuples()) { @@ -151,7 +184,7 @@ private static void HandleDefaultExpression(SyntaxNodeAnalysisContext context) } var defaultExpression = (DefaultExpressionSyntax)context.Node; - CheckType(context, defaultExpression.Type); + CheckType(context, expressionType, defaultExpression.Type); } private static void HandleDelegateDeclaration(SyntaxNodeAnalysisContext context) @@ -162,11 +195,11 @@ private static void HandleDelegateDeclaration(SyntaxNodeAnalysisContext context) } var delegateDeclaration = (DelegateDeclarationSyntax)context.Node; - CheckType(context, delegateDeclaration.ReturnType); + CheckType(context, expressionType: null, delegateDeclaration.ReturnType); CheckParameterList(context, delegateDeclaration.ParameterList); } - private static void HandleCastExpression(SyntaxNodeAnalysisContext context) + private static void HandleCastExpression(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType) { if (!context.SupportsTuples()) { @@ -174,46 +207,51 @@ private static void HandleCastExpression(SyntaxNodeAnalysisContext context) } var castExpression = (CastExpressionSyntax)context.Node; - CheckType(context, castExpression.Type); + CheckType(context, expressionType, castExpression.Type); } private static void CheckParameterList(SyntaxNodeAnalysisContext context, ParameterListSyntax parameterList) { foreach (var parameter in parameterList.Parameters) { - CheckType(context, parameter.Type); + CheckType(context, expressionType: null, parameter.Type); } } - private static void CheckType(SyntaxNodeAnalysisContext context, TypeSyntax typeSyntax, Location reportLocation = null) + private static void CheckType(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType, TypeSyntax typeSyntax, Location reportLocation = null) { + if (typeSyntax is null) + { + return; + } + switch (typeSyntax.Kind()) { case SyntaxKindEx.TupleType: - CheckTupleType(context, (TupleTypeSyntaxWrapper)typeSyntax, reportLocation); + CheckTupleType(context, expressionType, (TupleTypeSyntaxWrapper)typeSyntax, reportLocation); break; case SyntaxKind.QualifiedName: - CheckType(context, ((QualifiedNameSyntax)typeSyntax).Right, reportLocation ?? typeSyntax.GetLocation()); + CheckType(context, expressionType, ((QualifiedNameSyntax)typeSyntax).Right, reportLocation ?? typeSyntax.GetLocation()); break; case SyntaxKind.GenericName: - CheckGenericName(context, (GenericNameSyntax)typeSyntax, reportLocation); + CheckGenericName(context, expressionType, (GenericNameSyntax)typeSyntax, reportLocation); break; } } - private static void CheckTupleType(SyntaxNodeAnalysisContext context, TupleTypeSyntaxWrapper tupleTypeSyntax, Location reportLocation) + private static void CheckTupleType(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType, TupleTypeSyntaxWrapper tupleTypeSyntax, Location reportLocation) { foreach (var tupleElementSyntax in tupleTypeSyntax.Elements) { - CheckType(context, tupleElementSyntax.Type, reportLocation); + CheckType(context, expressionType, tupleElementSyntax.Type, reportLocation); } } - private static void CheckGenericName(SyntaxNodeAnalysisContext context, GenericNameSyntax genericNameSyntax, Location reportLocation) + private static void CheckGenericName(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType, GenericNameSyntax genericNameSyntax, Location reportLocation) { - if (IsValueTupleWithLanguageRepresentation(context, genericNameSyntax)) + if (IsValueTupleWithLanguageRepresentation(context, expressionType, genericNameSyntax)) { var location = reportLocation ?? genericNameSyntax.GetLocation(); context.ReportDiagnostic(Diagnostic.Create(Descriptor, location)); @@ -224,16 +262,17 @@ private static void CheckGenericName(SyntaxNodeAnalysisContext context, GenericN foreach (var typeArgument in genericNameSyntax.TypeArgumentList.Arguments) { - CheckType(context, typeArgument); + CheckType(context, expressionType, typeArgument); } } - private static bool IsValueTupleWithLanguageRepresentation(SyntaxNodeAnalysisContext context, ExpressionSyntax syntax) + private static bool IsValueTupleWithLanguageRepresentation(SyntaxNodeAnalysisContext context, INamedTypeSymbol expressionType, ExpressionSyntax syntax) { var symbolInfo = context.SemanticModel.GetSymbolInfo(syntax, context.CancellationToken); return symbolInfo.Symbol is INamedTypeSymbol typeSymbol && typeSymbol.IsTupleType() - && typeSymbol.TupleElements().Length > 1; + && typeSymbol.TupleElements().Length > 1 + && !syntax.IsInExpressionTree(context.SemanticModel, expressionType, context.CancellationToken); } } }