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

Support file-scoped namespaces in SA1516 #3513

Merged
merged 8 commits into from
Jun 21, 2023
Merged

Support file-scoped namespaces in SA1516 #3513

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cca960a
Add support for file scoped namespace to SA1516.
JakubLinhart May 22, 2022
edfbcf1
fix style
JakubLinhart May 23, 2022
1bcb856
add more tests and fix extern alias
JakubLinhart May 24, 2022
51dbe04
Merge remote-tracking branch 'remotes/origin/master' into fix/SA1516-…
JakubLinhart Jun 18, 2023
c2d0129
restore changes lost in merge
JakubLinhart Jun 18, 2023
a6efbfd
restore more changes lost in merge
JakubLinhart Jun 18, 2023
7a1ffdd
review fixes 1. part
JakubLinhart Jun 18, 2023
67bd1ba
remove parentheses from constructor call
JakubLinhart Jun 18, 2023
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
5 changes: 5 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1516CodeFixProvider.cs
View file
Open in desktop
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
233 changes: 233 additions & 0 deletions StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp10/LayoutRules/SA1516CSharp10UnitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,242 @@

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(LanguageVersion.CSharp10)
JakubLinhart marked this conversation as resolved.
Show resolved Hide resolved
{
ReferenceAssemblies = ReferenceAssemblies.Net.Net50,
JakubLinhart marked this conversation as resolved.
Show resolved Hide resolved
TestState =
{
OutputKind = OutputKind.DynamicallyLinkedLibrary,
JakubLinhart marked this conversation as resolved.
Show resolved Hide resolved
Sources = { testCode },
},
FixedCode = fixedCode,
};

return test.RunAsync(CancellationToken.None);
}
}
}
39 changes: 39 additions & 0 deletions ...eCop.Analyzers/StyleCop.Analyzers/LayoutRules/SA1516ElementsMustBeSeparatedByBlankLine.cs
View file
Open in desktop
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 @@ -127,6 +129,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 @@ -209,6 +212,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