Support running node_interop in null safety mode (#90)

* Fix broken lints and tests. * Support running node_interop in null-safety mode. * Temp fixes for build_node_compilers. These workarounds can be removed after migrating build_node_compilers to nnbd.
pulyaevskiy · Mar 17, 2021 · b6a99bb · b6a99bb
1 parent 2363f61
commit b6a99bb
Show file tree

Hide file tree

Showing 30 changed files with 67 additions and 61 deletions.
diff --git a/build_node_compilers/CHANGELOG.md b/build_node_compilers/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 0.3.2
+
+- Allowed Dart2JS to run in unsound null-safety mode.
+- Removed dependency on deprecated analyzer packages.
+
 # 0.3.1
 
 - Upgraded dependency on build_modules to `2.11.3` to fix issues with Dart SDK constraint.

diff --git a/build_node_compilers/lib/src/node_entrypoint_builder.dart b/build_node_compilers/lib/src/node_entrypoint_builder.dart
@@ -3,8 +3,8 @@
 
 import 'dart:async';
 
-// ignore: deprecated_member_use
-import 'package:analyzer/analyzer.dart';
+import 'package:analyzer/dart/analysis/utilities.dart';
+import 'package:analyzer/dart/ast/ast.dart';
 import 'package:build/build.dart';
 import 'package:build_modules/build_modules.dart';
 
@@ -47,7 +47,9 @@ class NodeEntrypointBuilder implements Builder {
   final WebCompiler webCompiler;
   final List<String> dart2JsArgs;
 
-  const NodeEntrypointBuilder(this.webCompiler, {this.dart2JsArgs = const []});
+  // TODO: Remove --no-sound-null-safety after migrating to nnbd.
+  const NodeEntrypointBuilder(this.webCompiler,
+      {this.dart2JsArgs = const ['--no-sound-null-safety']});
 
   factory NodeEntrypointBuilder.fromOptions(BuilderOptions options) {
     validateOptions(
@@ -113,14 +115,15 @@ Future<bool> _isAppEntryPoint(AssetId dartId, AssetReader reader) async {
   // Skip reporting errors here, dartdevc will report them later with nicer
   // formatting.
   // ignore: deprecated_member_use
-  var parsed = parseCompilationUnit(await reader.readAsString(dartId),
-      suppressErrors: true);
+  var parsed = parseString(
+      content: await reader.readAsString(dartId), throwIfDiagnostics: false);
+
   // Allow two or fewer arguments so that entrypoints intended for use with
   // [spawnUri] get counted.
   //
   // TODO: This misses the case where a Dart file doesn't contain main(),
   // but has a part that does, or it exports a `main` from another library.
-  return parsed.declarations.any((node) {
+  return parsed.unit.declarations.any((node) {
     return node is FunctionDeclaration &&
         node.name.name == 'main' &&
         node.functionExpression.parameters.parameters.length <= 2;

diff --git a/build_node_compilers/pubspec.yaml b/build_node_compilers/pubspec.yaml
@@ -1,5 +1,5 @@
 name: build_node_compilers
-version: 0.3.1
+version: 0.3.2
 description: Builders wrapping DDC and dart2js compilers configured to output Node modules.
 homepage: https://github.com/pulyaevskiy/node-interop
 author: Anatoly Pulyaevskiy <[email protected]>

diff --git a/e2e_interop_test/build.yaml b/e2e_interop_test/build.yaml
@@ -1,11 +1,14 @@
 targets:
   $default:
     sources:
+      - $package$
       - "lib/**"
       - "node/**"
     builders:
       build_node_compilers|entrypoint:
         options:
           compiler: dart2js
           dart2js_args:
+            # TODO: Remove after migrating build_node_compilers to nnbd.
+            - --no-sound-null-safety
             - -O4
diff --git a/e2e_interop_test/lib/e2e_interop_test.dart b/e2e_interop_test/lib/e2e_interop_test.dart
diff --git a/e2e_interop_test/pubspec.yaml b/e2e_interop_test/pubspec.yaml
@@ -1,16 +1,18 @@
 name: e2e_interop_test
 description: End-to-end test for node interop and builders
 version: 0.0.1
+publish_to: none
 
 environment:
   sdk: '>=2.4.0 <3.0.0'
 
-dependencies:
-  node_interop:
-    path: ../node_interop
-
 dev_dependencies:
   test: ^1.0.0
   build_runner: ^1.0.0
   build_node_compilers:
     path: ../build_node_compilers
+  # TODO: Unpin js and node_interop after migrating build_node_compilers to
+  # nnbd.
+  js: '<=0.6.2' # ^0.6.3
+  node_interop: '<=1.2.1'
+    # path: ../node_interop
diff --git a/node_interop/CHANGELOG.md b/node_interop/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 2.0.0-null-safety.0
+
+- Migrated to null-safety (requires \>=Dart 2.12).
+
 ## 1.2.1
 
 - Add `util.callbackToCompleter()`, `util.invokeAsync0()`, and `util.invokeAsync1()`

diff --git a/node_interop/lib/buffer.dart b/node_interop/lib/buffer.dart
@@ -13,7 +13,7 @@ import 'package:js/js.dart';
 import 'node.dart';
 
 BufferModule get buffer => _buffer ??= require('buffer');
-BufferModule _buffer;
+BufferModule? _buffer;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/child_process.dart b/node_interop/lib/child_process.dart
@@ -15,7 +15,7 @@ import 'stream.dart';
 
 ChildProcessModule get childProcess =>
     _childProcess ??= require('child_process');
-ChildProcessModule _childProcess;
+ChildProcessModule? _childProcess;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/console.dart b/node_interop/lib/console.dart
@@ -13,9 +13,9 @@ import 'stream.dart';
 import 'node.dart';
 
 ConsoleModule get console => _console ??= require('console');
-ConsoleModule _console;
+ConsoleModule? _console;
 
-Console createConsole(Writable stdout, [Writable stderr]) {
+Console createConsole(Writable stdout, [Writable? stderr]) {
   if (stderr == null) {
     return callConstructor(console.Console, [stdout]);
   } else {

diff --git a/node_interop/lib/dns.dart b/node_interop/lib/dns.dart
@@ -11,7 +11,7 @@ import 'node.dart';
 import 'util.dart';
 
 DNS get dns => _dns ??= require('dns');
-DNS _dns;
+DNS? _dns;
 
 /// Main entry point to Node's "dns" module functionality.
 ///
@@ -68,7 +68,7 @@ abstract class DNSAddress {
 }
 
 Resolver createDNSResolver() {
-  return callConstructor(dns.Resolver, null);
+  return callConstructor(dns.Resolver, []);
 }
 
 /// An independent resolver for DNS requests.

diff --git a/node_interop/lib/fs.dart b/node_interop/lib/fs.dart
@@ -14,7 +14,7 @@ import 'node.dart';
 import 'stream.dart';
 
 FS get fs => _fs ??= require('fs');
-FS _fs;
+FS? _fs;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/http.dart b/node_interop/lib/http.dart
@@ -17,12 +17,12 @@ import 'node.dart';
 import 'stream.dart';
 
 HTTP get http => _http ??= require('http');
-HTTP _http;
+HTTP? _http;
 
 /// Convenience method for creating instances of "http" module's Agent class.
 ///
 /// This is equivalent of Node's `new http.Agent([options])`.
-HttpAgent createHttpAgent([HttpAgentOptions options]) {
+HttpAgent createHttpAgent([HttpAgentOptions? options]) {
   var args = (options == null) ? [] : [options];
   return callConstructor(http.Agent, args);
 }

diff --git a/node_interop/lib/https.dart b/node_interop/lib/https.dart
@@ -25,12 +25,12 @@ export 'http.dart'
         ServerResponse;
 
 HTTPS get https => _https ??= require('https');
-HTTPS _https;
+HTTPS? _https;
 
 /// Convenience method for creating instances of "https" module's Agent class.
 ///
 /// This is equivalent of Node's `new https.Agent([options])`.
-HttpsAgent createHttpsAgent([HttpsAgentOptions options]) {
+HttpsAgent createHttpsAgent([HttpsAgentOptions? options]) {
   var args = (options == null) ? [] : [options];
   return callConstructor(https.Agent, args);
 }

diff --git a/node_interop/lib/net.dart b/node_interop/lib/net.dart
@@ -14,7 +14,7 @@ import 'node.dart';
 import 'stream.dart';
 
 Net get net => _net ??= require('net');
-Net _net;
+Net? _net;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/os.dart b/node_interop/lib/os.dart
@@ -12,7 +12,7 @@ import 'package:js/js.dart';
 import 'node.dart';
 
 OS get os => _os ??= require('os');
-OS _os;
+OS? _os;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/path.dart b/node_interop/lib/path.dart
@@ -12,7 +12,7 @@ import 'package:js/js.dart';
 import 'node.dart';
 
 Path get path => _path ??= require('path');
-Path _path;
+Path? _path;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/querystring.dart b/node_interop/lib/querystring.dart
@@ -12,7 +12,7 @@ import 'package:js/js.dart';
 import 'node.dart';
 
 QueryString get querystring => _querystring ??= require('querystring');
-QueryString _querystring;
+QueryString? _querystring;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/stream.dart b/node_interop/lib/stream.dart
@@ -14,7 +14,7 @@ import 'node.dart';
 
 /// The "stream" module's object as returned from [require] call.
 StreamModule get stream => _stream ??= require('stream');
-StreamModule _stream;
+StreamModule? _stream;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/tls.dart b/node_interop/lib/tls.dart
@@ -17,7 +17,7 @@ import 'node.dart';
 import 'stream.dart';
 
 TLS get tls => _tls ??= require('tls');
-TLS _tls;
+TLS? _tls;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/tty.dart b/node_interop/lib/tty.dart
@@ -33,7 +33,7 @@ import 'net.dart';
 import 'node.dart';
 
 TTY get tty => _tty ??= require('tls');
-TTY _tty;
+TTY? _tty;
 
 @JS()
 @anonymous

diff --git a/node_interop/lib/util.dart b/node_interop/lib/util.dart
@@ -16,7 +16,7 @@ import 'node.dart';
 export 'package:js/js_util.dart' hide jsify;
 
 Util get util => _util ??= require('util');
-Util _util;
+Util? _util;
 
 @JS()
 @anonymous
@@ -43,7 +43,7 @@ abstract class Util {
 ///
 /// See also:
 /// - [jsify]
-T dartify<T>(Object jsObject) {
+T dartify<T>(dynamic jsObject) {
   if (_isBasicType(jsObject)) {
     return jsObject as T;
   }
@@ -106,15 +106,15 @@ Future<T> promiseToFuture<T>(Promise promise) {
 /// - [promiseToFuture]
 Promise futureToPromise<T>(Future<T> future) {
   return Promise(allowInterop((Function resolve, Function reject) {
-    future.then(resolve, onError: reject);
+    future.then((result) => resolve(result), onError: reject);
   }));
 }
 
 /// Returns a function that can be passed to a Node.js-style asynchronous
 /// callback that will complete [completer] with that callback's error or
 /// success result.
 void Function(Object, T) callbackToCompleter<T>(Completer<T> completer) {
-  return allowInterop((error, [value]) {
+  return allowInterop((Object? error, [T? value]) {
     if (error != null) {
       completer.completeError(error);
     } else {

diff --git a/node_interop/lib/worker_threads.dart b/node_interop/lib/worker_threads.dart
@@ -45,7 +45,7 @@ MessageChannel createMessageChannel() =>
     callConstructor(worker.MessageChannel, []);
 
 /// Creates a [Worker] using `new Worker().
-Worker createWorker(String filename, [WorkerOptions options]) =>
+Worker createWorker(String filename, [WorkerOptions? options]) =>
     callConstructor(worker.Worker, [filename, options]);
 
 abstract class ReceivedMessage {

diff --git a/node_interop/pubspec.yaml b/node_interop/pubspec.yaml
@@ -1,11 +1,11 @@
 name: node_interop
 description: Provides Dart bindings and utility functions for core Node.js modules.
-version: 1.2.1
+version: 2.0.0-nullsafety.0
 homepage: https://github.com/pulyaevskiy/node-interop
 author: Anatoly Pulyaevskiy <[email protected]>
 
 environment:
-  sdk: '>=2.9.0 <3.0.0'
+  sdk: '>=2.12.0 <3.0.0'
 
 dependencies:
   js: ^0.6.1

diff --git a/node_interop/test/child_process_test.dart b/node_interop/test/child_process_test.dart
@@ -6,15 +6,16 @@ import 'dart:js';
 
 @TestOn('node')
 import 'package:node_interop/child_process.dart';
+import 'package:node_interop/node.dart';
 import 'package:test/test.dart';
 
 void main() {
   group('child_process', () {
     test('exec successful', () {
       final completer = Completer<int>();
       childProcess.exec('ls -la', ExecOptions(),
-          allowInterop((error, stdout, stderr) {
-        int result = (error == null) ? 0 : error.code;
+          allowInterop((NodeJsError? error, stdout, stderr) {
+        var result = (error == null) ? 0 : int.parse(error.code);
         completer.complete(result);
       }));
       expect(completer.future, completion(0));

diff --git a/node_interop/test/timers_test.dart b/node_interop/test/timers_test.dart
@@ -13,7 +13,7 @@ void main() {
     test('setImmediate', () async {
       var buffer = StringBuffer();
 
-      void writesToBuffer(StringBuffer buffer) async {
+      Future<void> writesToBuffer(StringBuffer buffer) async {
         buffer.writeln('before');
         await setImmediateFuture(buffer);
         buffer.writeln('after');
@@ -26,7 +26,7 @@ void main() {
 }
 
 Future setImmediateFuture(StringBuffer buffer) {
-  final completer = Completer<String>();
+  final completer = Completer();
   setImmediate(allowInterop((StringBuffer value) {
     value.writeln('hello world');
     completer.complete();

diff --git a/node_interop/test/worker_threads_test.dart b/node_interop/test/worker_threads_test.dart
@@ -24,7 +24,7 @@ void main() {
         allowInterop(Zone.current.bindUnaryCallback(expectAsync1((message) {
       expect(message, equals(13));
       var result = worker.terminate();
-      if (result != null) expect(promiseToFuture(result), completes);
+      expect(promiseToFuture(result), completes);
     }))));
   });
 }
diff --git a/node_io/lib/src/internet_address.dart b/node_io/lib/src/internet_address.dart
@@ -38,7 +38,7 @@ class InternetAddress implements io.InternetAddress {
   InternetAddress._(this.address, [this._host])
       : _inAddr = _inet_pton(address) {
     if (net.isIP(address) == 0) {
-      throw ArgumentError('${address} is not valid.');
+      throw ArgumentError('$address is not valid.');
     }
   }
 

diff --git a/node_io/lib/src/streams.dart b/node_io/lib/src/streams.dart
@@ -179,20 +179,11 @@ class NodeIOSink extends WritableStream<List<int>> implements IOSink {
     return Buffer.from(data);
   }
 
-  Encoding _encoding;
-
-  NodeIOSink(Writable nativeStream, {Encoding encoding = utf8})
-      : super(nativeStream, convert: _nodeIoSinkConvert) {
-    _encoding = encoding;
-  }
+  NodeIOSink(Writable nativeStream, {this.encoding = utf8})
+      : super(nativeStream, convert: _nodeIoSinkConvert);
 
   @override
-  Encoding get encoding => _encoding;
-
-  @override
-  set encoding(Encoding value) {
-    _encoding = value;
-  }
+  Encoding encoding;
 
   @override
   Future flush() => drain;