Correctly escape comments that may contain tags #1426

brianquinlan · 2024-08-13T17:44:37Z

Thanks for your contribution! Please replace this text with a description of what this PR is changing or adding and why, list any relevant issues, and review the contribution guidelines below.

I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

See our contributor guide for general expectations for PRs.
Larger or significant changes should be discussed in an issue before creating a PR.
Contributions to our repos should follow the Dart style guide and use dart format.
Most changes should add an entry to the changelog and may need to rev the pubspec package version.
Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Fixes dart-lang#1408

github-actions · 2024-08-13T17:46:14Z

PR Health

Changelog Entry ✔️

Details

Package	Changed Files

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

Coverage ⚠️

Details

File	Coverage
pkgs/jnigen/example/in_app_java/lib/android_utils.dart	💔 Not covered
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart	💔 Not covered
pkgs/jnigen/example/notification_plugin/lib/notifications.dart	💔 Not covered
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart	💔 Not covered
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart	💔 Not covered
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart	💔 Not covered
pkgs/jnigen/lib/src/bindings/dart_generator.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 ⚠️

Details

// 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
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.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/text/PDFTextStripper.dart

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/kotlin_plugin/example/lib/main.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/_package.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/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

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

Package publish validation ❗

Details

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-wip	WIP (no publish necessary)
package:native_assets_cli	0.7.3-wip	WIP (no publish necessary)
package:objective_c	1.1.0	already published at pub.dev
package:swift2objc	0.0.1-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)
package:jnigen	0.10.1	(error) pub publish dry-run failed; add the `publish-ignore-warnings` label to ignore

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

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

dcharkes · 2024-08-13T18:05:36Z

Thanks @brianquinlan!

I believe auto publishing doesn't work for packages that require the Flutter SDK. So this requires a manual merge (with red publish bot), and subsequently a manual flutter pub publish.

Hossein is on holiday this week. If this is important for you @brianquinlan, I can publish a new version with this change. Otherwise, it'll happen next week.

brianquinlan · 2024-08-13T20:54:06Z

I suspect that the test_jnigen failures are unrelated to my changes since the same tests pass on macOS and Windows:

Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
Note: javadoc: Loading source file example/in_app_java/android/app/src/main/java/com/example/in_app_java/AndroidUtils.java...
Note: javadoc: Loading source file /home/runner/.gradle/caches/modules-2/files-2.1/androidx.emoji2/emoji2/1.3.0/788179fcab96cd6ec9a3e284e2a509545dc0fb8d/emoji2-1.3.0-sources.jar/androidx/emoji2/text/EmojiCompat.java...
Note: javadoc: Loading source file /home/runner/.gradle/caches/modules-2/files-2.1/androidx.emoji2/emoji2/1.3.0/788179fcab96cd6ec9a3e284e2a509545dc0fb8d/emoji2-1.3.0-sources.jar/androidx/emoji2/text/DefaultEmojiCompatConfig.java...
/home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlinx/kotlinx-coroutines-android/1.7.1/d43e33a7c7ee4bdb6fb3210bfdffee23f275e3dc/kotlinx-coroutines-android-1.7.1-sources.jar(/module-info.java):5: error: module not found: kotlin.stdlib
    requires kotlin.stdlib;
                   ^
/home/runner/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlinx/kotlinx-coroutines-android/1.7.1/d43e33a7c7ee4bdb6fb3210bfdffee23f275e3dc/kotlinx-coroutines-android-1.7.1-sources.jar(/module-info.java):6: error: module not found: kotlinx.coroutines.core
    requires kotlinx.coroutines.core;
                               ^
error: cannot access kotlinx.coroutines.android.module-info
  cannot resolve modules
Note: javadoc: 3 errors
Exception in thread "main" java.lang.NullPointerException
	at java.base/java.util.HashMap.putMapEntries(HashMap.java:497)
	at java.base/java.util.HashMap.putAll(HashMap.java:781)
	at com.github.dart_lang.jnigen.apisummarizer.Main.main(Main.java:132)

Fatal: Cannot generate summary: FormatException: Unexpected end of input (at character 1)

^

Error: Process completed with exit code 1.

Hossein is on holiday this week. If this is important for you @brianquinlan, I can publish a new version with this change. Otherwise, it'll happen next week.

@dcharkes The CI for package:http is currently broken because the jnigened sources are not compatible with the unintended_html_in_doc_comment lint. Regenerating the sources would be the easiest path to a fix for us.

If doing a release is easy and low-risk, could you do one please?

dcharkes · 2024-08-14T07:25:37Z

@dcharkes The CI for package:http is currently broken because the jnigened sources are not compatible with the unintended_html_in_doc_comment lint. Regenerating the sources would be the easiest path to a fix for us.

Maybe you can ignore the lint in the generated file for now?

If doing a release is easy and low-risk, could you do one please?

I believe this knowledge is in @HosseinYousefi's brain. (We should document this @HosseinYousefi!)

HosseinYousefi · 2024-08-14T16:03:47Z

There is nothing worth documenting.

You can safely release it if the CI is failing even without your change. I don't know why the CI fails, I'll investigate when I get back.

brianquinlan · 2024-08-14T16:14:48Z

There is nothing worth documenting.

You can safely release it if the CI is failing even without your change. I don't know why the CI fails, I'll investigate when I get back.

So we just have to flutter pub publish? On a clean master?

brianquinlan · 2024-08-14T16:15:39Z

Maybe it is still worth waiting for #1430 to be fixed

HosseinYousefi · 2024-08-15T09:32:52Z

There is nothing worth documenting.

You can safely release it if the CI is failing even without your change. I don't know why the CI fails, I'll investigate when I get back.

So we just have to flutter pub publish? On a clean master?

Yes. We don't have a separate "stable" branch.

Correctly escape comments that may contain tags

4d5773a

Fixes dart-lang#1408

github-actions bot added the package:jnigen label Aug 13, 2024

brianquinlan added 2 commits August 13, 2024 10:48

CHANGELOG

c7fc087

Update simple_package.dart

308a865

dcharkes requested a review from HosseinYousefi August 13, 2024 18:00

dcharkes approved these changes Aug 13, 2024

View reviewed changes

brianquinlan added 3 commits August 13, 2024 12:39

Update dart_generator.dart

fa5b4c5

Update kotlin.dart

4f5772c

Fix remaining

13b7ca1

dcharkes mentioned this pull request Aug 14, 2024

[jnigen] null pointer at com.github.dart_lang.jnigen.apisummarizer.Main.main(Main.java:132) #1429

Closed

brianquinlan merged commit 4eaec5a into dart-lang:main Aug 14, 2024
22 of 26 checks passed

HosseinYousefi pushed a commit that referenced this pull request Aug 19, 2024

Correctly escape comments that may contain tags (#1426)

461a689

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly escape comments that may contain tags #1426

Correctly escape comments that may contain tags #1426

brianquinlan commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

dcharkes commented Aug 13, 2024

brianquinlan commented Aug 13, 2024

dcharkes commented Aug 14, 2024

HosseinYousefi commented Aug 14, 2024

brianquinlan commented Aug 14, 2024

brianquinlan commented Aug 14, 2024

HosseinYousefi commented Aug 15, 2024

Correctly escape comments that may contain tags #1426

Correctly escape comments that may contain tags #1426

Conversation

brianquinlan commented Aug 13, 2024 • edited Loading

github-actions bot commented Aug 13, 2024 • edited Loading

PR Health

Changelog Entry ✔️

Coverage ⚠️

License Headers ⚠️

Package publish validation ❗

dcharkes commented Aug 13, 2024

brianquinlan commented Aug 13, 2024

dcharkes commented Aug 14, 2024

HosseinYousefi commented Aug 14, 2024

brianquinlan commented Aug 14, 2024

brianquinlan commented Aug 14, 2024

HosseinYousefi commented Aug 15, 2024

brianquinlan commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading