From aaa9e606d4836dd1d458975896bcb7be25bc0be7 Mon Sep 17 00:00:00 2001
From: Slava Kim <slava@meteor.com>
Date: Mon, 20 Apr 2015 17:04:51 -0700
Subject: [PATCH] Fix bugs with running linters and passing imports

---
 packages/jshint/plugin/lint-jshint.js | 11 ++++-
 tools/compiler.js                     | 62 ++++++++++++++++-----------
 2 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/packages/jshint/plugin/lint-jshint.js b/packages/jshint/plugin/lint-jshint.js
index 83b78736f4..c1a4a52b5d 100644
--- a/packages/jshint/plugin/lint-jshint.js
+++ b/packages/jshint/plugin/lint-jshint.js
@@ -38,10 +38,19 @@ JsHintLinter.prototype.processFilesForTarget = function (files, globals) {
     }
   });
 
+  // JSHint has a particular format for defining globals. `false` means that the
+  // global is not allowed to be redefined. `true` means it is allowed to be
+  // redefined. Since the passed imports are probably not great for definition,
+  // mark them as false.
+  var predefinedGlobals = {};
+  globals.forEach(function (symbol) {
+    predefinedGlobals[symbol] = false;
+  });
+
   files.forEach(function (file) {
     if (file.getBasename() === '.jshintrc')
       return;
-    if (! jshint(file.getContentsAsString(), conf, globals)) {
+    if (! jshint(file.getContentsAsString(), conf, predefinedGlobals)) {
       jshint.errors.forEach(function (error) {
         file.error({
           message: error.reason,
diff --git a/tools/compiler.js b/tools/compiler.js
index afd4e32b77..6fefd5ed55 100644
--- a/tools/compiler.js
+++ b/tools/compiler.js
@@ -232,7 +232,7 @@ var compileUnibuild = function (options) {
   // XXX BBP redoc
   var allHandlersWithPkgs = {};
   var compilerPluginsByExtension = {};
-  var linterPluginsByExtension = {};
+  var allLinters = [];
   var sourceExtensions = {};  // maps source extensions to isTemplate
 
   sourceExtensions['js'] = false;
@@ -310,12 +310,8 @@ var compileUnibuild = function (options) {
       });
     });
 
-    // Iterate over the linters
     _.each(otherPkg.sourceProcessors.linter, function (linterPlugin, id) {
-      _.each(linterPlugin.extensions, function (ext) {
-        linterPluginsByExtension[ext] = linterPluginsByExtension[ext] || [];
-        linterPluginsByExtension[ext].push(linterPlugin);
-      });
+      allLinters.push(linterPlugin);
     });
   });
 
@@ -396,7 +392,7 @@ var compileUnibuild = function (options) {
     };
   });
 
-  runLinters(inputSourceArch, isopackCache, wrappedSourceItems, linterPluginsByExtension);
+  runLinters(inputSourceArch, isopackCache, wrappedSourceItems, allLinters);
 
   _.each(wrappedSourceItems, function (wrappedSource) {
     var source = wrappedSource.source;
@@ -952,10 +948,15 @@ var runLinters = function (
   inputSourceArch,
   isopackCache,
   wrappedSourceItems,
-  linterPluginsByExtension) {
+  linters) {
   // For linters, figure out what are the global imports from other packages
   // that we use directly, or are implied.
-  var globalImports = ['Package', 'Assets', 'Npm'];
+  var globalImports = ['Package'];
+
+  if (archinfo.matches(inputSourceArch.arch, "os")) {
+    globalImports = globalImports.concat(['Npm', 'Assets']);
+  }
+
   compiler.eachUsedUnibuild({
     dependencies: inputSourceArch.uses,
     arch: inputSourceArch.arch,
@@ -971,6 +972,14 @@ var runLinters = function (
     });
   });
 
+  var linterPluginsByExtension = {};
+  _.each(linters, function (linterPlugin) {
+    _.each(linterPlugin.extensions, function (ext) {
+      linterPluginsByExtension[ext] = linterPluginsByExtension[ext] || [];
+      linterPluginsByExtension[ext].push(linterPlugin);
+    });
+  });
+
   // For each file choose the longest extension handled by linters.
   var longestMatchingExt = {};
   _.each(wrappedSourceItems, function (wrappedSource) {
@@ -986,7 +995,11 @@ var runLinters = function (
   });
 
   // Run linters on files.
-  _.each(linterPluginsByExtension, function (linters, ext) {
+  _.each(linters, function (linterDef) {
+    // skip linters not relevant to the arch we are compiling for
+    if (! linterDef.relevantForArch(inputSourceArch.arch))
+      return;
+
     var sourcesToLint = [];
     _.each(wrappedSourceItems, function (wrappedSource) {
       var relPath = wrappedSource.relPath;
@@ -995,28 +1008,25 @@ var runLinters = function (
       var source = wrappedSource.source;
 
       // only run linters matching the longest handled extension
-      if (longestMatchingExt[relPath] !== ext)
+      if (! _.contains(linterDef.extensions, longestMatchingExt[relPath]))
         return;
 
       sourcesToLint.push(new linterPluginModule.LintingFile(wrappedSource));
     });
 
-    _.each(linters, function (linterDef) {
-      // skip linters not relevant to the arch we are compiling for
-      if (! linterDef.relevantForArch(inputSourceArch.arch))
-        return;
+    if (! sourcesToLint.length)
+      return;
 
-      var linter = linterDef.instantiatePlugin();
-      buildmessage.enterJob({
-        title: "linting files with " + linterDef.isopack.name
-      }, function () {
-        try {
-          var markedLinter = buildmessage.markBoundary(linter.run.bind(linter));
-          markedLinter(sourcesToLint, globalImports);
-        } catch (e) {
-          buildmessage.exception(e);
-        }
-      });
+    var linter = linterDef.instantiatePlugin();
+    buildmessage.enterJob({
+      title: "linting files with " + linterDef.isopack.name
+    }, function () {
+      try {
+        var markedLinter = buildmessage.markBoundary(linter.run.bind(linter));
+        markedLinter(sourcesToLint, globalImports);
+      } catch (e) {
+        buildmessage.exception(e);
+      }
     });
   });
 };