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

Add Support for 'package:' configs #205

Merged
merged 35 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8c53401
Add support for including 'package:' configs
Nov 23, 2023
98931bc
Dart SDK -> 3.2.0
Nov 23, 2023
d7acbd7
removed unnecessary check
Nov 23, 2023
408cbdc
isAbsolute check
Nov 23, 2023
0348204
Revert "Dart SDK -> 3.2.0"
Nov 23, 2023
96f3d2f
added test
Nov 23, 2023
1caf5aa
converted to async
Nov 24, 2023
2ec8d91
added test
Nov 24, 2023
e37117c
Merge pull request #1 from solid-software/add-support-for-package-con…
Nov 24, 2023
0259795
applied suggestions
Nov 24, 2023
40f7ffa
moved constant declaration
Nov 27, 2023
249aa75
Merge pull request #2 from solid-software/add-support-for-package-con…
Nov 27, 2023
489d744
Merge branch 'main' into main
illia-romanenko Dec 4, 2023
5a00e1a
applied suggestions
Dec 5, 2023
7c03f04
applied suggestions
Dec 5, 2023
0d44882
refactored previous approach
Dec 5, 2023
666da2a
added normalize
Dec 5, 2023
c8179f1
includeUri.toFilePath()
Dec 5, 2023
818a8c6
moved to variable
Dec 5, 2023
39af4e3
Merge pull request #3 from solid-software/applied-suggestions
Dec 5, 2023
df029bd
Using built-in package-config parser (#4)
Dec 7, 2023
ee12bf4
rm unnecessary async
Dec 7, 2023
f9cc496
Merge branch 'main' into main
illia-romanenko Dec 8, 2023
f03badf
Merge branch 'main' into main
illia-romanenko Dec 18, 2023
c8b0414
Update packages/custom_lint_core/lib/src/configs.dart
Jan 5, 2024
5c3f9aa
packageConfig as parameter
Jan 8, 2024
37b648a
parse package config in constructor
Jan 8, 2024
d329adf
revert
Jan 8, 2024
b6400a5
pass packageConfig directly
Jan 8, 2024
ffd5585
nullable parameter
Jan 8, 2024
db40222
rm nullable parameter
Jan 8, 2024
4784201
reverted nullable parameter
Jan 8, 2024
0909f51
using package_utils in tests
Jan 8, 2024
c0d820f
rm ?
Jan 8, 2024
bf493b1
rm unnecessary null checks
Jan 8, 2024
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
8 changes: 4 additions & 4 deletions packages/custom_lint_builder/lib/src/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,13 @@ class _CustomLintAnalysisConfigs {
this.pubspec,
);

factory _CustomLintAnalysisConfigs.from(
static Future<_CustomLintAnalysisConfigs> from(
Pubspec pubspecForContext,
AnalysisContext analysisContext,
CustomLintPluginClient client,
) {
) async {
final configs =
CustomLintConfigs.parse(analysisContext.contextRoot.optionsFile);
await CustomLintConfigs.parse(analysisContext.contextRoot.optionsFile);

final activePluginsForContext = Map.fromEntries(
client._channel.registeredPlugins.entries.where(
Expand Down Expand Up @@ -384,7 +384,7 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin {

_customLintConfigsForAnalysisContexts = {
for (final pubspecEntry in pubspecs.entries)
pubspecEntry.key: _CustomLintAnalysisConfigs.from(
pubspecEntry.key: await _CustomLintAnalysisConfigs.from(
await pubspecEntry.value,
pubspecEntry.key,
_client,
Expand Down
22 changes: 16 additions & 6 deletions packages/custom_lint_core/lib/src/configs.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:isolate';

import 'package:analyzer/file_system/file_system.dart';
import 'package:collection/collection.dart';
import 'package:meta/meta.dart';
Expand All @@ -18,7 +20,7 @@ class CustomLintConfigs {

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

final includeAbsolutePath = includeUri.isAbsolute
? includeUri.toFilePath()
: normalize(
absolute(
analysisOptionsFile.parent.path,
includeUri.path,
),
);
This conversation was marked as resolved.
Show resolved Hide resolved

includedOptions = await CustomLintConfigs.parse(
analysisOptionsFile.provider.getFile(includeAbsolutePath),
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/custom_lint_core/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ dev_dependencies:
build_runner: ^2.3.3
lint_visitor_generator:
path: ../lint_visitor_generator
package_config: ^2.1.0
test: ^1.22.2
128 changes: 106 additions & 22 deletions packages/custom_lint_core/test/configs_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:io' as io;
import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/file_system/physical_file_system.dart';
import 'package:custom_lint_core/custom_lint_core.dart';
import 'package:package_config/package_config.dart';
import 'package:path/path.dart';
import 'package:test/test.dart';

Expand All @@ -19,10 +20,63 @@ File createAnalysisOptions(String content) {
return PhysicalResourceProvider.INSTANCE.getFile(ioFile.path);
}

Future<String> createTempProject(String projectName, String tempDirPath) async {
final projectPath = join(tempDirPath, projectName);

final dir = io.Directory(projectPath);
await dir.create();

final libPath = join(dir.path, 'lib');
await io.Directory(libPath).create();

final analysisOptionsPath = join(libPath, 'analysis_options.yaml');
await io.File(analysisOptionsPath).writeAsString('''
custom_lint:
rules:
- from_package
''');

return dir.path;
}

Future<void> patchPackageConfig(
String packageName,
String packagePath,
) async {
final currentPackageConfig = await findPackageConfig(io.Directory.current);
if (currentPackageConfig == null) {
throw Exception('Could not find package config');
}

final patchedPackages = currentPackageConfig.packages.toList()
..add(
Package(
packageName,
Uri.file('$packagePath/'),
packageUriRoot: Uri.parse('lib/'),
),
);

final patchedPackageConfig = PackageConfig(
patchedPackages,
extraData: currentPackageConfig.extraData,
);

await savePackageConfig(
patchedPackageConfig,
io.Directory.current,
);

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

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

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

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

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

test('if file does not exist, defaults to empty ', () {
final configs = CustomLintConfigs.parse(
test('if file does not exist, defaults to empty ', () async {
final configs = await 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"', () {
test('if custom_lint not present in the option file, clones "include"',
() async {
final analysisOptions = createAnalysisOptions('''
include: ${includeFile.path}
linter:
rules:
public_member_api_docs: false
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = await CustomLintConfigs.parse(analysisOptions);

expect(configs, includeConfig);
});

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

expect(configs, includeConfig);
});

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

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

expect(configs, includeConfig);
});

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

expect(
configs.rules,
Expand All @@ -115,7 +171,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 @@ -125,15 +181,15 @@ linter:
custom_lint:
enable_all_lint_rules: false
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = await 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 @@ -144,15 +200,43 @@ custom_lint:
rules:
- a
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = await CustomLintConfigs.parse(analysisOptions);

expect(configs.enableAllLintRules, false);
expect(configs.rules, {
'a': const LintOptions.empty(enabled: true),
});
});

test('if custom_lint.rules is present, merges with "include"', () {
test('include config using package: uri', () async {
final dir = createDir();
final file = createAnalysisOptions('''
include: package:$testPackageName/analysis_options.yaml
''');

final tempProjectDir = await createTempProject(dir.path, testPackageName);
await patchPackageConfig(testPackageName, tempProjectDir);
final configs = await 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';

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

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

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

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

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

group('Handles errors', () {
test('Defaults to empty if yaml fails to parse', () {
final configs = CustomLintConfigs.parse(
test('Defaults to empty if yaml fails to parse', () async {
final configs = await CustomLintConfigs.parse(
createAnalysisOptions('''
foo:
bar:
Expand Down
8 changes: 4 additions & 4 deletions packages/custom_lint_core/test/lint_rule_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -101,25 +101,25 @@ void main() {
expect(offByDefault.isEnabled(CustomLintConfigs.empty), false);
});

test('always enabled if on in the config files', () {
test('always enabled if on in the config files', () async {
final analysisOptionFile = createAnalysisOptions('''
custom_lint:
rules:
- test_lint
''');
final configs = CustomLintConfigs.parse(analysisOptionFile);
final configs = await CustomLintConfigs.parse(analysisOptionFile);

expect(onByDefault.isEnabled(configs), true);
expect(offByDefault.isEnabled(configs), true);
});

test('always disabled if off in the config files', () {
test('always disabled if off in the config files', () async {
final analysisOptionFile = createAnalysisOptions('''
custom_lint:
rules:
- test_lint: false
''');
final configs = CustomLintConfigs.parse(analysisOptionFile);
final configs = await CustomLintConfigs.parse(analysisOptionFile);

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