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

Using built-in package-config parser #4

Merged
7 commits merged into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
31 changes: 16 additions & 15 deletions packages/custom_lint_core/lib/src/configs.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import 'dart:isolate';
import 'dart:io';

import 'package:analyzer/file_system/file_system.dart';
import 'package:collection/collection.dart';
// ignore: implementation_imports
import 'package:custom_lint/src/package_utils.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart';
import 'package:yaml/yaml.dart';
Expand All @@ -20,7 +22,7 @@ class CustomLintConfigs {

/// Decode a [CustomLintConfigs] from a file.
@internal
static Future<CustomLintConfigs> parse(File? analysisOptionsFile) async {
factory CustomLintConfigs.parse(File? analysisOptionsFile) {
if (analysisOptionsFile == null || !analysisOptionsFile.exists) {
return CustomLintConfigs.empty;
}
Expand All @@ -37,29 +39,28 @@ class CustomLintConfigs {
final include = yaml['include'] as Object?;
var includedOptions = CustomLintConfigs.empty;
if (include is String) {
var includeUri = Uri.parse(include);
final includeUri = Uri.parse(include);
String? includeAbsolutePath;

final packageUri = await Isolate.resolvePackageUri(includeUri);
if (packageUri != null) {
includeUri = packageUri;
}
if (include.startsWith('package:')) {
final packageConfig = parsePackageConfigSync(Directory.current);
final packageUri = packageConfig.resolve(includeUri);

final includePath = includeUri.toFilePath();
String includeAbsolutePath;
if (includeUri.isAbsolute) {
includeAbsolutePath = includePath;
includeAbsolutePath = packageUri?.toFilePath();
} else {
includeAbsolutePath = normalize(
absolute(
analysisOptionsFile.parent.path,
includePath,
includeUri.toFilePath(),
),
);
}

includedOptions = await CustomLintConfigs.parse(
analysisOptionsFile.provider.getFile(includeAbsolutePath),
);
if (includeAbsolutePath != null) {
includedOptions = CustomLintConfigs.parse(
analysisOptionsFile.provider.getFile(includeAbsolutePath),
);
}
}

final customLint = yaml['custom_lint'] as Object?;
Expand Down
2 changes: 1 addition & 1 deletion packages/custom_lint_core/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ dependencies:
custom_lint: 0.5.7
matcher: ^0.12.0
meta: ^1.7.0
package_config: ^2.1.0
path: ^1.8.0
pubspec_parse: ^1.2.2
source_span: ^1.8.0
Expand All @@ -24,5 +25,4 @@ dev_dependencies:
build_runner: ^2.3.3
lint_visitor_generator:
path: ../lint_visitor_generator
package_config: ^2.1.0
test: ^1.22.2
71 changes: 42 additions & 29 deletions packages/custom_lint_core/test/configs_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ Future<void> patchPackageConfig(
);

addTearDown(
() async => savePackageConfig(currentPackageConfig, io.Directory.current),
() => savePackageConfig(currentPackageConfig, io.Directory.current),
);
}

void main() {
late File includeFile;
late CustomLintConfigs includeConfig;
const testPackageName = 'test_package_with_config';
setUp(() async {
setUp(() {
includeFile = createAnalysisOptions(
'''
custom_lint:
Expand All @@ -89,7 +89,7 @@ custom_lint:
''',
);

includeConfig = await CustomLintConfigs.parse(includeFile);
includeConfig = CustomLintConfigs.parse(includeFile);
});

test('Empty config', () {
Expand All @@ -98,45 +98,43 @@ custom_lint:
});

group('parse', () {
test('if file is null, defaults to empty', () async {
final configs = await CustomLintConfigs.parse(null);
test('if file is null, defaults to empty', () {
final configs = CustomLintConfigs.parse(null);
expect(configs, same(CustomLintConfigs.empty));
});

test('if file does not exist, defaults to empty ', () async {
final configs = await CustomLintConfigs.parse(
test('if file does not exist, defaults to empty ', () {
final configs = CustomLintConfigs.parse(
PhysicalResourceProvider.INSTANCE.getFile('/this-does-no-exist'),
);
expect(configs, same(CustomLintConfigs.empty));
});

test('if custom_lint not present in the option file, clones "include"',
() async {
test('if custom_lint not present in the option file, clones "include"', () {
final analysisOptions = createAnalysisOptions('''
include: ${includeFile.path}
linter:
rules:
public_member_api_docs: false
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(configs, includeConfig);
});

test('if custom_lint not present in the option file, clones "include"',
() async {
test('if custom_lint not present in the option file, clones "include"', () {
final analysisOptions = createAnalysisOptions('''
include: ${includeFile.path}
linter:
rules:
public_member_api_docs: false
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(configs, includeConfig);
});

test('if custom_lint is present but empty, clones "include"', () async {
test('if custom_lint is present but empty, clones "include"', () {
final analysisOptions = createAnalysisOptions('''
include: ${includeFile.path}
linter:
Expand All @@ -145,18 +143,18 @@ linter:

custom_lint:
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(configs, includeConfig);
});

test('has an immutable list of rules', () async {
test('has an immutable list of rules', () {
final analysisOptions = createAnalysisOptions('''
custom_lint:
rules:
- a
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(
configs.rules,
Expand All @@ -171,7 +169,7 @@ custom_lint:

test(
'if custom_lint is present and defines some properties, merges with "include"',
() async {
() {
final analysisOptions = createAnalysisOptions('''
include: ${includeFile.path}
linter:
Expand All @@ -181,15 +179,15 @@ linter:
custom_lint:
enable_all_lint_rules: false
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(configs.enableAllLintRules, false);
expect(configs.rules, includeConfig.rules);
});

test(
'if custom_lint.enable_all_lint_rules is not present, uses value from "include"',
() async {
() {
final included = createAnalysisOptions('''
custom_lint:
enable_all_lint_rules: false
Expand All @@ -200,7 +198,7 @@ custom_lint:
rules:
- a
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(configs.enableAllLintRules, false);
expect(configs.rules, {
Expand All @@ -216,27 +214,42 @@ include: package:$testPackageName/analysis_options.yaml

final tempProjectDir = await createTempProject(dir.path, testPackageName);
await patchPackageConfig(testPackageName, tempProjectDir);
final configs = await CustomLintConfigs.parse(file);
final configs = CustomLintConfigs.parse(file);

expect(configs.rules.containsKey('from_package'), true);
});

test('if package: uri is not resolved default to empty', () async {
const notExistedFileName = 'this-does-not-exist';
const notExistingFileName = 'this-does-not-exist';

final file = createAnalysisOptions('''
include: package:$testPackageName/$notExistedFileName
include: package:$testPackageName/$notExistingFileName
''');
final dir = createDir();

final tempProjectDir = await createTempProject(dir.path, testPackageName);
await patchPackageConfig(testPackageName, tempProjectDir);
final configs = await CustomLintConfigs.parse(file);
final configs = CustomLintConfigs.parse(file);

expect(configs, same(CustomLintConfigs.empty));
});

test('if custom_lint.rules is present, merges with "include"', () async {
test('if package: uri is not valid default to empty', () async {
const notExistingPackage = 'this-package-does-not-exists';

final file = createAnalysisOptions('''
include: package:$notExistingPackage/analysis_options.yaml
''');
final dir = createDir();

final tempProjectDir = await createTempProject(dir.path, testPackageName);
await patchPackageConfig(testPackageName, tempProjectDir);
final configs = CustomLintConfigs.parse(file);

expect(configs, same(CustomLintConfigs.empty));
});

test('if custom_lint.rules is present, merges with "include"', () {
final analysisOptions = createAnalysisOptions('''
include: ${includeFile.path}
linter:
Expand All @@ -252,7 +265,7 @@ custom_lint:
foo: 21
- d
''');
final configs = await CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions);

expect(configs.enableAllLintRules, true);
expect(configs.rules, {
Expand All @@ -267,8 +280,8 @@ custom_lint:
});

group('Handles errors', () {
test('Defaults to empty if yaml fails to parse', () async {
final configs = await CustomLintConfigs.parse(
test('Defaults to empty if yaml fails to parse', () {
final configs = CustomLintConfigs.parse(
createAnalysisOptions('''
foo:
bar:
Expand Down
4 changes: 2 additions & 2 deletions packages/custom_lint_core/test/lint_rule_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ custom_lint:
rules:
- test_lint
''');
final configs = await CustomLintConfigs.parse(analysisOptionFile);
final configs = CustomLintConfigs.parse(analysisOptionFile);

expect(onByDefault.isEnabled(configs), true);
expect(offByDefault.isEnabled(configs), true);
Expand All @@ -119,7 +119,7 @@ custom_lint:
rules:
- test_lint: false
''');
final configs = await CustomLintConfigs.parse(analysisOptionFile);
final configs = CustomLintConfigs.parse(analysisOptionFile);

expect(onByDefault.isEnabled(configs), false);
expect(offByDefault.isEnabled(configs), false);
Expand Down