Skip to content

[ffigen] Method filtering #1511

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

Merged
merged 7 commits into from
Sep 6, 2024
Merged

[ffigen] Method filtering #1511

merged 7 commits into from
Sep 6, 2024

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Sep 5, 2024

Implement method filtering for ObjC interfaces and protocols:

The YAML schema is essentially copying the existing include/exclude keys, and nesting them below a regex key. Example from the test:

objc-interfaces:
  include:
    - MethodFilteringTestInterface
  member-filter:
    Metho.*ngTe.*rface: # The full match rule below takes precedence.
      include:
        - .*
    MethodFilteringTestInterface:
      include:
        - includedStaticMethod
        - excludedStaticMethod   # The exclude rule takes precedence.
        - inc.*Ins.*Me.*od:wi.*  # Methods use original names with : delimiters.
        - includedProperty
      exclude:
        - excluded.*
objc-protocols:
  include:
    - MethodFilteringTestProtocol
  member-filter:
    Met.*ringT.*ocol:
      include:
        - includedProtocolMethod
      # If include is non-empty, everything else is excluded.

The behavior is a combination of the existing includer logic and the member renaming logic:

  • The filter regexes select which interface/protocol to apply the include/exclude rules to, and are separated into "full matches" and regex patterns.
  • Full matchers are basically just any string that doesn't have regex symbols in it (this logic is copied directly from the member renaming code).
  • If an interface matches a regex pattern and a full match, the full match takes precedence (if it matches multiple regexes, the first one wins).
  • If a method matches both an include rule and an exclude rule, the exclude rule wins.
  • If any include rules are specified for an interface, its methods default to excluded by default. Otherwise it defaults to included (to match existing behavior).
  • All match rules see the full original names of the interfaces and methods. In particular, method names are presented in the ObjC format, with : separating the argument names.

Implementation:

  • Add config infrastructure for member filtering.
    • Added shouldIncludeMember method to DeclarationFilters
    • The yaml based implementation lives in YamlMemberIncluder
    • Parsing occurs in declarationConfigExtractor, and pretty much just repurposes the existing parsing logic for includers.
  • Apply method filters in the interface and protocol parsers.

Other changes:

  • I realized we had the infra for member renaming already, but weren't using it for interfaces, so I added that. We already had the config plumbing for it, so I just had to add an originalName/name distinction to ObjCMethod, apply the renaming in the parsers, and use the name instead of the originalName when generating the method name for codegen.
  • Small refactor to YamlIncluder to share logic between the include part and the exclude part. That logic now lives in the YamlFilter class.
  • Wherever the readme talks about nested keys in the YAML schema, I've changed them from outer->inner to outer.inner, as this is more idiomatic.

Fixes #251
Fixes #1269

Copy link

github-actions bot commented Sep 5, 2024

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

Coverage ⚠️
File Coverage
pkgs/ffigen/lib/src/code_generator/objc_methods.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/config_types.dart 💔 Not covered
pkgs/ffigen/lib/src/config_provider/spec_utils.dart 💔 Not covered
pkgs/ffigen/lib/src/config_provider/yaml_config.dart 💔 Not covered
pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart 💔 Not covered
pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart 💔 Not covered
pkgs/ffigen/lib/src/strings.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_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/lang/jcharacter.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/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_property_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_property.dart
Package publish validation ✔️
Package Version Status
package:ffi 2.1.3 already published at pub.dev
package:ffigen 14.0.0-wip WIP (no publish necessary)
package:jni 0.11.0 already published at pub.dev
package:jnigen 0.11.0 already published at pub.dev
package:native_assets_cli 0.8.0-wip WIP (no publish necessary)
package:objective_c 2.0.0-wip WIP (no publish necessary)
package:swift2objc 0.0.1-wip WIP (no publish necessary)
package:swiftgen 0.0.1-wip WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@coveralls
Copy link

coveralls commented Sep 5, 2024

Coverage Status

coverage: 91.466% (+13.0%) from 78.501%
when pulling 70e8f88 on methfilt
into 029838d on main.

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! Nice branch name :P

@liamappelbe liamappelbe merged commit f890a91 into main Sep 6, 2024
17 checks passed
@liamappelbe liamappelbe deleted the methfilt branch September 6, 2024 00:28
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.

Error generating Objective-C binding files [ObjectiveC] Filter interface members
3 participants