-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Support javax.annotation.Generated annotation in Kotlin generator #10961
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a feature to add a javax.annotation.Generated annotation to the generated Kotlin code, controlled by a new useGeneratedAnnotation flag. The implementation correctly adds the flag to the options, parses it from command-line arguments, and conditionally adds the annotation in the Kotlin generator. The changes are accompanied by corresponding tests. My review includes suggestions to improve code maintainability by refactoring a magic string into a constant and reducing code duplication in the newly added tests.
|
Having difficulties executing will set up dart env locally and reformat or manually make changes |
There will be hundreds of changes if you can't run the formatter. I would highly recommend not doing it manually. Or I can format your code and push it up. If that's easier for you. |
|
Managed to install dart! PTAL! (I don't know how to trigger AI review again) |
… setup
- Extract @javax.annotation.Generated("dev.flutter.pigeon") to shared constant in generator_tools.dart
- Update kotlin_generator.dart and java_generator.dart to use shared constant
- Refactor kotlin_generator_test.dart and java_generator_test.dart to use group() with setUp() pattern
- Eliminates code duplication in test initialization across generator tests
tarrinneal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good. The "magic strings" comment from the ai is pretty funny when you look at the rest of the generator files, but there will be some upcoming changes in that regard soon anyway, so might as well start here :)
I do think it would be a good idea to add the annotations to at least one of the generated test files, should be as simple as adding a parameter to the 'runPigeon' method in the 'generation.dart' file in the tool/shared directory. Then adding an argument to the call to that method in generateTestPigeons in the same file for one file.
- Add kotlinUseGeneratedAnnotation parameter to runPigeon method - Pass useGeneratedAnnotation to KotlinOptions - Enable annotation for core_tests in generated test files
- Add javaUseGeneratedAnnotation parameter to runPigeon method - Pass useGeneratedAnnotation to JavaOptions in PigeonOptions - Update test case to enable annotation for both Kotlin and Java - Generates CoreTestsWithAnnotation files for both languages
- Add type annotation for corePascalCaseName (String) - Make generateCodeWithAnnotation variable final - Remove redundant kotlinIncludeErrorClass: true (matches default)
Adds supports for a flag to add a javax.annotation.Generated annotation in the Kotlin generated code
Fixes flutter/flutter#181234