Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix 'move type' with file scoped namespaces #55020

Merged
merged 4 commits into from
Jul 22, 2021
Merged

Fix 'move type' with file scoped namespaces #55020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
92e24c0
Fix 'match folder and namespace' with file scoped namespaces
CyrusNajmabadi Jul 21, 2021
55227d8
Fix 'move type' with file scoped namespaces
CyrusNajmabadi Jul 21, 2021
00b7a7a
Fix test
CyrusNajmabadi Jul 21, 2021
26c36e3
Merge branch 'fileScopedNamespaceMatchFolder' into fileScopedNamespac…
CyrusNajmabadi Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions ...harp/Analyzers/MatchFolderAndNamespace/CSharpMatchFolderAndNamespaceDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,17 @@
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.LanguageServices;
using System.Collections.Immutable;

namespace Microsoft.CodeAnalysis.CSharp.Analyzers.MatchFolderAndNamespace
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class CSharpMatchFolderAndNamespaceDiagnosticAnalyzer : AbstractMatchFolderAndNamespaceDiagnosticAnalyzer<NamespaceDeclarationSyntax>
internal class CSharpMatchFolderAndNamespaceDiagnosticAnalyzer
: AbstractMatchFolderAndNamespaceDiagnosticAnalyzer<SyntaxKind, BaseNamespaceDeclarationSyntax>
{
protected override ISyntaxFacts GetSyntaxFacts() => CSharpSyntaxFacts.Instance;

protected override void InitializeWorker(AnalysisContext context)
{
context.RegisterSyntaxNodeAction(AnalyzeNamespaceNode, SyntaxKind.NamespaceDeclaration);
}
protected override ImmutableArray<SyntaxKind> GetSyntaxKindsToAnalyze()
=> ImmutableArray.Create(SyntaxKind.NamespaceDeclaration, SyntaxKind.FileScopedNamespaceDeclaration);
}
}
85 changes: 80 additions & 5 deletions src/Analyzers/CSharp/Tests/MatchFolderAndNamespace/CSharpMatchFolderAndNamespaceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,16 @@ private static Task RunTestAsync(IEnumerable<(string, string)> originalSources,
var testState = new VerifyCS.Test
{
EditorConfig = editorconfig ?? EditorConfig,
CodeFixTestBehaviors = CodeAnalysis.Testing.CodeFixTestBehaviors.SkipFixAllInDocumentCheck
CodeFixTestBehaviors = CodeAnalysis.Testing.CodeFixTestBehaviors.SkipFixAllInDocumentCheck,
LanguageVersion = LanguageVersion.CSharp10,
};

foreach (var (fileName, content) in originalSources)
{
testState.TestState.Sources.Add((fileName, content));
}

fixedSources ??= Array.Empty<(string, string)>();
foreach (var (fileName, content) in fixedSources)
{
testState.FixedState.Sources.Add((fileName, content));
}

return testState.RunAsync();
}
Expand All @@ -82,6 +79,26 @@ class Class1
directory: folder);
}

[Fact]
public Task InvalidFolderName1_NoDiagnostic_FileScopedNamespace()
{
// No change namespace action because the folder name is not valid identifier
var folder = CreateFolderPath(new[] { "3B", "C" });
var code =
@"
namespace A.B;

class Class1
{
}
";

return RunTestAsync(
"File1.cs",
code,
directory: folder);
}

[Fact]
public Task InvalidFolderName2_NoDiagnostic()
{
Expand Down Expand Up @@ -167,6 +184,32 @@ await RunTestAsync(
fixedCode: fixedCode);
}

[Fact]
public async Task SingleDocumentNoReference_FileScopedNamespace()
{
var folder = CreateFolderPath("B", "C");
var code =
@"namespace [|A.B|];

class Class1
{
}
";

var fixedCode =
@$"namespace {DefaultNamespace}.B.C;

class Class1
{{
}}
";
await RunTestAsync(
fileName: "Class1.cs",
fileContents: code,
directory: folder,
fixedCode: fixedCode);
}

[Fact]
public async Task SingleDocumentNoReference_NoDefaultNamespace()
{
Expand Down Expand Up @@ -199,6 +242,38 @@ await RunTestAsync(
editorConfig: editorConfig);
}

[Fact]
public async Task SingleDocumentNoReference_NoDefaultNamespace_FileScopedNamespace()
{
var editorConfig = @$"
is_global=true
build_property.ProjectDir = {Directory}
";

var folder = CreateFolderPath("B", "C");
var code =
@"namespace [|A.B|];

class Class1
{
}
";

var fixedCode =
@$"namespace B.C;

class Class1
{{
}}
";
await RunTestAsync(
fileName: "Class1.cs",
fileContents: code,
directory: folder,
fixedCode: fixedCode,
editorConfig: editorConfig);
}

