dartdev: Simplify some JSON-parsing code; remove dead code

* In `_handleServerResponse`, use if-case with map patterns to avoid
  casts and null checks and repeatedly fetching map values.
* Remove the castStringKeyedMap utility. After some other changes,
  most call-sites passed in a Map, so then half of the utility is not
  used. In the others, it seemed simplest to use `as Map` and `.cast()`
  inline.
* Change some `dynamic` local variables to be `Object?` instead.
* Replace `DartdevCommand.project` getter and `_project` field with a
  single final field.
* Remove unused properties from `Project` and unused `PackageConfig`
  class.
* Remove associated tests.

Change-Id: I1f626ecc0e6e4d27ef24f65959fd4cb54fb5fc92
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412980
Reviewed-by: Devon Carew <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/dartdev/lib/src/analysis_server.dart

@@ -69,8 +69,8 @@ class AnalysisServer {
     // {"event":"server.status","params":{"analysis":{"isAnalyzing":true}}}
     return _streamController('server.status')
         .stream
-        .where((event) => event!['analysis'] != null)
-        .map((event) => (event!['analysis']['isAnalyzing']!) as bool);
+        .where((event) => event['analysis'] != null)
+        .map((event) => (event['analysis']['isAnalyzing']!) as bool);
   }
 
   /// This future completes when we next receive an analysis finished event
