diff --git a/src/Features/CSharp/Portable/ConvertProgram/ConvertProgramTransform_TopLevelStatements.cs b/src/Features/CSharp/Portable/ConvertProgram/ConvertProgramTransform_TopLevelStatements.cs index 384f28f7892ea..417e32ed5203a 100644 --- a/src/Features/CSharp/Portable/ConvertProgram/ConvertProgramTransform_TopLevelStatements.cs +++ b/src/Features/CSharp/Portable/ConvertProgram/ConvertProgramTransform_TopLevelStatements.cs @@ -17,6 +17,7 @@ using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.RemoveUnnecessaryImports; +using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -33,7 +34,7 @@ public static async Task ConvertToTopLevelStatementsAsync( Contract.ThrowIfNull(typeDeclaration); // checked by analyzer var generator = document.GetRequiredLanguageService(); - var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var root = (CompilationUnitSyntax)await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var rootWithGlobalStatements = GetRootWithGlobalStatements( @@ -97,7 +98,7 @@ private static void AddUsingDirectives(NameSyntax name, SyntaxAnnotation annotat private static SyntaxNode GetRootWithGlobalStatements( SemanticModel semanticModel, SyntaxGenerator generator, - SyntaxNode root, + CompilationUnitSyntax root, TypeDeclarationSyntax typeDeclaration, MethodDeclarationSyntax methodDeclaration, CancellationToken cancellationToken) @@ -107,19 +108,17 @@ private static SyntaxNode GetRootWithGlobalStatements( semanticModel, typeDeclaration, methodDeclaration, cancellationToken); var namespaceDeclaration = typeDeclaration.Parent as BaseNamespaceDeclarationSyntax; + if (namespaceDeclaration != null && namespaceDeclaration.Members.Count >= 2) { // Our parent namespace has another symbol in it. Keep the namespace declaration around, removing only // the existing Program type from it. editor.RemoveNode(typeDeclaration); - editor.ReplaceNode( - root, - (current, _) => - { - var currentRoot = (CompilationUnitSyntax)current; - return currentRoot.WithMembers(currentRoot.Members.InsertRange(0, globalStatements)); - }); + editor.InsertBefore(namespaceDeclaration, globalStatements); + + // We want to place the trailing directive on the namespace declaration we're preceding. + AddDirectivesToNextMemberOrEndOfFile(root.Members.IndexOf(namespaceDeclaration)); } else if (namespaceDeclaration != null) { @@ -136,18 +135,60 @@ private static SyntaxNode GetRootWithGlobalStatements( globalStatements[0].WithPrependedLeadingTrivia(fileBanner)); } - editor.ReplaceNode( - root, - root.ReplaceNode(namespaceDeclaration, globalStatements)); + editor.ReplaceNode(namespaceDeclaration, (_, _) => globalStatements); + + // We're removing the namespace itself. So we want to place the trailing directive on the element that follows that. + AddDirectivesToNextMemberOrEndOfFile(root.Members.IndexOf(namespaceDeclaration) + 1); } else { // type wasn't in a namespace. just remove the type and replace it with the new global statements. - editor.ReplaceNode( - root, root.ReplaceNode(typeDeclaration, globalStatements)); + editor.ReplaceNode(typeDeclaration, (_, _) => globalStatements); + + // We're removing the namespace itself. So we want to place the trailing directive on the element that follows that. + AddDirectivesToNextMemberOrEndOfFile(root.Members.IndexOf(typeDeclaration) + 1); } return editor.GetChangedRoot(); + + void AddDirectivesToNextMemberOrEndOfFile(int memberIndexToPlaceTrailingDirectivesOn) + { + // If the method has trailing directive on the close brace, move them to whatever will come after the + // final global statement in the new file. That could be the next namespace/type member declaration. Or + // it could be the end of file token if there are no more members in the file. + if (methodDeclaration.Body is not BlockSyntax block) + return; + + var leadingCloseBraceTrivia = block.CloseBraceToken.LeadingTrivia; + if (!leadingCloseBraceTrivia.Any(t => t.IsDirective)) + return; + + if (memberIndexToPlaceTrailingDirectivesOn < root.Members.Count) + { + editor.ReplaceNode( + root.Members[memberIndexToPlaceTrailingDirectivesOn], + (current, _) => + { + var updated = current.WithPrependedLeadingTrivia(leadingCloseBraceTrivia); + updated = updated.ReplaceToken( + updated.GetFirstToken(), + updated.GetFirstToken().WithAdditionalAnnotations(Formatter.Annotation)); + return updated; + }); + } + else + { + editor.ReplaceNode( + root, + (current, _) => + { + var currentRoot = (CompilationUnitSyntax)current; + return currentRoot.WithEndOfFileToken(currentRoot.EndOfFileToken + .WithPrependedLeadingTrivia(leadingCloseBraceTrivia) + .WithAdditionalAnnotations(Formatter.Annotation)); + }); + } + } } private static ImmutableArray GetGlobalStatements( diff --git a/src/Features/CSharpTest/ConvertProgram/ConvertToTopLevelStatementsRefactoringTests.cs b/src/Features/CSharpTest/ConvertProgram/ConvertToTopLevelStatementsRefactoringTests.cs index c40f1be9fa6b5..3d0e05d22d4d5 100644 --- a/src/Features/CSharpTest/ConvertProgram/ConvertToTopLevelStatementsRefactoringTests.cs +++ b/src/Features/CSharpTest/ConvertProgram/ConvertToTopLevelStatementsRefactoringTests.cs @@ -10,6 +10,7 @@ using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Testing; +using Roslyn.Test.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ConvertProgram; @@ -22,9 +23,9 @@ public sealed class ConvertToTopLevelStatementsRefactoringTests [Fact] public async Task TestNotOnEmptyFile() { - var code = @" -$$ -"; + var code = """ + $$ + """; // default preference is to prefer top level namespaces. As such, we should not have the refactoring here // since the analyzer will take over. @@ -44,14 +45,15 @@ public async Task TestNotOnEmptyFile() [Fact] public async Task TestConvertToTopLevelStatementsWithDefaultTopLevelStatementPreference() { - var code = @" -class Program -{ - static void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -}"; + var code = """ + class Program + { + static void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """; // default preference is to prefer top level namespaces. As such, we should not have the refactoring here // since the analyzer will take over. @@ -69,18 +71,19 @@ public async Task TestConvertToTopLevelStatementsWithProgramMainPreferenceSugges // user actually prefers Program.Main. As such, we only offer to convert to the alternative as a refactoring. await new VerifyCS.Test { - TestCode = @" -class Program -{ - static void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -} -", - FixedCode = @" -System.Console.WriteLine(args[0]); -", + TestCode = """ + class Program + { + static void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """, + FixedCode = """ + System.Console.WriteLine(args[0]); + + """, LanguageVersion = LanguageVersion.CSharp10, TestState = { OutputKind = OutputKind.ConsoleApplication }, Options = @@ -93,15 +96,15 @@ class Program [Fact] public async Task TestNotOfferedInLibrary() { - var code = @" -class Program -{ - static void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -} -"; + var code = """ + class Program + { + static void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """; await new VerifyCS.Test { @@ -117,15 +120,15 @@ class Program [Fact] public async Task TestNotWithNonViableType() { - var code = @" -class Program -{ - void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -} -"; + var code = """ + class Program + { + void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """; await new VerifyCS.Test { @@ -147,15 +150,15 @@ class Program [Fact] public async Task TestNoConvertToTopLevelStatementsWithProgramMainPreferenceSuggestionBeforeCSharp9() { - var code = @" -class Program -{ - static void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -} -"; + var code = """ + class Program + { + static void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """; // user actually prefers Program.Main. As such, we only offer to convert to the alternative as a refactoring. await new VerifyCS.Test @@ -173,15 +176,15 @@ class Program [Fact] public async Task TestNoConvertToTopLevelStatementsWithTopLevelStatementsPreferenceSuggestion() { - var code = @" -class Program -{ - static void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -} -"; + var code = """ + class Program + { + static void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """; await new VerifyCS.Test { TestCode = code, @@ -197,15 +200,15 @@ class Program [Fact] public async Task TestNoConvertToTopLevelStatementsWithTopLevelStatementsPreferenceSilent() { - var code = @" -class Program -{ - static void $$Main(string[] args) - { - System.Console.WriteLine(args[0]); - } -} -"; + var code = """ + class Program + { + static void $$Main(string[] args) + { + System.Console.WriteLine(args[0]); + } + } + """; await new VerifyCS.Test { TestCode = code, @@ -224,23 +227,203 @@ public async Task TestConvertToTopLevelStatementWithTopLevelStatementPreferenceS // if the user has the analyzer suppressed, then we want to supply teh refactoring. await new VerifyCS.Test { - TestCode = @" -internal class Program -{ - private static void $$Main(string[] args) + TestCode = """ + internal class Program + { + private static void $$Main(string[] args) + { + System.Console.WriteLine(0); + } + } + """, + FixedCode = """ + System.Console.WriteLine(0); + + """, + LanguageVersion = LanguageVersion.CSharp10, + TestState = { OutputKind = OutputKind.ConsoleApplication }, + Options = + { + { CSharpCodeStyleOptions.PreferTopLevelStatements, true, NotificationOption2.None }, + } + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78002")] + public async Task TestPreserveStatementDirectives1() { - System.Console.WriteLine(0); + await new VerifyCS.Test + { + TestCode = """ + using System; + + class Program + { + static void $$Main(string[] args) + { + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + } + } + """, + FixedCode = """ + using System; + + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + + """, + LanguageVersion = LanguageVersion.CSharp10, + TestState = { OutputKind = OutputKind.ConsoleApplication }, + Options = + { + { CSharpCodeStyleOptions.PreferTopLevelStatements, false, NotificationOption2.Suggestion }, + } + }.RunAsync(); } -} -", - FixedCode = @" -System.Console.WriteLine(0); -", + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78002")] + public async Task TestPreserveStatementDirectives2() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + class Program + { + static void $$Main(string[] args) + { + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + } + } + + class Next + { + } + """, + FixedCode = """ + using System; + + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + + class Next + { + } + """, LanguageVersion = LanguageVersion.CSharp10, TestState = { OutputKind = OutputKind.ConsoleApplication }, Options = { - { CSharpCodeStyleOptions.PreferTopLevelStatements, true, NotificationOption2.None }, + { CSharpCodeStyleOptions.PreferTopLevelStatements, false, NotificationOption2.Suggestion }, + } + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78002")] + public async Task TestPreserveStatementDirectives3() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + namespace N + { + class Program + { + static void $$Main(string[] args) + { + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + } + } + } + """, + FixedCode = """ + using System; + + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + + """, + LanguageVersion = LanguageVersion.CSharp10, + TestState = { OutputKind = OutputKind.ConsoleApplication }, + Options = + { + { CSharpCodeStyleOptions.PreferTopLevelStatements, false, NotificationOption2.Suggestion }, + } + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78002")] + public async Task TestPreserveStatementDirectives4() + { + await new VerifyCS.Test + { + TestCode = """ + using System; + + namespace N + { + class Program + { + static void $$Main(string[] args) + { + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + } + } + + class Next + { + } + } + """, + FixedCode = """ + using System; + + #if true + Console.WriteLine("true"); + #else + Console.WriteLine("false"); + #endif + + namespace N + { + class Next + { + } + } + """, + LanguageVersion = LanguageVersion.CSharp10, + TestState = { OutputKind = OutputKind.ConsoleApplication }, + Options = + { + { CSharpCodeStyleOptions.PreferTopLevelStatements, false, NotificationOption2.Suggestion }, } }.RunAsync(); }