[Fact]
public async Task NamespaceWithSpaces_NoDiagnostic()
{
Expand Down
10 changes: 8 additions & 2 deletions ...re/Analyzers/MatchFolderAndNamespace/AbstractMatchFolderAndNamespaceDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

namespace Microsoft.CodeAnalysis.Analyzers.MatchFolderAndNamespace
{
internal abstract class AbstractMatchFolderAndNamespaceDiagnosticAnalyzer<TNamespaceSyntax> : AbstractBuiltInCodeStyleDiagnosticAnalyzer
internal abstract class AbstractMatchFolderAndNamespaceDiagnosticAnalyzer<TSyntaxKind, TNamespaceSyntax>
: AbstractBuiltInCodeStyleDiagnosticAnalyzer
where TSyntaxKind : struct
where TNamespaceSyntax : SyntaxNode
{
private static readonly LocalizableResourceString s_localizableTitle = new(
Expand All @@ -39,11 +41,15 @@ protected AbstractMatchFolderAndNamespaceDiagnosticAnalyzer()
}

protected abstract ISyntaxFacts GetSyntaxFacts();
protected abstract ImmutableArray<TSyntaxKind> GetSyntaxKindsToAnalyze();

protected sealed override void InitializeWorker(AnalysisContext context)
=> context.RegisterSyntaxNodeAction(AnalyzeNamespaceNode, GetSyntaxKindsToAnalyze());

public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
=> DiagnosticAnalyzerCategory.SemanticSpanAnalysis;

protected void AnalyzeNamespaceNode(SyntaxNodeAnalysisContext context)
private void AnalyzeNamespaceNode(SyntaxNodeAnalysisContext context)
{
// It's ok to not have a rootnamespace property, but if it's there we want to use it correctly
context.Options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue(MatchFolderAndNamespaceConstants.RootNamespaceOption, out var rootNamespace);
Expand Down
25 changes: 25 additions & 0 deletions src/EditorFeatures/CSharpTest/CodeActions/MoveType/MoveTypeTests.MoveToNewFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,31 @@ class Class1 { }
await TestMoveTypeToNewFileAsync(code, codeAfterMove, expectedDocumentName, destinationDocumentText);
}

[WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsMoveType)]
public async Task MoveTypeWithWithFileScopedNamespace()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a tests for moving Class1 and Class2 with this setup? Assuming this is valid with filescoped namespaces

namespace N1;

class Class1 
{
    Nested.Class2 C2 { get; }
}

namespace Nested
{
    class Class2 { }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not valid. it's illegal to mix/match a file-scoped namespace with any other sort of namespace (including another file scoped namespace).

{
var code =
@"namespace N1;

[||]class Class1 { }
class Class2 { }
";

var codeAfterMove =
@"namespace N1;
class Class2 { }
";

var expectedDocumentName = "Class1.cs";

var destinationDocumentText =
@"namespace N1;

class Class1 { }
";
await TestMoveTypeToNewFileAsync(code, codeAfterMove, expectedDocumentName, destinationDocumentText);
}

[WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsMoveType)]
public async Task MoveNestedTypeToNewFile_Simple()
{
Expand Down
2 changes: 1 addition & 1 deletion src/EditorFeatures/CSharpTest/Debugging/LocationInfoGetterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void Method()
{
}$$
}
", "Class.Method()", 2);
", "Namespace.Class.Method()", 2);
}

[Fact, Trait(Traits.Feature, Traits.Features.DebuggingLocationName)]
Expand Down
2 changes: 1 addition & 1 deletion src/Features/CSharp/Portable/CodeRefactorings/MoveType/CSharpMoveTypeService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.MoveType
{
[ExportLanguageService(typeof(IMoveTypeService), LanguageNames.CSharp), Shared]
internal class CSharpMoveTypeService :
AbstractMoveTypeService<CSharpMoveTypeService, BaseTypeDeclarationSyntax, NamespaceDeclarationSyntax, MemberDeclarationSyntax, CompilationUnitSyntax>
AbstractMoveTypeService<CSharpMoveTypeService, BaseTypeDeclarationSyntax, BaseNamespaceDeclarationSyntax, MemberDeclarationSyntax, CompilationUnitSyntax>
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
Expand Down
Loading