Merge pull request #28 from Workiva/null_safety

Migrate to null safety

.github/workflows/dart_ci.yml

@@ -15,10 +15,10 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        sdk: [ stable, 2.13.4 ]
+        sdk: [ 2.18.7, stable ]
     steps:
       - uses: actions/checkout@v2
-      - uses: dart-lang/setup-dart@v0.2
+      - uses: dart-lang/setup-dart@v1
         with:
           sdk: ${{ matrix.sdk }}
 
@@ -27,11 +27,10 @@ jobs:
 
       - name: Formatting
         run: dart format --output=none --set-exit-if-changed .
-        if: always() && ${{ matrix.sdk }} == 'stable'
+        if: ${{ matrix.sdk }} == '2.18.7'
 
       - name: Analysis
         run: dart analyze
 
       - name: Tests
         run: dart test --chain-stack-traces
-        if: ${{ matrix.sdk == '2.13.4' }}

Dockerfile

@@ -1,4 +1,4 @@
-FROM google/dart:2.13
+FROM drydock-prod.workiva.net/workiva/dart2_base_image:1
 WORKDIR /build/
 ADD pubspec.yaml .
 RUN dart pub get

analysis_options.yaml

@@ -1 +1 @@
-include: package:pedantic/analysis_options.yaml
+include: package:lints/recommended.yaml

lib/src/command_runner.dart

@@ -11,7 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-
+import 'dart:async';
 import 'package:args/args.dart';
 import 'package:args/command_runner.dart';
 import 'package:logging/logging.dart';
