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

Authoring: Diagnostic for private getter #651

Merged
merged 3 commits into from
Jan 6, 2021
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
10 changes: 10 additions & 0 deletions src/Authoring/DiagnosticTests/NegativeData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ namespace DiagnosticTests
{
public sealed partial class UnitTesting
{
private const string PrivateGetter = @"
namespace DiagnosticTests
{
public sealed class PrivateGetter
{
public int MyInt { private get; set; }
}
}";


private const string SameNameNamespacesDisjoint = @"
namespace DiagnosticTests
{
Expand Down
10 changes: 10 additions & 0 deletions src/Authoring/DiagnosticTests/PositiveData.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
namespace DiagnosticTests
{
public sealed partial class UnitTesting
{

private const string Valid_PrivateSetter = @"
namespace DiagnosticTests
{
public sealed class PrivateSetter
{
public int MyInt { get; private set; }
}
}";

private const string Valid_RollYourOwnAsyncAction = @"
using System;
using Windows.Foundation;
Expand Down
3 changes: 3 additions & 0 deletions src/Authoring/DiagnosticTests/UnitTesting.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ private static IEnumerable<TestCaseData> InvalidCases
{
get
{
// private getter
yield return new TestCaseData(PrivateGetter, WinRTRules.PrivateGetterRule).SetName("Property. PrivateGetter");
// 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(NamespacesDifferByCase, WinRTRules.NamespacesDifferByCase).SetName("Namespace. names only differ by case");
Expand Down Expand Up @@ -392,6 +394,7 @@ private static IEnumerable<TestCaseData> ValidCases
{
get
{
yield return new TestCaseData(Valid_PrivateSetter).SetName("Valid. Property. Private Setter");
yield return new TestCaseData(Valid_RollYourOwnAsyncAction).SetName("Valid. AsyncInterfaces. Implementing your own IAsyncAction");
yield return new TestCaseData(Valid_CustomDictionary).SetName("Valid. CustomProjection. IDictionary<string,BasicStruct>");
yield return new TestCaseData(Valid_CustomList).SetName("Valid. CustomProjection. IList<DisposableClass>");
Expand Down
16 changes: 11 additions & 5 deletions src/Authoring/WinRT.SourceGenerator/DiagnosticUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ public void FindDiagnostics()

// check for things in nonWindowsRuntimeInterfaces
ImplementsInvalidInterface(classSymbol, @class);

CheckPropertySignature(props, classId);


CheckProperties(props, classId);

// check types -- todo: check for !valid types
CheckMethods(publicMethods, classId);
}

Expand All @@ -87,7 +88,7 @@ public void FindDiagnostics()

ImplementsInvalidInterface(interfaceSym, @interface);

CheckPropertySignature(props, @interface.Identifier);
CheckProperties(props, @interface.Identifier);

CheckMethods(methods, @interface.Identifier);
}
Expand Down Expand Up @@ -222,13 +223,18 @@ private void CheckMethods(IEnumerable<MethodDeclarationSyntax> methodDeclaration
/// <summary>Looks at all the properties of the given class and checks them for improper array types (System.Array instances, multidimensional, jagged)</summary>
///<param name="props">collection of properties</param><param name="typeId">containing class/interface</param>
/// <returns>True iff any of the invalid array types are used in any of the propertyy signatures in the given class</returns>
private void CheckPropertySignature(IEnumerable<PropertyDeclarationSyntax> props, SyntaxToken typeId)
private void CheckProperties(IEnumerable<PropertyDeclarationSyntax> props, SyntaxToken typeId)
{
foreach (var prop in props)
{
var propSym = GetModel(prop.SyntaxTree).GetDeclaredSymbol(prop);
var loc = prop.GetLocation();

if (!propSym.GetMethod.DeclaredAccessibility.Equals(Accessibility.Public))
j0shuams marked this conversation as resolved.
Show resolved Hide resolved
{
Report(WinRTRules.PrivateGetterRule, loc, prop.Identifier);
}

ReportIfInvalidType(propSym.Type, loc, prop.Identifier, typeId);
foreach (var arg in propSym.Parameters)
{
Expand Down
5 changes: 5 additions & 0 deletions src/Authoring/WinRT.SourceGenerator/WinRTRules.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ private static DiagnosticDescriptor MakeRule(string id, string title, string mes
helpLinkUri: "https://docs.microsoft.com/en-us/previous-versions/hh977010(v=vs.110)");
}

public static DiagnosticDescriptor PrivateGetterRule = MakeRule(
"WME",
"Property must have public getter",
"Property '{0}' does not have a public getter method. Windows Metadata does not support setter-only properties.");

public static DiagnosticDescriptor DisjointNamespaceRule = MakeRule(
"WME1044",
"Namespace is disjoint from main (winmd) namespace",
Expand Down