From 7371844e928bec6620d241e58170a0117dd61a73 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 19 Jul 2021 12:47:33 -0700 Subject: [PATCH 1/4] Fix 'add accessibility' with file scoped namespaces --- ...ccessibilityModifiersDiagnosticAnalyzer.cs | 18 ++++------------- .../AddAccessibilityModifiersTests.cs | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs index b47871a2d1323..0a244181d2fbe 100644 --- a/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs @@ -2,16 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Collections.Immutable; using Microsoft.CodeAnalysis.AddAccessibilityModifiers; using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.CSharp.Extensions; -using Microsoft.CodeAnalysis.CSharp.Formatting; using Microsoft.CodeAnalysis.CSharp.LanguageServices; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.CSharp.AddAccessibilityModifiers { @@ -19,10 +17,6 @@ namespace Microsoft.CodeAnalysis.CSharp.AddAccessibilityModifiers internal class CSharpAddAccessibilityModifiersDiagnosticAnalyzer : AbstractAddAccessibilityModifiersDiagnosticAnalyzer { - public CSharpAddAccessibilityModifiersDiagnosticAnalyzer() - { - } - private static CSharpSyntaxFacts SyntaxFacts => CSharpSyntaxFacts.Instance; protected override void ProcessCompilationUnit( @@ -38,22 +32,18 @@ private void ProcessMembers( SyntaxList members) { foreach (var memberDeclaration in members) - { ProcessMemberDeclaration(context, option, memberDeclaration); - } } private void ProcessMemberDeclaration( SyntaxTreeAnalysisContext context, CodeStyleOption2 option, MemberDeclarationSyntax member) { - if (member.IsKind(SyntaxKind.NamespaceDeclaration, out NamespaceDeclarationSyntax namespaceDeclaration)) - { + if (member is BaseNamespaceDeclarationSyntax namespaceDeclaration) ProcessMembers(context, option, namespaceDeclaration.Members); - } // If we have a class or struct, recurse inwards. - if (member.IsKind(SyntaxKind.ClassDeclaration, out TypeDeclarationSyntax typeDeclaration) || + if (member.IsKind(SyntaxKind.ClassDeclaration, out TypeDeclarationSyntax? typeDeclaration) || member.IsKind(SyntaxKind.StructDeclaration, out typeDeclaration) || member.IsKind(SyntaxKind.RecordDeclaration, out typeDeclaration) || member.IsKind(SyntaxKind.RecordStructDeclaration, out typeDeclaration)) @@ -99,7 +89,7 @@ private void ProcessMemberDeclaration( return; } - var parentKind = member.Parent.Kind(); + var parentKind = member.GetRequiredParent().Kind(); switch (parentKind) { // Check for default modifiers in namespace and outside of namespace diff --git a/src/Analyzers/CSharp/Tests/AddAccessibilityModifiers/AddAccessibilityModifiersTests.cs b/src/Analyzers/CSharp/Tests/AddAccessibilityModifiers/AddAccessibilityModifiersTests.cs index e99a8daa52c2b..6b21e2923d94b 100644 --- a/src/Analyzers/CSharp/Tests/AddAccessibilityModifiers/AddAccessibilityModifiersTests.cs +++ b/src/Analyzers/CSharp/Tests/AddAccessibilityModifiers/AddAccessibilityModifiersTests.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.AddAccessibilityModifiers; using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; using Microsoft.CodeAnalysis.Test.Utilities; @@ -528,5 +529,24 @@ public class Derived : TestClass } "); } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddAccessibilityModifiers)] + public async Task TestFileScopedNamespaces() + { + await new VerifyCS.Test + { + TestCode = @" +namespace Test; + +struct [|S1|] { } +", + FixedCode = @" +namespace Test; + +internal struct S1 { } +", + LanguageVersion = LanguageVersion.CSharp10, + }.RunAsync(); + } } } From 28e0edc823c47369ed26072422f027c88a5dbbfe Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 19 Jul 2021 13:10:55 -0700 Subject: [PATCH 2/4] Fix 'misplaced usings' with file scoped namespaces --- ...placedUsingDirectivesDiagnosticAnalyzer.cs | 10 +-- ...MisplacedUsingDirectivesCodeFixProvider.cs | 40 ++++----- ...acedUsingDirectivesCodeFixProviderTests.cs | 85 +++++++++++++++++-- 3 files changed, 97 insertions(+), 38 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/MisplacedUsingDirectives/MisplacedUsingDirectivesDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/MisplacedUsingDirectives/MisplacedUsingDirectivesDiagnosticAnalyzer.cs index a5f566f4ad5d5..95def5d42e20f 100644 --- a/src/Analyzers/CSharp/Analyzers/MisplacedUsingDirectives/MisplacedUsingDirectivesDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/MisplacedUsingDirectives/MisplacedUsingDirectivesDiagnosticAnalyzer.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -52,7 +50,7 @@ public override DiagnosticAnalyzerCategory GetAnalyzerCategory() protected override void InitializeWorker(AnalysisContext context) { - context.RegisterSyntaxNodeAction(AnalyzeNamespaceNode, SyntaxKind.NamespaceDeclaration); + context.RegisterSyntaxNodeAction(AnalyzeNamespaceNode, SyntaxKind.NamespaceDeclaration, SyntaxKind.FileScopedNamespaceDeclaration); context.RegisterSyntaxNodeAction(AnalyzeCompilationUnitNode, SyntaxKind.CompilationUnit); } @@ -60,11 +58,9 @@ private void AnalyzeNamespaceNode(SyntaxNodeAnalysisContext context) { var option = context.Options.GetOption(CSharpCodeStyleOptions.PreferredUsingDirectivePlacement, context.Node.SyntaxTree, context.CancellationToken); if (option.Value != AddImportPlacement.OutsideNamespace) - { return; - } - var namespaceDeclaration = (NamespaceDeclarationSyntax)context.Node; + var namespaceDeclaration = (BaseNamespaceDeclarationSyntax)context.Node; ReportDiagnostics(context, s_outsideDiagnosticDescriptor, namespaceDeclaration.Usings, option); } @@ -89,7 +85,7 @@ private static bool ShouldSuppressDiagnostic(CompilationUnitSyntax compilationUn // Suppress if there are nodes other than usings and namespaces in the // compilation unit (including ExternAlias). return compilationUnit.ChildNodes().Any( - t => !t.IsKind(SyntaxKind.UsingDirective, SyntaxKind.NamespaceDeclaration)); + t => !t.IsKind(SyntaxKind.UsingDirective, SyntaxKind.NamespaceDeclaration, SyntaxKind.FileScopedNamespaceDeclaration)); } private static void ReportDiagnostics( diff --git a/src/Analyzers/CSharp/CodeFixes/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProvider.cs index 0c4c0153d8bbc..97ef03e6d2c58 100644 --- a/src/Analyzers/CSharp/CodeFixes/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProvider.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Collections.Immutable; @@ -62,7 +60,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) var document = context.Document; var cancellationToken = context.CancellationToken; - var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var syntaxRoot = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var compilationUnit = (CompilationUnitSyntax)syntaxRoot; #if CODE_STYLE @@ -76,9 +74,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) // it manually. var (placement, preferPreservation) = DeterminePlacement(compilationUnit, options); if (preferPreservation) - { return; - } foreach (var diagnostic in context.Diagnostics) { @@ -90,7 +86,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) internal static async Task TransformDocumentIfRequiredAsync(Document document, CancellationToken cancellationToken) { - var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var syntaxRoot = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var compilationUnit = (CompilationUnitSyntax)syntaxRoot; #if CODE_STYLE @@ -114,10 +110,10 @@ private static async Task GetTransformedDocumentAsync( AddImportPlacement placement, CancellationToken cancellationToken) { - var syntaxFactsService = document.GetLanguageService(); + var syntaxFactsService = document.GetRequiredLanguageService(); // Expand usings so that they can be properly simplified after they are relocated. - var compilationUnitWithExpandedUsings = (CompilationUnitSyntax)await ExpandUsingDirectivesAsync(document, compilationUnit, cancellationToken).ConfigureAwait(false); + var compilationUnitWithExpandedUsings = await ExpandUsingDirectivesAsync(document, compilationUnit, cancellationToken).ConfigureAwait(false); // Remove the file header from the compilation unit so that we do not lose it when making changes to usings. var (compilationUnitWithoutHeader, fileHeader) = RemoveFileHeader(compilationUnitWithExpandedUsings, syntaxFactsService); @@ -138,11 +134,12 @@ private static async Task GetTransformedDocumentAsync( return await Simplifier.ReduceAsync(newDocument, Simplifier.Annotation, options, cancellationToken).ConfigureAwait(false); } - private static async Task ExpandUsingDirectivesAsync(Document document, CompilationUnitSyntax containerNode, CancellationToken cancellationToken) + private static async Task ExpandUsingDirectivesAsync(Document document, CompilationUnitSyntax containerNode, CancellationToken cancellationToken) { // Get all using directives so they can be expanded at one time. - var allUsingDirectives = containerNode.DescendantNodes( - node => node.IsKind(SyntaxKind.CompilationUnit, SyntaxKind.NamespaceDeclaration)).OfType(); + var allUsingDirectives = containerNode + .DescendantNodes(node => node is CompilationUnitSyntax or BaseNamespaceDeclarationSyntax) + .OfType(); // Create a map between the original node and the future expanded node. var expandUsingDirectiveTasks = allUsingDirectives.ToDictionary( @@ -153,7 +150,7 @@ private static async Task ExpandUsingDirectivesAsync(Document docume await Task.WhenAll(expandUsingDirectiveTasks.Values).ConfigureAwait(false); // Replace using directives with their expanded version. - return ((SyntaxNode)containerNode).ReplaceNodes( + return containerNode.ReplaceNodes( expandUsingDirectiveTasks.Keys, (node, _) => expandUsingDirectiveTasks[node].Result); } @@ -176,7 +173,7 @@ private static CompilationUnitSyntax MoveUsingsInsideNamespace(CompilationUnitSy var compilationUnitWithoutBlankLine = RemoveLeadingBlankLinesFromFirstMember(compilationUnitWithoutUsings); // Fix the leading trivia for the namespace declaration. - var namespaceDeclaration = (NamespaceDeclarationSyntax)compilationUnitWithoutBlankLine.Members[0]; + var namespaceDeclaration = (BaseNamespaceDeclarationSyntax)compilationUnitWithoutBlankLine.Members[0]; var namespaceDeclarationWithBlankLine = EnsureLeadingBlankLineBeforeFirstMember(namespaceDeclaration); // Update the namespace declaration with the usings from the compilation unit. @@ -189,7 +186,7 @@ private static CompilationUnitSyntax MoveUsingsInsideNamespace(CompilationUnitSy private static CompilationUnitSyntax MoveUsingsOutsideNamespaces(CompilationUnitSyntax compilationUnit, SyntaxAnnotation warningAnnotation) { - var namespaceDeclarations = compilationUnit.Members.OfType(); + var namespaceDeclarations = compilationUnit.Members.OfType(); var namespaceDeclarationMap = namespaceDeclarations.ToDictionary( namespaceDeclaration => namespaceDeclaration, namespaceDeclaration => RemoveUsingsFromNamespace(namespaceDeclaration)); @@ -220,10 +217,10 @@ private static CompilationUnitSyntax MoveUsingsOutsideNamespaces(CompilationUnit return compilationUnitWithSeparatorLine.ReplaceNode(firstMember, firstMember.WithPrependedLeadingTrivia(orphanedTrivia)); } - private static (NamespaceDeclarationSyntax namespaceWithoutUsings, IEnumerable usingsFromNamespace) RemoveUsingsFromNamespace( - NamespaceDeclarationSyntax usingContainer) + private static (BaseNamespaceDeclarationSyntax namespaceWithoutUsings, IEnumerable usingsFromNamespace) RemoveUsingsFromNamespace( + BaseNamespaceDeclarationSyntax usingContainer) { - var namespaceDeclarations = usingContainer.Members.OfType(); + var namespaceDeclarations = usingContainer.Members.OfType(); var namespaceDeclarationMap = namespaceDeclarations.ToDictionary( namespaceDeclaration => namespaceDeclaration, namespaceDeclaration => RemoveUsingsFromNamespace(namespaceDeclaration)); @@ -281,7 +278,7 @@ private static SyntaxList GetMembers(SyntaxNode node) => node switch { CompilationUnitSyntax compilationUnit => compilationUnit.Members, - NamespaceDeclarationSyntax namespaceDeclaration => namespaceDeclaration.Members, + BaseNamespaceDeclarationSyntax namespaceDeclaration => namespaceDeclaration.Members, _ => throw ExceptionUtilities.UnexpectedValue(node) }; @@ -357,9 +354,7 @@ private static (AddImportPlacement placement, bool preferPreservation) Determine var preferPreservation = codeStyleOption.Notification == NotificationOption2.None; if (preferPreservation || placement == AddImportPlacement.OutsideNamespace) - { return (placement, preferPreservation); - } // Determine if we can safely apply the InsideNamespace preference. @@ -382,8 +377,9 @@ private static (AddImportPlacement placement, bool preferPreservation) Determine private static bool HasOneNamespace(CompilationUnitSyntax compilationUnit) { // Find all the NamespaceDeclarations - var allNamespaces = compilationUnit.DescendantNodes( - node => node.IsKind(SyntaxKind.CompilationUnit, SyntaxKind.NamespaceDeclaration)).OfType(); + var allNamespaces = compilationUnit + .DescendantNodes(node => node is CompilationUnitSyntax or BaseNamespaceDeclarationSyntax) + .OfType(); // To determine if there are multiple namespaces we only need to look for at least two. return allNamespaces.Take(2).Count() == 1; diff --git a/src/Analyzers/CSharp/Tests/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProviderTests.cs b/src/Analyzers/CSharp/Tests/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProviderTests.cs index 6ef1cc604c401..e26c013a693f4 100644 --- a/src/Analyzers/CSharp/Tests/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProviderTests.cs +++ b/src/Analyzers/CSharp/Tests/MisplacedUsingDirectives/MisplacedUsingDirectivesCodeFixProviderTests.cs @@ -2,12 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Threading.Tasks; using Microsoft.CodeAnalysis.AddImports; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.CodeStyle; using Microsoft.CodeAnalysis.CSharp.MisplacedUsingDirectives; using Microsoft.CodeAnalysis.Diagnostics; @@ -76,7 +75,8 @@ private protected Task TestInRegularAndScriptAsync(string initialMarkup, string { CSharpCodeStyleOptions.PreferredUsingDirectivePlacement, preferredPlacementOption }, { GenerationOptions.PlaceSystemNamespaceFirst, placeSystemNamespaceFirst }, }; - return TestInRegularAndScriptAsync(initialMarkup, expectedMarkup, options: options); + return TestInRegularAndScriptAsync( + initialMarkup, expectedMarkup, options: options, parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp10)); } #region Test Preserve @@ -97,6 +97,18 @@ public Task WhenPreserve_UsingsInNamespace_ValidUsingStatements() return TestDiagnosticMissingAsync(testCode, OutsidePreferPreservationOption); } + [Fact] + public Task WhenPreserve_UsingsInNamespace_ValidUsingStatements_FileScopedNamespace() + { + var testCode = @"namespace TestNamespace; + +[|using System; +using System.Threading;|] +"; + + return TestDiagnosticMissingAsync(testCode, OutsidePreferPreservationOption); + } + /// /// Verifies that having using statements in the compilation unit will not produce any diagnostics, nor will /// having using statements inside a namespace. @@ -194,6 +206,18 @@ namespace TestNamespace return TestDiagnosticMissingAsync(testCode, OutsideNamespaceOption); } + [Fact] + public Task WhenOutsidePreferred_UsingsInCompilationUnit_ValidUsingStatements_FileScopedNamespace() + { + var testCode = @"[|using System; +using System.Threading;|] + +namespace TestNamespace; +"; + + return TestDiagnosticMissingAsync(testCode, OutsideNamespaceOption); + } + /// /// Verifies that having using statements in the compilation unit will not produce any diagnostics when there are type definition present. /// @@ -237,10 +261,27 @@ namespace TestNamespace return TestInRegularAndScriptAsync(testCode, fixedTestCode, OutsideNamespaceOption, placeSystemNamespaceFirst: true); } + [Fact] + public Task WhenOutsidePreferred_UsingsInNamespace_UsingsMoved_FileScopedNamespace() + { + var testCode = @"namespace TestNamespace; + +[|using System; +using System.Threading;|] +"; + var fixedTestCode = @" +{|Warning:using System;|} +{|Warning:using System.Threading;|} + +namespace TestNamespace; +"; + + return TestInRegularAndScriptAsync(testCode, fixedTestCode, OutsideNamespaceOption, placeSystemNamespaceFirst: true); + } + /// /// Verifies that simplified using statements in a namespace are expanded during the code fix operation. /// - /// A representing the asynchronous unit test. [Fact] public Task WhenOutsidePreferred_SimplifiedUsingInNamespace_UsingsMovedAndExpanded() { @@ -320,7 +361,6 @@ namespace System.MyExtension /// /// Verifies that having using statements in the compilation unit will not produce any diagnostics when there are attributes present. /// - /// A representing the asynchronous unit test. [Fact] public Task WhenOutsidePreferred_UsingsInNamespaceAndCompilationUnitWithAttributes_UsingsMoved() { @@ -351,7 +391,6 @@ namespace TestNamespace /// /// Verifies that the file header of a file is properly preserved when moving using statements out of a namespace. /// - /// A representing the asynchronous unit test. [Fact] public Task WhenOutsidePreferred_UsingsInNamespaceAndCompilationUnitHasFileHeader_UsingsMovedAndHeaderPreserved() { @@ -499,7 +538,6 @@ public class Bar /// /// Verifies that simplified using statements in nested namespace are expanded during the code fix operation. /// - /// A representing the asynchronous unit test. [Fact] public Task WhenOutsidePreferred_UsingsInNestedNamespaces_UsingsMovedAndExpanded() { @@ -537,7 +575,6 @@ namespace OtherNamespace /// /// Verifies that simplified using statements in multiple namespaces are expanded during the code fix operation. /// - /// A representing the asynchronous unit test. [Fact] public Task WhenOutsidePreferred_UsingsInMultipleNamespaces_UsingsMovedAndExpanded() { @@ -576,7 +613,6 @@ namespace System.OtherNamespace /// /// Verifies that simplified using statements in multiple namespaces are deduplicated during the code fix operation. /// - /// A representing the asynchronous unit test. [Fact] public Task WhenOutsidePreferred_UsingsInMultipleNamespaces_UsingsMovedAndDeduplicated() { @@ -634,6 +670,18 @@ public Task WhenInsidePreferred_UsingsInNamespace_ValidUsingStatements() return TestDiagnosticMissingAsync(testCode, InsideNamespaceOption); } + [Fact] + public Task WhenInsidePreferred_UsingsInNamespace_ValidUsingStatements_FileScopedNamespace() + { + var testCode = @"namespace TestNamespace; + +[|using System; +using System.Threading;|] +"; + + return TestDiagnosticMissingAsync(testCode, InsideNamespaceOption); + } + /// /// Verifies that having using statements in the compilation unit will not produce any diagnostics when there are type definition present. /// @@ -818,6 +866,25 @@ namespace TestNamespace return TestInRegularAndScriptAsync(testCode, fixedTestCode, InsideNamespaceOption, placeSystemNamespaceFirst: true); } + [Fact] + public Task WhenInsidePreferred_UsingsInBoth_UsingsMoved_FileScopedNamespaec() + { + var testCode = @"[|using Microsoft.CodeAnalysis;|] + +namespace TestNamespace; + +using System; +"; + + var fixedTestCode = @"namespace TestNamespace; +{|Warning:using Microsoft.CodeAnalysis;|} + +using System; +"; + + return TestInRegularAndScriptAsync(testCode, fixedTestCode, InsideNamespaceOption, placeSystemNamespaceFirst: true); + } + /// /// Verifies that the code fix will properly move separated trivia, but will not move a file header comment. /// From 7ca6030311df7efdad164e3b240e03ee8ccc7dbb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 19 Jul 2021 13:42:11 -0700 Subject: [PATCH 3/4] Fix 'remove unused usings' with file scoped namespaces --- ...oveUnnecessaryImportsDiagnosticAnalyzer.cs | 10 +-- .../RemoveUnnecessaryImportsTests.cs | 88 +++++++++++++++++++ ...emoveUnnecessaryImportsService.Rewriter.cs | 11 ++- .../CSharpRemoveUnnecessaryImportsService.cs | 14 +-- 4 files changed, 104 insertions(+), 19 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryImports/CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryImports/CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer.cs index fc94ca6fbaf5f..5190e8eaac11c 100644 --- a/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryImports/CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryImports/CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -49,15 +47,15 @@ protected override IEnumerable GetFixableDiagnosticSpans( var nodesContainingUnnecessaryUsings = new HashSet(); foreach (var node in nodes) { - var nodeContainingUnnecessaryUsings = node.GetAncestors().First(n => n is NamespaceDeclarationSyntax || n is CompilationUnitSyntax); + var nodeContainingUnnecessaryUsings = node.GetAncestors().First(n => n is BaseNamespaceDeclarationSyntax || n is CompilationUnitSyntax); if (!nodesContainingUnnecessaryUsings.Add(nodeContainingUnnecessaryUsings)) { continue; } - yield return nodeContainingUnnecessaryUsings is NamespaceDeclarationSyntax ? - ((NamespaceDeclarationSyntax)nodeContainingUnnecessaryUsings).Usings.GetContainedSpan() : - ((CompilationUnitSyntax)nodeContainingUnnecessaryUsings).Usings.GetContainedSpan(); + yield return nodeContainingUnnecessaryUsings is BaseNamespaceDeclarationSyntax namespaceDeclaration + ? namespaceDeclaration.Usings.GetContainedSpan() + : ((CompilationUnitSyntax)nodeContainingUnnecessaryUsings).Usings.GetContainedSpan(); } } } diff --git a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs index 048b244fd37cb..4f2976d01d9f3 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryImports/RemoveUnnecessaryImportsTests.cs @@ -410,6 +410,45 @@ static void Main(string[] args) }"); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] + public async Task TestNestedUnusedUsings_FileScopedNamespace() + { + await new VerifyCS.Test + { + TestCode = +@"[|{|IDE0005:using System; +using System.Collections.Generic; +using System.Linq;|}|] + +namespace N; + +using System; + +class Program +{ + static void Main(string[] args) + { + DateTime d; + } +} +", + FixedCode = +@"namespace N; + +using System; + +class Program +{ + static void Main(string[] args) + { + DateTime d; + } +} +", + LanguageVersion = LanguageVersion.CSharp10, + }.RunAsync(); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] public async Task TestNestedUsedUsings() { @@ -504,6 +543,55 @@ class F }"); } + [WorkItem(712656, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/712656")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] + public async Task TestNestedUsedUsings2_FileScopedNamespace() + { + await new VerifyCS.Test + { + TestCode = +@"[|{|IDE0005:using System; +using System.Collections.Generic; +using System.Linq;|}|] + +namespace N; + +[|using System; +{|IDE0005:using System.Collections.Generic;|}|] + +class Program +{ + static void Main(string[] args) + { + DateTime d; + } +} + +class F +{ + DateTime d; +}", + FixedCode = +@"namespace N; + +using System; + +class Program +{ + static void Main(string[] args) + { + DateTime d; + } +} + +class F +{ + DateTime d; +}", + LanguageVersion = LanguageVersion.CSharp10, + }.RunAsync(); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryImports)] public async Task TestAttribute() { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs index d0a2830fa7bd3..87c00ef11ed5c 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.Rewriter.cs @@ -148,14 +148,19 @@ public override SyntaxNode VisitCompilationUnit(CompilationUnitSyntax node) return resultCompilationUnit; } + public override SyntaxNode VisitFileScopedNamespaceDeclaration(FileScopedNamespaceDeclarationSyntax node) + => VisitBaseNamespaceDeclaration(node, (BaseNamespaceDeclarationSyntax)base.VisitFileScopedNamespaceDeclaration(node)); + public override SyntaxNode VisitNamespaceDeclaration(NamespaceDeclarationSyntax node) + => VisitBaseNamespaceDeclaration(node, (BaseNamespaceDeclarationSyntax)base.VisitNamespaceDeclaration(node)); + + private SyntaxNode VisitBaseNamespaceDeclaration( + BaseNamespaceDeclarationSyntax node, + BaseNamespaceDeclarationSyntax namespaceDeclaration) { - var namespaceDeclaration = (NamespaceDeclarationSyntax)base.VisitNamespaceDeclaration(node); var usingsToRemove = GetUsingsToRemove(node.Usings, namespaceDeclaration.Usings); if (usingsToRemove.Count == 0) - { return namespaceDeclaration; - } ProcessUsings(namespaceDeclaration.Usings, usingsToRemove, out var finalUsings, out var finalTrivia); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.cs index 2a0c055bec68b..3a9ac27d3fa8e 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/LanguageServices/CSharpRemoveUnnecessaryImportsService.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System; using System.Collections.Generic; using System.Composition; @@ -50,7 +48,7 @@ public override async Task RemoveUnnecessaryImportsAsync( return document; } - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var oldRoot = (CompilationUnitSyntax)root; var newRoot = (CompilationUnitSyntax)new Rewriter(document, unnecessaryImports, cancellationToken).Visit(oldRoot); @@ -76,24 +74,20 @@ private void AddFormattingSpans( cancellationToken.ThrowIfCancellationRequested(); spans.Add(TextSpan.FromBounds(0, GetEndPosition(compilationUnit, compilationUnit.Members))); - foreach (var @namespace in compilationUnit.Members.OfType()) - { + foreach (var @namespace in compilationUnit.Members.OfType()) AddFormattingSpans(@namespace, spans, cancellationToken); - } } private void AddFormattingSpans( - NamespaceDeclarationSyntax namespaceMember, + BaseNamespaceDeclarationSyntax namespaceMember, List spans, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); spans.Add(TextSpan.FromBounds(namespaceMember.SpanStart, GetEndPosition(namespaceMember, namespaceMember.Members))); - foreach (var @namespace in namespaceMember.Members.OfType()) - { + foreach (var @namespace in namespaceMember.Members.OfType()) AddFormattingSpans(@namespace, spans, cancellationToken); - } } private static int GetEndPosition(SyntaxNode container, SyntaxList list) From 81da9d0121eb1cf4ec4c463c2c664d6257699e3f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 19 Jul 2021 14:05:44 -0700 Subject: [PATCH 4/4] Fix 'use auto property' with file scoped namespaces --- .../CSharpUseAutoPropertyAnalyzer.cs | 3 +- .../UseAutoProperty/UseAutoPropertyTests.cs | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseAutoProperty/CSharpUseAutoPropertyAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseAutoProperty/CSharpUseAutoPropertyAnalyzer.cs index 7b02446f0adbd..cece3d875b056 100644 --- a/src/Analyzers/CSharp/Analyzers/UseAutoProperty/CSharpUseAutoPropertyAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseAutoProperty/CSharpUseAutoPropertyAnalyzer.cs @@ -6,7 +6,6 @@ using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.CSharp.Extensions; -using Microsoft.CodeAnalysis.CSharp.Formatting; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.UseAutoProperty; @@ -47,7 +46,7 @@ private void AnalyzeMemberDeclaration( MemberDeclarationSyntax member, List analysisResults) { - if (member.IsKind(SyntaxKind.NamespaceDeclaration, out NamespaceDeclarationSyntax? namespaceDeclaration)) + if (member is BaseNamespaceDeclarationSyntax namespaceDeclaration) { AnalyzeMembers(context, namespaceDeclaration.Members, analysisResults); } diff --git a/src/Analyzers/CSharp/Tests/UseAutoProperty/UseAutoPropertyTests.cs b/src/Analyzers/CSharp/Tests/UseAutoProperty/UseAutoPropertyTests.cs index fdd6f5205399e..2a764d2991358 100644 --- a/src/Analyzers/CSharp/Tests/UseAutoProperty/UseAutoPropertyTests.cs +++ b/src/Analyzers/CSharp/Tests/UseAutoProperty/UseAutoPropertyTests.cs @@ -51,6 +51,34 @@ int P }"); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)] + public async Task TestSingleGetterFromField_FileScopedNamespace() + { + await TestInRegularAndScript1Async( +@" +namespace N; + +class Class +{ + [|int i|]; + + int P + { + get + { + return i; + } + } +}", +@" +namespace N; + +class Class +{ + int P { get; } +}"); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)] public async Task TestSingleGetterFromField_InRecord() {