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

Update the flutter pubspec-analyzing code to allow more complex asset declarations #54038

Closed
andrewkolos opened this issue Nov 13, 2023 · 5 comments
Assignees
Labels
analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@andrewkolos
Copy link

flutter/flutter#132985 will allow a new type of asset declaration within the pubspec.yaml file of Flutter projects. In particular, it will allow entries in the assets list to either be strings (as they are today), or maps with a path and a flavors list. Consider this example:

# pubspec.yaml file for a Flutter project
# ...
flutter:
# ...
  assets:
    - assets/common/ # Nothing new here.
    - path: assets/premium/ # Newly allowed asset declaration syntax.
      flavors: # This is a list of arbitrary, nonempty strings.
        - premium

Since the analyzer includes code to validate the assets section within Flutter pubspecs, the analyzer raises warnings when validating pubspecs such as example one just mentioned:

andrewkolos-macbookpro:flavor_specific_assets andrewkolos$ flutter analyze
Analyzing flavor_specific_assets...                                     

warning • Assets are required to be file paths (strings) • pubspec.yaml:22:7 • asset_not_string

Before this feature is landed, the analyzer code should be updated to account for the new syntax.

@bwilkerson bwilkerson added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-warning Issues with the analyzer's Warning codes labels Nov 14, 2023
@srawlins srawlins added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Nov 20, 2023
@srawlins srawlins self-assigned this Nov 23, 2023
@srawlins
Copy link
Member

@andrewkolos thanks for alerting us; can you say specifically what the new logic should be? I guess it could generally be strict or loose, so:

  • strict: assets must be a list; each item in the list can be a string or a map; map items must have a path key and a flavors key? Or are they optional?
  • loose: assets must be a list; each item in the list can be a string or a map; end of validation.

@andrewkolos
Copy link
Author

andrewkolos commented Nov 27, 2023

can you say specifically what the new logic should be?

Sure! Here's how I would implement it:

  • assets must be a list.
  • Each item in the list can be a string or a map.
  • Map entries must contain a path key, which has a string value; the value represents a path (relative to project root) to either 1) a file that exists on disk or 2) a directory with a trailing /1 (said more simply, apply the same validation logic that string entries within assets already have).

I would end validation there. Personally, I don't think it's worth the effort to add any additional validation, because the flutter CLI has to parse assets anyway.

Footnotes

  1. For example, assets/folder/.

@andrewkolos
Copy link
Author

Heya @srawlins, I followed the instructions at on CONTRIBUTING.md and the Building wiki page, and I think I have a working local dev environment. Unless you've already started development on this, I can try making my own PR.

@srawlins
Copy link
Member

I started development this morning as well, haha. I'm most of the way there, and plan to mail this out tonight :)

I hope you haven't gotten too far!

@srawlins
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants