Skip to content

Commit 049bb0a

Browse files
mandel-macaquemandel-macaque
authored
[RGen] Add two new validators one for arrays one for properties. (#23647)
Changes in this commit: 1. Added a generic array validator that will validate an array using an inner validator. 2. Added a property validator that validates the following from a property: - Export<Property> attribute is valid. - Must be declared partial. - Supported platforms shoul be correct for the accessors. - Validate each of the accessor selectors if they have been added. Extra changes: 1. While test the property validator I found out that we were duplicating errors in nested valiators. There is no need to add a new strategy when adding a nested validator. 2. Updated some of the error messages in the FieldValidator since we got one of the descriptors wrong. 3. Added a new string strategy to ensure that the number of ':' is a selector matches the number of arguments in a method (this includes getters and setters in properties).
2 parents c15d0ab + a7d57cb commit 049bb0a

File tree

11 files changed

+947
-97
lines changed

11 files changed

+947
-97
lines changed

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

Lines changed: 81 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: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,4 +393,41 @@
393393
<value>Ignored flag</value>
394394
</data>
395395

396+
<!-- RBI0029 -->
397+
398+
<data name="RBI0029Description" xml:space="preserve">
399+
<value>There is a mismatch between the arguments of a method/property and the number of ':' in a selector.</value>
400+
</data>
401+
<data name="RBI0029MessageFormat" xml:space="preserve">
402+
<value>There is a mismatch between the arguments of '{0}' (found {1}) and the selector '{2}' (found {3})</value>
403+
<comment>The {0} is the name of the method or property, {1} is the argument count. {2} is the selector and {3} is its argument count</comment>
404+
</data>
405+
<data name="RBI0029Title" xml:space="preserve">
406+
<value>Selector argument mismatch</value>
407+
</data>
408+
409+
<!-- RBI0030 -->
410+
411+
<data name="RBI0030Description" xml:space="preserve">
412+
<value>Field properties must be declared static.</value>
413+
</data>
414+
<data name="RBI0030MessageFormat" xml:space="preserve">
415+
<value>Field properties must be declared static</value>
416+
</data>
417+
<data name="RBI0030Title" xml:space="preserve">
418+
<value>Wrong field declaration</value>
419+
</data>
420+
421+
<!-- RBI0031 -->
422+
423+
<data name="RBI0031Description" xml:space="preserve">
424+
<value>Exported properties must be declared partial.</value>
425+
</data>
426+
<data name="RBI0031MessageFormat" xml:space="preserve">
427+
<value>Exported properties must be declared partial</value>
428+
</data>
429+
<data name="RBI0031Title" xml:space="preserve">
430+
<value>Wrong property declaration</value>
431+
</data>
432+
396433
</root>

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,4 +436,49 @@ public static class RgenDiagnostics {
436436
description: new LocalizableResourceString (nameof (Resources.RBI0028Description), Resources.ResourceManager,
437437
typeof (Resources))
438438
);
439+
440+
/// <summary>
441+
/// Diagnostic descriptor for when there is a mismatch between the number of arguments in a selector and the expected argument count.
442+
/// </summary>
443+
internal static readonly DiagnosticDescriptor RBI0029 = new (
444+
"RBI0029",
445+
new LocalizableResourceString (nameof (Resources.RBI0029Title), Resources.ResourceManager, typeof (Resources)),
446+
new LocalizableResourceString (nameof (Resources.RBI0029MessageFormat), Resources.ResourceManager,
447+
typeof (Resources)),
448+
"Usage",
449+
DiagnosticSeverity.Warning,
450+
isEnabledByDefault: true,
451+
description: new LocalizableResourceString (nameof (Resources.RBI0029Description), Resources.ResourceManager,
452+
typeof (Resources))
453+
);
454+
455+
/// <summary>
456+
/// Diagnostic descriptor for when a field property has not been marked as static.
457+
/// </summary>
458+
internal static readonly DiagnosticDescriptor RBI0030 = new (
459+
"RBI0030",
460+
new LocalizableResourceString (nameof (Resources.RBI0030Title), Resources.ResourceManager, typeof (Resources)),
461+
new LocalizableResourceString (nameof (Resources.RBI0030MessageFormat), Resources.ResourceManager,
462+
typeof (Resources)),
463+
"Usage",
464+
DiagnosticSeverity.Warning,
465+
isEnabledByDefault: true,
466+
description: new LocalizableResourceString (nameof (Resources.RBI0030Description), Resources.ResourceManager,
467+
typeof (Resources))
468+
);
469+
470+
/// <summary>
471+
/// Diagnostic descriptor for when a property has not been marked as partial.
472+
/// </summary>
473+
internal static readonly DiagnosticDescriptor RBI0031 = new (
474+
"RBI0031",
475+
new LocalizableResourceString (nameof (Resources.RBI0031Title), Resources.ResourceManager, typeof (Resources)),
476+
new LocalizableResourceString (nameof (Resources.RBI0031MessageFormat), Resources.ResourceManager,
477+
typeof (Resources)),
478+
"Usage",
479+
DiagnosticSeverity.Warning,
480+
isEnabledByDefault: true,
481+
description: new LocalizableResourceString (nameof (Resources.RBI0031Description), Resources.ResourceManager,
482+
typeof (Resources))
483+
);
439484
}

