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

Fix diagnostics for namespaces with internal partial types #1426

Merged
merged 3 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion src/Authoring/WinRT.SourceGenerator/DiagnosticUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ private void CheckNamespaces()
{
WinRTSyntaxReceiver syntaxReceiver = (WinRTSyntaxReceiver)_context.SyntaxReceiver;

// Used to check for conflicitng namespace names
// Used to check for conflicting namespace names
HashSet<string> namespaceNames = new();

foreach (var @namespace in syntaxReceiver.Namespaces)
Expand Down
11 changes: 10 additions & 1 deletion src/Authoring/WinRT.SourceGenerator/Generator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ public void OnVisitSyntaxNode(SyntaxNode syntaxNode)
// once from the namespace declaration and once from the member's declaration
if (syntaxNode is NamespaceDeclarationSyntax @namespace)
{
// We only include the namespace if it has a public type as otherwise it won't.
manodasanW marked this conversation as resolved.
Show resolved Hide resolved
// be projected. For partial types, there would be one instance that we encounter
// which declares the accessibility and we will use that to determine the accessibility
// of the type for the purpose of determining whether to include the namespace.
if (HasSomePublicTypes(syntaxNode))
{
Namespaces.Add(@namespace);
Expand All @@ -233,7 +237,7 @@ public void OnVisitSyntaxNode(SyntaxNode syntaxNode)
return;
}

if (syntaxNode is not MemberDeclarationSyntax declaration || !IsPublic(declaration))
if (syntaxNode is not MemberDeclarationSyntax declaration || !IsPublicOrPartial(declaration))
{
return;
}
Expand All @@ -249,6 +253,11 @@ syntaxNode is DelegateDeclarationSyntax ||
}

private bool IsPublic(MemberDeclarationSyntax member)
{
return member.Modifiers.Any(m => m.IsKind(SyntaxKind.PublicKeyword));
}

private bool IsPublicOrPartial(MemberDeclarationSyntax member)
{
// We detect whether partial types are public using symbol information later.
return member.Modifiers.Any(m => m.IsKind(SyntaxKind.PublicKeyword) || m.IsKind(SyntaxKind.PartialKeyword));
Expand Down
24 changes: 24 additions & 0 deletions src/Tests/AuthoringTest/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1569,4 +1569,28 @@ public partial struct PartialStruct
{
public double Z;
}
}

namespace AnotherNamespace
{
internal partial class PartialClass3
{
public void InternalFunction()
{
}
}

partial class PartialClass3
{
public void InternalFunction2()
{
}
}

internal class InternalClass
{
public void InternalFunction()
{
}
}
}
25 changes: 25 additions & 0 deletions src/Tests/DiagnosticTests/NegativeData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,31 @@ namespace A
public sealed class Blank { public Blank() {} }
}";

private const string UnrelatedNamespaceWithPublicPartialTypes = @"
namespace DiagnosticTests
{
namespace Foo
{
public sealed class Dog
{
public int Woof { get; set; }
}
}
}
namespace Utilities
{
public sealed partial class Sandwich
{
private int BreadCount { get; set; }
}

partial class Sandwich
{
private int BreadCount2 { get; set; }
}
}
";

// note the below test should only fail if the AssemblyName is "DiagnosticTests.A", this is valid under the default "DiagnosticTests"
private const string NamespaceDifferByDot = @"
namespace DiagnosticTests.A
Expand Down
24 changes: 24 additions & 0 deletions src/Tests/DiagnosticTests/PositiveData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,30 @@ private class Sandwich
}
}";

private const string Valid_UnrelatedNamespaceWithNoPublicTypes2 = @"
namespace DiagnosticTests
{
namespace Foo
{
public sealed class Dog
{
public int Woof { get; set; }
}
}
}
namespace Utilities
{
internal partial class Sandwich
{
private int BreadCount { get; set; }
}

partial class Sandwich
{
private int BreadCount2 { get; set; }
}
}";

private const string Valid_SubNamespacesWithOverlappingNames = @"
namespace DiagnosticTests
{
Expand Down
4 changes: 3 additions & 1 deletion src/Tests/DiagnosticTests/UnitTesting.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ private static IEnumerable<TestCaseData> InvalidCases
yield return new TestCaseData(PropertyNoGetter, WinRTRules.PrivateGetterRule).SetName("Property. No Get, public Setter");
// namespace tests
yield return new TestCaseData(SameNameNamespacesDisjoint, WinRTRules.DisjointNamespaceRule).SetName("Namespace. isn't accessible without Test prefix, doesn't use type");
yield return new TestCaseData(UnrelatedNamespaceWithPublicPartialTypes, WinRTRules.DisjointNamespaceRule).SetName("Namespace. Component has public types in different namespaces");
yield return new TestCaseData(NamespacesDifferByCase, WinRTRules.NamespacesDifferByCase).SetName("Namespace. names only differ by case");
yield return new TestCaseData(DisjointNamespaces, WinRTRules.DisjointNamespaceRule).SetName("Namespace. isn't accessible without Test prefix, doesn't use type");
yield return new TestCaseData(DisjointNamespaces2, WinRTRules.DisjointNamespaceRule).SetName("Namespace. using type from inaccessible namespace");
Expand Down Expand Up @@ -414,7 +415,8 @@ private static IEnumerable<TestCaseData> ValidCases
{
get
{
yield return new TestCaseData(Valid_UnrelatedNamespaceWithNoPublicTypes).SetName("Valid. Namespace. Helper namespace with no public types");
yield return new TestCaseData(Valid_UnrelatedNamespaceWithNoPublicTypes).SetName("Valid. Namespace. Helper namespace with no public types");
yield return new TestCaseData(Valid_UnrelatedNamespaceWithNoPublicTypes2).SetName("Valid. Namespace. Helper namespace with partial types but aren't public");
yield return new TestCaseData(Valid_SubNamespacesWithOverlappingNames).SetName("Valid. Namespace. Overlapping namesepaces names is OK if in different namespaces");
yield return new TestCaseData(Valid_PrivateSetter).SetName("Valid. Property. Private Setter");
yield return new TestCaseData(Valid_RollYourOwnAsyncAction).SetName("Valid. AsyncInterfaces. Implementing your own IAsyncAction");
Expand Down