Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/src/dependency_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ Future<bool> checkPackage({required String root}) async {
final packagesUsedOutsidePublicDirs = <String>{
// 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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why is this check necessary? Wouldn't ...[] do nothing?

...optionsIncludePackage,
};
for (final file in nonPublicDartFiles) {
packagesUsedOutsidePublicDirs.addAll(getDartDirectivePackageNames(file));
Expand Down
38 changes: 32 additions & 6 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,45 @@ final Logger logger = Logger('dependency_validator');
String bulletItems(Iterable<String> items) =>
items.map((l) => ' * $l').join('\n');

/// Returns the name of the package referenced in the `include:` directive in an
/// analysis_options.yaml file, or null if there is not one.
String? getAnalysisOptionsIncludePackage({String? path}) {
/// Returns the names of the packages referenced in the `include:` directive in an
/// analysis_options.yaml file, or null if there are none.
///
/// Supports both single string includes:
/// include: package:flutter_lints/flutter.yaml
///
/// And list includes:
/// include:
/// - package:flutter_lints/flutter.yaml
/// - package:another_package/config.yaml
Set<String>? getAnalysisOptionsIncludePackage({String? path}) {
final optionsFile = File(p.join(path ?? p.current, 'analysis_options.yaml'));
if (!optionsFile.existsSync()) return null;

final YamlMap? analysisOptions = loadYaml(optionsFile.readAsStringSync());
if (analysisOptions == null) return null;

final String? include = analysisOptions['include'];
if (include == null || !include.startsWith('package:')) return null;
final dynamic include = analysisOptions['include'];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Slight preference for Object? over dynamic.

if (include == null) return null;

final Set<String> packageNames = <String>{};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if (include is String) {
// Handle single string include
if (include.startsWith('package:')) {
final packageName = Uri.parse(include).pathSegments.first;
packageNames.add(packageName);
}
} else if (include is YamlList) {
// Handle list of includes
for (final dynamic item in include) {
if (item is String && item.startsWith('package:')) {
final packageName = Uri.parse(item).pathSegments.first;
packageNames.add(packageName);
}
}
}

return Uri.parse(include).pathSegments.first;
return packageNames.isEmpty ? null : packageNames;
}

/// Returns an iterable of all Dart files (files ending in .dart) in the given
Expand Down
39 changes: 38 additions & 1 deletion test/utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,44 @@ linter:
await d.file('analysis_options.yaml', '''
include: package:pedantic/analysis_options.1.8.0.yaml
''').create();
expect(getAnalysisOptionsIncludePackage(path: d.sandbox), 'pedantic');
expect(getAnalysisOptionsIncludePackage(path: d.sandbox), {'pedantic'});
});

test('returns package names from list `include:`', () async {
await d.file('analysis_options.yaml', '''
include:
- package:flutter_lints/flutter.yaml
- package:pedantic/analysis_options.1.8.0.yaml
''').create();
expect(getAnalysisOptionsIncludePackage(path: d.sandbox),
{'flutter_lints', 'pedantic'});
});

test('filters out non-package includes from list', () async {
await d.file('analysis_options.yaml', '''
include:
- package:flutter_lints/flutter.yaml
- analysis_options_shared.yaml
- package:pedantic/analysis_options.1.8.0.yaml
''').create();
expect(getAnalysisOptionsIncludePackage(path: d.sandbox),
{'flutter_lints', 'pedantic'});
});

test('returns null for list with no package includes', () async {
await d.file('analysis_options.yaml', '''
include:
- analysis_options_shared.yaml
- ../common_options.yaml
''').create();
expect(getAnalysisOptionsIncludePackage(path: d.sandbox), isNull);
});

test('handles empty list', () async {
await d.file('analysis_options.yaml', '''
include: []
''').create();
expect(getAnalysisOptionsIncludePackage(path: d.sandbox), isNull);
});
});

Expand Down
Loading