fix/nuwayze-dogfood-findings #11
6 changed files with 202 additions and 8 deletions
fix(queries): resolve a subdirectory path to its enclosing package
Found by dogfooding on nuwayze: `pipe queries lib` from the repo root
failed with
no resolved package graph at .dart_tool/package_config.json
remedy: run `flutter pub get` or `dart pub get` in .../nuwayze-w3/lib
It failed even with the package resolved, because the path was taken
as the project root: pipe looked for lib/.dart_tool/package_config.json,
which never exists, and the remedy sent the user to run pub get in a
directory that has no pubspec.
enclosingPackage() (project.dart) splits a path into the package that
contains it (the path itself when it holds a pubspec.yaml, else the
nearest ancestor that does) and the directory it names under that
root. `pipe queries` loads the package root and scans only that
directory, so `pipe queries lib/services` is `--dir lib/services` on
the package; an explicit --dir with such a path is relative to it.
Without a resolved graph the error now names the package root.
loadProject's own remedy uses the same lookup, so `pipe check lib`
(and the other commands, which still take the path as the root) now
says the path is inside the package at <root> and suggests passing
that with `--dir lib`, instead of pub get in lib.
Tests: enclosingPackage on a root, a nested directory, a nested
package and a missing path; loadProject's remedy for a directory
inside a package; and `pipe queries` on the fixture's lib/data
(root reported as the package, one file discovered), with
`lib --dir data`, and on an unresolved package where the remedy names
the root, not lib.
On nuwayze-w3: `pipe queries lib` exits 0 with 313 queries over 847
files, identical to `pipe queries .`; `pipe queries lib/services` scans
161 files and every one of its 212 queries sits under lib/services.
233 tests pass.
commit
31607b5bcb
|
|
@ -6,6 +6,11 @@ findings, no baseline, and deliberately no index analysis. Deciding whether a
|
|||
query needs a composite index, and probing a live project, belong to the
|
||||
consumer of this output, which can compare it against `firestore.indexes.json`.
|
||||
|
||||
The path may be a package root or a directory inside one: `pipe queries lib/data`
|
||||
resolves the package that contains `lib/data` (the nearest `pubspec.yaml`) and
|
||||
scans only that directory, as `--dir lib/data` would. `--dir` given with such a
|
||||
path is relative to it.
|
||||
|
||||
What it recognises:
|
||||
|
||||
- **cloud_firestore chains.** Receivers are identified by **static type and
|
||||
|
|
|
|||
|
|
@ -46,7 +46,8 @@ export 'src/facts.dart';
|
|||
export 'src/provider_rules.dart' show providerRuleNote;
|
||||
export 'src/recall.dart'
|
||||
show RecallCase, RecallOutcome, RecallReport, loadRecallCases, runRecall;
|
||||
export 'src/project.dart' show Project, loadProject;
|
||||
export 'src/project.dart'
|
||||
show PackageTarget, Project, enclosingPackage, loadProject;
|
||||
export 'src/sdk.dart' show SdkResolution, resolveSdk;
|
||||
export 'src/rules.dart'
|
||||
show
|
||||
|
|
|
|||
|
|
@ -337,9 +337,18 @@ class GraphCommand extends Command<int> {
|
|||
@override
|
||||
Future<int> run() async {
|
||||
final results = argResults!;
|
||||
// `pipe queries lib` means "the queries under lib of this package": the
|
||||
// package graph lives at the package root, and the path is the scope.
|
||||
final target = enclosingPackage(_target(results));
|
||||
final dirs = results['dir'] as List<String>;
|
||||
final scope = target.scope;
|
||||
final project = await loadProject(
|
||||
_target(results),
|
||||
directories: results['dir'] as List<String>,
|
||||
target.root,
|
||||
directories: scope == null
|
||||
? dirs
|
||||
: results.wasParsed('dir')
|
||||
? [for (final dir in dirs) p.join(scope, dir)]
|
||||
: [scope],
|
||||
);
|
||||
final run = await resolveProject(
|
||||
project,
|
||||
|
|
@ -393,9 +402,18 @@ class StateCommand extends Command<int> {
|
|||
@override
|
||||
Future<int> run() async {
|
||||
final results = argResults!;
|
||||
// `pipe queries lib` means "the queries under lib of this package": the
|
||||
// package graph lives at the package root, and the path is the scope.
|
||||
final target = enclosingPackage(_target(results));
|
||||
final dirs = results['dir'] as List<String>;
|
||||
final scope = target.scope;
|
||||
final project = await loadProject(
|
||||
_target(results),
|
||||
directories: results['dir'] as List<String>,
|
||||
target.root,
|
||||
directories: scope == null
|
||||
? dirs
|
||||
: results.wasParsed('dir')
|
||||
? [for (final dir in dirs) p.join(scope, dir)]
|
||||
: [scope],
|
||||
);
|
||||
final run = await resolveProject(
|
||||
project,
|
||||
|
|
@ -1009,9 +1027,18 @@ class QueriesCommand extends Command<int> {
|
|||
final adapters = configPath == null
|
||||
? const <QueryAdapter>[]
|
||||
: parseAdapterConfig(_readText(configPath, 'adapter config'));
|
||||
// `pipe queries lib` means "the queries under lib of this package": the
|
||||
// package graph lives at the package root, and the path is the scope.
|
||||
final target = enclosingPackage(_target(results));
|
||||
final dirs = results['dir'] as List<String>;
|
||||
final scope = target.scope;
|
||||
final project = await loadProject(
|
||||
_target(results),
|
||||
directories: results['dir'] as List<String>,
|
||||
target.root,
|
||||
directories: scope == null
|
||||
? dirs
|
||||
: results.wasParsed('dir')
|
||||
? [for (final dir in dirs) p.join(scope, dir)]
|
||||
: [scope],
|
||||
);
|
||||
final run = await resolveProject(
|
||||
project,
|
||||
|
|
|
|||
|
|
@ -56,6 +56,40 @@ bool _isHidden(String path, String root) {
|
|||
.any((segment) => segment.startsWith('.') && segment != '.');
|
||||
}
|
||||
|
||||
/// A path split into the package that contains it and the directory under
|
||||
/// that package's root it names.
|
||||
class PackageTarget {
|
||||
const PackageTarget(this.root, this.scope);
|
||||
|
||||
/// The package root: the path itself when it holds a `pubspec.yaml`, else
|
||||
/// the nearest ancestor that does, else the path unchanged.
|
||||
final String root;
|
||||
|
||||
/// The path relative to [root] when it named a directory inside the
|
||||
/// package (`lib`, `lib/data`), or null when it named the root itself.
|
||||
final String? scope;
|
||||
}
|
||||
|
||||
/// The package enclosing [inputPath].
|
||||
///
|
||||
/// `pipe queries lib`, run from a package root, used to treat `lib` as the
|
||||
/// project: it looked for `lib/.dart_tool/package_config.json` and told the
|
||||
/// user to run `pub get` in `lib`. The package graph and the remedy belong to
|
||||
/// the package root, and `lib` is only the scope within it.
|
||||
PackageTarget enclosingPackage(String inputPath) {
|
||||
final path = p.normalize(p.absolute(inputPath));
|
||||
if (!Directory(path).existsSync() ||
|
||||
File(p.join(path, 'pubspec.yaml')).existsSync()) {
|
||||
return PackageTarget(path, null);
|
||||
}
|
||||
for (var dir = p.dirname(path); ; dir = p.dirname(dir)) {
|
||||
if (File(p.join(dir, 'pubspec.yaml')).existsSync()) {
|
||||
return PackageTarget(dir, p.relative(path, from: dir));
|
||||
}
|
||||
if (p.dirname(dir) == dir) return PackageTarget(path, null);
|
||||
}
|
||||
}
|
||||
|
||||
/// Load and validate a target project without touching the filesystem.
|
||||
///
|
||||
/// Fails loudly when the project has not been resolved, because analyzing an
|
||||
|
|
@ -74,9 +108,16 @@ Future<Project> loadProject(
|
|||
p.join(root, '.dart_tool', 'package_config.json'),
|
||||
);
|
||||
if (!packageConfigFile.existsSync()) {
|
||||
// A directory inside a package has no graph of its own; resolving there
|
||||
// would not help, so point at the package that owns it.
|
||||
final package = enclosingPackage(root);
|
||||
throw PipeException(
|
||||
'no resolved package graph at .dart_tool/package_config.json',
|
||||
remedy: 'run `flutter pub get` or `dart pub get` in $root',
|
||||
remedy: package.scope == null
|
||||
? 'run `flutter pub get` or `dart pub get` in $root'
|
||||
: '$root is inside the package at ${package.root}: pass that path '
|
||||
'(with `--dir ${package.scope}` to narrow the scan), and run '
|
||||
'`flutter pub get` or `dart pub get` there if it has not been',
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -602,5 +602,61 @@ adapters:
|
|||
expect(strict.exitCode, 2);
|
||||
expect(strict.stderr as String, contains('--require-firestore'));
|
||||
});
|
||||
|
||||
test('a directory inside the package is a scope, not a project', () async {
|
||||
// `pipe queries lib` looked for lib/.dart_tool/package_config.json and
|
||||
// failed. The graph is the package's; the path only narrows the scan.
|
||||
final result = await pipe(['$fixturePath/lib/data']);
|
||||
expect(result.exitCode, 0, reason: '${result.stderr}');
|
||||
final json = jsonDecode(result.stdout as String) as Map<String, Object?>;
|
||||
expect(json['root'], fixturePath);
|
||||
final coverage = json['coverage'] as Map<String, Object?>;
|
||||
expect(coverage['discovered'], 1);
|
||||
expect(coverage['complete'], isTrue);
|
||||
// lib/data holds only the gateway, so the queries the rest of lib issues
|
||||
// are out of scope.
|
||||
final sites = [
|
||||
for (final query in json['queries'] as List<Object?>)
|
||||
(query as Map<String, Object?>)['site'] as String,
|
||||
];
|
||||
expect(sites, everyElement(startsWith('lib/data/')));
|
||||
expect(model.queries, isNotEmpty);
|
||||
expect(json['unresolvedFirestoreUris'], isEmpty);
|
||||
});
|
||||
|
||||
test('--dir under a scoped path is relative to that path', () async {
|
||||
final result = await pipe(['$fixturePath/lib', '--dir', 'data']);
|
||||
expect(result.exitCode, 0, reason: '${result.stderr}');
|
||||
final json = jsonDecode(result.stdout as String) as Map<String, Object?>;
|
||||
expect((json['coverage'] as Map)['discovered'], 1);
|
||||
});
|
||||
|
||||
test('an unresolved package is reported at its root, not the scope', () {
|
||||
final copy = Directory.systemTemp.createTempSync('pipe-queries-fresh');
|
||||
try {
|
||||
File('${copy.path}/pubspec.yaml').writeAsStringSync('name: fresh\n');
|
||||
File('${copy.path}/lib/fresh.dart')
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync('void main() {}\n');
|
||||
final root = copy.resolveSymbolicLinksSync();
|
||||
expect(
|
||||
() => PipeRunner().run(['queries', '$root/lib']),
|
||||
throwsA(
|
||||
isA<PipeException>()
|
||||
.having((e) => e.message, 'message', contains('package graph'))
|
||||
.having(
|
||||
(e) => e.remedy,
|
||||
'remedy',
|
||||
allOf(
|
||||
endsWith('pub get` in $root'),
|
||||
isNot(contains('$root/lib')),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
} finally {
|
||||
copy.deleteSync(recursive: true);
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
64
test/package_target_test.dart
Normal file
64
test/package_target_test.dart
Normal file
|
|
@ -0,0 +1,64 @@
|
|||
import 'dart:io';
|
||||
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:pipe/pipe.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
void main() {
|
||||
late String root;
|
||||
|
||||
setUp(() {
|
||||
root = Directory.systemTemp
|
||||
.createTempSync('pipe-package-target')
|
||||
.resolveSymbolicLinksSync();
|
||||
File(p.join(root, 'pubspec.yaml')).writeAsStringSync('name: outer\n');
|
||||
Directory(p.join(root, 'lib', 'data')).createSync(recursive: true);
|
||||
File(p.join(root, 'lib', 'data', 'a.dart')).writeAsStringSync('');
|
||||
File(p.join(root, 'nested', 'pubspec.yaml'))
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync('name: nested\n');
|
||||
Directory(p.join(root, 'nested', 'lib')).createSync();
|
||||
});
|
||||
|
||||
tearDown(() => Directory(root).deleteSync(recursive: true));
|
||||
|
||||
group('enclosingPackage', () {
|
||||
test('a package root is its own package, with no scope', () {
|
||||
final target = enclosingPackage(root);
|
||||
expect(target.root, root);
|
||||
expect(target.scope, isNull);
|
||||
});
|
||||
|
||||
test('a directory inside a package resolves to the package root', () {
|
||||
final target = enclosingPackage(p.join(root, 'lib', 'data'));
|
||||
expect(target.root, root);
|
||||
expect(target.scope, p.join('lib', 'data'));
|
||||
});
|
||||
|
||||
test('the nearest pubspec wins', () {
|
||||
final target = enclosingPackage(p.join(root, 'nested', 'lib'));
|
||||
expect(target.root, p.join(root, 'nested'));
|
||||
expect(target.scope, 'lib');
|
||||
});
|
||||
|
||||
test('a missing path is returned unchanged for loadProject to reject', () {
|
||||
final missing = p.join(root, 'nope');
|
||||
expect(enclosingPackage(missing).root, missing);
|
||||
expect(enclosingPackage(missing).scope, isNull);
|
||||
});
|
||||
});
|
||||
|
||||
test('loadProject on a directory inside a package names the package', () {
|
||||
// `pipe check lib` used to say "run `dart pub get` in .../lib".
|
||||
expect(
|
||||
() => loadProject(p.join(root, 'lib')),
|
||||
throwsA(
|
||||
isA<PipeException>().having(
|
||||
(e) => e.remedy,
|
||||
'remedy',
|
||||
allOf(contains('inside the package at $root'), contains('--dir lib')),
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue