From a3aba2c02540839ac7c36d589f4e46ed84cd3b7d Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Mon, 15 Aug 2022 14:32:13 +0200 Subject: [PATCH 1/3] Prefix identifier with '@' if necessary --- ...tchingInsteadOfIsAndCastCodeFixProvider.cs | 3 + ...ePatternMatchingInsteadOfIsAndCastTests.cs | 477 ++++++++++++++++-- 2 files changed, 430 insertions(+), 50 deletions(-) diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs index 674d8770d5..4ad9492259 100644 --- a/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs @@ -113,6 +113,9 @@ private static (IsPatternExpressionSyntax isPatternExpression, TNode newNode) Ge name = NameGenerator.Default.EnsureUniqueLocalName(name, semanticModel, node.SpanStart, cancellationToken: cancellationToken) ?? DefaultNames.Variable; + if (SyntaxFacts.GetKeywordKind(name) != SyntaxKind.None) + name = "@" + name; + IsPatternExpressionSyntax isPatternExpression = IsPatternExpression( isInfo.Expression, DeclarationPattern( diff --git a/src/Tests/Analyzers.Tests/RCS1220UsePatternMatchingInsteadOfIsAndCastTests.cs b/src/Tests/Analyzers.Tests/RCS1220UsePatternMatchingInsteadOfIsAndCastTests.cs index a5f60c1ed8..132d9cce95 100644 --- a/src/Tests/Analyzers.Tests/RCS1220UsePatternMatchingInsteadOfIsAndCastTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1220UsePatternMatchingInsteadOfIsAndCastTests.cs @@ -19,27 +19,134 @@ public async Task Test_LogicalAndExpression() await VerifyDiagnosticAndFixAsync(@" class C { - private readonly object _f = false; - public void M() { + object x = null; string s = null; - object x = null; if ([|x is string && ((string)x) == s|]) { } + } +} +", @" +class C +{ + public void M() + { + object x = null; + string s = null; + + + if (x is string x2 && (x2) == s) { } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_LogicalAndExpression2() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + public void M() + { + object x = null; if ([|x is string && ((string)x).Equals((string)x)|]) { } + } +} +", @" +class C +{ + public void M() + { + object x = null; + + if (x is string x2 && (x2).Equals(x2)) { } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_LogicalAndExpression3() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; if ([|_f is string && (string)(_f) == s|]) { } + } +} +", @" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; + + if (_f is string x2 && x2 == s) { } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_LogicalAndExpression4() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; if ([|this._f is string && (string)this._f == s|]) { } + } +} +", @" +class C +{ + private readonly object _f = false; - if ([|_f is string && (string)(this._f) == s|]) { } + public void M() + { + object x = null; + string s = null; - if ([|this._f is string && (string)_f == s|]) { } + if (this._f is string x2 && x2 == s) { } + } +} +"); + } - if ([|this._f is string && ((string)_f).Equals((string)this._f)|]) { } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_LogicalAndExpression5() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; + + if ([|_f is string && (string)(this._f) == s|]) { } } } ", @" @@ -49,75 +156,233 @@ class C public void M() { + object x = null; string s = null; + if (_f is string x2 && x2 == s) { } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_LogicalAndExpression6() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { object x = null; + string s = null; - if (x is string x2 && (x2) == s) { } + if ([|this._f is string && (string)_f == s|]) { } + } +} +", @" +class C +{ + private readonly object _f = false; - if (x is string x3 && (x3).Equals(x3)) { } + public void M() + { + object x = null; + string s = null; + + if (this._f is string x2 && x2 == s) { } + } +} +"); + } - if (_f is string x4 && x4 == s) { } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_LogicalAndExpression7() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; - if (this._f is string x5 && x5 == s) { } + public void M() + { + object x = null; - if (_f is string x6 && x6 == s) { } + if ([|this._f is string && ((string)_f).Equals((string)this._f)|]) { } + } +} +", @" +class C +{ + private readonly object _f = false; - if (this._f is string x7 && x7 == s) { } + public void M() + { + object x = null; - if (this._f is string x8 && (x8).Equals(x8)) { } + if (this._f is string x2 && (x2).Equals(x2)) { } } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] - public async Task Test_IfStatement() + public async Task Test_LogicalAndExpression_Enum() { await VerifyDiagnosticAndFixAsync(@" +using System; + class C { private readonly object _f = false; public void M() { - string s = null; + Enum e = null; + object x = null; + + if ([|this._f is Enum && ((Enum)_f).Equals((Enum)this._f)|]) { } + } +} +", @" +using System; + +class C +{ + private readonly object _f = false; + + public void M() + { + Enum e = null; + object x = null; + + if (this._f is Enum @enum && (@enum).Equals(@enum)) { } + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement1() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + public void M() + { object x = null; + string s = null; if ([|x is string|]) { if (((string)x) == s) { } } + } +} +", @" +class C +{ + public void M() + { + object x = null; + string s = null; + + if (x is string x2) + { + if ((x2) == s) { } + } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement2() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + public void M() + { + object x = null; if ([|x is string|]) { if (((string)x).Equals((string)x)) { } } + } +} +", @" +class C +{ + public void M() + { + object x = null; - if ([|_f is string|]) + if (x is string x2) { - if ((string)_f == s) { } + if ((x2).Equals(x2)) { } + } + } +} +"); } - if ([|this._f is string|]) + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement3() { - if ((string)this._f == s) { } - } + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; if ([|_f is string|]) { - if ((string)this._f == s) { } + if ((string)_f == s) { } } + } +} +", @" +class C +{ + private readonly object _f = false; - if ([|this._f is string|]) + public void M() + { + object x = null; + string s = null; + + if (_f is string x2) { - if ((string)_f == s) { } + if (x2 == s) { } + } + } +} +"); } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement4() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; + if ([|this._f is string|]) { - if (((string)_f).Equals((string)this._f)) { } + if ((string)this._f == s) { } } } } @@ -128,43 +393,126 @@ class C public void M() { + object x = null; string s = null; + if (this._f is string x2) + { + if (x2 == s) { } + } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement5() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { object x = null; + string s = null; - if (x is string x2) + if ([|_f is string|]) { - if ((x2) == s) { } + if ((string)this._f == s) { } } + } +} +", @" +class C +{ + private readonly object _f = false; - if (x is string x3) + public void M() + { + object x = null; + string s = null; + + if (_f is string x2) { - if ((x3).Equals(x3)) { } + if (x2 == s) { } + } + } +} +"); } - if (_f is string x4) + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement6() { - if (x4 == s) { } - } + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; + string s = null; - if (this._f is string x5) + + if ([|this._f is string|]) { - if (x5 == s) { } + if ((string)_f == s) { } } + } +} +", @" +class C +{ + private readonly object _f = false; - if (_f is string x6) + public void M() + { + object x = null; + string s = null; + + + if (this._f is string x2) { - if (x6 == s) { } + if (x2 == s) { } } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task Test_IfStatement7() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + private readonly object _f = false; - if (this._f is string x7) + public void M() + { + object x = null; + + if ([|this._f is string|]) { - if (x7 == s) { } + if (((string)_f).Equals((string)this._f)) { } } + } +} +", @" +class C +{ + private readonly object _f = false; + + public void M() + { + object x = null; - if (this._f is string x8) + if (this._f is string x2) { - if ((x8).Equals(x8)) { } + if ((x2).Equals(x2)) { } } } } @@ -172,7 +520,7 @@ public void M() } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] - public async Task Test_IfStatement2() + public async Task Test_IfStatement8() { await VerifyDiagnosticAndFixAsync(@" using System.Dynamic; @@ -206,18 +554,52 @@ bool M(dynamic @object, string name) } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] - public async Task TestNoDiagnostic_LogicalAndExpression() + public async Task Test_IfStatement_Enum() { - await VerifyNoDiagnosticAsync(@" + await VerifyDiagnosticAndFixAsync(@" +using System; + class C { private readonly object _f = false; public void M() { - string s = null; + if ([|this._f is Enum|]) + { + if (((Enum)_f).Equals((Enum)this._f)) { } + } + } +} +", @" +using System; + +class C +{ + private readonly object _f = false; + + public void M() + { + if (this._f is Enum @enum) + { + if ((@enum).Equals(@enum)) { } + } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UsePatternMatchingInsteadOfIsAndCast)] + public async Task TestNoDiagnostic_LogicalAndExpression() + { + await VerifyNoDiagnosticAsync(@" +class C +{ + public void M() + { object x = null; object x2 = null; + string s = null; if (x is string && ReferenceEquals(((string)x), x)) { } @@ -235,13 +617,11 @@ public async Task TestNoDiagnostic_IfStatement() await VerifyNoDiagnosticAsync(@" class C { - private readonly object _f = false; - public void M() { - string s = null; object x = null; object x2 = null; + string s = null; if (x is string) { @@ -309,13 +689,10 @@ public async Task TestNoDiagnostic_LanguageVersion() await VerifyNoDiagnosticAsync(@" class C { - private readonly object _f = false; - public void M() { - string s = null; - object x = null; + string s = null; if (x is string && ((string)x) == s) { } } From 8039dfdd48bffac4aff7136fc49d8edb7ee55ba0 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Mon, 15 Aug 2022 14:34:57 +0200 Subject: [PATCH 2/3] Update changelog --- ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog.md b/ChangeLog.md index 0b945cb865..256a8add26 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Do not simplify default expression if it would change semantics ([RCS1244](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1244.md)) ([#939](https://github.com/josefpihrt/roslynator/pull/939). - 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). ----- From c4bedaad8e881b4af6321bff057dabfa613d88a1 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Mon, 22 Aug 2022 15:14:29 +0200 Subject: [PATCH 3/3] Add CSharpNameGenerator --- ...tchingInsteadOfIsAndCastCodeFixProvider.cs | 5 +- src/CSharp/CSharp/CSharpNameGenerator.cs | 161 ++++++++++++++++++ 2 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 src/CSharp/CSharp/CSharpNameGenerator.cs diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs index 4ad9492259..bb95099523 100644 --- a/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/UsePatternMatchingInsteadOfIsAndCastCodeFixProvider.cs @@ -111,10 +111,7 @@ private static (IsPatternExpressionSyntax isPatternExpression, TNode newNode) Ge string name = NameGenerator.CreateName(typeSymbol, firstCharToLower: true) ?? DefaultNames.Variable; - name = NameGenerator.Default.EnsureUniqueLocalName(name, semanticModel, node.SpanStart, cancellationToken: cancellationToken) ?? DefaultNames.Variable; - - if (SyntaxFacts.GetKeywordKind(name) != SyntaxKind.None) - name = "@" + name; + name = CSharpNameGenerator.Default.EnsureUniqueLocalName(name, semanticModel, node.SpanStart, cancellationToken: cancellationToken) ?? DefaultNames.Variable; IsPatternExpressionSyntax isPatternExpression = IsPatternExpression( isInfo.Expression, diff --git a/src/CSharp/CSharp/CSharpNameGenerator.cs b/src/CSharp/CSharp/CSharpNameGenerator.cs new file mode 100644 index 0000000000..939bc4b87b --- /dev/null +++ b/src/CSharp/CSharp/CSharpNameGenerator.cs @@ -0,0 +1,161 @@ +// 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.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace Roslynator.CSharp +{ + //TODO: make public + /// + /// Provides methods to obtain an unique C# identifier. + /// + internal class CSharpNameGenerator + { + private static readonly NameGenerator _generator = new NumberSuffixCSharpNameGenerator(); + + /// + /// Default implementation of that adds number suffix to ensure uniqueness. + /// + public static CSharpNameGenerator Default { get; } = new(); + + /// + /// Returns a name that will be unique at the specified position. + /// + /// + /// + /// + public string EnsureUniqueName(string baseName, SemanticModel semanticModel, int position) + { + return _generator.EnsureUniqueName(baseName, semanticModel, position); + } + + /// + /// Returns unique enum member name for a specified enum type. + /// + /// + /// + public string EnsureUniqueEnumMemberName(string baseName, INamedTypeSymbol enumType) + { + return _generator.EnsureUniqueEnumMemberName(baseName, enumType, isCaseSensitive: true); + } + + /// + /// Return a local name that will be unique at the specified position. + /// + /// + /// + /// + /// + public string EnsureUniqueLocalName( + string baseName, + SemanticModel semanticModel, + int position, + CancellationToken cancellationToken = default) + { + return _generator.EnsureUniqueLocalName(baseName, semanticModel, position, isCaseSensitive: true, cancellationToken); + } + + /// + /// Return a local names that will be unique at the specified position. + /// + /// + /// + /// + /// + /// + public ImmutableArray EnsureUniqueLocalNames( + string baseName, + SemanticModel semanticModel, + int position, + int count, + CancellationToken cancellationToken = default) + { + return _generator.EnsureUniqueLocalNames(baseName, semanticModel, position, count, isCaseSensitive: true, cancellationToken); + } + + /// + /// Return a parameter name that will be unique at the specified position. + /// + /// + /// + /// + /// + public string EnsureUniqueParameterName( + string baseName, + ISymbol containingSymbol, + SemanticModel semanticModel, + CancellationToken cancellationToken = default) + { + return _generator.EnsureUniqueParameterName(baseName, containingSymbol, semanticModel, isCaseSensitive: true, cancellationToken); + } + + internal string CreateUniqueLocalName( + ITypeSymbol typeSymbol, + SemanticModel semanticModel, + int position, + CancellationToken cancellationToken = default) + { + return _generator.CreateUniqueLocalName(typeSymbol, semanticModel, position, isCaseSensitive: true, cancellationToken); + } + + internal string CreateUniqueLocalName( + ITypeSymbol typeSymbol, + string oldName, + SemanticModel semanticModel, + int position, + CancellationToken cancellationToken = default) + { + return _generator.CreateUniqueLocalName(typeSymbol, oldName, semanticModel, position, isCaseSensitive: true, cancellationToken); + } + + internal string CreateUniqueParameterName( + string oldName, + IParameterSymbol parameterSymbol, + SemanticModel semanticModel, + CancellationToken cancellationToken = default) + { + return _generator.CreateUniqueParameterName(oldName, parameterSymbol, semanticModel, isCaseSensitive: true, cancellationToken); + } + + private class NumberSuffixCSharpNameGenerator : NameGenerator + { + public override string EnsureUniqueName(string baseName, IEnumerable reservedNames, bool isCaseSensitive = true) + { + int suffix = 1; + + string name = baseName; + + while (!IsUniqueName(name, reservedNames, isCaseSensitive)) + { + suffix++; + name = baseName + suffix.ToString(); + } + + return CheckKeyword(name); + } + + public override string EnsureUniqueName(string baseName, ImmutableArray symbols, bool isCaseSensitive = true) + { + int suffix = 1; + + string name = baseName; + + while (!IsUniqueName(name, symbols, isCaseSensitive)) + { + suffix++; + name = baseName + suffix.ToString(); + } + + return CheckKeyword(name); + } + + private string CheckKeyword(string name) + { + return (SyntaxFacts.GetKeywordKind(name) != SyntaxKind.None) ? "@" + name : name; + } + } + } +}