diff --git a/ChangeLog.md b/ChangeLog.md index 256a8add26..935b14192b 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix NullReferenceException in [RCS1198](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1198.md) ([#940](https://github.com/josefpihrt/roslynator/pull/940). - Order named arguments even if optional arguments are not specified [RCS1205](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1205.md) ([#941](https://github.com/josefpihrt/roslynator/pull/941). - Prefix identifier with `@` if necessary ([RCS1220](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1220.md)) ([#943](https://github.com/josefpihrt/roslynator/pull/943). +- Do not suggest to make local variable a const when it is used in ref extension method ([RCS1118](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1118.md)) ([#948](https://github.com/josefpihrt/roslynator/pull/948). ----- diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs index 2dbbbf9710..c6489f0b18 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs @@ -83,7 +83,7 @@ private static void AnalyzeLocalDeclarationStatement(SyntaxNodeAnalysisContext c return; } - if (!CanBeMarkedAsConst(localInfo.Variables, statements, index + 1)) + if (!CanBeMarkedAsConst(context, localInfo.Variables, statements, index + 1)) return; if (((CSharpParseOptions)context.Node.SyntaxTree.Options).LanguageVersion <= LanguageVersion.CSharp9 @@ -96,31 +96,43 @@ private static void AnalyzeLocalDeclarationStatement(SyntaxNodeAnalysisContext c } private static bool CanBeMarkedAsConst( + SyntaxNodeAnalysisContext context, SeparatedSyntaxList variables, SyntaxList statements, int startIndex) { - MarkLocalVariableAsConstWalker walker = MarkLocalVariableAsConstWalker.GetInstance(); + MarkLocalVariableAsConstWalker walker = null; - foreach (VariableDeclaratorSyntax variable in variables) - walker.Identifiers.Add(variable.Identifier.ValueText); + try + { + walker = MarkLocalVariableAsConstWalker.GetInstance(); - var result = true; + walker.SemanticModel = context.SemanticModel; + walker.CancellationToken = context.CancellationToken; - for (int i = startIndex; i < statements.Count; i++) - { - walker.Visit(statements[i]); + foreach (VariableDeclaratorSyntax variable in variables) + { + var symbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken) as ILocalSymbol; + + if (symbol is not null) + walker.Identifiers[variable.Identifier.ValueText] = symbol; + } - if (walker.Result) + for (int i = startIndex; i < statements.Count; i++) { - result = false; - break; + walker.Visit(statements[i]); + + if (walker.Result) + return false; } } + finally + { + if (walker != null) + MarkLocalVariableAsConstWalker.Free(walker); + } - MarkLocalVariableAsConstWalker.Free(walker); - - return result; + return true; } private static bool HasConstantValue( diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs index 8b42b3668d..6c8f2fda58 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs @@ -3,6 +3,9 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslynator.CSharp.SyntaxWalkers; @@ -14,36 +17,59 @@ internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker [ThreadStatic] private static MarkLocalVariableAsConstWalker _cachedInstance; - public HashSet Identifiers { get; } = new(); + public Dictionary Identifiers { get; } = new(); + + public SemanticModel SemanticModel { get; set; } + + public CancellationToken CancellationToken { get; set; } public bool Result { get; set; } - protected override bool ShouldVisit - { - get { return !Result; } - } + protected override bool ShouldVisit => !Result; public override void VisitAssignedExpression(ExpressionSyntax expression) { - if (expression is IdentifierNameSyntax identifierName - && Identifiers.Contains(identifierName.Identifier.ValueText)) - { + if (IsLocalReference(expression)) Result = true; - } } - public override void VisitPrefixUnaryExpression(PrefixUnaryExpressionSyntax node) + public override void VisitIdentifierName(IdentifierNameSyntax node) { - if (node.Kind() == SyntaxKind.AddressOfExpression - && node.Operand is IdentifierNameSyntax identifierName) + if (node.IsParentKind(SyntaxKind.SimpleMemberAccessExpression, SyntaxKind.AddressOfExpression) + && IsLocalReference(node)) { - if (Identifiers.Contains(identifierName.Identifier.ValueText)) + if (node.IsParentKind(SyntaxKind.SimpleMemberAccessExpression)) + { + var methodSymbol = SemanticModel.GetSymbol(node.Parent, CancellationToken) as IMethodSymbol; + + if (methodSymbol? + .ReducedFrom? + .Parameters + .FirstOrDefault()? + .IsRefOrOut() == true) + { + Result = true; + } + } + else if (node.IsParentKind(SyntaxKind.AddressOfExpression)) + { Result = true; + } } - else - { - base.VisitPrefixUnaryExpression(node); - } + + base.VisitIdentifierName(node); + } + + private bool IsLocalReference(SyntaxNode node) + { + return node is IdentifierNameSyntax identifierName + && IsLocalReference(identifierName); + } + + private bool IsLocalReference(IdentifierNameSyntax identifierName) + { + return Identifiers.TryGetValue(identifierName.Identifier.ValueText, out ILocalSymbol symbol) + && SymbolEqualityComparer.Default.Equals(symbol, SemanticModel.GetSymbol(identifierName, CancellationToken)); } public static MarkLocalVariableAsConstWalker GetInstance() @@ -65,6 +91,8 @@ public static MarkLocalVariableAsConstWalker GetInstance() public static void Free(MarkLocalVariableAsConstWalker walker) { walker.Identifiers.Clear(); + walker.SemanticModel = null; + walker.CancellationToken = default; walker.Result = false; _cachedInstance = walker; diff --git a/src/Tests/Analyzers.Tests/RCS1118MarkLocalVariableAsConstTests.cs b/src/Tests/Analyzers.Tests/RCS1118MarkLocalVariableAsConstTests.cs index 08f98f8096..f9bf15d7c2 100644 --- a/src/Tests/Analyzers.Tests/RCS1118MarkLocalVariableAsConstTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1118MarkLocalVariableAsConstTests.cs @@ -80,5 +80,47 @@ void M() } ", options: WellKnownCSharpTestOptions.Default_CSharp9); } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.MarkLocalVariableAsConst)] + public async Task TestNoDiagnostic_RefParameter() + { + await VerifyNoDiagnosticAsync(@" +public static class C +{ + static int Foo(int p) + { + int x = default; + Bar(ref x, p); + + return x; + } + + public static int Bar(this ref int p1, int p2) + { + return p1 + p2; + } +}"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.MarkLocalVariableAsConst)] + public async Task TestNoDiagnostic_RefParameter_ExtensionMethod() + { + await VerifyNoDiagnosticAsync(@" +public static class C +{ + static int Foo(int p) + { + int x = default; + x.Bar(p); + + return x; + } + + public static int Bar(this ref int p1, int p2) + { + return p1 + p2; + } +}"); + } } }