use cli_utils to get SDK dir

also a small cleanup to make it easier to construct TypeResolver (fixes a TODO) and fix a couple of typos

[email protected]

Review URL: https://chromereviews.googleplex.com/145437014

Author: John Messerly

bin/devc.dart

@@ -6,11 +6,11 @@ library ddc.bin.checker;
 import 'dart:io';
 
 import 'package:args/args.dart';
+import 'package:cli_util/cli_util.dart' show getSdkDir;
 import 'package:logging/logging.dart' show Logger, Level;
 
 import 'package:ddc/devc.dart';
-import 'package:ddc/src/checker/dart_sdk.dart'
-    show dartSdkDirectory, mockSdkSources;
+import 'package:ddc/src/checker/dart_sdk.dart' show mockSdkSources;
 import 'package:ddc/src/checker/resolver.dart' show TypeResolver;
 
 final ArgParser argParser = new ArgParser()
@@ -55,13 +55,13 @@ void main(List<String> argv) {
   bool shouldMockSdk = args['mock-sdk'];
   String dartSdkPath;
   if (!shouldMockSdk) {
-    dartSdkPath = args['dart-sdk'];
-    if (dartSdkPath == null) dartSdkPath = dartSdkDirectory;
-    if (dartSdkPath == null) {
+    var sdkDir = getSdkDir(argv);
+    if (sdkDir == null) {
       print('Could not automatically find dart sdk path.');
       print('Please pass in explicitly: --dart-sdk <path>');
       exit(1);
     }
+    dartSdkPath = sdkDir.path;
   }
 
   if (args.rest.length == 0) {
@@ -75,9 +75,9 @@ void main(List<String> argv) {
   var useColors = stdioType(stdout) == StdioType.TERMINAL;
   if (!args['dump-info']) setupLogger(level, print);
 
-  var typeResolver = new TypeResolver(shouldMockSdk
-      ? TypeResolver.sdkResolverFromMock(mockSdkSources)
-      : TypeResolver.sdkResolverFromDir(dartSdkPath));
+  var typeResolver = shouldMockSdk
+      ? new TypeResolver.fromMock(mockSdkSources)
+      : new TypeResolver.fromDir(dartSdkPath);
 
   var filename = args.rest.first;
   var result = compile(filename, typeResolver,

bin/edit_files.dart

@@ -11,12 +11,11 @@ import 'dart:io';
 import 'dart:convert';
 
 import 'package:args/args.dart';
-import 'package:ddc/src/report.dart';
+import 'package:cli_util/cli_util.dart' show getSdkDir;
 import 'package:source_maps/refactor.dart';
 import 'package:source_span/source_span.dart';
 
-import 'package:ddc/src/checker/dart_sdk.dart'
-    show dartSdkDirectory, mockSdkSources;
+import 'package:ddc/src/report.dart';
 import 'package:ddc/src/checker/resolver.dart' show TypeResolver;
 
 final ArgParser argParser = new ArgParser()
@@ -31,6 +30,7 @@ final ArgParser argParser = new ArgParser()
       help: 'regular expression of file names to include', defaultsTo: null)
   ..addOption('exclude-pattern',
       help: 'regular expression of file names to exclude', defaultsTo: null)
+  ..addOption('dart-sdk', help: 'Dart SDK Path', defaultsTo: null)
   ..addFlag('help', abbr: 'h', help: 'Display this message');
 
 void _showUsageAndExit() {
@@ -88,8 +88,8 @@ class EditFileSummaryVisitor extends RecursiveSummaryVisitor {
       Process.runSync(
           checkoutFilesExecutable, [checkoutFilesArg]..addAll(_files.keys));
     }
-    _files.forEach((name, transacation) {
-      var nestedPrinter = transacation.commit()..build(name);
+    _files.forEach((name, transaction) {
+      var nestedPrinter = transaction.commit()..build(name);
       new File(name).writeAsStringSync(nestedPrinter.text, flush: true);
     });
   }
@@ -104,9 +104,15 @@ void main(List<String> argv) {
     _showUsageAndExit();
   }
 
+  var sdkDir = getSdkDir(argv);
+  if (sdkDir == null) {
+    print('Could not automatically find dart sdk path.');
+    print('Please pass in explicitly: --dart-sdk <path>');
+    exit(1);
+  }
+
   var filename = args.rest.first;
-  var typeResolver =
-      new TypeResolver(TypeResolver.sdkResolverFromDir(dartSdkDirectory));
+  var typeResolver = new TypeResolver.fromDir(sdkDir.path);
 
   Map json = JSON.decode(new File(filename).readAsStringSync());
   var summary = GlobalSummary.parse(json);

lib/src/checker/dart_sdk.dart

@@ -3,54 +3,10 @@
 /// definitions to provide mock sdks.
 library ddc.src.checker.dart_sdk;
 
-import 'dart:convert' as convert;
-import 'dart:io' show File, Link, Platform, Process;
-import 'package:path/path.dart' as path;
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/sdk.dart';
 import 'package:analyzer/src/generated/source.dart';
 
-/// Attempts to provide the current Dart SDK directory.  Returns null if the SDK
-/// cannot be found.
-final String dartSdkDirectory = () {
-  bool isSdkDir(String dirname) =>
-      new File(path.join(dirname, 'lib', '_internal', 'libraries.dart'))
-          .existsSync();
-
-  String executable = Platform.executable;
-  if (path.split(executable).length == 1) {
-    // TODO(sigmund,blois): make this cross-platform.
-    // HACK: A single part, hope it's on the path.
-    executable = Process.runSync('which', ['dart'],
-        stdoutEncoding: convert.UTF8).stdout.trim();
-    // In case Dart is symlinked (e.g. homebrew on Mac) follow symbolic links.
-    var link = new Link(executable);
-    if (link.existsSync()) {
-      executable = link.resolveSymbolicLinksSync();
-    }
-    var sdkDir = path.dirname(path.dirname(executable));
-    if (isSdkDir(sdkDir)) return sdkDir;
-  }
-
-  var dartDir = path.dirname(path.absolute(executable));
-  // If there's a sub-dir named dart-sdk then we're most likely executing from
-  // a dart enlistment build directory.
-  if (isSdkDir(path.join(dartDir, 'dart-sdk'))) {
-    return path.join(dartDir, 'dart-sdk');
-  }
-  // If we can find libraries.dart then it's the root of the SDK.
-  if (isSdkDir(dartDir)) return dartDir;
-
-  var parts = path.split(dartDir);
-  // If the dart executable is within the sdk dir then get the root.
-  if (parts.contains('dart-sdk')) {
-    var dartSdkDir = path.joinAll(parts.take(parts.indexOf('dart-sdk') + 1));
-    if (isSdkDir(dartSdkDir)) return dartSdkDir;
-  }
-
-  return null;
-}();
-
 /// Dart SDK which contains a mock implementation of the SDK libraries. May be
 /// used to speed up execution when most of the core libraries is not needed.
 class MockDartSdk implements DartSdk {

lib/src/checker/resolver.dart

@@ -43,6 +43,16 @@ class TypeResolver {
     context.sourceFactory = new SourceFactory(resolvers);
   }
 
+  /// Creates a [TypeResolver] that uses a mock 'dart:' library contents.
+  TypeResolver.fromMock(Map<String, String> mockSources, [List otherResolvers])
+      : this(new MockDartSdk(mockSources, reportMissing: true).resolver,
+          otherResolvers);
+
+  /// Creates a [TypeResolver] that uses the SDK at the given [sdkPath].
+  TypeResolver.fromDir(String sdkPath, [List otherResolvers]) : this(
+          new DartUriResolver(
+              new DirectoryBasedDartSdk(new JavaFile(sdkPath))));
+
   /// Find the corresponding [Source] for [uri].
   Source findSource(Uri uri) {
     var source = _sources[uri];
@@ -67,21 +77,6 @@ class TypeResolver {
     }
     return failure;
   }
-
-  // TODO(jmesserly): in practice these are passed to `new TypeResolver` so
-  // that makes me think these should just be named constructors, rather than
-  // `new TypeResolver(TypeResolver.sdkResolverFromDir((...)) which repeats
-  // the name twice.
-
-  /// Creates a [DartUriResolver] that uses the SDK at the given [sdkPath].
-  static DartUriResolver sdkResolverFromDir(String sdkPath) =>
-      new DartUriResolver(new DirectoryBasedDartSdk(new JavaFile(sdkPath)));
-
-  /// Creates a [DartUriResolver] that uses a mock 'dart:' library contents.
-  static DartUriResolver sdkResolverFromMock(
-      Map<String, String> mockSdkSources) {
-    return new MockDartSdk(mockSdkSources, reportMissing: true).resolver;
-  }
 }
 
 /// Creates an analysis context that contains our restricted typing rules.

lib/src/testing.dart

@@ -30,7 +30,7 @@ import 'package:ddc/devc.dart' show compile;
 ///   logging level were the error would be reported at, and `Type` is the
 ///   concrete subclass of [StaticInfo] that denotes the error.
 ///
-/// For example, to check that an assigment produces a warning about a boxing
+/// For example, to check that an assignment produces a warning about a boxing
 /// conversion, you can describe the test as follows:
 ///
 ///     testChecker({
@@ -41,17 +41,16 @@ import 'package:ddc/devc.dart' show compile;
 ///       '''
 ///     });
 ///
-CheckerResults testChecker(Map<String, String> testFiles, {bool mockSdk: true,
+CheckerResults testChecker(Map<String, String> testFiles, {String sdkDir,
     CheckerReporter reporter, covariantGenerics: true, relaxedCasts: true}) {
   expect(testFiles.containsKey('/main.dart'), isTrue,
       reason: '`/main.dart` is missing in testFiles');
 
   // Create a resolver that can load test files from memory.
-  var dartUriResolver = mockSdk
-      ? TypeResolver.sdkResolverFromMock(mockSdkSources)
-      : TypeResolver.sdkResolverFromDir(dartSdkDirectory);
   var testUriResolver = new _TestUriResolver(testFiles);
-  var resolver = new TypeResolver(dartUriResolver, [testUriResolver]);
+  var resolver = sdkDir == null
+      ? new TypeResolver.fromMock(mockSdkSources, [testUriResolver])
+      : new TypeResolver.fromDir(sdkDir, [testUriResolver]);
 
   // Run the checker on /main.dart.
   var mainFile = new Uri.file('/main.dart');

pubspec.yaml

@@ -6,6 +6,7 @@ dependencies:
       url: http://github.com/dart-lang/analyzer-for-ddc.git
       ref: master
   args: ^0.12.1
+  cli_util: ^0.0.1
   dart_style:
     git:
       url: http://github.com/dart-lang/dart_style.git

test/checker/self_host_test.dart

@@ -2,19 +2,18 @@
 library ddc.test.end_to_end;
 
 import 'dart:io';
+import 'package:cli_util/cli_util.dart' show getSdkDir;
 import 'package:ddc/devc.dart' show compile;
 import 'package:ddc/src/report.dart';
-import 'package:ddc/src/checker/dart_sdk.dart' show dartSdkDirectory;
 import 'package:ddc/src/checker/resolver.dart' show TypeResolver;
 import 'package:path/path.dart' as path;
 import 'package:unittest/compact_vm_config.dart';
 import 'package:unittest/unittest.dart';
 
-main() {
+main(args) {
   useCompactVMConfiguration();
-  var realSdk =
-      new TypeResolver(TypeResolver.sdkResolverFromDir(dartSdkDirectory));
 
+  var realSdk = new TypeResolver.fromDir(getSdkDir(args).path);
   var testDir = path.absolute(path.dirname(Platform.script.path));
 
   test('checker can run on itself ', () {

test/codegen_test.dart

@@ -5,14 +5,15 @@ library ddc.test.codegen_test;
 
 import 'dart:io';
 import 'package:args/args.dart';
+import 'package:cli_util/cli_util.dart' show getSdkDir;
 import 'package:ddc/devc.dart';
-import 'package:ddc/src/checker/dart_sdk.dart' show dartSdkDirectory;
 import 'package:ddc/src/checker/resolver.dart' show TypeResolver;
 import 'package:logging/logging.dart' show Level;
 import 'package:path/path.dart' as path;
 import 'package:unittest/unittest.dart';
 
 final ArgParser argParser = new ArgParser()
+  ..addOption('dart-sdk', help: 'Dart SDK Path', defaultsTo: null)
   ..addFlag('dart-gen',
       abbr: 'd', help: 'Generate dart output', defaultsTo: false);
 
@@ -48,8 +49,7 @@ main(arguments) {
       .map((f) => f.path)
       .where((p) => p.endsWith('.dart') && filePattern.hasMatch(p));
 
-  var realSdk =
-      new TypeResolver(TypeResolver.sdkResolverFromDir(dartSdkDirectory));
+  var realSdk = new TypeResolver.fromDir(getSdkDir(arguments).path);
 
   // Validate that old output is gone before running.
   // TODO(jmesserly): it'd be nice to do all cleanup here, including removing
@@ -86,10 +86,11 @@ main(arguments) {
   }
 
   test('devc dart:core', () {
+    // Get the test SDK. We use a checked in copy so test expectations can be
+    // generated against a specific SDK version.
     // TODO(jmesserly): eventually we should track compiler messages.
     // For now we're just trying to get decent code generation.
-    var testSdk = new TypeResolver(
-        TypeResolver.sdkResolverFromDir(path.join(testDir, 'sdk')));
+    var testSdk = new TypeResolver.fromDir(path.join(testDir, 'sdk'));
 
     var result = compile('dart:core', testSdk,
         outputDir: actualDir,

test/end_to_end_test.dart

@@ -11,8 +11,7 @@ import 'package:path/path.dart' as path;
 import 'package:unittest/unittest.dart';
 
 main() {
-  var mockSdk =
-      new TypeResolver(TypeResolver.sdkResolverFromMock(mockSdkSources));
+  var mockSdk = new TypeResolver.fromMock(mockSdkSources);
 
   var testDir = path.absolute(path.dirname(Platform.script.path));