Skip to content

[pigeon] Adds Dart implementation of ProxyApi #6043

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.

Sign up for GitHub

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

Merged
auto-submit merged 45 commits into from
Mar 19, 2024
Merged

[pigeon] Adds Dart implementation of ProxyApi #6043

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
6e385f1
update ast
bparrishMines Jan 10, 2024
abc026a
constructor extend method and make required default to true
bparrishMines Jan 10, 2024
870cec3
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Jan 10, 2024
0c69e1d
use helper method
bparrishMines Jan 10, 2024
eb14522
fix some validation and add tests
bparrishMines Jan 12, 2024
155469f
change required to isRequired
bparrishMines Jan 12, 2024
3a45694
improve docs
bparrishMines Jan 12, 2024
6350353
fix lint errors and hide ProxyApi
bparrishMines Jan 12, 2024
5cfcb3a
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Jan 12, 2024
4ce1e21
add hide comment
bparrishMines Jan 12, 2024
22c3065
test for recursive looks
bparrishMines Jan 12, 2024
a14485e
fix tools version
bparrishMines Jan 12, 2024
360dd7a
review comments
bparrishMines Jan 23, 2024
ea9f7c3
switch to a method that looks for matching prefix
bparrishMines Jan 24, 2024
33b2e07
avoid loops
bparrishMines Jan 24, 2024
6c79879
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Jan 24, 2024
c8db9c8
fix test
bparrishMines Jan 24, 2024
b414381
change superClass and interfaces to TypeDeclarations
bparrishMines Jan 25, 2024
00db939
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Jan 25, 2024
54c7ac2
add deps
bparrishMines Jan 25, 2024
d05bebf
add proxyapi ast helper gen methods
bparrishMines Jan 27, 2024
dfb7e45
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Jan 30, 2024
8015a9a
implementation of proxyapis stuff for dart
bparrishMines Jan 31, 2024
2dac15d
add proxy api base class
bparrishMines Jan 31, 2024
36bbca1
add documentation
bparrishMines Feb 2, 2024
afa62e8
add tests and some minor validatation improvements
bparrishMines Feb 2, 2024
1cd4d95
add tests actually
bparrishMines Feb 2, 2024
b08d017
generate test api file
bparrishMines Feb 2, 2024
fc3df6f
method improvements
bparrishMines Feb 2, 2024
0caa963
add protected for more methods
bparrishMines Feb 2, 2024
1c61c51
improvement of docs
bparrishMines Feb 2, 2024
cb2e654
change enum name
bparrishMines Feb 4, 2024
12d9c7d
dont gen method without apis
bparrishMines Feb 4, 2024
8abdb3f
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Feb 4, 2024
1d2c549
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Feb 27, 2024
c0aa558
fix class name generation without underscore
bparrishMines Feb 27, 2024
1f0c6ce
change to indexMap and fix extra space
bparrishMines Feb 27, 2024
7f1f70e
review comments and regen
bparrishMines Feb 27, 2024
2b04aa5
`Merge branch 'main' of github.com:flutter/packages into pigeon_wrapp…
bparrishMines Feb 27, 2024
94106ae
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Feb 28, 2024
796d336
move methods
bparrishMines Feb 28, 2024
dcbd085
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Feb 28, 2024
358b6d7
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Mar 5, 2024
0138563
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Mar 14, 2024
ea338c8
Merge branch 'main' of github.com:flutter/packages into pigeon_wrappe…
bparrishMines Mar 14, 2024
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
Prev Previous commit
Next Next commit
change to indexMap and fix extra space
  • Loading branch information
@bparrishMines
bparrishMines committed Feb 27, 2024
commit 1f0c6ce90159d5e851f5bea75eda868adec27dc9
2 changes: 1 addition & 1 deletion packages/pigeon/lib/dart/templates.dart
View file
Open in desktop
Copy link
Contributor

@tarrinneal tarrinneal Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see these templates changing much in the future? I get a bit nervous thinking about adding logic into this later. Just based on doing that with previously written pigeon code.

Copy link
Contributor Author

@bparrishMines bparrishMines Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't expect these to have many changes in the future. And if they do, it shouldn't be changes that require adding logic when generating them. This should at least be true for InstanceManager, InstanceManagerApi, and ProxyApiBaseClass. I expect all of these to always be basic templates.

However, ProxyApiBaseCodec could require logic later to solve some complex issues, but I don't have any plans for it yet.

Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ String instanceManagerApiTemplate({
return '''
/// Generated API for managing the Dart and native `$instanceManagerClassName`s.
class _$apiName {
/// Constructor for [_$apiName ].
/// Constructor for [_$apiName].
_$apiName({BinaryMessenger? binaryMessenger})
: _binaryMessenger = binaryMessenger;

Expand Down
19 changes: 12 additions & 7 deletions packages/pigeon/lib/dart_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// found in the LICENSE file.

import 'package:code_builder/code_builder.dart' as cb;
import 'package:collection/collection.dart' as collection;
import 'package:dart_style/dart_style.dart';
import 'package:path/path.dart' as path;

Expand Down Expand Up @@ -1075,7 +1074,8 @@ if (${_varNamePrefix}replyList == null) {
..toThis = true
..required = method.isRequired,
),
...constructor.parameters.mapIndexed(
...indexMap(
constructor.parameters,
(int index, NamedType parameter) => cb.Parameter(
(cb.ParameterBuilder builder) => builder
..name = _getParameterName(index, parameter)
Expand Down Expand Up @@ -1296,7 +1296,8 @@ if (${_varNamePrefix}replyList == null) {
..isNullable = !method.isRequired
..requiredParameters.addAll(<cb.Reference>[
cb.refer('$apiName ${classMemberNamePrefix}instance'),
...method.parameters.mapIndexed(
...indexMap(
method.parameters,
(int index, NamedType parameter) {
return cb.refer(
'${_addGenericTypesNullable(parameter.type)} ${_getParameterName(index, parameter)}',
Expand Down Expand Up @@ -1342,7 +1343,8 @@ if (${_varNamePrefix}replyList == null) {
cb.refer(
'${proxyApi.name} ${classMemberNamePrefix}instance',
),
...method.parameters.mapIndexed(
...indexMap(
method.parameters,
(int index, NamedType parameter) {
return cb.refer(
'${_addGenericTypesNullable(parameter.type)} ${_getParameterName(index, parameter)}',
Expand Down Expand Up @@ -1437,7 +1439,8 @@ if (${_varNamePrefix}replyList == null) {
..returnType = cb.refer(apiName)
..isNullable = true
..requiredParameters.addAll(
unattachedFields.mapIndexed(
indexMap(
unattachedFields,
(int index, ApiField field) {
return cb.refer(
'${_addGenericTypesNullable(field.type)} ${_getParameterName(index, field)}',
Expand All @@ -1460,7 +1463,8 @@ if (${_varNamePrefix}replyList == null) {
..isNullable = true
..requiredParameters.addAll(<cb.Reference>[
cb.refer('$apiName ${classMemberNamePrefix}instance'),
...method.parameters.mapIndexed(
...indexMap(
method.parameters,
(int index, NamedType parameter) {
return cb.refer(
'${_addGenericTypesNullable(parameter.type)} ${_getParameterName(index, parameter)}',
Expand Down Expand Up @@ -1702,7 +1706,8 @@ if (${_varNamePrefix}replyList == null) {
))
..returns = _refer(method.returnType, asFuture: true)
..requiredParameters.addAll(
method.parameters.mapIndexed(
indexMap(
method.parameters,
(int index, NamedType parameter) => cb.Parameter(
(cb.ParameterBuilder builder) => builder
..name = _getParameterName(index, parameter)
Expand Down
2 changes: 1 addition & 1 deletion .../pigeon/platform_tests/shared_test_plugin_code/lib/src/generated/proxy_api_tests.gen.dart
View file
Open in desktop
Copy link
Contributor

@tarrinneal tarrinneal Feb 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file makes me want to quit my job. Generally seems like everything is there and makes sense.

I'm torn about whether or not to keep it apart from the other core_tests file.

Copy link
Contributor Author

@bparrishMines bparrishMines Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this file is all generated so that I can write the integration tests and verify the generated code doesn't cause any lint warnings. I assumed the review for this file would only require someone to skim it.

Copy link
Contributor

@tarrinneal tarrinneal Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a sense this is the code that needs the most review, since it's the actual functioning code from what you've written. The rest of it is just preference and future work looking.

Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ class PigeonInstanceManager {

/// Generated API for managing the Dart and native `PigeonInstanceManager`s.
class _PigeonInstanceManagerApi {
/// Constructor for [_PigeonInstanceManagerApi ].
/// Constructor for [_PigeonInstanceManagerApi].
_PigeonInstanceManagerApi({BinaryMessenger? binaryMessenger})
: _binaryMessenger = binaryMessenger;

Expand Down