diff --git a/ChangeLog.md b/ChangeLog.md index 8bb10fd1ad..56ba8c41c6 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Recognize more shapes of IAsyncEnumerable as being Async ([RCS1047](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1047.md)) ([#1084](https://github.com/josefpihrt/roslynator/pull/1084)). - Fix [RCS1197](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1197.md) ([#1093](https://github.com/JosefPihrt/Roslynator/pull/1093)). - Fix [RCS1056](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1056.md) ([#1096](https://github.com/JosefPihrt/Roslynator/pull/1096)). +- Fix [RCS1216](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1216.md) ([#1094](https://github.com/JosefPihrt/Roslynator/pull/1094)). ## [4.3.0] - 2023-04-24 diff --git a/src/Analyzers/CSharp/Analysis/UnnecessaryUnsafeContextAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnnecessaryUnsafeContextAnalyzer.cs index ea87f8a483..c70060c6b3 100644 --- a/src/Analyzers/CSharp/Analysis/UnnecessaryUnsafeContextAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnnecessaryUnsafeContextAnalyzer.cs @@ -58,7 +58,7 @@ private static void AnalyzeUnsafeStatement(SyntaxNodeAnalysisContext context) if (!unsafeStatement.Block.Statements.Any()) return; - if (!ParentDeclarationsContainsUnsafeModifier(unsafeStatement)) + if (!AncestorContainsUnsafeModifier(unsafeStatement.Parent)) return; DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.UnnecessaryUnsafeContext, unsafeStatement.UnsafeKeyword); @@ -68,26 +68,7 @@ private static void AnalyzeLocalFunctionStatement(SyntaxNodeAnalysisContext cont { var localFunctionStatement = (LocalFunctionStatementSyntax)context.Node; - SyntaxTokenList modifiers = localFunctionStatement.Modifiers; - - int index = modifiers.IndexOf(SyntaxKind.UnsafeKeyword); - - if (index == -1) - return; - - SyntaxNode parent = localFunctionStatement.Parent; - - SyntaxDebug.Assert(parent.IsKind(SyntaxKind.Block), parent); - - if (parent is not BlockSyntax) - return; - - parent = parent.Parent; - - if (!ParentDeclarationsContainsUnsafeModifier(parent)) - return; - - DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.UnnecessaryUnsafeContext, modifiers[index]); + AnalyzeMemberDeclaration(context, localFunctionStatement, localFunctionStatement.Modifiers); } private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) @@ -176,7 +157,7 @@ private static void AnalyzeIndexerDeclaration(SyntaxNodeAnalysisContext context) private static void AnalyzeMemberDeclaration( SyntaxNodeAnalysisContext context, - MemberDeclarationSyntax memberDeclaration, + SyntaxNode node, SyntaxTokenList modifiers) { int index = modifiers.IndexOf(SyntaxKind.UnsafeKeyword); @@ -184,94 +165,37 @@ private static void AnalyzeMemberDeclaration( if (index == -1) return; - if (!ParentTypeDeclarationsContainsUnsafeModifier(memberDeclaration)) + if (!AncestorContainsUnsafeModifier(node.Parent)) return; DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.UnnecessaryUnsafeContext, modifiers[index]); } - private static bool ParentDeclarationsContainsUnsafeModifier(UnsafeStatementSyntax unsafeStatement) + private static bool AncestorContainsUnsafeModifier(SyntaxNode node) { - SyntaxNode parent = unsafeStatement.Parent; - - while (parent is not null) + while (node is not null) { - SyntaxKind kind = parent.Kind(); - - if (kind == SyntaxKind.UnsafeStatement) - { - return true; - } - else if (kind == SyntaxKind.LocalFunctionStatement) + switch (node) { - break; + case UnsafeStatementSyntax: + return true; + case MemberDeclarationSyntax memberDeclarationSyntax: + { + if (memberDeclarationSyntax.Modifiers.Contains(SyntaxKind.UnsafeKeyword)) + return true; + + break; + } + case LocalFunctionStatementSyntax localFunctionStatement: + { + if (localFunctionStatement.Modifiers.Contains(SyntaxKind.UnsafeKeyword)) + return true; + + break; + } } - if (parent is AccessorDeclarationSyntax) - { - parent = parent.Parent; - - if (parent is AccessorListSyntax) - parent = parent.Parent; - - break; - } - - if (parent is MemberDeclarationSyntax) - break; - - parent = parent.Parent; - } - - return ParentDeclarationsContainsUnsafeModifier(parent); - } - - private static bool ParentDeclarationsContainsUnsafeModifier(SyntaxNode node) - { - while (node.IsKind(SyntaxKind.LocalFunctionStatement)) - { - var localFunction = (LocalFunctionStatementSyntax)node; - - if (localFunction.Modifiers.Contains(SyntaxKind.UnsafeKeyword)) - return true; - node = node.Parent; - - SyntaxDebug.Assert(node.IsKind(SyntaxKind.Block), node); - - if (!node.IsKind(SyntaxKind.Block)) - break; - - node = node.Parent; - } - - SyntaxDebug.Assert(node is MemberDeclarationSyntax, node); - - if (node is MemberDeclarationSyntax memberDeclaration) - { - if (SyntaxInfo.ModifierListInfo(memberDeclaration).IsUnsafe) - return true; - - return ParentTypeDeclarationsContainsUnsafeModifier(memberDeclaration); - } - - return false; - } - - private static bool ParentTypeDeclarationsContainsUnsafeModifier(MemberDeclarationSyntax memberDeclaration) - { - SyntaxNode parent = memberDeclaration.Parent; - - while (parent.IsKind( - SyntaxKind.ClassDeclaration, - SyntaxKind.StructDeclaration, - SyntaxKind.RecordStructDeclaration, - SyntaxKind.InterfaceDeclaration)) - { - if (((TypeDeclarationSyntax)parent).Modifiers.Contains(SyntaxKind.UnsafeKeyword)) - return true; - - parent = parent.Parent; } return false; diff --git a/src/Tests/Analyzers.Tests/RCS1216UnnecessaryUnsafeContextTests.cs b/src/Tests/Analyzers.Tests/RCS1216UnnecessaryUnsafeContextTests.cs new file mode 100644 index 0000000000..25862738df --- /dev/null +++ b/src/Tests/Analyzers.Tests/RCS1216UnnecessaryUnsafeContextTests.cs @@ -0,0 +1,445 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Roslynator.CSharp.CodeFixes; +using Roslynator.Testing.CSharp; +using Xunit; + +namespace Roslynator.CSharp.Analysis.Tests; + +public class RCS1216UnnecessaryUnsafeContextTests : AbstractCSharpDiagnosticVerifier +{ + public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.UnnecessaryUnsafeContext; + + [Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Class() + { + await VerifyDiagnosticAndFixAsync(@" + unsafe class C + { + void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + unsafe class C + { + void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Interface() + { + await VerifyDiagnosticAndFixAsync(@" + unsafe interface C + { + void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + unsafe interface C + { + void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Record() + { + await VerifyDiagnosticAndFixAsync(@" + unsafe record C + { + void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + unsafe record C + { + void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Struct() + { + await VerifyDiagnosticAndFixAsync(@" + unsafe struct C + { + void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + unsafe struct C + { + void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_RecordStruct() + { + await VerifyDiagnosticAndFixAsync(@" + unsafe record struct C + { + void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + unsafe record struct C + { + void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Method() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + unsafe void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + class C + { + unsafe void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Constructor() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + unsafe C() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + class C + { + unsafe C() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_StaticMember() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + unsafe static void M() + { + [|unsafe|] + { + var x = 1; + } + } + } +", @" + class C + { + unsafe static void M() + { + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_UnsafeLocalFunction() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + unsafe void M() + { + for(int y = 0; y < 10; y ++) + { + [|unsafe|] void M2() + { + var x = 1; + } + } + } + } +", @" + class C + { + unsafe void M() + { + for(int y = 0; y < 10; y ++) + { + void M2() + { + var x = 1; + } + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_UnsafeBlock() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + void M() + { + unsafe + { + [|unsafe|] + { + var x = 1; + } + } + } + } +", @" + class C + { + void M() + { + unsafe + { + { + var x = 1; + } + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Property() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + unsafe string X + { + get + { + [|unsafe|] + { + var x = 1; + } + return ""1""; + } + } + } +", @" + class C + { + unsafe string X + { + get + { + { + var x = 1; + } + return ""1""; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Operator() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + public unsafe static C operator +(C c1, C c2) + { + [|unsafe|] + { + var x = 1; + } + return c1; + } + } +", @" + class C + { + public unsafe static C operator +(C c1, C c2) + { + { + var x = 1; + } + return c1; + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Indexer() + { + await VerifyDiagnosticAndFixAsync(@" + class C + { + unsafe string this[int i] + { + get + { + [|unsafe|] + { + var x = 1; + } + return ""1""; + } + } + } +", @" + class C + { + unsafe string this[int i] + { + get + { + { + var x = 1; + } + return ""1""; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_Local() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M() + { + unsafe void Local() + { + [|unsafe|] + { + var x = 1; + } + } + } +} +", @" +class C +{ + void M() + { + unsafe void Local() + { + { + var x = 1; + } + } + } +} +", options: Options.WithAllowUnsafe(true)); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryUnsafeContext)] + public async Task Test_NoDiagnostic_UnwrappedUnsafeBlock() + { + await VerifyNoDiagnosticAsync(@" + class C + { + void M() + { + unsafe + { + var x = 1; + } + } + } +", options: Options.WithAllowUnsafe(true)); + } +}