Skip to content

First pass at strict dependencies #1508

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

Merged
merged 6 commits into from
Jan 30, 2017
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
6 changes: 6 additions & 0 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -786,3 +786,9 @@ void fail(String message, [innerError, StackTrace innerTrace]) {
///
/// This will report the error and cause pub to exit with [exit_codes.DATA].
void dataError(String message) => throw new DataException(message);

/// Returns an iterable that contains the elements of [iter1] followed by those
/// of [iter2].
Iterable/*<T>*/ combineIterables/*<T>*/(
Iterable/*<T>*/ iter1, Iterable/*<T>*/ iter2) =>
iter1.toList()..addAll(iter2);
159 changes: 159 additions & 0 deletions lib/src/validator/strict_dependencies.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';

import 'package:analyzer/analyzer.dart';
import 'package:path/path.dart' as p;
import 'package:pub/src/dart.dart';
import 'package:pub/src/entrypoint.dart';
import 'package:pub/src/io.dart';
import 'package:pub/src/log.dart' as log;
import 'package:pub/src/utils.dart';
import 'package:pub/src/validator.dart';
import 'package:source_span/source_span.dart';
import 'package:stack_trace/stack_trace.dart';

/// Validates that Dart source files only import declared dependencies.
class StrictDependenciesValidator extends Validator {
Copy link
Member

Choose a reason for hiding this comment

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

Document this, and also the class below and its members.

StrictDependenciesValidator(Entrypoint entrypoint) : super(entrypoint);

/// Lazily returns all dependency uses in [files].
///
/// Files that do not parse and directives that don't import or export
/// `package:` URLs are ignored.
Iterable<_Usage> _findPackages(Iterable<String> files) sync* {
for (var file in files) {
List<UriBasedDirective> directives;
var contents = readTextFile(file);
try {
directives = parseImportsAndExports(contents, name: file);
} on AnalyzerErrorGroup catch (e, s) {
// Ignore files that do not parse.
log.fine(getErrorMessage(e));
log.fine(new Chain.forTrace(s).terse);
continue;
}

for (var directive in directives) {
Uri url;
try {
url = Uri.parse(directive.uri.stringValue);
} on FormatException catch (_) {
// Ignore a format exception. [url] will be null, and we'll emit an
// "Invalid URL" warning below.
}

// If the URL could not be parsed or it is a `package:` URL AND there
// are no segments OR any segment are empty, it's invalid.
if (url == null ||
(url.scheme == 'package' &&
(url.pathSegments.length < 2 ||
url.pathSegments.any((s) => s.isEmpty)))) {
warnings.add(_Usage.errorMessage(
'Invalid URL.', file, contents, directive));
} else if (url.scheme == 'package') {
yield new _Usage(file, contents, directive, url);
}
}
}
}

Future validate() async {
var dependencies = entrypoint.root.dependencies
.map((d) => d.name)
.toSet()
..add(entrypoint.root.name);
var devDependencies = entrypoint.root.devDependencies
.map((d) => d.name)
.toSet();
_validateLibBin(dependencies, devDependencies);
_validateTestTool(dependencies, devDependencies);
}

/// Validates that no Dart files in `lib/` or `bin/` have dependencies that
/// aren't in [deps].
///
/// The [devDeps] are used to generate special warnings for files that import
/// dev dependencies.
void _validateLibBin(Set<String> deps, Set<String> devDeps) {
var libFiles = entrypoint.root.listFiles(beneath: 'lib').where(_isDart);
var binFiles = entrypoint.root.listFiles(beneath: 'bin').where(_isDart);
for (var usage in _findPackages(combineIterables(libFiles, binFiles))) {
if (!deps.contains(usage.package)) {
if (devDeps.contains(usage.package)) {
warnings.add(usage.dependencyMisplaceMessage());
} else {
warnings.add(usage.dependencyMissingMessage());
}
}
}
}

/// Validates that no Dart files in `test/` or `tool/` have dependencies that
/// aren't in [deps] or [devDeps].
void _validateTestTool(Set<String> deps, Set<String> devDeps) {
var testFiles = entrypoint.root.listFiles(beneath: 'test').where(_isDart);
var toolFiles = entrypoint.root.listFiles(beneath: 'tool').where(_isDart);
for (var usage in _findPackages(combineIterables(testFiles, toolFiles))) {
if (!deps.contains(usage.package) && !devDeps.contains(usage.package)) {
warnings.add(usage.dependencyMissingMessage());
}
}
}

/// Returns whether [file] is a Dart file.
bool _isDart(String file) => p.extension(file) == '.dart';
}

/// A parsed import or export directive in a D source file.
class _Usage {
/// Returns a formatted error message highlighting [directive] in [file].
static String errorMessage(
String message,
String file,
String contents,
UriBasedDirective directive) {
return new SourceFile(contents, url: file)
.span(directive.offset, directive.offset + directive.length)
.message(message);
}

/// The path to the file from which [_directive] was parsed.
final String _file;

/// The contents of [_file].
final String _contents;

/// The URI parsed from [_directive].
final Uri _url;

/// The directive that uses [_url].
final UriBasedDirective _directive;

_Usage(this._file, this._contents, this._directive, this._url);

/// The name of the package referred to by this usage..
String get package => _url.pathSegments.first;

/// Returns a message associated with [_directive].
///
/// We assume that normally all directives are valid and we won't see an error
/// message, so we create the SourceFile lazily to avoid parsing line endings
/// in the case of only valid directives.
String _toMessage(String message) =>
errorMessage(message, _file, _contents, _directive);

/// Returns an error message saying the package is not listed in dependencies.
String dependencyMissingMessage() =>
_toMessage("This package doesn't depend on $package.");

/// Returns an error message saying the package should be in `dependencies`.
String dependencyMisplaceMessage() {
var shortFile = p.split(p.relative(_file)).first;
return _toMessage(
'$package is a dev dependency. Packages used in $shortFile/ must be '
'declared as normal dependencies.');
}
}
7 changes: 5 additions & 2 deletions test/descriptor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ Descriptor appPubspec([Map dependencies]) {
/// Describes a file named `pubspec.yaml` for a library package with the given
/// [name], [version], and [deps]. If "sdk" is given, then it adds an SDK
/// constraint on that version.
Descriptor libPubspec(String name, String version, {Map deps, String sdk}) {
var map = packageMap(name, version, deps);
Descriptor libPubspec(String name, String version, {
Map deps,
Map devDeps,
String sdk}) {
var map = packageMap(name, version, deps, devDeps);
if (sdk != null) map["environment"] = {"sdk": sdk};
return pubspec(map);
}
Expand Down
6 changes: 4 additions & 2 deletions test/test_pub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,9 @@ void useMockClient(MockClient client) {

/// Describes a map representing a library package with the given [name],
/// [version], and [dependencies].
Map packageMap(String name, String version, [Map dependencies]) {
Map packageMap(String name, String version, [
Map dependencies,
Map devDependencies,]) {
var package = <String, dynamic>{
"name": name,
"version": version,
Expand All @@ -609,7 +611,7 @@ Map packageMap(String name, String version, [Map dependencies]) {
};

if (dependencies != null) package["dependencies"] = dependencies;

if (devDependencies != null) package["dev_dependencies"] = devDependencies;
return package;
}

Expand Down
Loading