Skip to content

Commit 3f38423

Browse files
[pigeon] Improves documentation of ProxyApi and moves helper functions to a separate file (#9756)
Follow up to #9515 that moves Dart ProxyApi methods to a separate file. ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent d1ca502 commit 3f38423

File tree

12 files changed

+967
-947
lines changed

12 files changed

+967
-947
lines changed

packages/pigeon/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 26.0.1
2+
3+
* Improves documentation of `ProxyApi` annotation and internal Dart ProxyAPI helper functions.
4+
* Moves helper functions for generating Dart portion of ProxyAPIs.
5+
16
## 26.0.0
27

38
* **Breaking Change** [dart] Changes name of constructors used to create subclasses of ProxyApis to

packages/pigeon/lib/src/ast.dart

Lines changed: 40 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ class Method extends Node {
8282

8383
/// Whether this method is required to be implemented.
8484
///
85-
/// This flag is typically used to determine whether a callback method for
86-
/// a `ProxyApi` is nullable or not.
85+
/// This flag is typically only used to determine whether a callback method
86+
/// for an instance of a Dart proxy class of a ProxyAPI is nonnull.
8787
bool isRequired;
8888

89-
/// Whether this is a static method of a ProxyApi.
89+
/// Whether the method of an [AstProxyApi] is denoted with [static].
9090
bool isStatic;
9191

9292
@override
@@ -134,7 +134,7 @@ class AstFlutterApi extends Api {
134134
}
135135
}
136136

137-
/// Represents an API that wraps a native class.
137+
/// Represents the AST for the class denoted with the ProxyAPI annotation.
138138
class AstProxyApi extends Api {
139139
/// Parametric constructor for [AstProxyApi].
140140
AstProxyApi({
@@ -149,16 +149,16 @@ class AstProxyApi extends Api {
149149
this.kotlinOptions,
150150
});
151151

152-
/// List of constructors inside the API.
152+
/// List of constructors declared in the class.
153153
final List<Constructor> constructors;
154154

155-
/// List of fields inside the API.
155+
/// List of fields declared in the class.
156156
List<ApiField> fields;
157157

158-
/// Name of the class this class considers the super class.
158+
/// A [TypeDeclaration] of the parent class if the class had one.
159159
TypeDeclaration? superClass;
160160

161-
/// Name of the classes this class considers to be implemented.
161+
/// A set of [TypeDeclaration]s that this class implements.
162162
Set<TypeDeclaration> interfaces;
163163

164164
/// Options that control how Swift code will be generated for a specific
@@ -169,12 +169,12 @@ class AstProxyApi extends Api {
169169
/// ProxyApi.
170170
final KotlinProxyApiOptions? kotlinOptions;
171171

172-
/// Methods implemented in the host platform language.
172+
/// Methods that handled by an implementation of the native type api.
173173
Iterable<Method> get hostMethods => methods.where(
174174
(Method method) => method.location == ApiLocation.host,
175175
);
176176

177-
/// Methods implemented in Flutter.
177+
/// Methods that are handled by an instance of the Dart proxy class.
178178
Iterable<Method> get flutterMethods => methods.where(
179179
(Method method) => method.location == ApiLocation.flutter,
180180
);
@@ -193,15 +193,16 @@ class AstProxyApi extends Api {
193193
(ApiField field) => !field.isAttached,
194194
);
195195

196-
/// A list of AstProxyApis where each `extends` the API that follows it.
196+
/// A list of [AstProxyApi]s where each is the [superClass] of the one
197+
/// proceeding it.
197198
///
198-
/// Returns an empty list if this api does not extend a ProxyApi.
199+
/// Returns an empty list if this class did not provide a [superClass].
199200
///
200-
/// This method assumes the super classes of each ProxyApi doesn't create a
201-
/// loop. Throws a [ArgumentError] if a loop is found.
201+
/// This method assumes the [superClass] of each class doesn't lead to a loop
202+
/// Throws a [ArgumentError] if a loop is found.
202203
///
203-
/// This method also assumes that all super classes are ProxyApis. Otherwise,
204-
/// throws an [ArgumentError].
204+
/// This method also assumes that the type of [superClass] is annotated with
205+
/// `@ProxyApi`. Otherwise, throws an [ArgumentError].
205206
Iterable<AstProxyApi> allSuperClasses() {
206207
final List<AstProxyApi> superClassChain = <AstProxyApi>[];
207208

@@ -236,12 +237,12 @@ class AstProxyApi extends Api {
236237
return superClassChain;
237238
}
238239

239-
/// All ProxyApis this API `implements` and all the interfaces those APIs
240+
/// All classes this class `implements` and all the interfaces those classes
240241
/// `implements`.
241242
Iterable<AstProxyApi> apisOfInterfaces() => _recursiveFindAllInterfaceApis();
242243

243-
/// Returns a record for each method inherited from an interface and its
244-
/// corresponding ProxyApi.
244+
/// Returns a record for each Flutter method inherited from an interface and
245+
/// the AST of its corresponding class.
245246
Iterable<(Method, AstProxyApi)> flutterMethodsFromInterfacesWithApis() sync* {
246247
for (final AstProxyApi proxyApi in apisOfInterfaces()) {
247248
yield* proxyApi.methods.map((Method method) => (method, proxyApi));
@@ -250,8 +251,7 @@ class AstProxyApi extends Api {
250251

251252
/// Returns a record for each Flutter method inherited from [superClass].
252253
///
253-
/// This also includes methods that super classes inherited from interfaces
254-
/// with `implements`.
254+
/// This also includes methods that the [superClass] inherits from interfaces.
255255
Iterable<(Method, AstProxyApi)>
256256
flutterMethodsFromSuperClassesWithApis() sync* {
257257
for (final AstProxyApi proxyApi in allSuperClasses().toList().reversed) {
@@ -266,51 +266,50 @@ class AstProxyApi extends Api {
266266
}
267267
}
268268

269-
/// All methods inherited from interfaces and the interfaces of interfaces.
269+
/// All methods inherited from interfaces.
270270
Iterable<Method> flutterMethodsFromInterfaces() sync* {
271271
yield* flutterMethodsFromInterfacesWithApis().map(
272272
((Method, AstProxyApi) method) => method.$1,
273273
);
274274
}
275275

276-
/// A list of Flutter methods inherited from the ProxyApi that this ProxyApi
277-
/// `extends`.
276+
/// A list of Flutter methods inherited from [superClass].
278277
///
279-
/// This also recursively checks the ProxyApi that the super class `extends`
280-
/// and so on.
278+
/// This also recursively checks the [superClass] of [superClass].
281279
///
282-
/// This also includes methods that super classes inherited from interfaces
283-
/// with `implements`.
280+
/// This also includes methods that [superClass] inherits from interfaces with
281+
/// `implements`.
284282
Iterable<Method> flutterMethodsFromSuperClasses() sync* {
285283
yield* flutterMethodsFromSuperClassesWithApis().map(
286284
((Method, AstProxyApi) method) => method.$1,
287285
);
288286
}
289287

290-
/// Whether the API has a method that callbacks to Dart to add a new instance
291-
/// to the InstanceManager.
288+
/// Whether the generated ProxyAPI should generate a method in the native type
289+
/// API that calls to Dart to instantiate a Dart proxy class instance.
292290
///
293-
/// This is possible as long as no callback methods are required to
294-
/// instantiate the class.
291+
/// This is possible as the class does not contain a method that is required
292+
/// to be handled by an instance of the Dart proxy class.
295293
bool hasCallbackConstructor() {
296294
return flutterMethods
297295
.followedBy(flutterMethodsFromSuperClasses())
298296
.followedBy(flutterMethodsFromInterfaces())
299297
.every((Method method) => !method.isRequired);
300298
}
301299

302-
/// Whether the API has any message calls from Dart to host.
300+
/// Whether the Dart proxy class makes any message calls to the native type
301+
/// API.
303302
bool hasAnyHostMessageCalls() =>
304303
constructors.isNotEmpty ||
305304
attachedFields.isNotEmpty ||
306305
hostMethods.isNotEmpty;
307306

308-
/// Whether the API has any message calls from host to Dart.
307+
/// Whether the native type API makes any message calls to the Dart proxy
308+
/// class or calls to instantiate a Dart proxy class instance.
309309
bool hasAnyFlutterMessageCalls() =>
310310
hasCallbackConstructor() || flutterMethods.isNotEmpty;
311311

312-
/// Whether the host proxy API class will have methods that need to be
313-
/// implemented.
312+
/// Whether the native type API will have methods that need to be implemented.
314313
bool hasMethodsRequiringImplementation() =>
315314
hasAnyHostMessageCalls() || unattachedFields.isNotEmpty;
316315

@@ -407,7 +406,7 @@ class Constructor extends Method {
407406
}
408407
}
409408

410-
/// Represents a field of an API.
409+
/// Represents a field declared in a class denoted with the ProxyApi annotation.
411410
class ApiField extends NamedType {
412411
/// Constructor for [ApiField].
413412
ApiField({
@@ -419,17 +418,17 @@ class ApiField extends NamedType {
419418
this.isStatic = false,
420419
}) : assert(!isStatic || isAttached);
421420

422-
/// Whether this is an attached field for a [AstProxyApi].
421+
/// Whether this represents an attached field of an [AstProxyApi].
423422
///
424423
/// See [attached].
425424
final bool isAttached;
426425

427-
/// Whether this is a static field of a [AstProxyApi].
426+
/// Whether this represents a static field of an [AstProxyApi].
428427
///
429-
/// A static field must also be attached. See [attached].
428+
/// A static field must also be attached. See [static].
430429
final bool isStatic;
431430

432-
/// Returns a copy of [Parameter] instance with new attached [TypeDeclaration].
431+
/// Returns a copy of an [ApiField] with the new [TypeDeclaration].
433432
@override
434433
ApiField copyWithType(TypeDeclaration type) {
435434
return ApiField(

0 commit comments

Comments
 (0)