‎src/rgen/Microsoft.Macios.Bindings.Analyzer/Validator.cs‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -195,22 +195,7 @@ public void AddNestedValidator<TField> (
195195
Validator<TField> nestedValidator)
196196
{
197197
var fieldName = GetPropertyName (selector);
198-
199198
nestedValidators [fieldName] = nestedValidator;
200-
201-
AddStrategy (selector, nestedValidator.Descriptors, NestedValidation);
202-
203-
bool NestedValidation (TField? data, RootContext context, out ImmutableArray<Diagnostic> diagnostic, Location? location = null)
204-
{
205-
diagnostic = [];
206-
if (data is null)
207-
return true; // null nested = valid
208-
209-
var nestedErrors = nestedValidator.ValidateAll (data, context);
210-
// flatten the diagnostics
211-
diagnostic = [.. nestedErrors.SelectMany (x => x.Value)];
212-
return nestedErrors.Count == 0;
213-
}
214199
}
215200

216201
/// <summary>

‎src/rgen/Microsoft.Macios.Bindings.Analyzer/Validators/ArrayValidator.cs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System.Collections.Immutable;
5+
using System.Linq;
6+
using Microsoft.CodeAnalysis;
7+
using Microsoft.Macios.Generator.Context;
8+
9+
namespace Microsoft.Macios.Bindings.Analyzer.Validators;
10+
11+
/// <summary>
12+
/// Validator for arrays of items, which validates each item in the array using an inner validator.
13+
/// </summary>
14+
/// <typeparam name="T">The type of items in the array to validate.</typeparam>
15+
class ArrayValidator<T> : Validator<ImmutableArray<T>> {
16+
/// <summary>
17+
/// Validates that all items in the array are valid using the inner validator.
18+
/// </summary>
19+
/// <param name="property">The array of items to validate.</param>
20+
/// <param name="context">The root context for validation.</param>
21+
/// <param name="diagnostics">When this method returns, contains an array of diagnostics if any items are invalid; otherwise, an empty array.</param>
22+
/// <param name="location">The code location to be used for the diagnostics.</param>
23+
/// <returns><c>true</c> if all items in the array are valid; otherwise, <c>false</c>.</returns>
24+
bool AllItemsAreValid (ImmutableArray<T> property, RootContext context,
25+
out ImmutableArray<Diagnostic> diagnostics, Location? location = null)
26+
{
27+
diagnostics = ImmutableArray<Diagnostic>.Empty;
28+
var builder = ImmutableArray.CreateBuilder<Diagnostic> ();
29+
// loop over all the variables, validate them and collect the diagnostics
30+
foreach (var prop in property) {
31+
var errors = validator.ValidateAll (prop, context);
32+
if (errors.Count > 0) {
33+
// flatten and add all the errors in the builder
34+
builder.AddRange (errors.SelectMany (x => x.Value));
35+
}
36+
}
37+
diagnostics = builder.ToImmutable ();
38+
return diagnostics.Length == 0;
39+
}
40+
41+
/// <summary>
42+
/// The validator used internally to validate each of the items in the array.
43+
/// </summary>
44+
readonly Validator<T> validator;
45+
46+
/// <summary>
47+
/// Initializes a new instance of the <see cref="ArrayValidator{T}"/> class.
48+
/// </summary>
49+
/// <param name="innerValidator">The validator to use for validating each item in the array.</param>
50+
public ArrayValidator (Validator<T> innerValidator)
51+
{
52+
validator = innerValidator;
53+
// add a global strategy that will validate each of the properties in the array
54+
AddGlobalStrategy (validator.Descriptors, AllItemsAreValid);
55+
}
56+
}

‎src/rgen/Microsoft.Macios.Bindings.Analyzer/Validators/FieldValidator.cs‎

Lines changed: 3 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.Macios.Generator.Context;
77
using Microsoft.Macios.Generator.DataModel;
88
using static Microsoft.Macios.Generator.RgenDiagnostics;
9+
using static Microsoft.Macios.Bindings.Analyzer.Validators.PropertyStrategies;
910

1011
namespace Microsoft.Macios.Bindings.Analyzer.Validators;
1112
using PropertyFlag = ObjCBindings.Property;
@@ -48,30 +49,6 @@ internal static bool SelectorHasNoWhitespace (string? selector, RootContext cont
4849
location: location
4950
);
5051