@@ -33,11 +33,11 @@ class WebdevProxy extends CommandRunner<int> {
   @override
   Future<int> run(Iterable<String> args) async {
     // The help command returns null, so return success code in that case.
-    return await super.run(args) ?? 0;
+    return (await super.run(args)) ?? 0;
   }
 
   @override
-  Future<int> runCommand(ArgResults topLevelResults) async {
+  Future<int?> runCommand(ArgResults topLevelResults) async {
     final verbose = topLevelResults[verboseFlag] == true;
     Logger.root.level = verbose ? Level.ALL : Level.INFO;
     Logger.root.onRecord.listen(stdIOLogListener(verbose: verbose));

lib/src/logging.dart

@@ -28,9 +28,7 @@ const _logSuffix = '\n';
 
 final Logger log = Logger('Proxy');
 
-StringBuffer colorLog(LogRecord record, {bool verbose}) {
-  verbose ??= false;
-
+StringBuffer colorLog(LogRecord record, {bool verbose = false}) {
   AnsiCode color;
   if (record.level < Level.WARNING) {
     color = cyan;
@@ -44,13 +42,13 @@ StringBuffer colorLog(LogRecord record, {bool verbose}) {
   final lines = <Object>[
     '$eraseLine$level ${_loggerName(record, verbose)}${record.message}'
   ];
-
-  if (record.error != null) {
-    lines.add(record.error);
+  var error = record.error;
+  if (error != null) {
+    lines.add(error);
   }
 
   if (record.stackTrace != null && verbose) {
-    final trace = Trace.from(record.stackTrace).terse;
+    final trace = Trace.from(record.stackTrace!).terse;
     lines.add(trace);
   }
 
@@ -130,7 +128,7 @@ T logTimedSync<T>(
   return result;
 }
 
-Function(LogRecord) stdIOLogListener({bool verbose}) =>
+Function(LogRecord) stdIOLogListener({bool verbose = false}) =>
     (record) => io.stdout.write(colorLog(record, verbose: verbose));
 
 String _loggerName(LogRecord record, bool verbose) {

lib/src/serve_command.dart

@@ -18,7 +18,6 @@ import 'dart:io';
 import 'package:args/command_runner.dart';
 import 'package:io/ansi.dart';
 import 'package:io/io.dart';
-import 'package:pedantic/pedantic.dart';
 
 import 'package:webdev_proxy/src/command_runner.dart';
 import 'package:webdev_proxy/src/command_utils.dart';
@@ -52,15 +51,15 @@ class ServeCommand extends Command<int> {
       '[-- [webdev serve arguments]]';
 
   @override
-  String get usageFooter => _webdevCompatibilityHelp;
+  String? get usageFooter => _webdevCompatibilityHelp;
 
   @override
   String get name => 'serve';
 
   bool get _canUseWebdev => !_missingWebdev && _hasCompatibleWebdev;
 
   bool get _hasCompatibleWebdev =>
-      webdevCompatibility.allows(getGlobalWebdevVersion());
+      webdevCompatibility.allows(getGlobalWebdevVersion()!);
 
   bool get _missingWebdev => getGlobalWebdevVersion() == null;
 
@@ -78,7 +77,7 @@ class ServeCommand extends Command<int> {
     }
   }
 
-  String get _webdevCompatibilityHelp {
+  String? get _webdevCompatibilityHelp {
     if (_missingWebdev) {
       return red.wrap(
         'This command requires that `webdev` be activated globally.\n'
@@ -109,19 +108,19 @@ class ServeCommand extends Command<int> {
     //
     // To enforce this, we validate that [argResults.rest] is exactly equal to
     // all the arguments after the `--`.
-    assertNoPositionalArgsBeforeSeparator(name, argResults, usageException);
+    assertNoPositionalArgsBeforeSeparator(name, argResults!, usageException);
 
     final exitCodeCompleter = Completer<int>();
     var interruptReceived = false;
     final proxies = <WebdevProxyServer>[];
     var proxiesFailed = false;
-    StreamSubscription sigintSub;
-    WebdevServer webdevServer;
+    StreamSubscription? sigintSub;
+    WebdevServer? webdevServer;
 
     void shutDown(int code) async {
       await Future.wait([
-        sigintSub?.cancel(),
-        webdevServer?.close(),
+        if (sigintSub != null) sigintSub.cancel(),
+        if (webdevServer != null) webdevServer.close(),
         ...proxies.map((proxy) => proxy.close()),
       ]);
       if (!exitCodeCompleter.isCompleted) {
@@ -137,13 +136,13 @@ class ServeCommand extends Command<int> {
     });
 
     // Parse the hostname to serve each dir on (defaults to 0.0.0.0)
-    final hostnameResults = parseHostname(argResults.rest);
+    final hostnameResults = parseHostname(argResults!.rest);
     final hostname = hostnameResults.hostname;
     final remainingArgs = hostnameResults.remainingArgs;
 
     // Parse the directory:port mappings that will be used by the proxy servers.
     // Each proxy will be mapped to a `webdev serve` instance on another port.
-    final portsToServeByDir = parseDirectoryArgs(argResults.rest);
+    final portsToServeByDir = parseDirectoryArgs(argResults!.rest);
 
     // Find open ports for each of the directories to be served by webdev.
     final portsToProxyByDir = {
@@ -173,8 +172,8 @@ class ServeCommand extends Command<int> {
           dir: dir,
           hostname: hostname,
           portToProxy: portsToProxyByDir[dir],
-          portToServe: portsToServeByDir[dir],
-          rewrite404s: argResults[rewrite404sFlag] == true,
+          portToServe: portsToServeByDir[dir]!,
+          rewrite404s: argResults![rewrite404sFlag] == true,
         ));
       } catch (e, stackTrace) {
         proxiesFailed = true;

lib/src/sse_proxy_handler.dart

@@ -34,14 +34,19 @@ String _sseHeaders(String origin) => 'HTTP/1.1 200 OK\r\n'
 /// simply forwards data back and forth between clients and the actual server.
 class SseProxyHandler {
   final _httpClient = http.Client();
-  shelf.Handler _incomingMessageProxyHandler;
-  final String _proxyName;
+  late final shelf.Handler _incomingMessageProxyHandler =
+      shelf_proxy.proxyHandler(
+    _serverUri,
+    client: _httpClient,
+    proxyName: _proxyName,
+  );
+  final String? _proxyName;
   final Uri _proxyUri;
   final Uri _serverUri;
 
   /// Creates an SSE proxy handler that will handle EventSource requests to
   /// [proxyUri] by proxying them to [serverUri].
-  SseProxyHandler(Uri proxyUri, Uri serverUri, {String proxyName})
+  SseProxyHandler(Uri proxyUri, Uri serverUri, {String? proxyName})
       : _proxyUri = proxyUri,
         _serverUri = serverUri,
         _proxyName = proxyName;
@@ -61,10 +66,10 @@ class SseProxyHandler {
 
     req.hijack((channel) {
       final sink = utf8.encoder.startChunkedConversion(channel.sink)
-        ..add(_sseHeaders(req.headers['origin']));
+        ..add(_sseHeaders(req.headers['origin'] ?? ''));
 
       StreamSubscription serverSseSub;
-      StreamSubscription reqChannelSub;
+      StreamSubscription? reqChannelSub;
 
       serverSseSub =
           utf8.decoder.bind(serverResponse.stream).listen(sink.add, onDone: () {
@@ -75,7 +80,7 @@ class SseProxyHandler {
       reqChannelSub = channel.stream.listen((_) {
         // SSE is unidirectional.
       }, onDone: () {
-        serverSseSub?.cancel();
+        serverSseSub.cancel();
         sink.close();
       });
     });
@@ -100,11 +105,6 @@ class SseProxyHandler {
   }
 
   Future<shelf.Response> _handleIncomingMessage(shelf.Request req) async {
-    _incomingMessageProxyHandler ??= shelf_proxy.proxyHandler(
-      _serverUri,
-      client: _httpClient,
-      proxyName: _proxyName,
-    );
     return _incomingMessageProxyHandler(req);
   }
 }

lib/src/webdev_proc_utils.dart

@@ -24,20 +24,20 @@ import 'package:pub_semver/pub_semver.dart';
 final webdevCompatibility = VersionConstraint.parse('>=1.0.1 <3.0.0');
 
 @visibleForTesting
-ProcessResult cachedWebdevVersionResult;
+ProcessResult? cachedWebdevVersionResult;
 
 /// Returns the version of the `webdev` package that is currently globally
 /// activated, or `null` if it is not activated.
-Version getGlobalWebdevVersion() {
+Version? getGlobalWebdevVersion() {
   cachedWebdevVersionResult ??= Process.runSync(
     'dart',
     ['pub', 'global', 'run', 'webdev', '--version'],
     stdoutEncoding: utf8,
   );
-  if (cachedWebdevVersionResult.exitCode != 0) {
+  if (cachedWebdevVersionResult!.exitCode != 0) {
     return null;
   }
-  return Version.parse(cachedWebdevVersionResult.stdout.toString().trim());
+  return Version.parse(cachedWebdevVersionResult!.stdout.toString().trim());
 }
 
 /// Prints the output from `webdev help serve` with a header that explains how

lib/src/webdev_proxy_server.dart

@@ -16,7 +16,6 @@ import 'dart:io';
 
 import 'package:http_multi_server/http_multi_server.dart';
 import 'package:io/ansi.dart';
-import 'package:meta/meta.dart';
 import 'package:shelf/shelf.dart' as shelf;
 import 'package:shelf/shelf_io.dart' as shelf_io;
 import 'package:shelf_proxy/shelf_proxy.dart' as shelf_proxy;
@@ -37,7 +36,7 @@ class WebdevProxyServer {
 
   /// Permanently stops this proxy server from listening for new connections and
   /// closes all active connections immediately.
-  Future<Null> close() async {
+  Future<void> close() async {
     await _server.close(force: true);
   }
 
@@ -51,15 +50,12 @@ class WebdevProxyServer {
   /// request that fetches the root index path (`/`) unless [rewrite404s] is
   /// false.
   static Future<WebdevProxyServer> start({
-    @required String dir,
-    @required String hostname,
-    @required int portToProxy,
-    int portToServe,
-    bool rewrite404s,
+    required String dir,
+    required String hostname,
+    required int? portToProxy,
+    int portToServe = 0,
+    bool rewrite404s = true,
   }) async {
-    portToServe ??= 0;
-    rewrite404s ??= true;
-
     final serverHostname = hostname == 'any' ? 'localhost' : hostname;
     final serverUri = Uri.parse('http://$serverHostname:$portToProxy');
     final serverSseUri = serverUri.replace(path: r'/$sseHandler');
@@ -77,9 +73,8 @@ class WebdevProxyServer {
     final server = await HttpMultiServer.bind(hostname, portToServe);
     shelf_io.serveRequests(server, cascade.handler);
     final proxyHostname = hostname == 'any' ? '::' : hostname;
-    log.info(green.wrap('Serving `$dir` proxy on '
-            'http://$proxyHostname:$portToServe') +
-        '\n');
+    log.info(green
+        .wrap('Serving `$dir` proxy on http://$proxyHostname:$portToServe\n'));
     log.fine('... forwards to http://$serverHostname:$portToProxy');
     return WebdevProxyServer._(server);
   }

lib/src/webdev_server.dart

@@ -30,20 +30,21 @@ class WebdevServer {
 
   /// Permanently stops this proxy server from listening for new connections and
   /// closes all active connections immediately.
-  Future<Null> close() async {
+  Future<void> close() async {
     _process.kill();
     await _process.exitCode;
   }
 
   /// Starts a `webdev serve` process with the given [args] and returns a
   /// [WebdevServer] abstraction over said process.
-  static Future<WebdevServer> start(List<String> args) async {
+  static Future<WebdevServer> start(List<String> args,
+      {ProcessStartMode mode = ProcessStartMode.inheritStdio}) async {
     final webdevArgs = ['pub', 'global', 'run', 'webdev', 'serve', ...args];
     log.fine('Running `dart ${webdevArgs.join(' ')}');
     final process = await Process.start(
       'dart',
       webdevArgs,
-      mode: ProcessStartMode.inheritStdio,
+      mode: mode,
     );
     return WebdevServer._(process);
   }

pubspec.yaml

@@ -8,7 +8,7 @@ description: >
   routing by rewriting 404s to the root index.
 
 environment:
-  sdk: ">=2.11.0 <3.0.0"
+  sdk: '>=2.12.0 <3.0.0'
 
 dependencies:
   args: ^2.3.1
@@ -18,7 +18,7 @@ dependencies:
   io: ^1.0.3
   logging: ^1.1.0
   meta: ^1.8.0
-  pedantic: ^1.11.1
+  lints: ^2.0.1
   pub_semver: ^2.1.2
   shelf: ^1.2.0
   shelf_proxy: ^1.0.0

test/chromedriver_utils.dart

@@ -12,7 +12,11 @@ Future<void> startChromeDriver() async {
   try {
     final chromeDriver = await Process.start(
         'chromedriver', ['--port=4444', '--url-base=wd/hub']);
-    addTearDown(chromeDriver.kill);
+    addTearDown(() {
+      if (!chromeDriver.kill()) {
+        chromeDriver.kill(ProcessSignal.sigkill);
+      }
+    });
 
     // On windows this takes a while to boot up, wait for the first line
     // of stdout as a signal that it is ready.
@@ -51,7 +55,7 @@ Future<wd.WebDriver> createWebDriver() async {
       spec: wd.WebDriverSpec.JsonWire,
       desired: capabilities,
       uri: Uri.parse(
-          'http://127.0.0.1:$chromeDriverPort/$chromeDriverUrlBase/'));
+          'http://localhost:$chromeDriverPort/$chromeDriverUrlBase/'));
   addTearDown(webDriver.quit);
   return webDriver;
 }