Skip to content

Commit

Permalink
Merge pull request #54946 from CyrusNajmabadi/fileScopedNamespaceUseA…
Browse files Browse the repository at this point in the history
…utoProp

Fix 'use auto property' with file scoped namespaces
  • Loading branch information
CyrusNajmabadi authored Jul 19, 2021
2 parents 1de6081 + 81da9d0 commit 83d7c5c
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,19 +50,17 @@ 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);
}

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);
}

Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -49,15 +47,15 @@ protected override IEnumerable<TextSpan> GetFixableDiagnosticSpans(
var nodesContainingUnnecessaryUsings = new HashSet<SyntaxNode>();
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();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,7 +46,7 @@ private void AnalyzeMemberDeclaration(
MemberDeclarationSyntax member,
List<AnalysisResult> analysisResults)
{
if (member.IsKind(SyntaxKind.NamespaceDeclaration, out NamespaceDeclarationSyntax? namespaceDeclaration))
if (member is BaseNamespaceDeclarationSyntax namespaceDeclaration)
{
AnalyzeMembers(context, namespaceDeclaration.Members, analysisResults);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand All @@ -90,7 +86,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)

internal static async Task<Document> 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
Expand All @@ -114,10 +110,10 @@ private static async Task<Document> GetTransformedDocumentAsync(
AddImportPlacement placement,
CancellationToken cancellationToken)
{
var syntaxFactsService = document.GetLanguageService<ISyntaxFactsService>();
var syntaxFactsService = document.GetRequiredLanguageService<ISyntaxFactsService>();

// 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);
Expand All @@ -138,11 +134,12 @@ private static async Task<Document> GetTransformedDocumentAsync(
return await Simplifier.ReduceAsync(newDocument, Simplifier.Annotation, options, cancellationToken).ConfigureAwait(false);
}

private static async Task<SyntaxNode> ExpandUsingDirectivesAsync(Document document, CompilationUnitSyntax containerNode, CancellationToken cancellationToken)
private static async Task<CompilationUnitSyntax> 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<UsingDirectiveSyntax>();
var allUsingDirectives = containerNode
.DescendantNodes(node => node is CompilationUnitSyntax or BaseNamespaceDeclarationSyntax)
.OfType<UsingDirectiveSyntax>();

// Create a map between the original node and the future expanded node.
var expandUsingDirectiveTasks = allUsingDirectives.ToDictionary(
Expand All @@ -153,7 +150,7 @@ private static async Task<SyntaxNode> 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);
}
Expand All @@ -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.
Expand All @@ -189,7 +186,7 @@ private static CompilationUnitSyntax MoveUsingsInsideNamespace(CompilationUnitSy

private static CompilationUnitSyntax MoveUsingsOutsideNamespaces(CompilationUnitSyntax compilationUnit, SyntaxAnnotation warningAnnotation)
{
var namespaceDeclarations = compilationUnit.Members.OfType<NamespaceDeclarationSyntax>();
var namespaceDeclarations = compilationUnit.Members.OfType<BaseNamespaceDeclarationSyntax>();
var namespaceDeclarationMap = namespaceDeclarations.ToDictionary(
namespaceDeclaration => namespaceDeclaration, namespaceDeclaration => RemoveUsingsFromNamespace(namespaceDeclaration));

Expand Down Expand Up @@ -220,10 +217,10 @@ private static CompilationUnitSyntax MoveUsingsOutsideNamespaces(CompilationUnit
return compilationUnitWithSeparatorLine.ReplaceNode(firstMember, firstMember.WithPrependedLeadingTrivia(orphanedTrivia));
}

private static (NamespaceDeclarationSyntax namespaceWithoutUsings, IEnumerable<UsingDirectiveSyntax> usingsFromNamespace) RemoveUsingsFromNamespace(
NamespaceDeclarationSyntax usingContainer)
private static (BaseNamespaceDeclarationSyntax namespaceWithoutUsings, IEnumerable<UsingDirectiveSyntax> usingsFromNamespace) RemoveUsingsFromNamespace(
BaseNamespaceDeclarationSyntax usingContainer)
{
var namespaceDeclarations = usingContainer.Members.OfType<NamespaceDeclarationSyntax>();
var namespaceDeclarations = usingContainer.Members.OfType<BaseNamespaceDeclarationSyntax>();
var namespaceDeclarationMap = namespaceDeclarations.ToDictionary(
namespaceDeclaration => namespaceDeclaration, namespaceDeclaration => RemoveUsingsFromNamespace(namespaceDeclaration));

Expand Down Expand Up @@ -281,7 +278,7 @@ private static SyntaxList<MemberDeclarationSyntax> GetMembers(SyntaxNode node)
=> node switch
{
CompilationUnitSyntax compilationUnit => compilationUnit.Members,
NamespaceDeclarationSyntax namespaceDeclaration => namespaceDeclaration.Members,
BaseNamespaceDeclarationSyntax namespaceDeclaration => namespaceDeclaration.Members,
_ => throw ExceptionUtilities.UnexpectedValue(node)
};

Expand Down Expand Up @@ -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.

Expand All @@ -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<NamespaceDeclarationSyntax>();
var allNamespaces = compilationUnit
.DescendantNodes(node => node is CompilationUnitSyntax or BaseNamespaceDeclarationSyntax)
.OfType<BaseNamespaceDeclarationSyntax>();

// To determine if there are multiple namespaces we only need to look for at least two.
return allNamespaces.Take(2).Count() == 1;
Expand Down
Loading

0 comments on commit 83d7c5c

Please sign in to comment.