DDC: mostly incremental compilation, fixes #223

lib/src/compiler.dart

Index: lib/src/compiler.dart
diff --git a/lib/src/compiler.dart b/lib/src/compiler.dart
index f5a0aecf82f7936cfba2fab49069c0eab25b1b64..06b47ada0ddb9bd7b712e9afb63dc5354baa8cae 100644
--- a/lib/src/compiler.dart
+++ b/lib/src/compiler.dart
@@ -10,14 +10,13 @@ import 'dart:collection';
 import 'dart:math' as math;
 import 'dart:io';
 
-import 'package:analyzer/src/generated/ast.dart' show CompilationUnit;
-import 'package:analyzer/src/generated/element.dart';
+import 'package:analyzer/src/generated/ast.dart'
+    show CompilationUnit, NamespaceDirective, PartDirective, UriBasedDirective;
 import 'package:analyzer/src/generated/engine.dart'
     show AnalysisEngine, AnalysisContext, ChangeSet, ParseDartTask;
 import 'package:analyzer/src/generated/error.dart'
     show AnalysisError, ErrorSeverity, ErrorType;
 import 'package:analyzer/src/generated/error.dart';
-import 'package:analyzer/src/generated/resolver.dart' show TypeProvider;
 import 'package:analyzer/src/generated/source.dart' show Source;
 import 'package:analyzer/src/task/html.dart';
 import 'package:html/dom.dart' as html;
@@ -25,8 +24,6 @@ import 'package:html/parser.dart' as html;
 import 'package:logging/logging.dart' show Level, Logger, LogRecord;
 import 'package:path/path.dart' as path;
 
-import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions;
-
 import 'analysis_context.dart';
 import 'checker/checker.dart';
 import 'checker/rules.dart';
@@ -73,11 +70,11 @@ bool compile(CompilerOptions options) {
 
 class BatchCompiler extends AbstractCompiler {
   JSGenerator _jsGen;
-  LibraryElement _dartCore;
+  Source _dartCore;
   String _runtimeOutputDir;
 
   /// Already compiled sources, so we don't compile them again.
-  final _compiled = new HashSet<LibraryElement>();
+  final _compiled = new HashSet<Uri>();
   bool _sdkCopied = false;
 
   bool _failure = false;
@@ -88,17 +85,15 @@ class BatchCompiler extends AbstractCompiler {
       : super(
             context,
             options,
-            new ErrorCollector(
-                reporter ?? AnalysisErrorListener.NULL_LISTENER)) {
-    _inputBaseDir = options.inputBaseDir;
+            new ErrorCollector(context, reporter, options.logLevel,
+                saveMessages: options.saveMessages)) {
     if (outputDir != null) {
-      _jsGen = new JSGenerator(this);
       _runtimeOutputDir = path.join(outputDir, 'dev_compiler', 'runtime');
     }
-    _dartCore = context.typeProvider.objectType.element.library;
+    _dartCore = context.sourceFactory.forUri('dart:core');
   }
 
-  ErrorCollector get reporter => checker.reporter;
+  ErrorCollector get reporter => super.reporter;
 
   void reset() {
     _compiled.clear();
@@ -113,7 +108,6 @@ class BatchCompiler extends AbstractCompiler {
     clock.stop();
     var time = (clock.elapsedMilliseconds / 1000).toStringAsFixed(2);
     _log.fine('Compiled ${_compiled.length} libraries in ${time} s\n');
-
     return !_failure;
   }
 
@@ -136,66 +130,109 @@ class BatchCompiler extends AbstractCompiler {
     if (AnalysisEngine.isHtmlFileName(source.uri.path)) {
       _compileHtml(source);
     } else {
-      _compileLibrary(context.computeLibraryElement(source));
+      _compileLibrary(source);
     }
-    reporter.flush();
   }
 
-  void _compileLibrary(LibraryElement library) {
-    if (!_compiled.add(library)) return;
+  bool _compileLibrary(Source source) {
+    if (!_compiled.add(source.uri)) return false;

[Leaf, 2015/09/04 21:46:31]

I'm worried about this doing the right thing in the presence of cycles.  If I
have a cycle of libraries A and B, and I modify A, then I think this code does
the following:

1) Adds A to _compiled
2) Tries to compile B
3) Adds B to _compiled
4) Tries to compile A
5) Notices that A is in _compiled, so returns false for changed,
6) Does a time stamp check on B, sees that it hasn't itself changed, so doesn't
recompile it
7) Recompiles A based on timestamps

