Skip to content
This repository was archived by the owner on May 24, 2023. It is now read-only.

null safety #52

Merged
merged 20 commits into from
Feb 25, 2021
Merged
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ env:

jobs:
# Check code formatting and static analysis on a single OS (linux)
# against Dart dev.
# against Dart beta.
analyze:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
sdk: [dev]
sdk: [beta]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/[email protected]
Expand All @@ -38,7 +38,7 @@ jobs:

# Run tests on a matrix consisting of two dimensions:
# 1. OS: ubuntu-latest, (macos-latest, windows-latest)
# 2. release channel: dev
# 2. release channel: beta
test:
needs: analyze
runs-on: ${{ matrix.os }}
Expand All @@ -47,7 +47,7 @@ jobs:
matrix:
# Add macos-latest and/or windows-latest if relevant for this package.
os: [ubuntu-latest]
sdk: [2.3.0, dev]
sdk: [beta]
steps:
- uses: actions/checkout@v2
- uses: dart-lang/[email protected]
Expand Down
4 changes: 2 additions & 2 deletions benchmark/shortest_path_benchmark.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void main() {
}
}

int minTicks;
int? minTicks;
var maxIteration = 0;

final testOutput =
Expand All @@ -34,7 +34,7 @@ void main() {
watch
..reset()
..start();
final length = shortestPath(0, size - 1, (e) => graph[e] ?? []).length;
final length = shortestPath(0, size - 1, (e) => graph[e] ?? [])?.length;
watch.stop();
assert(length == 4, '$length');

Expand Down
28 changes: 16 additions & 12 deletions example/crawl_async_example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ Future<Null> main() async {
print(allImports.map((s) => s.uri).toList());
}

AnalysisContext _analysisContext;
AnalysisContext? _analysisContext;

Future<AnalysisContext> get analysisContext async {
Future<AnalysisContext?> get analysisContext async {
if (_analysisContext == null) {
var libUri = Uri.parse('package:graphs/');
var libPath = await pathForUri(libUri);
Expand All @@ -39,25 +39,29 @@ Future<AnalysisContext> get analysisContext async {
throw StateError('Expected to find exactly one context root, got $roots');
}

_analysisContext = ContextBuilder().createContext(contextRoot: roots[0]);
final analysisContext =
ContextBuilder().createContext(contextRoot: roots[0]);
_analysisContext = analysisContext;
return analysisContext;
}

return _analysisContext;
return null;
}

Future<Iterable<Uri>> findImports(Uri from, Source source) async {
return source.unit.directives
Future<Iterable<Uri>?> findImports(Uri from, Source source) async {
return source.unit?.directives
.whereType<UriBasedDirective>()
.map((d) => d.uri.stringValue)
.where((uri) => !uri.startsWith('dart:'))
.whereType<String>()
.where((uri) => uri.startsWith('dart:') == false)
.map((import) => resolveImport(import, from));
}

Future<CompilationUnit> parseUri(Uri uri) async {
Future<CompilationUnit?> parseUri(Uri uri) async {
var path = await pathForUri(uri);
var analysisSession = (await analysisContext).currentSession;
var parseResult = analysisSession.getParsedUnit(path);
return parseResult.unit;
var analysisSession = (await analysisContext)?.currentSession;
var parseResult = analysisSession?.getParsedUnit(path);
return parseResult?.unit;
}

Future<String> pathForUri(Uri uri) async {
Expand All @@ -81,7 +85,7 @@ Uri resolveImport(String import, Uri from) {

class Source {
final Uri uri;
final CompilationUnit unit;
final CompilationUnit? unit;

Source(this.uri, this.unit);
}
2 changes: 1 addition & 1 deletion example/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void main() {
});

var components = stronglyConnectedComponents<Node>(
graph.nodes.keys, (node) => graph.nodes[node]);
graph.nodes.keys, (node) => graph.nodes[node] ?? []);

print(components);
}
4 changes: 2 additions & 2 deletions lib/src/crawl_async.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ final _empty = Future<Null>.value(null);
/// have not completed. If the [edges] callback needs to be limited or throttled
/// that must be done by wrapping it before calling [crawlAsync].
Stream<V> crawlAsync<K, V>(Iterable<K> roots, FutureOr<V> Function(K) readNode,
FutureOr<Iterable<K>> Function(K, V) edges) {
FutureOr<Iterable<K>?> Function(K, V) edges) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@natebosch I am a bit surprised we allow returning null here? should we retain that behavior?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I think we should restrict it - we previously allowed null and [] to mean the same thing, and there is no need to do that in a null safe world.

I'm on the fence about statically typing it as non-nullable, but dynamically allowing null to maintain backwards compatibility for opt-out code. Even with a major version bump it might be friendly to do so.

final crawl = _CrawlAsync(roots, readNode, edges)..run();
return crawl.result.stream;
}
Expand All @@ -43,7 +43,7 @@ class _CrawlAsync<K, V> {
final result = StreamController<V>();

final FutureOr<V> Function(K) readNode;
final FutureOr<Iterable<K>> Function(K, V) edges;
final FutureOr<Iterable<K>?> Function(K, V) edges;
final Iterable<K> roots;

final _seen = HashSet<K>();
Expand Down
29 changes: 14 additions & 15 deletions lib/src/shortest_path.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ import 'dart:collection';
///
/// If you supply one of [equals] or [hashCode], you should generally also to
/// supply the other.
List<T> shortestPath<T>(
List<T>? shortestPath<T>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be <T extends Object>? I don't know that null is ever useful value here... but maybe we don't want to be overly opinionated about it. There is an assert below that start is not null though.

Copy link
Contributor

Choose a reason for hiding this comment

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

My gut reaction is that most of these should be <T extends Object>, but I could also be convinced otherwise.

T start,
T target,
Iterable<T> Function(T) edges, {
bool Function(T, T) equals,
int Function(T) hashCode,
bool Function(T?, T?)? equals,
int Function(T)? hashCode,
}) =>
_shortestPaths<T>(
start,
Expand Down Expand Up @@ -61,8 +61,8 @@ List<T> shortestPath<T>(
Map<T, List<T>> shortestPaths<T>(
T start,
Iterable<T> Function(T) edges, {
bool Function(T, T) equals,
int Function(T) hashCode,
bool Function(T?, T?)? equals,
int Function(T)? hashCode,
}) =>
_shortestPaths<T>(
start,
Expand All @@ -74,29 +74,28 @@ Map<T, List<T>> shortestPaths<T>(
Map<T, List<T>> _shortestPaths<T>(
T start,
Iterable<T> Function(T) edges, {
T target,
bool Function(T, T) equals,
int Function(T) hashCode,
T? target,
bool Function(T?, T?)? equals,
int Function(T)? hashCode,
}) {
assert(start != null, '`start` cannot be null');
assert(edges != null, '`edges` cannot be null');

final distances = HashMap<T, List<T>>(equals: equals, hashCode: hashCode);
distances[start] = const [];

equals ??= _defaultEquals;
if (equals(start, target)) {
if (equals.call(start, target)) {
return distances;
}

final toVisit = ListQueue<T>()..add(start);

List<T> bestOption;
List<T>? bestOption;

while (toVisit.isNotEmpty) {
final current = toVisit.removeFirst();
final currentPath = distances[current];
final currentPathLength = currentPath.length;
final currentPathLength = currentPath?.length ?? 0;

if (bestOption != null && (currentPathLength + 1) >= bestOption.length) {
// Skip any existing `toVisit` items that have no chance of being
Expand All @@ -111,13 +110,13 @@ Map<T, List<T>> _shortestPaths<T>(
assert(existingPath == null ||
existingPath.length <= (currentPathLength + 1));

if (existingPath == null) {
if (existingPath == null && currentPath != null) {
final newOption = [
...currentPath,
edge,
];

if (equals(edge, target)) {
if (target != null && equals(edge, target)) {
assert(bestOption == null || bestOption.length > newOption.length);
bestOption = newOption;
}
Expand All @@ -135,4 +134,4 @@ Map<T, List<T>> _shortestPaths<T>(
return distances;
}

bool _defaultEquals(Object a, Object b) => a == b;
bool _defaultEquals<T>(T a, T b) => a == b;
22 changes: 15 additions & 7 deletions lib/src/strongly_connected_components.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ import 'dart:math' show min;
List<List<T>> stronglyConnectedComponents<T>(
Iterable<T> nodes,
Iterable<T> Function(T) edges, {
bool Function(T, T) equals,
int Function(T) hashCode,
bool Function(T?, T?)? equals,
int Function(T)? hashCode,
}) {
final result = <List<T>>[];
final lowLinks = HashMap<T, int>(equals: equals, hashCode: hashCode);
Expand All @@ -52,12 +52,20 @@ List<List<T>> stronglyConnectedComponents<T>(
lastVisited.addLast(node);
onStack.add(node);
// ignore: omit_local_variable_types
for (final T next in edges(node) ?? const []) {
for (final T next in edges(node)) {
if (!indexes.containsKey(next)) {
strongConnect(next);
lowLinks[node] = min(lowLinks[node], lowLinks[next]);
final n = lowLinks[node];
final x = lowLinks[next];
if (n != null && x != null) {
lowLinks[node] = min(n, x);
}
} else if (onStack.contains(next)) {
lowLinks[node] = min(lowLinks[node], indexes[next]);
final n = lowLinks[node];
final x = indexes[next];
if (n != null && x != null) {
lowLinks[node] = min(n, x);
}
}
}
if (lowLinks[node] == indexes[node]) {
Expand All @@ -67,7 +75,7 @@ List<List<T>> stronglyConnectedComponents<T>(
next = lastVisited.removeLast();
onStack.remove(next);
component.add(next);
} while (!equals(next, node));
} while (equals?.call(next, node) == false);
result.add(component);
}
}
Expand All @@ -78,4 +86,4 @@ List<List<T>> stronglyConnectedComponents<T>(
return result;
}

bool _defaultEquals(Object a, Object b) => a == b;
bool _defaultEquals<T>(T a, T b) => a == b;
16 changes: 8 additions & 8 deletions pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
name: graphs
version: 0.2.1-dev
version: 0.3.0-nullsafety.0
description: Graph algorithms that operate on graphs in any representation
homepage: https://github.com/dart-lang/graphs

environment:
sdk: '>=2.3.0 <3.0.0'

sdk: '>=2.12.0-0 <3.0.0'
dependency_overrides:
analyzer: ^0.42.0-nullsafety.0
dev_dependencies:
pedantic: ^1.3.0
test: ^1.5.1
pedantic: ^1.10.0
test: ^1.16.0

# For examples
analyzer: '>=0.35.0 <0.42.0'
path: ^1.1.0
pool: ^1.3.0
path: ^1.8.0
pool: ^1.5.0
7 changes: 3 additions & 4 deletions test/crawl_async_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,15 @@
// 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 'package:test/test.dart';

import 'package:graphs/graphs.dart';
import 'package:test/test.dart';

import 'utils/graph.dart';

void main() {
group('asyncCrawl', () {
Future<List<String>> crawl(
Map<String, List<String>> g, Iterable<String> roots) {
Future<List<String?>> crawl(
Map<String, List<String>?> g, Iterable<String> roots) {
var graph = AsyncGraph(g);
return crawlAsync(roots, graph.readNode, graph.edges).toList();
}
Expand Down
Loading