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.

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

Support file-scoped namespaces in SA1516 #3513

Merged
merged 8 commits into from
Jun 21, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ private static SyntaxNode GetRelevantNode(SyntaxNode innerNode)
return currentNode;
}

if (currentNode is ExternAliasDirectiveSyntax)
{
return currentNode;
}

currentNode = currentNode.Parent;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,241 @@

namespace StyleCop.Analyzers.Test.CSharp10.LayoutRules
{
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Testing;
using StyleCop.Analyzers.Test.CSharp9.LayoutRules;
using Xunit;
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
StyleCop.Analyzers.LayoutRules.SA1516ElementsMustBeSeparatedByBlankLine,
StyleCop.Analyzers.LayoutRules.SA1516CodeFixProvider>;

public class SA1516CSharp10UnitTests : SA1516CSharp9UnitTests
{
/// <summary>
/// Verifies that SA1516 is reported for usings and extern alias outside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnUsingsAndExternAliasOutsideFileScopedNamespaceAsync()
{
var testCode = @"extern alias corlib;
[|using|] System;
using System.Linq;
using a = System.Collections;
[|namespace|] Foo;
";

var fixedCode = @"extern alias corlib;

using System;
using System.Linq;
using a = System.Collections;

namespace Foo;
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

/// <summary>
/// Verifies that SA1516 is reported for usings inside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnSpacingWithUsingsInsideFileScopedNamespaceAsync()
{
var testCode = @"namespace Foo;
[|using|] System;
using System.Linq;
using a = System.Collections;
";

var fixedCode = @"namespace Foo;

using System;
using System.Linq;
using a = System.Collections;
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

/// <summary>
/// Verifies that SA1516 is reported for member declarations inside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnMemberDeclarationsInsideFileScopedNamespaceAsync()
{
var testCode = @"namespace Foo;
[|public|] class Bar
{
}
[|public|] enum Foobar
{
}
";

var fixedCode = @"namespace Foo;

public class Bar
{
}

public enum Foobar
{
}
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

/// <summary>
/// Verifies that SA1516 is reported for usings and member declarations inside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnUsingsAndMemberDeclarationsInsideFileScopedNamespaceAsync()
{
var testCode = @"namespace Foo;
[|using|] System;
using System.Linq;
using a = System.Collections;
[|public|] class Bar
{
}
[|public|] enum Foobar
{
}
";

var fixedCode = @"namespace Foo;

using System;
using System.Linq;
using a = System.Collections;

public class Bar
{
}

public enum Foobar
{
}
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

/// <summary>
/// Verifies that SA1516 is reported extern alias inside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnExternAliasInsideFileScopedNamespaceAsync()
{
var testCode = @"namespace Foo;
[|extern|] alias corlib;
";

var fixedCode = @"namespace Foo;

extern alias corlib;
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

/// <summary>
/// Verifies that SA1516 is reported extern alias and usings inside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnExternAliasAndUsingsInsideFileScopedNamespaceAsync()
{
var testCode = @"namespace Foo;
[|extern|] alias corlib;
[|using|] System;
using System.Linq;
using a = System.Collections;
";

var fixedCode = @"namespace Foo;

extern alias corlib;

using System;
using System.Linq;
using a = System.Collections;
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

/// <summary>
/// Verifies that SA1516 is reported extern alias, usings and member declarations
/// inside a file scoped namespace.
/// </summary>
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
[Fact]
[WorkItem(3512, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3512")]
public async Task TestThatDiagnosticIIsReportedOnExternAliasUsingsAndMemberDeclarationsInsideFileScopedNamespaceAsync()
{
var testCode = @"namespace Foo;
[|extern|] alias corlib;
[|using|] System;
using System.Linq;
using a = System.Collections;
[|public|] class Bar
{
}
[|public|] enum Foobar
{
}
";

var fixedCode = @"namespace Foo;

extern alias corlib;

using System;
using System.Linq;
using a = System.Collections;

public class Bar
{
}

public enum Foobar
{
}
";

await VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
}

private static Task VerifyCSharpFixAsync(string testCode, string fixedCode)
{
var test = new CSharpTest
{
ReferenceAssemblies = ReferenceAssemblies.Net.Net60,
TestState =
{
Sources = { testCode },
},
FixedCode = fixedCode,
};

return test.RunAsync(CancellationToken.None);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ namespace StyleCop.Analyzers.LayoutRules
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Text;
using StyleCop.Analyzers.Helpers;
using StyleCop.Analyzers.Lightup;
using StyleCop.Analyzers.Settings.ObjectModel;

/// <summary>
Expand Down Expand Up @@ -87,6 +88,7 @@ internal class SA1516ElementsMustBeSeparatedByBlankLine : DiagnosticAnalyzer
private static readonly Action<SyntaxNodeAnalysisContext> TypeDeclarationAction = HandleTypeDeclaration;
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> CompilationUnitAction = HandleCompilationUnit;
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> NamespaceDeclarationAction = HandleNamespaceDeclaration;
private static readonly Action<SyntaxNodeAnalysisContext, StyleCopSettings> FileScopedNamespaceDeclarationAction = HandleFileScopedNamespaceDeclaration;
private static readonly Action<SyntaxNodeAnalysisContext> BasePropertyDeclarationAction = HandleBasePropertyDeclaration;

private static readonly ImmutableDictionary<string, string> DiagnosticProperties = ImmutableDictionary<string, string>.Empty.Add(CodeFixActionKey, InsertBlankLineValue);
Expand Down Expand Up @@ -129,6 +131,7 @@ public override void Initialize(AnalysisContext context)
context.RegisterSyntaxNodeAction(TypeDeclarationAction, SyntaxKinds.TypeDeclaration);
context.RegisterSyntaxNodeAction(CompilationUnitAction, SyntaxKind.CompilationUnit);
context.RegisterSyntaxNodeAction(NamespaceDeclarationAction, SyntaxKind.NamespaceDeclaration);
context.RegisterSyntaxNodeAction(FileScopedNamespaceDeclarationAction, SyntaxKindEx.FileScopedNamespaceDeclaration);
context.RegisterSyntaxNodeAction(BasePropertyDeclarationAction, SyntaxKinds.BasePropertyDeclaration);
});
}
Expand Down Expand Up @@ -212,6 +215,42 @@ private static void HandleCompilationUnit(SyntaxNodeAnalysisContext context, Sty
}
}

private static void HandleFileScopedNamespaceDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
{
var namespaceDeclaration = (BaseNamespaceDeclarationSyntaxWrapper)context.Node;

var usings = namespaceDeclaration.Usings;
var members = namespaceDeclaration.Members;

HandleUsings(context, usings, settings);
HandleMemberList(context, members);

if (namespaceDeclaration.Externs.Count > 0)
{
ReportIfThereIsNoBlankLine(context, namespaceDeclaration.Name, namespaceDeclaration.Externs[0]);
}

if (namespaceDeclaration.Usings.Count > 0)
{
ReportIfThereIsNoBlankLine(context, namespaceDeclaration.Name, namespaceDeclaration.Usings[0]);

if (namespaceDeclaration.Externs.Count > 0)
{
ReportIfThereIsNoBlankLine(context, namespaceDeclaration.Externs[namespaceDeclaration.Externs.Count - 1], namespaceDeclaration.Usings[0]);
}
}

if (members.Count > 0)
{
ReportIfThereIsNoBlankLine(context, namespaceDeclaration.Name, members[0]);

if (namespaceDeclaration.Usings.Count > 0)
{
ReportIfThereIsNoBlankLine(context, usings[usings.Count - 1], members[0]);
}
}
Comment on lines +243 to +251
Copy link
Member

Choose a reason for hiding this comment

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

❔ Is this needed, or is it duplicating work done in HandleMemberList?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While HandleMemberList checks blank lines between two consecutive members, this does the check for the namespace declaration/usings and the 1. member. This is a core part of the fix I guess 😄. Not sure how to merge it into HandleMemberList in a nice way.

}

private static void HandleNamespaceDeclaration(SyntaxNodeAnalysisContext context, StyleCopSettings settings)
{
var namespaceDeclaration = (NamespaceDeclarationSyntax)context.Node;
Expand Down