Am I missing something? 

 
-    if (!options.checkSdk && library.source.uri.scheme == 'dart') {
-      if (_jsGen != null) _copyDartRuntime();
-      return;
+    if (!options.checkSdk && source.uri.scheme == 'dart') {
+      return outputDir != null && _copyDartRuntime();
     }
 
-    // TODO(jmesserly): in incremental mode, we can skip the transitive
-    // compile of imports/exports.
-    _compileLibrary(_dartCore); // implicit dart:core dependency
-    library.importedLibraries.forEach(_compileLibrary);
-    library.exportedLibraries.forEach(_compileLibrary);
+    var sources = <Source>[source];
+    if (!source.exists()) return false;
 
-    var unitElements = [library.definingCompilationUnit]..addAll(library.parts);
-    var units = <CompilationUnit>[];
+    int lastEdit = context.getModificationStamp(source);
 
-    bool failureInLib = false;
-    for (var element in unitElements) {
-      var unit = context.resolveCompilationUnit(element.source, library);
-      units.add(unit);
-      failureInLib = logErrors(element.source) || failureInLib;
-      checker.visitCompilationUnit(unit);
-      if (checker.failure) failureInLib = true;
+    // implicit dart:core dependency
+    bool changed = _compileLibrary(_dartCore);
+
+    var definingUnit = context.parseCompilationUnit(source);
+    for (var d in definingUnit.directives) {
+      if (d is UriBasedDirective) {
+        var src = context.sourceFactory.resolveUri(source, d.uri.stringValue);
+        if (src == null) continue;
+
+        if (d is NamespaceDirective) {
+          if (_compileLibrary(src)) {
+            changed = true;
+          }
+        } else if (d is PartDirective) {
+          sources.add(src);
+          lastEdit = math.max(lastEdit, context.getModificationStamp(src));
+        }
+      }
     }
 
-    if (failureInLib) {
-      _failure = true;
-      if (!options.codegenOptions.forceCompile) return;
+    // Take into account if the compiler itself was edited.
+    if (compilerLastModified != null) {
+      lastEdit =
+          math.max(lastEdit, compilerLastModified.millisecondsSinceEpoch);
     }
 
-    if (_jsGen != null) {
-      // TODO(jmesserly): full incremental support would avoid checking as well,
-      // however, we'd lose compiler messages in that case.
-
-      // Note: analyzer's modification stamp is millisecondsSinceEpoch
-      int lastModifyTime = unitElements
-          .map((e) => context.getModificationStamp(e.source))
-          .reduce(math.max);
-      var outFile = new File(getOutputPath(library.source.uri));
-      if (outFile.existsSync() &&
-          outFile.lastModifiedSync().millisecondsSinceEpoch >= lastModifyTime) {
-        // Output already up to date.
-        return;
+    String messageFilePath;
+    File outFile;
+    if (outputDir != null) {
+      messageFilePath =
+          path.withoutExtension(getOutputPath(source.uri)) + '.txt';
+      outFile = new File(getOutputPath(source.uri));
+    } else {
+      // if no output directory is specified, we're in checker mode, and should
+      // run the full compilation.
+      // TODO(jmesserly): deprecate check only mode; this should go through
+      // analyzer_cli instead. BatchCompiler would be simpler if it was always
+      // a compiler that produced output.
+      changed = true;
+    }
+    if (!changed) {
+      var msgFile = new File(messageFilePath);
+
+      if (outFile.existsSync()) {
+        changed = outFile.lastModifiedSync().millisecondsSinceEpoch < lastEdit;
+      } else if (msgFile.existsSync()) {
+        changed = msgFile.lastModifiedSync().millisecondsSinceEpoch < lastEdit;
+      } else {
+        // Output files do not exist.
+        changed = true;
       }
+      // If output is already up to date, we can skip remaining steps.
+      if (!changed) return false;
+    }
+
+    var units = <CompilationUnit>[];
+    for (var src in sources) {
+      var unit = context.resolveCompilationUnit2(src, source);
+      units.add(unit);
+      if (logErrors(src)) _failure = true;
+      checker.visitCompilationUnit(unit);
+      if (checker.failure) _failure = true;
+    }
 
-      var unit = units.first;
-      var parts = units.skip(1).toList();
-      _jsGen.generateLibrary(new LibraryUnit(unit, parts));
+    if (outputDir != null) {
+      if (!_failure || options.codegenOptions.forceCompile) {
+        if (_jsGen == null) _jsGen = new JSGenerator(this);
+        var unit = units.first;
+        var parts = units.skip(1).toList();
+        _jsGen.generateLibrary(new LibraryUnit(unit, parts));
+      } else {
+        // Delete stale output.
+        if (outFile.existsSync()) outFile.deleteSync();
+      }
     }
+
+    reporter.flush(messageFilePath);
+    return true;
   }
 
-  void _copyDartRuntime() {
-    if (_sdkCopied) return;
+  bool _copyDartRuntime() {
+    if (_sdkCopied) return false;
+    if (options.runtimeDir == null) return false;
+
     _sdkCopied = true;
+
+    bool changed = false;
     for (var file in defaultRuntimeFiles) {
       var input = new File(path.join(options.runtimeDir, file));
       var output = new File(path.join(_runtimeOutputDir, file));
@@ -203,19 +240,27 @@ class BatchCompiler extends AbstractCompiler {
           output.lastModifiedSync() == input.lastModifiedSync()) {
         continue;
       }
+
+      changed = true;
       new Directory(path.dirname(output.path)).createSync(recursive: true);
       input.copySync(output.path);
     }
+    return changed;
   }
 
-  void _compileHtml(Source source) {
+  bool _compileHtml(Source source) {
     // TODO(jmesserly): reuse DartScriptsTask instead of copy/paste.
     var contents = context.getContents(source);
     var document = html.parse(contents.data, generateSpans: true);
     var scripts = document.querySelectorAll('script[type="application/dart"]');
 
-    var loadedLibs = new LinkedHashSet<Uri>();
+    var outFile = new File(getOutputPath(source.uri));
+
+    bool changed = !outFile.existsSync() ||
+        context.getModificationStamp(source) >
+            outFile.lastModifiedSync().millisecondsSinceEpoch;
 
+    var scriptSources = <Source>[];
     var htmlOutDir = path.dirname(getOutputPath(source.uri));
     for (var script in scripts) {
       Source scriptSource = null;
@@ -235,22 +280,33 @@ class BatchCompiler extends AbstractCompiler {
       } else if (AnalysisEngine.isDartFileName(srcAttr)) {
         scriptSource = context.sourceFactory.resolveUri(source, srcAttr);
       }
+      scriptSources.add(scriptSource);
 
-      if (scriptSource != null) {
-        var lib = context.computeLibraryElement(scriptSource);
-        _compileLibrary(lib);
-        script.replaceWith(_linkLibraries(lib, loadedLibs, from: htmlOutDir));
-      }
+      if (scriptSource != null && _compileLibrary(scriptSource)) changed = true;
     }
 
-    new File(getOutputPath(source.uri)).openSync(mode: FileMode.WRITE)
+    if (!changed) {
+      return false;
+    }
+
+    var loadedLibs = new LinkedHashSet<Uri>();
+    for (int i = 0; i < scripts.length; i++) {
+      var src = scriptSources[i];
+      if (src == null) continue;
+      scripts[i].replaceWith(_linkLibraries(src, loadedLibs, from: htmlOutDir));
+    }
+
+    outFile.openSync(mode: FileMode.WRITE)
       ..writeStringSync(document.outerHtml)
       ..writeStringSync('\n')
       ..closeSync();
+
+    reporter.flush(getOutputPath(source.uri) + '.txt');
+    return true;
   }
 
   html.DocumentFragment _linkLibraries(
-      LibraryElement mainLib, LinkedHashSet<Uri> loaded,
+      Source mainLib, LinkedHashSet<Uri> loaded,
       {String from}) {
     assert(from != null);
     var alreadyLoaded = loaded.length;
@@ -279,12 +335,19 @@ class BatchCompiler extends AbstractCompiler {
     return df;
   }
 
-  void _collectLibraries(LibraryElement lib, LinkedHashSet<Uri> loaded) {
-    var uri = lib.source.uri;
+  void _collectLibraries(Source source, LinkedHashSet<Uri> loaded) {
+    var uri = source.uri;
     if (!loaded.add(uri)) return;
     _collectLibraries(_dartCore, loaded);
-    for (var l in lib.importedLibraries) _collectLibraries(l, loaded);
-    for (var l in lib.exportedLibraries) _collectLibraries(l, loaded);
+
+    var definingUnit = context.parseCompilationUnit(source);
+    for (var d in definingUnit.directives) {
+      if (d is NamespaceDirective) {
+        var src = context.sourceFactory.resolveUri(source, d.uri.stringValue);
+        if (src != null) _collectLibraries(src, loaded);
+      }
+    }
+
     // Move the item to the end of the list.
     loaded.remove(uri);
     loaded.add(uri);
@@ -294,26 +357,30 @@ class BatchCompiler extends AbstractCompiler {
 abstract class AbstractCompiler {
   final CompilerOptions options;
   final AnalysisContext context;
-  final CodeChecker checker;
+  final AnalysisErrorListener reporter;
+  CodeChecker _checker;
 
-  AbstractCompiler(AnalysisContext context, CompilerOptions options,
+  AbstractCompiler(this.context, CompilerOptions options,
       [AnalysisErrorListener reporter])
-      : context = context,
+      : reporter = reporter ?? AnalysisErrorListener.NULL_LISTENER,
         options = options,
-        checker = createChecker(context.typeProvider, options.strongOptions,
-            reporter ?? AnalysisErrorListener.NULL_LISTENER) {
+        _inputBaseDir = options.inputBaseDir {
     enableDevCompilerInference(context, options.strongOptions);
   }
 
-  static CodeChecker createChecker(TypeProvider typeProvider,
-      StrongModeOptions options, AnalysisErrorListener reporter) {
-    return new CodeChecker(
-        new RestrictedRules(typeProvider, options: options), reporter, options);
+  CodeChecker get checker {
+    if (_checker == null) {
+      var opts = options.strongOptions;
+      _checker = new CodeChecker(
+          new RestrictedRules(context.typeProvider, options: opts),
+          reporter,
+          opts);
+    }
+    return _checker;
   }
 
   String get outputDir => options.codegenOptions.outputDir;
   TypeRules get rules => checker.rules;
-  AnalysisErrorListener get reporter => checker.reporter;
 
   Uri stringToUri(String uriString) {
     var uri = uriString.startsWith('dart:') || uriString.startsWith('package:')
@@ -421,9 +488,8 @@ abstract class AbstractCompiler {
 
 AnalysisErrorListener createErrorReporter(
     AnalysisContext context, CompilerOptions options) {
-  return options.dumpInfo
-      ? new SummaryReporter(context, options.logLevel)
-      : new LogReporter(context, useColors: options.useColors);
+  if (options.dumpInfo) return new SummaryReporter(context, options.logLevel);
+  return new LogReporter(context);
 }
 
 // TODO(jmesserly): find a better home for these.