@@ -79,15 +79,15 @@ class AnalysisServer {
   Future<bool>? get analysisFinished => _analysisFinished?.future;
 
   Stream<FileAnalysisErrors> get onErrors {
-    // {"event":"analysis.errors","params":{"file":"/Users/.../lib/main.dart","errors":[]}}
+    // {"event":"analysis.errors",
+    //  "params":{"file":"/Users/.../lib/main.dart","errors":[]}}
     return _streamController('analysis.errors').stream.map((event) {
-      final file = event!['file'] as String;
+      final file = event['file'] as String;
       final errorsList = event['errors'] as List<dynamic>;
-      final errors = errorsList
-          .map<Map<String, dynamic>>(castStringKeyedMap)
-          .map<AnalysisError>(
-              (Map<String, dynamic> json) => AnalysisError(json))
-          .toList();
+      final errors = [
+        for (final error in errorsList)
+          AnalysisError((error as Map).cast<String, dynamic>())
+      ];
       return FileAnalysisErrors(file, errors);
     });
   }
@@ -281,24 +281,18 @@ class AnalysisServer {
   void _handleServerResponse(String line) {
     log.trace('<== $line');
 
-    final dynamic response = json.decode(line);
+    final response = json.decode(line) as Object?;
 
     if (response is Map<String, dynamic>) {
-      if (response['event'] != null) {
-        final event = response['event'] as String;
-        final dynamic params = response['params'];
-
+      if (response
+          case {'event': final String event, 'params': final Object? params}) {
         if (params is Map<String, dynamic>) {
-          _streamController(event).add(castStringKeyedMap(params));
+          _streamController(event).add(params.cast<String, dynamic>());
         }
-      } else if (response['id'] != null) {
-        final id = response['id'];
-
-        if (response['error'] != null) {
-          final error = castStringKeyedMap(response['error']);
-          _requestCompleters
-              .remove(id)
-              ?.completeError(RequestError.parse(error));
+      } else if (response case {'id': final String id}) {
+        if (response case {'error': final Map error}) {
+          _requestCompleters.remove(id)?.completeError(
+              RequestError.parse(error.cast<String, dynamic>()));
         } else {
           _requestCompleters.remove(id)?.complete(response['result'] ?? {});
         }
@@ -315,12 +309,13 @@ class AnalysisServer {
         'details:\n');
     // Fields are 'isFatal', 'message', and 'stackTrace'.
     log.stderr(err['message']);
-    if (err['stackTrace'] != null) {
-      log.stderr(err['stackTrace'] as String);
+    final stackTrace = err['stackTrace'];
+    if (stackTrace is String && stackTrace.isNotEmpty) {
+      log.stderr(stackTrace);
     }
   }
 
-  StreamController<Map<String, dynamic>?> _streamController(String streamId) {
+  StreamController<Map<String, dynamic>> _streamController(String streamId) {
     return _streamControllers.putIfAbsent(
         streamId, () => StreamController<Map<String, dynamic>>.broadcast());
   }

pkg/dartdev/lib/src/core.dart

@@ -32,8 +32,7 @@ abstract class DartdevCommand extends Command<int> {
   final String _name;
   final String _description;
   final bool _verbose;
-
-  Project? _project;
+  final Project project = Project();
 
   @override
   final bool hidden;
@@ -70,8 +69,6 @@ abstract class DartdevCommand extends Command<int> {
   /// Subclasses can override this in order to create a customized ArgParser.
   ArgParser createArgParser() =>
       ArgParser(usageLineLength: dartdevUsageLineLength);
-
-  Project get project => _project ??= Project();
 }
 
 extension DartDevCommand on Command {
@@ -147,8 +144,6 @@ Future _streamLineTransform(
 class Project {
   final Directory dir;
 
-  PackageConfig? _packageConfig;
-
   Project() : dir = Directory.current;
 
   Project.fromDirectory(this.dir);
@@ -157,37 +152,4 @@ class Project {
       FileSystemEntity.isFileSync(path.join(dir.path, 'pubspec.yaml'));
 
   File get pubspecFile => File(path.join(dir.path, 'pubspec.yaml'));
-
-  bool get hasPackageConfigFile => packageConfig != null;
-
-  PackageConfig? get packageConfig {
-    if (_packageConfig == null) {
-      File file =
-          File(path.join(dir.path, '.dart_tool', 'package_config.json'));
-
-      if (file.existsSync()) {
-        try {
-          dynamic contents = json.decode(file.readAsStringSync());
-          _packageConfig = PackageConfig(contents);
-        } catch (_) {}
-      }
-    }
-
-    return _packageConfig;
-  }
-}
-
-/// A simple representation of a `package_config.json` file.
-class PackageConfig {
-  final Map<String, dynamic> contents;
-
-  PackageConfig(this.contents);
-
-  List<Map<String, dynamic>?> get packages {
-    List<dynamic> packages = contents['packages'];
-    return packages.map<Map<String, dynamic>?>(castStringKeyedMap).toList();
-  }
-
-  bool hasDependency(String packageName) =>
-      packages.any((element) => element!['name'] == packageName);
 }

pkg/dartdev/lib/src/utils.dart

@@ -109,13 +109,6 @@ ArgParser globalDartdevOptionsParser({bool verbose = false}) {
   return argParser;
 }
 
-/// Given a data structure which is a Map of String to dynamic values, return
-/// the same structure (`Map<String, dynamic>`) with the correct runtime types.
-Map<String, dynamic> castStringKeyedMap(dynamic untyped) {
-  final Map<dynamic, dynamic> map = untyped! as Map<dynamic, dynamic>;
-  return map.cast<String, dynamic>();
-}
-
 /// Emit the given word with the correct pluralization.
 String pluralize(String word, int count) => count == 1 ? word : '${word}s';
 

pkg/dartdev/test/core_test.dart

@@ -2,7 +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.
 
-import 'dart:convert';
 import 'dart:io';
 
 import 'package:dartdev/src/commands/analyze.dart';
@@ -20,7 +19,6 @@ import 'utils.dart';
 void main() {
   initGlobalState();
   group('DartdevCommand', _dartdevCommand);
-  group('PackageConfig', _packageConfig);
   group('Project', _project);
 }
 
@@ -110,19 +108,6 @@ void _dartdevCommand() {
   });
 }
 
-void _packageConfig() {
-  test('packages', () {
-    PackageConfig packageConfig = PackageConfig(jsonDecode(_packageData));
-    expect(packageConfig.packages, isNotEmpty);
-  });
-
-  test('hasDependency', () {
-    PackageConfig packageConfig = PackageConfig(jsonDecode(_packageData));
-    expect(packageConfig.hasDependency('test'), isFalse);
-    expect(packageConfig.hasDependency('lints'), isTrue);
-  });
-}
-
 void _project() {
   test('hasPubspecFile positive', () {
     final p = project();
@@ -138,43 +123,4 @@ void _project() {
     Project coreProj = Project.fromDirectory(p.dir);
     expect(coreProj.hasPubspecFile, isFalse);
   });
-
-  test('hasPackageConfigFile positive', () {
-    final p = project();
-    Project coreProj = Project.fromDirectory(p.dir);
-    expect(coreProj.hasPackageConfigFile, isTrue);
-    expect(coreProj.packageConfig, isNotNull);
-    expect(coreProj.packageConfig!.packages, isNotEmpty);
-  });
-
-  test('hasPackageConfigFile negative', () {
-    final p = project();
-    var packageConfig =
-        File(path.join(p.dirPath, '.dart_tool/package_config.json'));
-    packageConfig.deleteSync();
-    Project coreProj = Project.fromDirectory(p.dir);
-    expect(coreProj.hasPackageConfigFile, isFalse);
-  });
-}
-
-const String _packageData = '''{
-  "configVersion": 2,
-  "packages": [
-    {
-      "name": "lints",
-      "rootUri": "file:///Users/.../.pub-cache/hosted/pub.dartlang.org/lints-1.0.1",
-      "packageUri": "lib/",
-      "languageVersion": "2.1"
-    },
-    {
-      "name": "args",
-      "rootUri": "../",
-      "packageUri": "lib/",
-      "languageVersion": "2.3"
-    }
-  ],
-  "generated": "2020-03-01T03:38:14.906205Z",
-  "generator": "pub",
-  "generatorVersion": "2.8.0-dev.10.0"
 }
-''';

pkg/dartdev/test/utils_test.dart

@@ -2,7 +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.
 
-import 'dart:convert';
 import 'dart:io';
 
 import 'package:dartdev/src/utils.dart';
@@ -54,29 +53,6 @@ void main() {
     });
   });
 
-  group('castStringKeyedMap', () {
-    test('fails', () {
-      dynamic contents = json.decode(_packageData);
-      List<dynamic> packages = contents['packages'];
-      try {
-        // ignore: unused_local_variable
-        List<Map<String, dynamic>> mappedPackages =
-            packages as List<Map<String, dynamic>>;
-        fail('expected implicit cast to fail');
-      } on TypeError {
-        // TypeError is expected
-      }
-    });
-
-    test('succeeds', () {
-      dynamic contents = json.decode(_packageData);
-      List<dynamic> packages = contents['packages'];
-      List<Map<String, dynamic>> mappedPackages =
-          packages.map<Map<String, dynamic>>(castStringKeyedMap).toList();
-      expect(mappedPackages, isList);
-    });
-  });
-
   group('FileSystemEntityExtension', () {
     test('isDartFile', () {
       expect(File('foo.dart').isDartFile, isTrue);
@@ -185,25 +161,3 @@ void main() {
     });
   });
 }
-
-const String _packageData = '''{
-  "configVersion": 2,
-  "packages": [
-    {
-      "name": "lints",
-      "rootUri": "file:///Users/.../.pub-cache/hosted/pub.dartlang.org/lints-1.0.1",
-      "packageUri": "lib/",
-      "languageVersion": "2.1"
-    },
-    {
-      "name": "args",
-      "rootUri": "../",
-      "packageUri": "lib/",
-      "languageVersion": "2.3"
-    }
-  ],
-  "generated": "2020-03-01T03:38:14.906205Z",
-  "generator": "pub",
-  "generatorVersion": "2.8.0-dev.10.0"
-}
-''';