Skip to content

Commit

Permalink
Add some stricter analysis (flutter#64)
Browse files Browse the repository at this point in the history
- Enable errors that would fail internally
- Add `comment_references` lint - this makes DartDocs output more usable
  and there isn't any reason to have unclickable references.
- Add `prefer_typing_uninitialized_variables` lint - this prevents some
  unintentionally dynamic behavior.
- Disable implicit casts.
- Use function type syntax for the ManuallyClosedWatcher factory.
- Remove some unused utilities.
  • Loading branch information
natebosch authored Sep 11, 2018
1 parent 539ed40 commit 1a52033
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 179 deletions.
14 changes: 14 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
analyzer:
strong-mode:
implicit-casts: false
errors:
todo: ignore
unused_import: error
unused_element: error
unused_local_variable: error
dead_code: error

linter:
rules:
- comment_references
- prefer_typing_uninitialized_variables
2 changes: 1 addition & 1 deletion benchmark/path_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ abstract class PathSetBenchmark extends BenchmarkBase {
/// Each virtual directory contains ten entries: either subdirectories or
/// files.
void walkTree(int depth, callback(String path)) {
recurse(path, remainingDepth) {
recurse(String path, remainingDepth) {
for (var i = 0; i < 10; i++) {
var padded = i.toString().padLeft(2, '0');
if (remainingDepth == 0) {
Expand Down
12 changes: 7 additions & 5 deletions lib/src/directory_watcher/linux.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,14 @@ class _LinuxDirectoryWatcher
.transform(new BatchedStreamTransformer<FileSystemEvent>());
_listen(innerStream, _onBatch, onError: _eventsController.addError);

_listen(new Directory(path).list(recursive: true), (entity) {
_listen(new Directory(path).list(recursive: true),
(FileSystemEntity entity) {
if (entity is Directory) {
_watchSubdir(entity.path);
} else {
_files.add(entity.path);
}
}, onError: (error, stackTrace) {
}, onError: (error, StackTrace stackTrace) {
_eventsController.addError(error, stackTrace);
close();
}, onDone: () {
Expand Down Expand Up @@ -207,14 +208,15 @@ class _LinuxDirectoryWatcher

/// Emits [ChangeType.ADD] events for the recursive contents of [path].
void _addSubdir(String path) {
_listen(new Directory(path).list(recursive: true), (entity) {
_listen(new Directory(path).list(recursive: true),
(FileSystemEntity entity) {
if (entity is Directory) {
_watchSubdir(entity.path);
} else {
_files.add(entity.path);
_emit(ChangeType.ADD, entity.path);
}
}, onError: (error, stackTrace) {
}, onError: (error, StackTrace stackTrace) {
// Ignore an exception caused by the dir not existing. It's fine if it
// was added and then quickly removed.
if (error is FileSystemException) return;
Expand Down Expand Up @@ -252,7 +254,7 @@ class _LinuxDirectoryWatcher
/// [_subscriptions] so that it can be canceled when [close] is called.
void _listen<T>(Stream<T> stream, void onData(T event),
{Function onError, void onDone(), bool cancelOnError}) {
var subscription;
StreamSubscription subscription;
subscription = stream.listen(onData, onError: onError, onDone: () {
_subscriptions.remove(subscription);
if (onDone != null) onDone();
Expand Down
8 changes: 4 additions & 4 deletions lib/src/directory_watcher/mac_os.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ class _MacOSDirectoryWatcher

/// The subscription to the stream returned by [Directory.watch].
///
/// This is separate from [_subscriptions] because this stream occasionally
/// needs to be resubscribed in order to work around issue 14849.
/// This is separate from [_listSubscriptions] because this stream
/// occasionally needs to be resubscribed in order to work around issue 14849.
StreamSubscription<List<FileSystemEvent>> _watchSubscription;

/// The subscription to the [Directory.list] call for the initial listing of
Expand Down Expand Up @@ -143,7 +143,7 @@ class _MacOSDirectoryWatcher

_emitEvent(ChangeType.ADD, entity.path);
_files.add(entity.path);
}, onError: (e, stackTrace) {
}, onError: (e, StackTrace stackTrace) {
_emitError(e, stackTrace);
}, onDone: () {
_listSubscriptions.remove(subscription);
Expand Down Expand Up @@ -212,7 +212,7 @@ class _MacOSDirectoryWatcher
/// one exists.
///
/// If [batch] doesn't contain any contradictory events (e.g. DELETE and
/// CREATE, or events with different values for [isDirectory]), this returns a
/// CREATE, or events with different values for `isDirectory`), this returns a
/// single event that describes what happened to the path in question.
///
/// If [batch] does contain contradictory events, this returns `null` to
Expand Down
12 changes: 6 additions & 6 deletions lib/src/directory_watcher/polling.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class PollingDirectoryWatcher extends ResubscribableWatcher

/// Creates a new polling watcher monitoring [directory].
///
/// If [_pollingDelay] is passed, it specifies the amount of time the watcher
/// If [pollingDelay] is passed, it specifies the amount of time the watcher
/// will pause between successive polls of the directory contents. Making this
/// shorter will give more immediate feedback at the expense of doing more IO
/// and higher CPU usage. Defaults to one second.
Expand Down Expand Up @@ -68,13 +68,13 @@ class _PollingDirectoryWatcher

/// The set of files that have been seen in the current directory listing.
///
/// Used to tell which files have been removed: files that are in [_statuses]
/// but not in here when a poll completes have been removed.
/// Used to tell which files have been removed: files that are in
/// [_lastModifieds] but not in here when a poll completes have been removed.
final _polledFiles = new Set<String>();

_PollingDirectoryWatcher(this.path, this._pollingDelay) {
_filesToProcess =
new AsyncQueue<String>(_processFile, onError: (e, stackTrace) {
_filesToProcess = new AsyncQueue<String>(_processFile,
onError: (e, StackTrace stackTrace) {
if (!_events.isClosed) _events.addError(e, stackTrace);
});

Expand Down Expand Up @@ -113,7 +113,7 @@ class _PollingDirectoryWatcher

if (entity is! File) return;
_filesToProcess.add(entity.path);
}, onError: (error, stackTrace) {
}, onError: (error, StackTrace stackTrace) {
if (!isDirectoryNotFoundException(error)) {
// It's some unknown error. Pipe it over to the event stream so the
// user can see it.
Expand Down
6 changes: 3 additions & 3 deletions lib/src/directory_watcher/windows.dart
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ class _WindowsDirectoryWatcher
_files.add(entity.path);
}, onDone: () {
_listSubscriptions.remove(subscription);
}, onError: (e, stackTrace) {
}, onError: (e, StackTrace stackTrace) {
_listSubscriptions.remove(subscription);
_emitError(e, stackTrace);
}, cancelOnError: true);
Expand Down Expand Up @@ -253,7 +253,7 @@ class _WindowsDirectoryWatcher
/// one exists.
///
/// If [batch] doesn't contain any contradictory events (e.g. DELETE and
/// CREATE, or events with different values for [isDirectory]), this returns a
/// CREATE, or events with different values for `isDirectory`), this returns a
/// single event that describes what happened to the path in question.
///
/// If [batch] does contain contradictory events, this returns `null` to
Expand Down Expand Up @@ -382,7 +382,7 @@ class _WindowsDirectoryWatcher
_files.clear();
var completer = new Completer();
var stream = new Directory(path).list(recursive: true);
void handleEntity(entity) {
void handleEntity(FileSystemEntity entity) {
if (entity is! Directory) _files.add(entity.path);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/src/file_watcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import 'file_watcher/polling.dart';
/// it will emit a single [ChangeType.REMOVE] event and then close the stream.
///
/// If the file is deleted and quickly replaced (when a new file is moved in its
/// place, for example) this will emit a [ChangeTime.MODIFY] event.
/// place, for example) this will emit a [ChangeType.MODIFY] event.
abstract class FileWatcher implements Watcher {
/// Creates a new [FileWatcher] monitoring [file].
///
Expand Down
2 changes: 1 addition & 1 deletion lib/src/file_watcher/polling.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class _PollingFileWatcher implements FileWatcher, ManuallyClosedWatcher {
return;
}

var modified;
DateTime modified;
try {
try {
modified = await getModificationTime(path);
Expand Down
16 changes: 8 additions & 8 deletions lib/src/path_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ class PathSet {
/// empty set.
Set<String> remove(String path) {
path = _normalize(path);
var parts = new Queue.from(p.split(path));
var parts = new Queue.of(p.split(path));

// Remove the children of [dir], as well as [dir] itself if necessary.
//
// [partialPath] is the path to [dir], and a prefix of [path]; the remaining
// components of [path] are in [parts].
Set<String> recurse(dir, partialPath) {
Set<String> recurse(_Entry dir, String partialPath) {
if (parts.length > 1) {
// If there's more than one component left in [path], recurse down to
// the next level.
Expand Down Expand Up @@ -97,7 +97,7 @@ class PathSet {
/// [dirPath] should be the path to [dir].
Set<String> _explicitPathsWithin(_Entry dir, String dirPath) {
var paths = new Set<String>();
recurse(dir, path) {
recurse(_Entry dir, String path) {
dir.contents.forEach((name, entry) {
var entryPath = p.join(path, name);
if (entry.isExplicit) paths.add(p.join(root, entryPath));
Expand All @@ -110,9 +110,9 @@ class PathSet {
return paths;
}

/// Returns whether [this] contains [path].
/// Returns whether this set contains [path].
///
/// This only returns true for paths explicitly added to [this].
/// This only returns true for paths explicitly added to this set.
/// Implicitly-added directories can be inspected using [containsDir].
bool contains(String path) {
path = _normalize(path);
Expand All @@ -126,7 +126,7 @@ class PathSet {
return entry.isExplicit;
}

/// Returns whether [this] contains paths beneath [path].
/// Returns whether this set contains paths beneath [path].
bool containsDir(String path) {
path = _normalize(path);
var entry = _entries;
Expand All @@ -143,7 +143,7 @@ class PathSet {
List<String> get paths {
var result = <String>[];

recurse(dir, path) {
recurse(_Entry dir, String path) {
for (var name in dir.contents.keys) {
var entry = dir.contents[name];
var entryPath = p.join(path, name);
Expand All @@ -156,7 +156,7 @@ class PathSet {
return result;
}

/// Removes all paths from [this].
/// Removes all paths from this set.
void clear() {
_entries.contents.clear();
}
Expand Down
8 changes: 3 additions & 5 deletions lib/src/resubscribable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import 'dart:async';
import '../watcher.dart';
import 'watch_event.dart';

typedef ManuallyClosedWatcher WatcherFactory();

/// A wrapper for [ManuallyClosedWatcher] that encapsulates support for closing
/// the watcher when it has no subscribers and re-opening it when it's
/// re-subscribed.
Expand All @@ -24,7 +22,7 @@ typedef ManuallyClosedWatcher WatcherFactory();
/// takes a factory function that produces instances of the inner class.
abstract class ResubscribableWatcher implements Watcher {
/// The factory function that produces instances of the inner class.
final WatcherFactory _factory;
final ManuallyClosedWatcher Function() _factory;

final String path;

Expand All @@ -39,8 +37,8 @@ abstract class ResubscribableWatcher implements Watcher {
/// Creates a new [ResubscribableWatcher] wrapping the watchers
/// emitted by [_factory].
ResubscribableWatcher(this.path, this._factory) {
var watcher;
var subscription;
ManuallyClosedWatcher watcher;
StreamSubscription subscription;

_eventsController = new StreamController<WatchEvent>.broadcast(
onListen: () {
Expand Down
51 changes: 0 additions & 51 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import 'dart:async';
import 'dart:io';
import 'dart:collection';

import 'package:async/async.dart';

/// Returns `true` if [error] is a [FileSystemException] for a missing
/// directory.
bool isDirectoryNotFoundException(error) {
Expand All @@ -22,55 +20,6 @@ bool isDirectoryNotFoundException(error) {
Set<T> unionAll<T>(Iterable<Set<T>> sets) =>
sets.fold(new Set<T>(), (union, set) => union.union(set));

/// Returns a buffered stream that will emit the same values as the stream
/// returned by [future] once [future] completes.
///
/// If [future] completes to an error, the return value will emit that error and
/// then close.
///
/// If [broadcast] is true, a broadcast stream is returned. This assumes that
/// the stream returned by [future] will be a broadcast stream as well.
/// [broadcast] defaults to false.
Stream<T> futureStream<T>(Future<Stream<T>> future, {bool broadcast: false}) {
var subscription;
StreamController<T> controller;

future = DelegatingFuture.typed(future.catchError((e, stackTrace) {
// Since [controller] is synchronous, it's likely that emitting an error
// will cause it to be cancelled before we call close.
if (controller != null) controller.addError(e, stackTrace);
if (controller != null) controller.close();
controller = null;
}));

onListen() {
future.then((stream) {
if (controller == null) return;
subscription = stream.listen(controller.add,
onError: controller.addError, onDone: controller.close);
});
}

onCancel() {
if (subscription != null) subscription.cancel();
subscription = null;
controller = null;
}

if (broadcast) {
controller = new StreamController.broadcast(
sync: true, onListen: onListen, onCancel: onCancel);
} else {
controller = new StreamController(
sync: true, onListen: onListen, onCancel: onCancel);
}
return controller.stream;
}

/// Like [new Future], but avoids around issue 11911 by using [new Future.value]
/// under the covers.
Future newFuture(callback()) => new Future.value().then((_) => callback());

/// A stream transformer that batches all events that are sent at the same time.
///
/// When multiple events are synchronously added to a stream controller, the
Expand Down
Loading

0 comments on commit 1a52033

Please sign in to comment.