51-
/// <summary>
52-
/// Validates that a field property has the partial modifier.
53-
/// </summary>
54-
/// <param name="property">The property to validate.</param>
55-
/// <param name="context">The root context for validation.</param>
56-
/// <param name="diagnostics">When this method returns, contains an array of diagnostics if the property is invalid; otherwise, an empty array.</param>
57-
/// <param name="location">The code location to be used for the diagnostics.</param>
58-
/// <returns><c>true</c> if the property has the partial modifier; otherwise, <c>false</c>.</returns>
59-
internal static bool IsPartial (Property property, RootContext context, out ImmutableArray<Diagnostic> diagnostics,
60-
Location? location = null)
61-
=> ModifiersStrategies.IsPartial (property.Modifiers, RBI0004, out diagnostics, location, property.Name);
62-
63-
/// <summary>
64-
/// Validates that a field property has the static modifier.
65-
/// </summary>
66-
/// <param name="property">The property to validate.</param>
67-
/// <param name="context">The root context for validation.</param>
68-
/// <param name="diagnostics">When this method returns, contains an array of diagnostics if the property is invalid; otherwise, an empty array.</param>
69-
/// <param name="location">The code location to be used for the diagnostics.</param>
70-
/// <returns><c>true</c> if the property has the static modifier; otherwise, <c>false</c>.</returns>
71-
internal static bool IsStatic (Property property, RootContext context, out ImmutableArray<Diagnostic> diagnostics,
72-
Location? location = null)
73-
=> ModifiersStrategies.IsStatic (property.Modifiers, RBI0004, out diagnostics, location, property.Name);
74-
7552
/// <summary>
7653
/// Validates that field property flags are appropriate for field properties.
7754
/// Many property flags are ignored when used on fields, and this method validates that only valid flags are used.
@@ -132,62 +109,6 @@ internal static bool FlagsAreValid (Property property, RootContext context,
132109
return diagnostics.Length == 0;
133110
}
134111

135-
/// <summary>
136-
/// Validates that the field property and its accessors are available on the current platform.
137-
/// This method checks platform availability for the property itself, its getter (if present), and its setter (if present).
138-
/// </summary>
139-
/// <param name="property">The property to validate.</param>
140-
/// <param name="context">The root context for validation.</param>
141-
/// <param name="diagnostics">When this method returns, contains an array of diagnostics if the property or its accessors are not available on the current platform; otherwise, an empty array.</param>
142-
/// <param name="location">The code location to be used for the diagnostics.</param>
143-
/// <returns><c>true</c> if the property and all its accessors are available on the current platform; otherwise, <c>false</c>.</returns>
144-
internal static bool IsValidPlatform (Property property, RootContext context,
145-
out ImmutableArray<Diagnostic> diagnostics, Location? location = null)
146-
{
147-
diagnostics = ImmutableArray<Diagnostic>.Empty;
148-
if (!property.IsField) {
149-
// this is a bug, return the diagnostic for it
150-
diagnostics = [Diagnostic.Create (
151-
RBI0000, // An unexpected error occurred while processing '{0}'. Please fill a bug report at https://github.com/dotnet/macios/issues/new.
152-
location: location,
153-
messageArgs: property.Name
154-
)];
155-
return false;
156-
}
157-
158-
// there are several locations in which we need to check if the field is valid for the platform
159-
// 1. the property itself
160-
// 2. the getter if present
161-
// 3. the setter if present
162-
if (!SupportedPlatformStrategies.IsValidPlatform (property.SymbolAvailability,
163-
context, out diagnostics, property.Name, property.Location)) {
164-
return false;
165-
}
166-
167-
var builder = ImmutableArray.CreateBuilder<Diagnostic> ();
168-
// if we have a getter or a setter, we want to validate them as well but we will return those merged in the
169-
// diagnostics rather than returning false if one of them is invalid
170-
var getter = property.GetAccessor (AccessorKind.Getter);
171-
if (!getter.IsNullOrDefault) {
172-
if (!SupportedPlatformStrategies.IsValidPlatform (getter.SymbolAvailability, context,
173-
out var getterDiagnostics,
174-
$"{property.Name}.get", getter.Location)) {
175-
builder.AddRange (getterDiagnostics);
176-
}
177-
}
178-
179-
var setter = property.GetAccessor (AccessorKind.Setter);
180-
if (!setter.IsNullOrDefault) {
181-
if (!SupportedPlatformStrategies.IsValidPlatform (setter.SymbolAvailability, context,
182-
out var setterDiagnostics,
183-
$"{property.Name}.set", setter.Location)) {
184-
builder.AddRange (setterDiagnostics);
185-
}
186-
}
187-
diagnostics = builder.ToImmutable ();
188-
return diagnostics.Length == 0;
189-
}
190-
191112
/// <summary>
192113
/// Initializes a new instance of the <see cref="FieldValidator"/> class.
193114
/// </summary>
@@ -198,9 +119,9 @@ public FieldValidator () : base (p => p.Location)
198119
AddStrategy (p => p.Selector, RBI0019, SelectorHasNoWhitespace);
199120

200121
// fields have to be partial
201-
AddGlobalStrategy (RBI0001, IsPartial);
122+
AddGlobalStrategy (RBI0031, IsPartial);
202123
// fields have to be static
203-
AddGlobalStrategy (RBI0004, IsStatic);
124+
AddGlobalStrategy (RBI0030, IsStatic);
204125

205126
// ensure that the flags are valid
206127
AddGlobalStrategy ([RBI0000, RBI0028], FlagsAreValid);

0 commit comments

Comments
 (0)