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 32 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
9 changes: 7 additions & 2 deletions packages/custom_lint_builder/lib/src/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import 'package:custom_lint_core/src/resolver.dart';
import 'package:glob/glob.dart';
import 'package:hotreloader/hotreloader.dart';
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart' show PackageConfig;
import 'package:path/path.dart';
import 'package:pubspec_parse/pubspec_parse.dart';
import 'package:rxdart/subjects.dart';
Expand Down Expand Up @@ -235,11 +236,14 @@ class _CustomLintAnalysisConfigs {

factory _CustomLintAnalysisConfigs.from(
Pubspec pubspecForContext,
PackageConfig packageConfig,
AnalysisContext analysisContext,
CustomLintPluginClient client,
) {
final configs =
CustomLintConfigs.parse(analysisContext.contextRoot.optionsFile);
final configs = CustomLintConfigs.parse(
analysisContext.contextRoot.optionsFile,
packageConfig,
);

final activePluginsForContext = Map.fromEntries(
client._channel.registeredPlugins.entries.where(
Expand Down Expand Up @@ -386,6 +390,7 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin {
for (final pubspecEntry in pubspecs.entries)
pubspecEntry.key: _CustomLintAnalysisConfigs.from(
await pubspecEntry.value,
await parsePackageConfig(io.Directory.current),
pubspecEntry.key,
_client,
),
Expand Down
1 change: 1 addition & 0 deletions packages/custom_lint_builder/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ dependencies:
glob: ^2.1.1
hotreloader: ">=3.0.5 <5.0.0"
meta: ^1.7.0
package_config: ^2.1.0
path: ^1.8.0
pubspec_parse: ^1.2.0
rxdart: ^0.27.7
Expand Down
34 changes: 26 additions & 8 deletions packages/custom_lint_core/lib/src/configs.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:analyzer/file_system/file_system.dart';
import 'package:collection/collection.dart';
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:path/path.dart';
import 'package:yaml/yaml.dart';

Expand All @@ -18,7 +19,10 @@ class CustomLintConfigs {

/// Decode a [CustomLintConfigs] from a file.
@internal
factory CustomLintConfigs.parse(File? analysisOptionsFile) {
factory CustomLintConfigs.parse(
File? analysisOptionsFile,
PackageConfig? packageConfig,
This conversation was marked as resolved.
Show resolved Hide resolved
) {
if (analysisOptionsFile == null || !analysisOptionsFile.exists) {
return CustomLintConfigs.empty;
}
Expand All @@ -35,13 +39,27 @@ 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(
analysisOptionsFile.provider.getFile(includeAbsolutePath),
);
final includeUri = Uri.parse(include);
String? includeAbsolutePath;

if (includeUri.scheme == 'package') {
final packageUri = packageConfig?.resolve(includeUri);
includeAbsolutePath = packageUri?.toFilePath();
} else {
includeAbsolutePath = normalize(
absolute(
analysisOptionsFile.parent.path,
includeUri.toFilePath(),
),
);
}

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

final customLint = yaml['custom_lint'] as Object?;
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 @@ -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 Down
141 changes: 129 additions & 12 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,9 +20,54 @@ File createAnalysisOptions(String content) {
return PhysicalResourceProvider.INSTANCE.getFile(ioFile.path);
}

void main() {
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;
}

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

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

return patchedPackageConfig;
}

void main() async {
late File includeFile;
late CustomLintConfigs includeConfig;

final packageConfig = await findPackageConfig(io.Directory.current);

const testPackageName = 'test_package_with_config';
setUp(() {
includeFile = createAnalysisOptions(
'''
Expand All @@ -35,7 +81,7 @@ custom_lint:
''',
);

includeConfig = CustomLintConfigs.parse(includeFile);
includeConfig = CustomLintConfigs.parse(includeFile, packageConfig);
});

test('Empty config', () {
Expand All @@ -45,13 +91,14 @@ custom_lint:

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

test('if file does not exist, defaults to empty ', () {
final configs = CustomLintConfigs.parse(
PhysicalResourceProvider.INSTANCE.getFile('/this-does-no-exist'),
packageConfig,
);
expect(configs, same(CustomLintConfigs.empty));
});
Expand All @@ -63,7 +110,7 @@ linter:
rules:
public_member_api_docs: false
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions, packageConfig);

expect(configs, includeConfig);
});
Expand All @@ -75,7 +122,7 @@ linter:
rules:
public_member_api_docs: false
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions, packageConfig);

expect(configs, includeConfig);
});
Expand All @@ -89,7 +136,7 @@ linter:

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

expect(configs, includeConfig);
});
Expand All @@ -100,7 +147,7 @@ custom_lint:
rules:
- a
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions, packageConfig);

expect(
configs.rules,
Expand All @@ -125,7 +172,7 @@ linter:
custom_lint:
enable_all_lint_rules: false
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions, packageConfig);

expect(configs.enableAllLintRules, false);
expect(configs.rules, includeConfig.rules);
Expand All @@ -144,14 +191,81 @@ custom_lint:
rules:
- a
''');
final configs = CustomLintConfigs.parse(analysisOptions);
final configs = CustomLintConfigs.parse(analysisOptions, packageConfig);

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

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);
if (packageConfig == null) {
throw Exception('Package config is not loaded');
}

final patchedPackageConfig = patchPackageConfig(
packageConfig,
testPackageName,
tempProjectDir,
);
final configs = CustomLintConfigs.parse(file, patchedPackageConfig);

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

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

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

final tempProjectDir = await createTempProject(dir.path, testPackageName);

if (packageConfig == null) {
throw Exception('Package config is not loaded');
}
final patchedPackageConfig = patchPackageConfig(
packageConfig,
testPackageName,
tempProjectDir,
);
final configs = CustomLintConfigs.parse(file, patchedPackageConfig);

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

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);

if (packageConfig == null) {
throw Exception('Package config is not loaded');
}
final patchedPackageConfig = patchPackageConfig(
packageConfig,
testPackageName,
tempProjectDir,
);
final configs = CustomLintConfigs.parse(file, patchedPackageConfig);

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

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

expect(configs.enableAllLintRules, true);
expect(configs.rules, {
Expand All @@ -185,11 +299,14 @@ custom_lint:
group('Handles errors', () {
test('Defaults to empty if yaml fails to parse', () {
final configs = CustomLintConfigs.parse(
createAnalysisOptions('''
createAnalysisOptions(
'''
foo:
bar:
baz:
'''),
''',
),
packageConfig,
);
expect(configs, CustomLintConfigs.empty);
});
Expand Down
12 changes: 9 additions & 3 deletions packages/custom_lint_core/test/lint_rule_test.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import 'dart:io';

import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:analyzer/error/listener.dart';
import 'package:custom_lint_core/custom_lint_core.dart';
import 'package:package_config/package_config.dart';
import 'package:test/test.dart';

import 'assist_test.dart';
Expand Down Expand Up @@ -51,9 +54,10 @@ class MyLintRule extends DartLintRule {
}
}

void main() {
void main() async {
const onByDefault = TestLintRule(enabledByDefault: true);
const offByDefault = TestLintRule(enabledByDefault: false);
final packageConfig = await findPackageConfig(Directory.current);

test('LintRule.testRun', () async {
const assist = MyLintRule();
Expand Down Expand Up @@ -107,7 +111,8 @@ custom_lint:
rules:
- test_lint
''');
final configs = CustomLintConfigs.parse(analysisOptionFile);
final configs =
CustomLintConfigs.parse(analysisOptionFile, packageConfig);

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

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