From e3b983547cc07fe28c8386ca20d26a99e5238f45 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Sun, 6 Aug 2023 20:35:21 +0000 Subject: [PATCH 01/10] fix and test --- .../CSharp/SyntaxLogicalInverter.cs | 16 ++-- .../RCS1208ReduceIfNestingTests.cs | 76 +++++++++++++++++++ 2 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index 78a1ff0d14..86c50bd585 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -128,13 +128,15 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.IsExpression: { var isExpression = (BinaryExpressionSyntax)expression; - - return IsPatternExpression( - isExpression.Left, - isExpression.OperatorToken.WithTrailingTrivia(Space), - UnaryPattern( - Token(SyntaxKind.NotKeyword).WithTrailingTrivia(isExpression.OperatorToken.TrailingTrivia), - TypePattern((TypeSyntax)isExpression.Right))); + return (Options.UseNotPattern) + ? IsPatternExpression( + isExpression.Left, + isExpression.OperatorToken.WithTrailingTrivia(Space), + UnaryPattern( + Token(SyntaxKind.NotKeyword) + .WithTrailingTrivia(isExpression.OperatorToken.TrailingTrivia), + TypePattern((TypeSyntax)isExpression.Right))) + : DefaultInvert(expression); } case SyntaxKind.AsExpression: { diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index 54a9e8298e..d411f4fc17 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -614,6 +614,82 @@ void M2() { } } +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_WhenIsExpressionCsharp8() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(object o) + { + [|if|] (o is string) + { + M2(); + } + } + + void M2() + { + } +} +", @" +class C +{ + void M(object o) + { + if (!(o is string)) + { + return; + } + + M2(); + } + + void M2() + { + } +} +", options: WellKnownCSharpTestOptions.Default_CSharp8); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_WhenIsExpression() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M(object o) + { + [|if|] (o is string) + { + M2(); + } + } + + void M2() + { + } +} +", @" +class C +{ + void M(object o) + { + if (o is not string) + { + return; + } + + M2(); + } + + void M2() + { + } +} "); } } From b4ff0826cbde7be2cf4952fa85b63a8154590203 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Sun, 6 Aug 2023 20:37:58 +0000 Subject: [PATCH 02/10] change log --- ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index 1b69e018e2..73c6bf0ed4 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -33,7 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix [RCS0005](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0005) ([#1114](https://github.com/JosefPihrt/Roslynator/pull/1114)). - Fix [RCS1176](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1176.md) ([#1122](https://github.com/JosefPihrt/Roslynator/pull/1122), [#1140](https://github.com/JosefPihrt/Roslynator/pull/1140)). - Fix [RCS1085](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1085.md) ([#1120](https://github.com/josefpihrt/roslynator/pull/1120)). -- Fix [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md) ([#1119](https://github.com/JosefPihrt/Roslynator/pull/1119)). +- Fix [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md) ([#1119](https://github.com/JosefPihrt/Roslynator/pull/1119),[#1153](https://github.com/JosefPihrt/Roslynator/pull/1153)). - [CLI] Fix member full declaration in generated documentation (command `generate-doc`) ([#1130](https://github.com/josefpihrt/roslynator/pull/1130)). - Append `?` to nullable reference types. - Fix [RCS1179](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1179.md) ([#1129](https://github.com/JosefPihrt/Roslynator/pull/1129)). From a6e7c54098a56638b7852bd7c0a0246f4f40e834 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 9 Aug 2023 18:05:49 +0000 Subject: [PATCH 03/10] CR --- ChangeLog.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ChangeLog.md b/ChangeLog.md index 19bd5cba49..98d230319a 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add SECURITY.md ([#1147](https://github.com/josefpihrt/roslynator/pull/1147)) +### Fixed + +- Fix [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md) ([#1153](https://github.com/JosefPihrt/Roslynator/pull/1153)) + ## [4.4.0] - 2023-08-01 ### Added @@ -40,7 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix [RCS0005](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0005) ([#1114](https://github.com/JosefPihrt/Roslynator/pull/1114)). - Fix [RCS1176](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1176.md) ([#1122](https://github.com/JosefPihrt/Roslynator/pull/1122), [#1140](https://github.com/JosefPihrt/Roslynator/pull/1140)). - Fix [RCS1085](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1085.md) ([#1120](https://github.com/josefpihrt/roslynator/pull/1120)). -- Fix [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md) ([#1119](https://github.com/JosefPihrt/Roslynator/pull/1119),[#1153](https://github.com/JosefPihrt/Roslynator/pull/1153)). +- Fix [RCS1208](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1208.md) ([#1119](https://github.com/JosefPihrt/Roslynator/pull/1119)). - [CLI] Fix member full declaration in generated documentation (command `generate-doc`) ([#1130](https://github.com/josefpihrt/roslynator/pull/1130)). - Append `?` to nullable reference types. - Fix [RCS1179](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1179.md) ([#1129](https://github.com/JosefPihrt/Roslynator/pull/1129)). From a394a37ce3178ea68448f1a9e7481132ca26e85c Mon Sep 17 00:00:00 2001 From: James Date: Wed, 9 Aug 2023 18:19:04 +0000 Subject: [PATCH 04/10] Fix the is to is not inconsitent behaviour bug --- .../CSharp/SyntaxLogicalInverter.cs | 4 +- .../RCS1208ReduceIfNestingTests.cs | 51 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index 2ae5e528ba..2697dfb9c9 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -128,6 +128,8 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.IsExpression: { var isExpression = (BinaryExpressionSyntax)expression; + string fullyQualifiedName = semanticModel.GetSymbolInfo(isExpression.Right, cancellationToken).Symbol! + .ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); return (Options.UseNotPattern) ? IsPatternExpression( isExpression.Left, @@ -135,7 +137,7 @@ private ExpressionSyntax LogicallyInvertImpl( UnaryPattern( Token(SyntaxKind.NotKeyword) .WithTrailingTrivia(isExpression.OperatorToken.TrailingTrivia), - TypePattern((TypeSyntax)isExpression.Right))) + TypePattern(ParseTypeName(fullyQualifiedName)))) : DefaultInvert(expression); } case SyntaxKind.AsExpression: diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index d411f4fc17..7590598146 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -50,6 +50,56 @@ void M2() "); } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] + public async Task Test_IsWhenIsNotIsInvalid() + { + await VerifyDiagnosticAndFixAsync(@" +class X +{ +} + +class C +{ + public X X { get; set; } + + C(object o) + { + [|if|] (o is X) + { + M2(); + } + } + + void M2() + { + } +} +", @" +class X +{ +} + +class C +{ + public X X { get; set; } + + C(object o) + { + if (o is not global::X) + { + return; + } + + M2(); + } + + void M2() + { + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.ReduceIfNesting)] public async Task Test_WhenParentIsConversionOperator() { @@ -693,3 +743,4 @@ void M2() "); } } + From 7a16721c853806e75f7dff255f41c8f120529925 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Wed, 9 Aug 2023 19:31:27 +0100 Subject: [PATCH 05/10] Update RCS1208ReduceIfNestingTests.cs --- src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs index 7590598146..a6c5a1d3bd 100644 --- a/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1208ReduceIfNestingTests.cs @@ -743,4 +743,3 @@ void M2() "); } } - From 3a27d6673018765c92015abbe9489954ad7bbf05 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 10 Aug 2023 20:02:09 +0000 Subject: [PATCH 06/10] CR comments --- src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index 2697dfb9c9..e0c0f48316 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -128,8 +128,7 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.IsExpression: { var isExpression = (BinaryExpressionSyntax)expression; - string fullyQualifiedName = semanticModel.GetSymbolInfo(isExpression.Right, cancellationToken).Symbol! - .ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); + ITypeSymbol rightTypeSymbol = (ITypeSymbol)semanticModel.GetSymbolInfo(isExpression.Right, cancellationToken).Symbol!; return (Options.UseNotPattern) ? IsPatternExpression( isExpression.Left, @@ -137,7 +136,7 @@ private ExpressionSyntax LogicallyInvertImpl( UnaryPattern( Token(SyntaxKind.NotKeyword) .WithTrailingTrivia(isExpression.OperatorToken.TrailingTrivia), - TypePattern(ParseTypeName(fullyQualifiedName)))) + TypePattern(rightTypeSymbol.ToTypeSyntax(SymbolDisplayFormat.FullyQualifiedFormat)))) : DefaultInvert(expression); } case SyntaxKind.AsExpression: From e11e614e12bd6651afc7ff719b9ccb3361222752 Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Thu, 10 Aug 2023 21:08:39 +0100 Subject: [PATCH 07/10] Update SyntaxLogicalInverter.cs --- src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index e0c0f48316..cf703c5bd3 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -128,7 +128,7 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.IsExpression: { var isExpression = (BinaryExpressionSyntax)expression; - ITypeSymbol rightTypeSymbol = (ITypeSymbol)semanticModel.GetSymbolInfo(isExpression.Right, cancellationToken).Symbol!; + var rightTypeSymbol = (ITypeSymbol)semanticModel.GetSymbolInfo(isExpression.Right, cancellationToken).Symbol!; return (Options.UseNotPattern) ? IsPatternExpression( isExpression.Left, From 4b9505fea2249b93e84a7535c9e4d2145c33e79c Mon Sep 17 00:00:00 2001 From: James Hargreaves Date: Fri, 11 Aug 2023 19:41:43 +0100 Subject: [PATCH 08/10] Update SyntaxLogicalInverter.cs --- src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index cf703c5bd3..abc1b0b5d2 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -128,7 +128,7 @@ private ExpressionSyntax LogicallyInvertImpl( case SyntaxKind.IsExpression: { var isExpression = (BinaryExpressionSyntax)expression; - var rightTypeSymbol = (ITypeSymbol)semanticModel.GetSymbolInfo(isExpression.Right, cancellationToken).Symbol!; + var rightTypeSymbol = (ITypeSymbol)semanticModel.GetSymbol(isExpression.Right, cancellationToken)!; return (Options.UseNotPattern) ? IsPatternExpression( isExpression.Left, From 0cf9d4b2b8d1a1ee4364af8e97cda5441609ac14 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 15 Aug 2023 20:29:03 +0000 Subject: [PATCH 09/10] CR comments --- .../CSharp/SyntaxLogicalInverter.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index abc1b0b5d2..e7e4ad8dc7 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -129,6 +129,16 @@ private ExpressionSyntax LogicallyInvertImpl( { var isExpression = (BinaryExpressionSyntax)expression; var rightTypeSymbol = (ITypeSymbol)semanticModel.GetSymbol(isExpression.Right, cancellationToken)!; + + TypeSyntax type = rightTypeSymbol.ToTypeSyntax(); + + if (SymbolEqualityComparer.Default.Equals( + semanticModel.GetSpeculativeSymbolInfo(isExpression.SpanStart, isExpression.Right, SpeculativeBindingOption.BindAsExpression).Symbol, + semanticModel.GetSpeculativeSymbolInfo(isExpression.SpanStart, isExpression.Right, SpeculativeBindingOption.BindAsTypeOrNamespace).Symbol)) + { + type = type.WithSimplifierAnnotation(); + } + return (Options.UseNotPattern) ? IsPatternExpression( isExpression.Left, @@ -136,7 +146,7 @@ private ExpressionSyntax LogicallyInvertImpl( UnaryPattern( Token(SyntaxKind.NotKeyword) .WithTrailingTrivia(isExpression.OperatorToken.TrailingTrivia), - TypePattern(rightTypeSymbol.ToTypeSyntax(SymbolDisplayFormat.FullyQualifiedFormat)))) + TypePattern(type))) : DefaultInvert(expression); } case SyntaxKind.AsExpression: From 92a76035b3bee40ed03b95de33039668a455bce8 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Wed, 16 Aug 2023 23:50:18 +0200 Subject: [PATCH 10/10] Update src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs --- src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs index e7e4ad8dc7..e46506796d 100644 --- a/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs +++ b/src/CSharp.Workspaces/CSharp/SyntaxLogicalInverter.cs @@ -133,8 +133,8 @@ private ExpressionSyntax LogicallyInvertImpl( TypeSyntax type = rightTypeSymbol.ToTypeSyntax(); if (SymbolEqualityComparer.Default.Equals( - semanticModel.GetSpeculativeSymbolInfo(isExpression.SpanStart, isExpression.Right, SpeculativeBindingOption.BindAsExpression).Symbol, - semanticModel.GetSpeculativeSymbolInfo(isExpression.SpanStart, isExpression.Right, SpeculativeBindingOption.BindAsTypeOrNamespace).Symbol)) + semanticModel.GetSpeculativeSymbolInfo(isExpression.SpanStart, isExpression.Right, SpeculativeBindingOption.BindAsExpression).Symbol, + semanticModel.GetSpeculativeSymbolInfo(isExpression.SpanStart, isExpression.Right, SpeculativeBindingOption.BindAsTypeOrNamespace).Symbol)) { type = type.WithSimplifierAnnotation(); }