[ddc] Migrate command to null safety

Revert small diffs introduced in earlier migrations to avoid an
import of command.dart before it was migrated.

Issue: #46617
Change-Id: Ia728fd727417fd4fdf8c454a0044e096e9de5756
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/248948
Reviewed-by: Anna Gringauze <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>

Author: nshahan

pkg/dev_compiler/lib/src/kernel/command.dart

@@ -2,8 +2,6 @@
 // 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.
 
-// @dart = 2.9
-
 import 'dart:async';
 import 'dart:convert' show json;
 import 'dart:io';
@@ -46,8 +44,8 @@ const _binaryName = 'dartdevc -k';
 /// The result may also contain a [previousResult], which can be passed back in
 /// for batch/worker executions to attempt to existing state.
 Future<CompilerResult> compile(ParsedArguments args,
-    {fe.InitializedCompilerState compilerState,
-    Map<Uri, List<int>> inputDigests}) {
+    {fe.InitializedCompilerState? compilerState,
+    Map<Uri, List<int>>? inputDigests}) {
   if (compilerState != null && !args.isBatchOrWorker) {
     throw ArgumentError(
         'previousResult requires --batch or --bazel_worker mode/');
@@ -83,10 +81,10 @@ String _usageMessage(ArgParser ddcArgParser) =>
     '${ddcArgParser.usage}';
 
 Future<CompilerResult> _compile(List<String> args,
-    {fe.InitializedCompilerState compilerState,
+    {fe.InitializedCompilerState? compilerState,
     bool isWorker = false,
     bool useIncrementalCompiler = false,
-    Map<Uri, List<int>> inputDigests}) async {
+    Map<Uri, List<int>>? inputDigests}) async {
   // TODO(jmesserly): refactor options to share code with dartdevc CLI.
   var argParser = ArgParser(allowTrailingOptions: true)
     ..addFlag('help',
@@ -98,7 +96,9 @@ Future<CompilerResult> _compile(List<String> args,
         defaultsTo: false,
         hide: true)
     ..addFlag('track-widget-creation',
-        help: 'Enable inspecting of Flutter widgets.', hide: true)
+        help: 'Enable inspecting of Flutter widgets.',
+        defaultsTo: false,
+        hide: true)
     // TODO(jmesserly): add verbose help to show hidden options
     ..addOption('dart-sdk-summary',
         help: 'The path to the Dart SDK summary file.', hide: true)
@@ -198,9 +198,10 @@ Future<CompilerResult> _compile(List<String> args,
 
   var summaryPaths = options.summaryModules.keys.toList();
   var summaryModules = Map.fromIterables(
-      summaryPaths.map(sourcePathToUri), options.summaryModules.values);
-  var sdkSummaryPath = argResults['dart-sdk-summary'] as String;
-  var librarySpecPath = argResults['libraries-file'] as String;
+      summaryPaths.map(sourcePathToUri).cast<Uri>(),
+      options.summaryModules.values);
+  var sdkSummaryPath = argResults['dart-sdk-summary'] as String?;
+  var librarySpecPath = argResults['libraries-file'] as String?;
   if (sdkSummaryPath == null) {
     sdkSummaryPath =
         defaultSdkSummaryPath(soundNullSafety: options.soundNullSafety);
@@ -246,7 +247,8 @@ Future<CompilerResult> _compile(List<String> args,
   // TODO(jmesserly): conceptually CFE should not need a .packages file to
   // resolve package URIs that are in the input summaries, but it seems to.
   // This needs further investigation.
-  var packageFile = argResults['packages'] as String ?? _findPackagesFilePath();
+  var packageFile =
+      argResults['packages'] as String? ?? _findPackagesFilePath();
 
   var succeeded = true;
   void diagnosticMessageHandler(fe.DiagnosticMessage message) {
@@ -259,14 +261,13 @@ Future<CompilerResult> _compile(List<String> args,
   var explicitExperimentalFlags = fe.parseExperimentalFlags(options.experiments,
       onError: stderr.writeln, onWarning: print);
 
-  var trackWidgetCreation =
-      argResults['track-widget-creation'] as bool ?? false;
+  var trackWidgetCreation = argResults['track-widget-creation'] as bool;
 
   var compileSdk = argResults['compile-sdk'] == true;
   var oldCompilerState = compilerState;
-  List<Component> doneAdditionalDills;
-  fe.IncrementalCompiler incrementalCompiler;
-  fe.WorkerInputComponent cachedSdkInput;
+  var doneAdditionalDills = <Component>[];
+  fe.IncrementalCompiler? incrementalCompiler;
+  fe.WorkerInputComponent? cachedSdkInput;
   var recordUsedInputs = argResults['used-inputs-file'] != null;
   var additionalDills = summaryModules.keys.toList();
 
@@ -291,18 +292,17 @@ Future<CompilerResult> _compile(List<String> args,
     // If digests weren't given and if not in worker mode, create fake data and
     // ensure we don't have a previous state (as that wouldn't be safe with
     // fake input digests).
-    if (!isWorker && (inputDigests == null || inputDigests.isEmpty)) {
+    inputDigests ??= {};
+    if (!isWorker && inputDigests.isEmpty) {
       oldCompilerState = null;
-      inputDigests ??= {};
+
       if (!compileSdk) {
         inputDigests[sourcePathToUri(sdkSummaryPath)] = const [0];
       }
       for (var uri in summaryModules.keys) {
         inputDigests[uri] = const [0];
       }
     }
-
-    doneAdditionalDills = List<Component>(summaryModules.length);
     compilerState = await fe.initializeIncrementalCompiler(
         oldCompilerState,
         {
@@ -329,7 +329,7 @@ Future<CompilerResult> _compile(List<String> args,
             options.soundNullSafety ? fe.NnbdMode.Strong : fe.NnbdMode.Weak);
     incrementalCompiler = compilerState.incrementalCompiler;
     cachedSdkInput =
-        compilerState.workerInputCache[sourcePathToUri(sdkSummaryPath)];
+        compilerState.workerInputCache![sourcePathToUri(sdkSummaryPath)];
   }
 
   // TODO(jmesserly): is there a cleaner way to do this?
@@ -338,20 +338,20 @@ Future<CompilerResult> _compile(List<String> args,
   // `initializeCompiler`. Also we should be able to pass down Components for
   // SDK and summaries.
   //
-  fe.DdcResult result;
+  fe.DdcResult? result;
   if (!useIncrementalCompiler) {
     result = await fe.compile(compilerState, inputs, diagnosticMessageHandler);
   } else {
     compilerState.options.onDiagnostic = diagnosticMessageHandler;
-    var incrementalCompilerResult = await incrementalCompiler.computeDelta(
+    var incrementalCompilerResult = await incrementalCompiler!.computeDelta(
         entryPoints: inputs,
         fullComponent: true,
         trackNeededDillLibraries: recordUsedInputs);
     result = fe.DdcResult(
         incrementalCompilerResult.component,
-        cachedSdkInput.component,
+        cachedSdkInput!.component,
         doneAdditionalDills,
-        incrementalCompilerResult.classHierarchy,
+        incrementalCompilerResult.classHierarchy!,
         incrementalCompilerResult.neededDillLibraries);
   }
   compilerState.options.onDiagnostic = null; // See http://dartbug.com/36983.
@@ -393,7 +393,7 @@ Future<CompilerResult> _compile(List<String> args,
     kernel.BinaryPrinter(sink).writeComponentFile(component);
     outFiles.add(sink.flush().then((_) => sink.close()));
   }
-  String fullDillUri;
+  String? fullDillUri;
   if (argResults['experimental-output-compiled-kernel'] as bool) {
     if (outPaths.length > 1) {
       print(
@@ -433,7 +433,7 @@ Future<CompilerResult> _compile(List<String> args,
   final summaryToModule = Map<Component, String>.identity();
   for (var i = 0; i < result.additionalDills.length; i++) {
     var additionalDill = result.additionalDills[i];
-    var moduleImport = summaryModules[additionalDills[i]];
+    var moduleImport = summaryModules[additionalDills[i]]!;
     for (var l in additionalDill.libraries) {
       assert(!importToSummary.containsKey(l));
       importToSummary[l] = additionalDill;
@@ -487,12 +487,12 @@ Future<CompilerResult> _compile(List<String> args,
   if (recordUsedInputs) {
     var usedOutlines = <Uri>{};
     if (useIncrementalCompiler) {
-      var neededDillLibraries = result.neededDillLibraries;
-      compilerState.incrementalCompiler.updateNeededDillLibrariesWithHierarchy(
+      var neededDillLibraries = result.neededDillLibraries!;
+      compilerState.incrementalCompiler!.updateNeededDillLibrariesWithHierarchy(
           neededDillLibraries, result.classHierarchy);
       for (var lib in neededDillLibraries) {
         if (lib.importUri.isScheme('dart')) continue;
-        var uri = compilerState.libraryToInputDill[lib.importUri];
+        var uri = compilerState.libraryToInputDill![lib.importUri];
         if (uri == null) {
           throw StateError('Library ${lib.importUri} was recorded as used, '
               'but was not in the list of known libraries.');
@@ -650,20 +650,20 @@ class JSCode {
   ///
   /// The source paths will initially be absolute paths. They can be adjusted
   /// using [placeSourceMap].
-  final Map sourceMap;
+  final Map? sourceMap;
 
   /// Module and library information
   ///
   /// The [metadata] is a contract between compiler and the debugger,
   /// helping the debugger map between libraries, modules, source paths.
   /// see: https://goto.google.com/dart-web-debugger-metadata
-  final ModuleMetadata metadata;
+  final ModuleMetadata? metadata;
 
   /// Module debug symbols.
   ///
   /// The [symbols] is a contract between compiler and the debugger,
   /// helping the debugger map between dart and JS objects.
-  final ModuleSymbols symbols;
+  final ModuleSymbols? symbols;
 
   JSCode(this.code, this.sourceMap, {this.symbols, this.metadata});
 }
@@ -677,18 +677,18 @@ JSCode jsProgramToCode(js_ast.Program moduleTree, ModuleFormat format,
     bool inlineSourceMap = false,
     bool emitDebugMetadata = false,
     bool emitDebugSymbols = false,
-    String jsUrl,
-    String mapUrl,
-    String fullDillUri,
-    String sourceMapBase,
-    String customScheme,
-    String multiRootOutputPath,
-    ProgramCompiler compiler,
-    Component component}) {
+    String? jsUrl,
+    String? mapUrl,
+    String? fullDillUri,
+    String? sourceMapBase,
+    String? customScheme,
+    String? multiRootOutputPath,
+    ProgramCompiler? compiler,
+    Component? component}) {
   var opts = js_ast.JavaScriptPrintingOptions(
       allowKeywordsInProperties: true, allowSingleLineIfStatements: true);
   js_ast.SimpleJavaScriptPrintingContext printer;
-  SourceMapBuilder sourceMap;
+  SourceMapBuilder? sourceMap;
   if (buildSourceMap) {
     var sourceMapContext = SourceMapPrintingContext();
     sourceMap = sourceMapContext.sourceMap;
@@ -702,9 +702,9 @@ JSCode jsProgramToCode(js_ast.Program moduleTree, ModuleFormat format,
   tree.accept(js_ast.Printer(opts, printer,
       localNamer: js_ast.TemporaryNamer(tree, nameListener)));
 
-  Map builtMap;
+  Map? builtMap;
   if (buildSourceMap && sourceMap != null) {
-    builtMap = placeSourceMap(sourceMap.build(jsUrl), mapUrl, customScheme,
+    builtMap = placeSourceMap(sourceMap.build(jsUrl!), mapUrl!, customScheme,
         multiRootOutputPath: multiRootOutputPath, sourceMapBase: sourceMapBase);
     var jsDir = p.dirname(p.fromUri(jsUrl));
     var relative = p.relative(p.fromUri(mapUrl), from: jsDir);
@@ -733,19 +733,19 @@ JSCode jsProgramToCode(js_ast.Program moduleTree, ModuleFormat format,
   // our runtime metrics gathering would obtain this information from the
   // compilation server, not the browser.  We don't yet have the infra for that.
   var compileTimeStatistics = {
-    'dartSize': component != null ? _computeDartSize(component) : null,
+    'dartSize': _computeDartSize(component!),
     'sourceMapSize': encodedMap.length
   };
   text = text.replaceFirst(
       SharedCompiler.metricsLocationID, '$compileTimeStatistics');
 
   var debugMetadata = emitDebugMetadata
-      ? _emitMetadata(moduleTree, component, mapUrl, jsUrl, fullDillUri)
+      ? _emitMetadata(moduleTree, component, mapUrl!, jsUrl!, fullDillUri)
       : null;
 
   var debugSymbols = emitDebugSymbols
       ? _emitSymbols(
-          compiler, moduleTree.name, nameListener.identifierNames, component)
+          compiler!, moduleTree.name!, nameListener!.identifierNames, component)
       : null;
 
   return JSCode(text, builtMap, symbols: debugSymbols, metadata: debugMetadata);
@@ -759,17 +759,26 @@ JSCode jsProgramToCode(js_ast.Program moduleTree, ModuleFormat format,
 /// names used when outputting the JavaScript.
 ModuleSymbols _emitSymbols(ProgramCompiler compiler, String moduleName,
     Map<js_ast.Identifier, String> identifierNames, Component component) {
+  /// Returns the name selected in the final Javascript for [id].
+  String lookupName(js_ast.Identifier id) {
+    var name = identifierNames[id];
+    if (name == null) {
+      throw Exception('No recorded naming decision found for Identifier with '
+          'name: ${id.name}');
+    }
+    return name;
+  }
+
   var classJsNames = <Class, String>{
-    for (var e in compiler.classIdentifiers.entries)
-      e.key: identifierNames[e.value],
+    for (var e in compiler.classIdentifiers.entries) e.key: lookupName(e.value),
   };
   var procedureJsNames = <Procedure, String>{
     for (var e in compiler.procedureIdentifiers.entries)
-      e.key: identifierNames[e.value],
+      e.key: lookupName(e.value),
   };
   var variableJsNames = <VariableDeclaration, String>{
     for (var e in compiler.variableIdentifiers.entries)
-      e.key: identifierNames[e.value],
+      e.key: lookupName(e.value),
   };
 
   return ModuleSymbolsCollector(moduleName, classJsNames, compiler.memberNames,
@@ -778,10 +787,10 @@ ModuleSymbols _emitSymbols(ProgramCompiler compiler, String moduleName,
 }
 
 ModuleMetadata _emitMetadata(js_ast.Program program, Component component,
-    String sourceMapUri, String moduleUri, String fullDillUri) {
+    String sourceMapUri, String moduleUri, String? fullDillUri) {
   var metadata = ModuleMetadata(
-      program.name,
-      loadFunctionName(program.name),
+      program.name!,
+      loadFunctionName(program.name!),
       sourceMapUri,
       moduleUri,
       fullDillUri,
@@ -803,7 +812,7 @@ Map<String, String> parseAndRemoveDeclaredVariables(List<String> args) {
   var declaredVariables = <String, String>{};
   for (var i = 0; i < args.length;) {
     var arg = args[i];
-    String rest;
+    String? rest;
     const defineFlag = '--define';
     if (arg.startsWith('-D') && arg.length > 2) {
       rest = arg.substring(2);
@@ -835,7 +844,7 @@ Map<String, String> parseAndRemoveDeclaredVariables(List<String> args) {
 
 /// The default path of the kernel summary for the Dart SDK given the
 /// [soundNullSafety] mode.
-String defaultSdkSummaryPath({bool soundNullSafety}) {
+String defaultSdkSummaryPath({required bool soundNullSafety}) {
   var outlineDill = soundNullSafety ? 'ddc_outline_sound.dill' : 'ddc_sdk.dill';
   return p.join(getSdkPath(), 'lib', '_internal', outlineDill);
 }
@@ -850,7 +859,7 @@ String getSdkPath() => p.dirname(p.dirname(Platform.resolvedExecutable));
 ///
 /// Checks for a `.dart_tool/package_config.json` file in the current working
 /// directory, or in any parent directory.
-String _findPackagesFilePath() {
+String? _findPackagesFilePath() {
   // TODO(jmesserly): this was copied from package:package_config/discovery.dart
   // Unfortunately the relevant function is not public. CFE APIs require a URI
   // to the .packages file, rather than letting us provide the package map data.
@@ -872,9 +881,9 @@ String _findPackagesFilePath() {
 }
 
 /// Inputs must be absolute paths. Returns null if no prefixing path is found.
-String _longestPrefixingPath(Uri baseUri, List<Uri> prefixingPaths) {
+String? _longestPrefixingPath(Uri baseUri, List<Uri> prefixingPaths) {
   var basePath = baseUri.path;
-  return prefixingPaths.fold(null, (String previousValue, Uri element) {
+  return prefixingPaths.fold(null, (String? previousValue, Uri element) {
     if (basePath.startsWith(element.path) &&
         (previousValue == null || previousValue.length < element.path.length)) {
       return element.path;

pkg/dev_compiler/test/nullable_inference_test.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:io';
 
+import 'package:dev_compiler/src/kernel/command.dart' show getSdkPath;
 import 'package:dev_compiler/src/kernel/js_typerep.dart';
 import 'package:dev_compiler/src/kernel/nullable_inference.dart';
 import 'package:dev_compiler/src/kernel/target.dart';
@@ -18,8 +19,6 @@ import 'package:kernel/type_environment.dart';
 import 'package:path/path.dart' as p;
 import 'package:test/test.dart';
 
-import 'shared_test_options.dart' show getSdkPath;
-
 const AstTextStrategy astTextStrategy = AstTextStrategy(
     includeLibraryNamesInTypes: true,
     includeLibraryNamesInMembers: true,
@@ -657,17 +656,13 @@ Future<CompileResult> kernelCompile(String code) async {
   var sdkUri = Uri.file('/memory/dart_sdk.dill');
   var sdkFile = _fileSystem.entityForUri(sdkUri);
   if (!await sdkFile.exists()) {
-    // TODO(46617) Call getSdkPath() from command.dart instead.
-    var sdkPath = getSdkPath();
-    var outlineDill = p.join(sdkPath, 'lib', '_internal', 'ddc_sdk.dill');
+    var outlineDill = p.join(getSdkPath(), 'lib', '_internal', 'ddc_sdk.dill');
     sdkFile.writeAsBytesSync(File(outlineDill).readAsBytesSync());
   }
   var librariesUri = Uri.file('/memory/libraries.json');
   var librariesFile = _fileSystem.entityForUri(librariesUri);
   if (!await librariesFile.exists()) {
-    // TODO(46617) Call getSdkPath() from command.dart instead.
-    var sdkPath = getSdkPath();
-    var librariesJson = p.join(sdkPath, 'lib', 'libraries.json');
+    var librariesJson = p.join(getSdkPath(), 'lib', 'libraries.json');
     librariesFile.writeAsBytesSync(File(librariesJson).readAsBytesSync());
   }
   var packagesUri = Uri.file('/memory/.packages');