From 25dd6442557384791d647b14888eb34b661eefcf Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sat, 12 Nov 2022 22:31:07 +0100 Subject: [PATCH 1/3] Fix RR0014 and RR0180 --- .../AddUsingStaticDirectiveRefactoring.cs | 11 +- .../InlineUsingStaticDirectiveRefactoring.cs | 3 +- .../AddUsingStaticDirectiveRefactoring.cs | 15 -- .../InlineUsingStaticRefactoring.cs | 16 --- ...sts.cs => RR0013AddUsingDirectiveTests.cs} | 2 +- .../RR0014AddUsingStaticDirectiveTests.cs | 136 ++++++++++++++++++ .../RR0180InlineUsingStaticDirectiveTests.cs | 64 +++++++++ 7 files changed, 210 insertions(+), 37 deletions(-) delete mode 100644 src/Tests.Old/Refactorings.Tests.Old/AddUsingStaticDirectiveRefactoring.cs delete mode 100644 src/Tests.Old/Refactorings.Tests.Old/InlineUsingStaticRefactoring.cs rename src/Tests/Refactorings.Tests/{RR0014AddUsingDirectiveTests.cs => RR0013AddUsingDirectiveTests.cs} (97%) create mode 100644 src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs diff --git a/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs b/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs index aa95c7e210..fa76e026ce 100644 --- a/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs +++ b/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs @@ -18,10 +18,16 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Me if (memberAccess.Name?.IsMissing != false) return; - memberAccess = GetTopmostMemberAccessExpression(memberAccess); + if (context.Span.IsBetweenSpans(memberAccess)) + { + if (memberAccess.IsParentKind(SyntaxKind.SimpleMemberAccessExpression)) + memberAccess = (MemberAccessExpressionSyntax)memberAccess.Parent; + } if (!context.Span.IsBetweenSpans(memberAccess.Expression)) + { return; + } SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); @@ -30,9 +36,6 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Me if (typeSymbol?.TypeKind != TypeKind.Class) return; - if (!typeSymbol.IsStatic) - return; - if (!typeSymbol.DeclaredAccessibility.Is(Accessibility.Public, Accessibility.Internal)) return; diff --git a/src/Refactorings/CSharp/Refactorings/InlineUsingStaticDirectiveRefactoring.cs b/src/Refactorings/CSharp/Refactorings/InlineUsingStaticDirectiveRefactoring.cs index 0036c72951..d17f55f065 100644 --- a/src/Refactorings/CSharp/Refactorings/InlineUsingStaticDirectiveRefactoring.cs +++ b/src/Refactorings/CSharp/Refactorings/InlineUsingStaticDirectiveRefactoring.cs @@ -56,7 +56,8 @@ internal static class InlineUsingStaticDirectiveRefactoring foreach (SyntaxNode descendant in node.DescendantNodes()) { - if (!descendant.IsParentKind(SyntaxKind.SimpleMemberAccessExpression) + if ((!descendant.IsParentKind(SyntaxKind.SimpleMemberAccessExpression) + || ((MemberAccessExpressionSyntax)descendant.Parent).Name != descendant) && (descendant is SimpleNameSyntax name)) { ISymbol symbol = semanticModel.GetSymbol(name, cancellationToken); diff --git a/src/Tests.Old/Refactorings.Tests.Old/AddUsingStaticDirectiveRefactoring.cs b/src/Tests.Old/Refactorings.Tests.Old/AddUsingStaticDirectiveRefactoring.cs deleted file mode 100644 index 9cf7b50457..0000000000 --- a/src/Tests.Old/Refactorings.Tests.Old/AddUsingStaticDirectiveRefactoring.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Roslynator.CSharp.Refactorings.Tests -{ - internal class AddUsingStaticDirectiveRefactoring - { - public void Do() - { - int max = Math.Max(1, 2); - - max = System.Math.Max(1, 2); - max = global::System.Math.Max(1, 2); - } - } -} diff --git a/src/Tests.Old/Refactorings.Tests.Old/InlineUsingStaticRefactoring.cs b/src/Tests.Old/Refactorings.Tests.Old/InlineUsingStaticRefactoring.cs deleted file mode 100644 index 3a9c8e6b80..0000000000 --- a/src/Tests.Old/Refactorings.Tests.Old/InlineUsingStaticRefactoring.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using static System.Math; - -namespace Roslynator.CSharp.Refactorings.Tests -{ - internal static class InlineUsingStaticRefactoring - { - private static void Foo() - { - var max = Max(1, 2); - var min = Min(1, 2); - } - } -} diff --git a/src/Tests/Refactorings.Tests/RR0014AddUsingDirectiveTests.cs b/src/Tests/Refactorings.Tests/RR0013AddUsingDirectiveTests.cs similarity index 97% rename from src/Tests/Refactorings.Tests/RR0014AddUsingDirectiveTests.cs rename to src/Tests/Refactorings.Tests/RR0013AddUsingDirectiveTests.cs index 49b186168b..b82f1928a2 100644 --- a/src/Tests/Refactorings.Tests/RR0014AddUsingDirectiveTests.cs +++ b/src/Tests/Refactorings.Tests/RR0013AddUsingDirectiveTests.cs @@ -6,7 +6,7 @@ namespace Roslynator.CSharp.Refactorings.Tests { - public class RR0014AddUsingDirectiveTests : AbstractCSharpRefactoringVerifier + public class RR0013AddUsingDirectiveTests : AbstractCSharpRefactoringVerifier { public override string RefactoringId { get; } = RefactoringIdentifiers.AddUsingDirective; diff --git a/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs b/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs new file mode 100644 index 0000000000..6cd4455135 --- /dev/null +++ b/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs @@ -0,0 +1,136 @@ +// Copyright (c) Josef Pihrt and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Roslynator.Testing.CSharp; +using Xunit; + +namespace Roslynator.CSharp.Refactorings.Tests +{ + public class RR0014AddUsingStaticDirectiveTests : AbstractCSharpRefactoringVerifier + { + public override string RefactoringId { get; } = RefactoringIdentifiers.AddUsingStaticDirective; + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.AddUsingStaticDirective)] + public async Task Test_Math_Max() + { + await VerifyRefactoringAsync(@" +using System; + +class C +{ + void M() + { + int max = [|Math|].Max(1, 2); + } +} +", @" +using System; +using static System.Math; + +class C +{ + void M() + { + int max = Max(1, 2); + } +} +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.AddUsingStaticDirective)] + public async Task Test_Math_Max2() + { + await VerifyRefactoringAsync(@" +class C +{ + void M() + { + int max = [|System.Math|].Max(1, 2); + } +} +", @"using static System.Math; + +class C +{ + void M() + { + int max = Max(1, 2); + } +} +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.AddUsingStaticDirective)] + public async Task Test_Math_Max3() + { + await VerifyRefactoringAsync(@" +class C +{ + void M() + { + int max = [|global::System.Math|].Max(1, 2); + } +} +", @"using static System.Math; + +class C +{ + void M() + { + int max = Max(1, 2); + } +} +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.AddUsingStaticDirective)] + public async Task Test_SimpleMemberAccessExpression() + { + await VerifyRefactoringAsync(@" +using System; + +class C +{ + void M() + { + var x = [|StringComparer|].CurrentCulture.GetHashCode(); + } +} +", @" +using System; +using static System.StringComparer; + +class C +{ + void M() + { + var x = CurrentCulture.GetHashCode(); + } +} +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.AddUsingStaticDirective)] + public async Task Test_SimpleMemberAccessExpression2() + { + await VerifyRefactoringAsync(@" +class C +{ + void M() + { + var x = [|System.StringComparer|].CurrentCulture.GetHashCode(); + } +} +", @"using static System.StringComparer; + +class C +{ + void M() + { + var x = CurrentCulture.GetHashCode(); + } +} +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + } +} diff --git a/src/Tests/Refactorings.Tests/RR0180InlineUsingStaticDirectiveTests.cs b/src/Tests/Refactorings.Tests/RR0180InlineUsingStaticDirectiveTests.cs index 1b78101972..761995b61f 100644 --- a/src/Tests/Refactorings.Tests/RR0180InlineUsingStaticDirectiveTests.cs +++ b/src/Tests/Refactorings.Tests/RR0180InlineUsingStaticDirectiveTests.cs @@ -42,6 +42,70 @@ void M() Enumerable.Empty(); } } +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.InlineUsingStaticDirective)] + public async Task Test2() + { + await VerifyRefactoringAsync(@" +using System; +using [||]static System.StringComparer; + +class C +{ + void M() + { + var a = CurrentCulture.GetHashCode(); + var b = CurrentCulture; + var c = StringComparer.CurrentCulture; + var d = System.StringComparer.CurrentCulture; + var e = global::System.StringComparer.CurrentCulture; + } +} +", @" +using System; + +class C +{ + void M() + { + var a = StringComparer.CurrentCulture.GetHashCode(); + var b = StringComparer.CurrentCulture; + var c = StringComparer.CurrentCulture; + var d = System.StringComparer.CurrentCulture; + var e = global::System.StringComparer.CurrentCulture; + } +} +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.InlineUsingStaticDirective)] + public async Task Test_Math_Max() + { + await VerifyRefactoringAsync(@" +using System; +using [||]static System.Math; + +class C +{ + void M() + { + var max = Max(1, 2); + var min = Min(1, 2); + } +} +", @" +using System; + +class C +{ + void M() + { + var max = Math.Max(1, 2); + var min = Math.Min(1, 2); + } +} ", equivalenceKey: EquivalenceKey.Create(RefactoringId)); } } From 5ea3eb4b6653cbe5988b6007970208a4d7a665a9 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sat, 12 Nov 2022 22:32:53 +0100 Subject: [PATCH 2/3] changelog --- ChangeLog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index 7e7ec8cb8d..7f1d3d6ed4 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix ([RCS1080](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1080.md)) when collection is derived from `List` ([#986](https://github.com/josefpihrt/roslynator/pull/986). +- Fix refactoring ([RR0014](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RR0014.md)) ([#988](https://github.com/josefpihrt/roslynator/pull/988). +- Fix refactoring ([RR0180](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RR0180.md)) ([#988](https://github.com/josefpihrt/roslynator/pull/988). ## [4.1.2] - 2022-10-31 From 48d8baef864a219a9e55b95cfba25d2de38d42cc Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 13 Nov 2022 20:26:11 +0100 Subject: [PATCH 3/3] update --- .../AddUsingStaticDirectiveRefactoring.cs | 13 +++++++-- .../RR0014AddUsingStaticDirectiveTests.cs | 27 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs b/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs index fa76e026ce..3c3528b168 100644 --- a/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs +++ b/src/Refactorings/CSharp/Refactorings/AddUsingStaticDirectiveRefactoring.cs @@ -31,14 +31,23 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Me SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); - var typeSymbol = semanticModel.GetSymbol(memberAccess.Expression, context.CancellationToken) as INamedTypeSymbol; + if (semanticModel.GetSymbol(memberAccess.Expression, context.CancellationToken) is not INamedTypeSymbol typeSymbol) + { + return; + } - if (typeSymbol?.TypeKind != TypeKind.Class) + if (typeSymbol.TypeKind != TypeKind.Class + && typeSymbol.TypeKind != TypeKind.Struct) + { return; + } if (!typeSymbol.DeclaredAccessibility.Is(Accessibility.Public, Accessibility.Internal)) return; + if (semanticModel.GetSymbol(memberAccess.Name, context.CancellationToken)?.IsStatic != true) + return; + if (CSharpUtility.IsStaticClassInScope(memberAccess, typeSymbol, semanticModel, context.CancellationToken)) return; diff --git a/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs b/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs index 6cd4455135..2dd0f88f9c 100644 --- a/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs +++ b/src/Tests/Refactorings.Tests/RR0014AddUsingStaticDirectiveTests.cs @@ -130,6 +130,33 @@ void M() var x = CurrentCulture.GetHashCode(); } } +", equivalenceKey: EquivalenceKey.Create(RefactoringId)); + } + + [Fact, Trait(Traits.Refactoring, RefactoringIdentifiers.AddUsingStaticDirective)] + public async Task Test_Struct() + { + await VerifyRefactoringAsync(@" +using System; + +class C +{ + void M() + { + var x = [|TimeSpan|].Zero; + } +} +", @" +using System; +using static System.TimeSpan; + +class C +{ + void M() + { + var x = Zero; + } +} ", equivalenceKey: EquivalenceKey.Create(RefactoringId)); } }