Skip to content
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.

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

[ffigen] Add flags controlling how transitive deps are pulled in #1687

Merged
merged 11 commits into from
Oct 31, 2024

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Oct 30, 2024

Add includeTransitiveObjCInterfaces and includeTransitiveObjCProtocols flags to the config.

#1678

To understand these flags, there are 4 different kinds of AST nodes to think about:

  • Nodes explicitly included by the config filters: includes
  • Nodes referred to directly by the includes: directTransitives
  • Nodes indirectly referred to by the includes: transitives
  • Other nodes, which are never included in the bindings in any form

includeTransitiveObjCInterfaces:

  • Interfaces can be fully code-genned, stubbed, or omitted
  • When this flag is true, includes, directTransitives, and transitives are all fully code-genned (this is the status quo behavior)
  • When false, includes are fully code-genned, directTransitives are stubbed, and transitives are omitted
  • Defaults to false, which is a breaking change.
  • Super types of fully code-genned nodes are also fully code-genned, and super types of stubs are also stubs.

includeTransitiveObjCProtocols:

  • Protocols can be fully code-genned or omitted. There's no stubbing atm
  • When this flag is true, includes, directTransitives', and transitives` are all fully code-genned
  • When false, includes are fully code-genned, and directTransitives and transitives are omitted
  • Defaults to false, which is consistent with existing behavior.
  • Currently transitive deps due to methods are ignored. This is because of Obj-C protocol conformance is dropped from types #1462

Copy link

github-actions bot commented Oct 30, 2024

PR Health

Coverage ⚠️
File Coverage
pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/objc_interface.dart 💔 Not covered
pkgs/ffigen/lib/src/code_generator/pointer.dart 💔 Not covered
pkgs/ffigen/lib/src/config_provider/config.dart 💔 Not covered
pkgs/ffigen/lib/src/config_provider/config_impl.dart 💔 Not covered
pkgs/ffigen/lib/src/config_provider/yaml_config.dart 💔 Not covered
pkgs/ffigen/lib/src/header_parser/parser.dart 💔 Not covered
pkgs/ffigen/lib/src/strings.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/apply_config_filters.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/find_transitive_deps.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/list_bindings.dart 💔 Not covered
pkgs/ffigen/lib/src/visitor/visitor.dart 💔 Not covered
pkgs/objective_c/lib/src/objective_c_bindings_generated.dart 💔 Not covered

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

License Headers ✔️
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/generate.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/base_gen.dart
pkgs/ffigen/example/simple/generated_bindings.dart
pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_decl_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_symbol_address_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_type_name_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_reserved_keyword_collision_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_comment_markup_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_dart_handle_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_enum_int_mimic_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_forward_decl_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_functions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_imported_types_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_native_func_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_opaque_dependencies_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_packed_structs_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_regress_384_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_sort_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_struct_fptr_fields_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_unions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_libclang_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.dart
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/jni/lib/src/third_party/global_env_extensions.dart
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart
pkgs/jnigen/android_test_runner/lib/main.dart
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/example/lib/main.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/pdfbox_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/_package.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/_package.dart
pkgs/jnigen/lib/src/bindings/descriptor.dart
pkgs/jnigen/lib/src/elements/elements.g.dart
pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart
pkgs/native_assets_builder/test_data/native_dynamic_linking/bin/native_dynamic_linking.dart
pkgs/objective_c/lib/src/ns_input_stream.dart
pkgs/swift2objc/lib/src/config.dart
pkgs/swift2objc/lib/src/generate_wrapper.dart
pkgs/swift2objc/lib/src/generator/_core/utils.dart
pkgs/swift2objc/lib/src/generator/generator.dart
pkgs/swift2objc/lib/src/generator/generators/class_generator.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_initializer_declaration.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_variable_declaration.dart
pkgs/swift2objc/lib/src/transformer/_core/unique_namer.dart
pkgs/swift2objc/lib/src/transformer/_core/utils.dart
pkgs/swift2objc/lib/src/transformer/transformers/transform_globals.dart
pkgs/swift2objc/lib/src/transformer/transformers/transform_variable.dart
pkgs/swift2objc/test/unit/parse_initializer_param_test.dart

@coveralls
Copy link

coveralls commented Oct 30, 2024

Coverage Status

coverage: 90.847% (+0.6%) from 90.284%
when pulling ea5b529 on transitive
into b90f4f2 on main.

@liamappelbe liamappelbe changed the title [ffigen] Add flags controlling how transitive deps are pulled in WIP: [ffigen] Add flags controlling how transitive deps are pulled in Oct 30, 2024
@liamappelbe liamappelbe changed the title WIP: [ffigen] Add flags controlling how transitive deps are pulled in [ffigen] Add flags controlling how transitive deps are pulled in Oct 31, 2024
@liamappelbe liamappelbe marked this pull request as ready for review October 31, 2024 03:32
Copy link
Member

@HosseinYousefi HosseinYousefi left a comment

Choose a reason for hiding this comment

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

LGTM! After your refactoring the changes are much nicer 👍

_visitation.visitor = this;
}

final Visitation _visitation;
final _seen = <AstNode>{};
final bool _debug;
int _ind = 0;
Copy link
Member

Choose a reason for hiding this comment

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

nit: better to write the full _indentLevel, I initially read it as _index

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@liamappelbe liamappelbe merged commit 7855c80 into main Oct 31, 2024
20 checks passed
@liamappelbe liamappelbe deleted the transitive branch October 31, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants