Commit 2c4ec1c9 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Avoid self-referential imports. (#11045)

And add a test to verify we don't do this again.
parent f02e9acc
......@@ -53,6 +53,8 @@ Future<Null> _generateDocs() async {
}
Future<Null> _analyzeRepo() async {
await _verifyNoBadImports(flutterRoot);
// Analyze all the Dart code in the repo.
await _runFlutterAnalyze(flutterRoot,
options: <String>['--flutter-repo'],
......@@ -241,6 +243,132 @@ Future<Null> _runFlutterAnalyze(String workingDirectory, {
);
}
Future<Null> _verifyNoBadImports(String workingDirectory) async {
final List<String> errors = <String>[];
final String libPath = path.join(workingDirectory, 'packages', 'flutter', 'lib');
final String srcPath = path.join(workingDirectory, 'packages', 'flutter', 'lib', 'src');
// Verify there's one libPath/*.dart for each srcPath/*/.
<String>[];
final List<String> packages = new Directory(libPath).listSync()
.where((FileSystemEntity entity) => entity is File && path.extension(entity.path) == '.dart')
.map<String>((FileSystemEntity entity) => path.basenameWithoutExtension(entity.path))
.toList()..sort();
final List<String> directories = new Directory(srcPath).listSync()
.where((FileSystemEntity entity) => entity is Directory)
.map<String>((FileSystemEntity entity) => path.basename(entity.path))
.toList()..sort();
if (!_matches(packages, directories)) {
errors.add(
'flutter/lib/*.dart does not match flutter/lib/src/*/:\n'
'These are the exported packages:\n' +
packages.map((String path) => ' lib/$path.dart').join('\n') +
'These are the directories:\n' +
directories.map((String path) => ' lib/src/$path/').join('\n')
);
}
// Verify that the imports are well-ordered.
final Map<String, Set<String>> dependencyMap = new Map<String, Set<String>>.fromIterable(
directories,
key: (String directory) => directory,
value: (String directory) => _findDependencies(path.join(srcPath, directory), errors, checkForMeta: directory != 'foundation'),
);
for (String package in dependencyMap.keys) {
if (dependencyMap[package].contains(package)) {
errors.add(
'One of the files in the $yellow$package$reset package imports that package recursively.'
);
}
}
for (String package in dependencyMap.keys) {
final List<String> loop = _deepSearch(dependencyMap, package);
if (loop != null) {
errors.add(
'${yellow}Dependency loop:$reset ' +
loop.join(' depends on ')
);
}
}
// Fail if any errors
if (errors.isNotEmpty) {
print('$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset');
if (errors.length == 1) {
print('${bold}An error was detected when looking at import dependencies within the Flutter package:$reset\n');
} else {
print('${bold}Multiple errors were detected when looking at import dependencies within the Flutter package:$reset\n');
}
print(errors.join('\n\n'));
print('$red━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$reset\n');
exit(1);
}
}
bool _matches<T>(List<T> a, List<T> b) {
assert(a != null);
assert(b != null);
if (a.length != b.length)
return false;
for (int index = 0; index < a.length; index += 1) {
if (a[index] != b[index])
return false;
}
return true;
}
final RegExp _importPattern = new RegExp(r"import 'package:flutter/([^.]+)\.dart'");
final RegExp _importMetaPattern = new RegExp(r"import 'package:meta/meta.dart'");
Set<String> _findDependencies(String srcPath, List<String> errors, { bool checkForMeta: false }) {
return new Directory(srcPath).listSync().where((FileSystemEntity entity) {
return entity is File && path.extension(entity.path) == '.dart';
}).map<Set<String>>((FileSystemEntity entity) {
final Set<String> result = new Set<String>();
final File file = entity;
for (String line in file.readAsLinesSync()) {
Match match = _importPattern.firstMatch(line);
if (match != null)
result.add(match.group(1));
if (checkForMeta) {
match = _importMetaPattern.firstMatch(line);
if (match != null) {
errors.add(
'${file.path}\nThis package imports the ${yellow}meta$reset package.\n'
'You should instead import the "foundation.dart" library.'
);
}
}
}
return result;
}).reduce((Set<String> value, Set<String> element) {
value ??= new Set<String>();
value.addAll(element);
return value;
});
}
List<T> _deepSearch<T>(Map<T, Set<T>> map, T start, [ Set<T> seen ]) {
for (T key in map[start]) {
if (key == start)
continue; // we catch these separately
if (seen != null && seen.contains(key))
return <T>[start, key];
final List<T> result = _deepSearch(
map,
key,
(seen == null ? new Set<T>.from(<T>[start]) : new Set<T>.from(seen))..add(key),
);
if (result != null) {
result.insert(0, start);
// Only report the shortest chains.
// For example a->b->a, rather than c->a->b->a.
// Since we visit every node, we know the shortest chains are those
// that start and end on the loop.
if (result.first == result.last)
return result;
}
}
return null;
}
void _printProgress(String action, String workingDir, String command) {
const String arrow = '⏩';
print('$arrow $action: cd $cyan$workingDir$reset; $yellow$command$reset');
......
......@@ -14,7 +14,8 @@ export 'package:meta/meta.dart' show
mustCallSuper,
optionalTypeArgs,
protected,
required;
required,
visibleForTesting;
// Examples can assume:
// String _name;
......
......@@ -4,10 +4,11 @@
import 'dart:async';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
import 'theme.dart';
/// Provides platform-specific acoustic and/or haptic feedback for certain
/// actions.
///
......
......@@ -5,8 +5,12 @@
import 'dart:ui' show lerpDouble;
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'material.dart';
import 'shadows.dart';
import 'theme.dart';
/// The base type for [MaterialSlice] and [MaterialGap].
///
......
......@@ -3,9 +3,14 @@
// found in the LICENSE file.
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/widgets.dart';
import 'colors.dart';
import 'debug.dart';
import 'drawer_header.dart';
import 'icons.dart';
import 'ink_well.dart';
import 'theme.dart';
class _AccountPictures extends StatelessWidget {
const _AccountPictures({
......
......@@ -2,8 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter/foundation.dart';
import 'package:flutter/painting.dart';
import 'package:flutter/rendering.dart';
import 'package:vector_math/vector_math_64.dart';
export 'package:flutter/foundation.dart' show debugPrint;
......
......@@ -4,9 +4,9 @@
import 'dart:ui' as ui show Gradient, Shader, TextBox;
import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/services.dart';
import 'package:meta/meta.dart';
import 'box.dart';
import 'debug.dart';
......
......@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter/services.dart';
import 'text_editing.dart';
import 'text_input.dart';
/// A [TextInputFormatter] can be optionally injected into an [EditableText]
/// to provide as-you-type validation and formatting of the text being edited.
......
......@@ -9,8 +9,8 @@
import 'dart:async' show Future, Stream, StreamSubscription;
import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart';
import 'package:meta/meta.dart' show required;
import 'framework.dart';
// Examples can assume:
// dynamic _lot;
......
......@@ -7,7 +7,6 @@ import 'dart:async';
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:meta/meta.dart';
import 'basic.dart';
import 'focus_manager.dart';
......
......@@ -5,11 +5,17 @@
import 'dart:async' show Timer;
import 'dart:math' as math;
import 'package:flutter/animation.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/physics.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart';
import 'basic.dart';
import 'framework.dart';
import 'notification_listener.dart';
import 'scroll_notification.dart';
import 'ticker_provider.dart';
/// A visual indication that a scroll view has overscrolled.
///
......
......@@ -9,7 +9,6 @@ import 'package:flutter/gestures.dart';
import 'package:flutter/physics.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:meta/meta.dart';
import 'basic.dart';
import 'framework.dart';
......
......@@ -4,7 +4,9 @@
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'framework.dart';
import 'notification_listener.dart';
/// Indicates that the size of one of the descendants of the object receiving
/// this notification has changed, and that therefore any assumptions about that
......
......@@ -4,7 +4,9 @@
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
import 'basic.dart';
import 'framework.dart';
/// A widget that describes this app in the operating system.
class Title extends StatelessWidget {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment