Make parts hermetic, fixes #531

lib/src/compiler/compiler.dart

Index: lib/src/compiler/compiler.dart
diff --git a/lib/src/compiler/compiler.dart b/lib/src/compiler/compiler.dart
index 969cd6d3418fe877024ea24b64ff2cdede1b7a08..17d72eec202dfb1e2649a50ae36e37642b2174c7 100644
--- a/lib/src/compiler/compiler.dart
+++ b/lib/src/compiler/compiler.dart
@@ -2,9 +2,10 @@
 // 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:collection' show HashSet;
 import 'package:args/args.dart' show ArgParser, ArgResults;
 import 'package:analyzer/analyzer.dart'
-    show AnalysisError, CompilationUnit, ErrorSeverity;
+    show AnalysisError, CompilationUnit, CompileTimeErrorCode, ErrorSeverity;
 import 'package:analyzer/src/generated/engine.dart' show AnalysisContext;
 import 'package:analyzer/src/generated/java_engine.dart' show AnalysisException;
 import 'package:analyzer/src/generated/source_io.dart' show Source, SourceKind;
@@ -52,6 +53,10 @@ class ModuleCompiler {
     var trees = <CompilationUnit>[];
     var errors = <AnalysisError>[];
 
+    // Validate that all parts were explicitly passed in.
+    // If not, it's an error.
+    var explicitParts = new HashSet<Source>();
+    var implicitParts = new HashSet<Source>();

[Paul Berry, 2016/04/28 20:12:33]

Nit: this is a confusing name.  It seems to imply that we are binning parts into
two categories (those that are explicit and those that are implicit), so no part
should ever wind up in both sets.  But that's not what's happening; under normal
conditions both sets will be equal.

Consider renaming to something like `usedParts`.

[Jennifer Messerly, 2016/04/28 20:18:23]

good idea, done

     for (var sourcePath in unit.sources) {
       var sourceUri = Uri.parse(sourcePath);
       if (sourceUri.scheme == '') {
@@ -64,7 +69,8 @@ class ModuleCompiler {
       }
 
       // Ignore parts. They need to be handled in the context of their library.
-      if (context.getKindOf(source) == SourceKind.PART) {
+      if (context.computeKindOf(source) == SourceKind.PART) {
+        explicitParts.add(source);
         continue;
       }
 
@@ -74,11 +80,21 @@ class ModuleCompiler {
 
       var library = resolvedTree.element.library;
       for (var part in library.parts) {
+        implicitParts.add(part.source);
         trees.add(context.resolveCompilationUnit(part.source, library));
         errors.addAll(context.computeErrors(part.source));
       }
     }
 
+    // Check if all parts were explicitly passed in.
+    // Also verify all explicitly parts were used.
+    var missingParts = implicitParts.difference(explicitParts);
+    var unusedParts = explicitParts.difference(implicitParts);
+    errors.addAll(missingParts
+        .map((s) => new AnalysisError(s, 0, 0, missingPartErrorCode)));
+    errors.addAll(unusedParts
+        .map((s) => new AnalysisError(s, 0, 0, unusedPartErrorCode)));
+
     sortErrors(context, errors);
     var messages = <String>[];
     for (var e in errors) {
@@ -269,3 +285,11 @@ class JSModuleFile {
     return map;
   }
 }
+
+/// (Public for tests) the error code used when a part is missing.
+final missingPartErrorCode = const CompileTimeErrorCode(
+    'MISSING_PART', 'The part was not supplied as an input to the compiler.');
+
+/// (Public for tests) the error code used when a part is unused.
+final unusedPartErrorCode = const CompileTimeErrorCode(

[Paul Berry, 2016/04/28 20:12:33]

Should this really be an error?  It's harmless, and I could see it arising as
part of a normal iterative development process.

[Jennifer Messerly, 2016/04/28 20:18:23]

Yeah, that's a good question. I'm fine taking it out. It just seemed odd we were
asked to compile a file and decided to silently ignore it. :)

Changed to be a Warning. Happy to downgrade to a hint too.

+    'UNUSED_PART', 'The part was not used by any libraries being compiled.');