Skip to content

Commit 2b6451b

Browse files
mandel-macaqueGitHub Actions AutoformatterCopilotrolfbjarne
authored
[RGen] Add an analyzer error for protocol inline constructors (#23788)
Add a new analyzer warning that will let the user know when a class constructor hides an inline constructor from a protocol. This will allow the user to decide if they ignore the warning or remove the constructor so that the inline version is used. --------- Co-authored-by: GitHub Actions Autoformatter <github-actions-autoformatter@xamarin.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
1 parent 33eab2c commit 2b6451b

File tree

13 files changed

+221
-291
lines changed

13 files changed

+221
-291
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: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,11 +540,24 @@
540540
<value>No result type or result type name was provided for an async method.</value>
541541
</data>
542542
<data name="RBI0040MessageFormat" xml:space="preserve">
543-
<value>The method '{0}' was marked as async and has multiple parameters but does not provide a return type name, a nameless tuple will be generated for the async method</value>
543+
<value>The method '{0}' was marked as async and has multiple parameters but does not provide a return type name, a nameless tuple will be generated for the async method</value>
544544
<comment>{0} is the name of the async method.</comment>
545545
</data>
546546
<data name="RBI0040Title" xml:space="preserve">
547547
<value>Not specified return type</value>
548548
</data>
549549

550+
<!-- RBI0040 -->
551+
552+
<data name="RBI0041Description" xml:space="preserve">
553+
<value>Protocol inline constructor is hidden.</value>
554+
</data>
555+
<data name="RBI0041MessageFormat" xml:space="preserve">
556+
<value>The class '{0}' contains a constructor with the selector '{1}' that hides a inline constructor from protocol '{2}'</value>
557+
<comment>{0} is the name of the class, {1} is the selector, {2} is the name of the protocol</comment>
558+
</data>
559+
<data name="RBI0041Title" xml:space="preserve">
560+
<value>Protocol constructor overlap:</value>
561+
</data>
562+
550563
</root>

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ public static class RgenDiagnostics {
603603
);
604604

605605
/// <summary>
606-
/// Disgnostic descriptor for when a method marked as async has does not provide a return type or return type name.
606+
/// Diagnostic descriptor for when a method marked as async has does not provide a return type or return type name.
607607
/// </summary>
608608
internal static readonly DiagnosticDescriptor RBI0040 = new (
609609
"RBI0040",
@@ -616,4 +616,19 @@ public static class RgenDiagnostics {
616616
description: new LocalizableResourceString (nameof (Resources.RBI0040Description), Resources.ResourceManager,
617617
typeof (Resources))
618618
);
619+
620+
/// <summary>
621+
/// Diagnostic descriptor for when a class inherits from UIView and is missing the initWithFrame: constructor.
622+
/// </summary>
623+
internal static readonly DiagnosticDescriptor RBI0041 = new (
624+
"RBI0041",
625+
new LocalizableResourceString (nameof (Resources.RBI0041Title), Resources.ResourceManager, typeof (Resources)),
626+
new LocalizableResourceString (nameof (Resources.RBI0041MessageFormat), Resources.ResourceManager,
627+
typeof (Resources)),
628+
"Usage",
629+
DiagnosticSeverity.Warning,
630+
isEnabledByDefault: true,
631+
description: new LocalizableResourceString (nameof (Resources.RBI0041Description), Resources.ResourceManager,
632+
typeof (Resources))
633+
);
619634
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,42 @@ bool ValidAsyncMethods (Binding binding, RootContext context,
318318
return diagnostics.Length == 0;
319319
}
320320

321+
bool ValidProtocolInlineConstructors (Binding binding, RootContext context,
322+
out ImmutableArray<Diagnostic> diagnostics, Location? location = null)
323+
{
324+
diagnostics = [];
325+
// ensure that if there are any constructors that are going to be inlined from the protocols that they
326+
// do not conflict with the constructors that are already defined in the class. This is a warning, and we only
327+
// are about those constructors that have the same selectors. The user can disable the warning if he really has
328+
var builder = ImmutableArray.CreateBuilder<Diagnostic> ();
329+
// get all the selectors from the constructors defined in the class as well as the protocol ones and find
330+
// the duplicates
331+
var constructorSelectorsSet = binding.Constructors.ToDictionary (x => x.Selector!, x => x);
332+
var duplicates = binding.ProtocolConstructors
333+
.Where (x => x.Selector is not null && constructorSelectorsSet.ContainsKey (x.Selector))
334+
.Select (x => (Selector: x.Selector!, Constructor: x))
335+
.ToArray ();
336+
if (duplicates.Length > 0) {
337+
// we have duplicates, create a warning for each of them
338+
foreach (var (selector, protocolConstructor) in duplicates) {
339+
// use the class constructor location
340+
var constructorLocation = constructorSelectorsSet.TryGetValue (selector, out var constructor)
341+
? constructor.Location : location;
342+
var protocolName = protocolConstructor.IsProtocolConstructor ? protocolConstructor.ProtocolType : "unknown";
343+
builder.Add (Diagnostic.Create (
344+
descriptor: RBI0041, // The class '{0}' contains a constructor with the selector '{1}' that hides a inline constructor from a protocol
345+
location: constructorLocation,
346+
messageArgs: [
347+
binding.Name,
348+
selector,
349+
protocolName
350+
]));
351+
}
352+
}
353+
diagnostics = builder.ToImmutable ();
354+
return diagnostics.Length == 0;
355+
}
356+
321357
/// <summary>
322358
/// Initializes a new instance of the <see cref="ClassValidator"/> class.
323359
/// </summary>
@@ -332,6 +368,9 @@ public ClassValidator ()
332368
// validate that the selectors are not duplicated, this includes properties and methods
333369
AddGlobalStrategy ([RBI0034], SelectorsAreUnique);
334370

371+
// validate that we have the required constructors for certain base classes like UIView
372+
AddGlobalStrategy ([RBI0041], ValidProtocolInlineConstructors);
373+
335374
// validate async methods. This is a global strategy because it needs to look at all the methods in the binding
336375
// are validated together so that async methods do not have the same names
337376
AddGlobalStrategy ([RBI0035, RBI0036, RBI0037, RBI0038, RBI0039, RBI0040], ValidAsyncMethods);

0 commit comments

Comments
 (0)