Support YAML list format for analysis_options includes, and update tests accordingly#158
Open
leventkantaroglu wants to merge 3 commits intoWorkiva:masterfrom
Open
Conversation
…ncludes, and update tests accordingly
alanknight-wk
left a comment
alanknight-wk
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
A couple of very small style nits.
The checker also wants the files formatted, and an entry in the CHANGELOG.
lib/src/dependency_validator.dart
Outdated
| // For more info on analysis options: | ||
| // https://dart.dev/guides/language/analysis-options#the-analysis-options-file | ||
| if (optionsIncludePackage != null) optionsIncludePackage, | ||
| if (optionsIncludePackage != null && optionsIncludePackage.isNotEmpty) |
There was a problem hiding this comment.
nit: why is this check necessary? Wouldn't ...[] do nothing?
lib/src/utils.dart
Outdated
| final dynamic include = analysisOptions['include']; | ||
| if (include == null) return null; | ||
|
|
||
| final Set<String> packageNames = <String>{}; |
There was a problem hiding this comment.
nit: We generally follow this rule https://dart.dev/effective-dart/design#dont-redundantly-type-annotate-initialized-local-variables so the explicit declaration isn't necessary.
lib/src/utils.dart
Outdated
|
|
||
| final String? include = analysisOptions['include']; | ||
| if (include == null || !include.startsWith('package:')) return null; | ||
| final dynamic include = analysisOptions['include']; |
There was a problem hiding this comment.
nit: Slight preference for Object? over dynamic.
|
Hi, what's the status on this issue? We recently ran into the same problem |
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
3 participants
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.
The
getAnalysisOptionsIncludePackagemethod previously only supported single string includes in analysis_options.yaml files, but modern Dart/Flutter projects often use YAML list format for includes to reference multiple configuration packages. This enhancement adds support for the list format commonly used in projects like:This change improves the dependency validator's ability to detect analysis option packages that should be included in dependency validation, regardless of the YAML format used.