[jnigen] Report an empty config list entry as a config error - #3514
Open
Yusufihsangorgel wants to merge 1 commit into
Open
[jnigen] Report an empty config list entry as a config error#3514Yusufihsangorgel wants to merge 1 commit into
Yusufihsangorgel wants to merge 1 commit into
Conversation
An entry left empty under a list key reads as null, and the list it lands in is cast lazily, so nothing fails until something reaches for the element: `type 'Null' is not a subtype of type 'String' in type cast`, from a stack trace that names neither the key nor the file. Check the entries in `getStringList` and `getPathList` and name the one that is empty. A list that came from `-D` holds its own strings, so an override still wins.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
An entry left empty under a list key reads as null, and the list it lands in is cast lazily, so nothing fails until something reaches for the element:
Nothing there names the key or the file, which is what the issue reports.
The entries are now checked where they are read, and the one that is empty is named:
This covers both list readers, so
source_path,class_pathandimportreport the same way rather than reachingoptionalPathListand failing there.The check runs on whichever source won, and only a list read from the file can carry a null: a
-Doverride holds its own strings, so-Dclasses=com.example.Barstill overrides a file with an empty entry.The test writes its own config file, since
-Dcannot express an empty entry: the override parser requires a value after=.Related Issues
Fixes #1802
PR Checklist
dart tool/ci.dart --alllocally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevelpubspec.yamlworkspace.CHANGELOG.mdfor the relevant packages. (Not needed for small changes such as doc typos).On the three unchecked boxes:
pkgs/jnigenis not in the toplevel workspace, sodart tool/ci.dart --alldoes not cover it; I ran the steps from.github/workflows/jnigen.yamlinstead.flutter analyze --fatal-infosis clean for the package, anddart formatleaves the two files here alone. It does want to reformat three checked-in bindings, but it does that on an untouched checkout too, on the SDK I have. I left the CHANGELOG and the version alone because 0.17.0 is published and opening the next-wipmeans bumpingversionindart_generator.dartto match, which rewrites the header of every checked-in binding. Say the word and I will add both.