Skip to content

Commit ad507e3

Browse files
mandel-macaqueGitHub Actions Autoformatter
andauthored
[RGen] Add the validation for weak delegates. (#23651)
Weak delegate properties have to start with 'Weak' or provide a strong delegate name for the rgen to generate a unique valid name. Added tests to ensure that both the PropertyValidator and PropertyOrFieldValidator return the correct error. --------- Co-authored-by: GitHub Actions Autoformatter <[email protected]>
1 parent 6cf7a82 commit ad507e3

File tree

7 files changed

+121
-3
lines changed

7 files changed

+121
-3
lines changed

src/rgen/Microsoft.Macios.Bindings.Analyzer/Resources.Designer.cs

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/rgen/Microsoft.Macios.Bindings.Analyzer/Resources.resx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,4 +430,17 @@
430430
<value>Wrong property declaration</value>
431431
</data>
432432

433+
<!-- RBI0032 -->
434+
435+
<data name="RBI0032Description" xml:space="preserve">
436+
<value>A weak delegate has to have a strong delegate name if it does not start with 'Weak'.</value>
437+
</data>
438+
<data name="RBI0032MessageFormat" xml:space="preserve">
439+
<value>The weak delegate '{0}' is missing a strong delegate name, provide one or use the 'Weak' prefix</value>
440+
<comment>{0} is the name of the property that does not start with 'Weak'.</comment>
441+
</data>
442+
<data name="RBI0032Title" xml:space="preserve">
443+
<value>Wrong weak delegate</value>
444+
</data>
445+
433446
</root>

src/rgen/Microsoft.Macios.Bindings.Analyzer/RgenDiagnostics.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,4 +481,19 @@ public static class RgenDiagnostics {
481481
description: new LocalizableResourceString (nameof (Resources.RBI0031Description), Resources.ResourceManager,
482482
typeof (Resources))
483483
);
484+
485+
/// <summary>
486+
/// Diagnostic descriptor for when a weak delegate does not start with "Weak".
487+
/// </summary>
488+
internal static readonly DiagnosticDescriptor RBI0032 = new (
489+
"RBI0032",
490+
new LocalizableResourceString (nameof (Resources.RBI0032Title), Resources.ResourceManager, typeof (Resources)),
491+
new LocalizableResourceString (nameof (Resources.RBI0032MessageFormat), Resources.ResourceManager,
492+
typeof (Resources)),
493+
"Usage",
494+
DiagnosticSeverity.Warning,
495+
isEnabledByDefault: true,
496+
description: new LocalizableResourceString (nameof (Resources.RBI0032Description), Resources.ResourceManager,
497+
typeof (Resources))
498+
);
484499
}

src/rgen/Microsoft.Macios.Bindings.Analyzer/Validators/PropertyValidator.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System;
45
using System.Collections.Immutable;
56
using Microsoft.CodeAnalysis;
67
using Microsoft.Macios.Generator.Context;
@@ -70,6 +71,32 @@ internal static bool AccessorIsValid ((string PropertyName, Accessor Accessor) d
7071
out diagnostics);
7172
}
7273

74+
/// <summary>
75+
/// Validates that a weak delegate property's name starts with "Weak" or that it has a corresponding strong delegate name defined.
76+
/// </summary>
77+
/// <param name="data">The property to validate.</param>
78+
/// <param name="context">The root context for validation.</param>
79+
/// <param name="diagnostic">When this method returns, contains a diagnostic if the property name is invalid; otherwise, an empty array.</param>
80+
/// <param name="location">The code location to be used for the diagnostics.</param>
81+
/// <returns><c>true</c> if the property is not a weak delegate or if its name is valid; otherwise, <c>false</c>.</returns>
82+
internal static bool WeakPropertyNameStartsWithWeak (Property data, RootContext context,
83+
out ImmutableArray<Diagnostic> diagnostic, Location? location)
84+
{
85+
diagnostic = ImmutableArray<Diagnostic>.Empty;
86+
if (data.IsWeakDelegate
87+
&& !data.Name.StartsWith ("Weak", StringComparison.Ordinal)
88+
&& string.IsNullOrEmpty (data.ExportPropertyData.StrongDelegateName)) {
89+
// if the property is weak, it must start with Weak
90+
diagnostic = [Diagnostic.Create (
91+
descriptor: RBI0032,
92+
location: data.Location,
93+
messageArgs: data.Name)];
94+
return false;
95+
}
96+
// we do not care about non-weak properties
97+
return true;
98+
}
99+
73100
/// <summary>
74101
/// Initializes a new instance of the <see cref="PropertyValidator"/> class.
75102
/// </summary>
@@ -96,5 +123,10 @@ public PropertyValidator () : base (p => p.Location)
96123
descriptor: [RBI0018, RBI0019, RBI0029],
97124
validation: AccessorIsValid,
98125
propertyName: "setter");
126+
127+
// if a property is weak ensure that it starts the name with Weak or set the strondelegatename in the
128+
// export property data
129+
AddGlobalStrategy (RBI0032, WeakPropertyNameStartsWithWeak);
99130
}
131+
100132
}

tests/rgen/Microsoft.Macios.Bindings.Analyzer.Tests/Validators/PropertyOrFieldValidatorTests.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,12 @@ public void ValidateValidPropertyTests ()
152152
[Fact]
153153
public void ValidateValidFieldPropertyTests ()
154154
=> testLogic.ValidateValidFieldPropertyTestsImpl ();
155+
156+
[Theory]
157+
[InlineData ("MyProperty", false, null, 0)] // Not weak, should pass
158+
[InlineData ("WeakMyProperty", true, null, 0)] // Weak, starts with "Weak", should pass
159+
[InlineData ("MyProperty", true, "StrongDelegateName", 0)] // Weak, doesn't start with "Weak", but has StrongDelegateName, should pass
160+
[InlineData ("MyProperty", true, null, 1)] // Weak, doesn't start with "Weak", no StrongDelegateName, should fail
161+
public void WeakPropertyNameStartsWithWeakTests (string propertyName, bool isWeak, string? strongDelegateName, int expectedDiagnosticsCount)
162+
=> testLogic.WeakPropertyNameStartsWithWeakTestsImpl (propertyName, isWeak, strongDelegateName, expectedDiagnosticsCount);
155163
}

tests/rgen/Microsoft.Macios.Bindings.Analyzer.Tests/Validators/PropertyValidatorTestLogic.cs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ Property CreateProperty (string name = "TestProperty",
5151
string? getterSelector = null,
5252
string? setterSelector = null,
5353
bool hasGetter = true,
54-
bool hasSetter = true)
54+
bool hasSetter = true,
55+
string? strongDelegateName = null)
5556
{
5657
var modifiers = ImmutableArray.CreateBuilder<SyntaxToken> ();
5758
modifiers.Add (SyntaxFactory.Token (SyntaxKind.PublicKeyword));
@@ -103,7 +104,9 @@ Property CreateProperty (string name = "TestProperty",
103104
accessors: accessors.ToImmutable ()
104105
) {
105106
ExportPropertyData = new ExportData<ObjCBindings.Property> (propertySelector,
106-
argumentSemantic, propertyFlags)
107+
argumentSemantic, propertyFlags) {
108+
StrongDelegateName = strongDelegateName
109+
}
107110
};
108111
}
109112

@@ -318,5 +321,17 @@ public void ValidateValidFieldPropertyTestsImpl ()
318321
var result = validator.ValidateAll (property, context);
319322
Assert.Empty (result);
320323
}
321-
}
322324

325+
public void WeakPropertyNameStartsWithWeakTestsImpl (string propertyName, bool isWeak, string? strongDelegateName, int expectedDiagnosticsCount)
326+
{
327+
var flags = isWeak ? ObjCBindings.Property.WeakDelegate : ObjCBindings.Property.Default;
328+
var property = CreateProperty (name: propertyName, propertyFlags: flags, strongDelegateName: strongDelegateName);
329+
var result = validator.ValidateAll (property, context);
330+
var totalDiagnostics = result.Values.Sum (x => x.Count);
331+
Assert.Equal (expectedDiagnosticsCount, totalDiagnostics);
332+
if (expectedDiagnosticsCount > 0) {
333+
var diagnostic = result.Values.SelectMany (x => x).First ();
334+
Assert.Equal ("RBI0032", diagnostic.Id);
335+
}
336+
}
337+
}

tests/rgen/Microsoft.Macios.Bindings.Analyzer.Tests/Validators/PropertyValidatorTests.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,12 @@ public void PropertyAccessorPresenceTests (bool hasGetter, bool hasSetter, int e
8888
[InlineData (false, "invalid getter", "invalid setter:", 3)] // Not partial and multiple issues
8989
public void CombinedValidationTests (bool isPartial, string? getterSelector, string? setterSelector, int expectedDiagnosticsCount)
9090
=> testLogic.CombinedPropertyValidationTestsImpl (isPartial, getterSelector, setterSelector, expectedDiagnosticsCount);
91+
92+
[Theory]
93+
[InlineData ("MyProperty", false, null, 0)] // Not weak, should pass
94+
[InlineData ("WeakMyProperty", true, null, 0)] // Weak, starts with "Weak", should pass
95+
[InlineData ("MyProperty", true, "StrongDelegateName", 0)] // Weak, doesn't start with "Weak", but has StrongDelegateName, should pass
96+
[InlineData ("MyProperty", true, null, 1)] // Weak, doesn't start with "Weak", no StrongDelegateName, should fail
97+
public void WeakPropertyNameStartsWithWeakTests (string propertyName, bool isWeak, string? strongDelegateName, int expectedDiagnosticsCount)
98+
=> testLogic.WeakPropertyNameStartsWithWeakTestsImpl (propertyName, isWeak, strongDelegateName, expectedDiagnosticsCount);
9199
}

0 commit comments

Comments
 (0)