dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.06k stars 1.55k forks source link

[feat] new --exclude flag for `flutter/dart analyze` #54826

Open opxdelwin opened 7 months ago

opxdelwin commented 7 months ago

Use case

Rather than excluding a certain file from analysis for the entire project, We can enable the devs to exclude certain file from analysis only for a single instance of flutter analyze.

Ran into this trouble where I wanted to exclude one file from analyze when done via github actions, as certain firebase_config files aren't in the repo, but kept locally.

Proposal

Create a new flag --exclude for flutter/dart analyze to exclude this file for this specific flutter analyze run

system details * Dart version and tooling diagnostic info (`dart info`) - Dart 3.2.6 (stable) - ``` | Memory | CPU | Elapsed time | Command line | | -----: | --: | -----------: | ------------ | | 580 MB | -- | | dart.exe | | 16 MB | -- | | dart.exe | | 77 MB | -- | | dart.exe | | 105 MB | -- | | dart.exe | | 150 MB | -- | | dart.exe | | 468 MB | -- | | dart.exe | | 83 MB | -- | | dart.exe | ``` * Platform: Windows/Linux * Whether you are using Chrome, Safari, Firefox, Edge (if applicable): NA/general
bkonyi commented 7 months ago

cc @srawlins

srawlins commented 7 months ago

Thanks for opening the request. Can you explain a little more what is desired? Is the code broken, like with compile-time errors, from the point of view of CI, with the missing Firebase config files?

opxdelwin commented 7 months ago

The thing is, config files are tracked locally and isn't checkout into the repo.

Hence dart analyse breaks on CI as specific files aren't available on remote repo.

hope it makes sense

On Tue, Feb 6, 2024, 6:14 AM Sam Rawlins @.***> wrote:

Thanks for opening the request. Can you explain a little more what is desired? Is the code broken, like with compile-time errors, from the point of view of CI, with the missing Firebase config files?

— Reply to this email directly, view it on GitHub https://github.com/dart-lang/sdk/issues/54826#issuecomment-1928572960, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUB2DO7WXRRXII7BXACEZHLYSF4H5AVCNFSM6AAAAABC2BXUI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRYGU3TEOJWGA . You are receiving this because you authored the thread.Message ID: @.***>

opxdelwin commented 7 months ago

Something similar can also be achieved by modifying lint rules in pubspec.yaml, but that causes the IDE to ignore these errors as well (dev's should know if the environment is broken).

Suitable implementation for this issue would be to add a --exclude that takes in a file dir, such that all errors from a certain file would be ignored for the runtime of command.

Alternatively, we can save the exact error raised into an error-config-file and pass this file into the command, so all mentioned errors from error-config-file would be ignored.

Whatd'y think?

srawlins commented 3 weeks ago

Are these firebase_config files added as part of some source code configuration, effectively being vendored in? E.g. are they synced with gclient sync or some other mechanism of fetching and pointing at third party code?

If this is the case, the solution is to exclude: those files via analysis_options.yaml. The analyzer does not report any static analysis issues with pub dependencies (files that live in the pub cache), and users are recommended to emulate this functionality with exclude:.

If you do want to keep seeing static errors, warnings, and lints in your dependencies, you could instead not use exclude: and enhance your CI tooling to filter out any issues reported by dart analyze for the firebase_config files.

opxdelwin commented 2 weeks ago

firebase_config is handled by flutterfire_cli. I did do a workaround for the time being, stored all required params in an .env file, and made firebase_configto get values from env.

This doesn't break the analysis, and gets the job done. Though I think this flag might be useful :)