Run linter on build-system (#11325)

build-system/.eslintrc

@@ -0,0 +1,8 @@
+{
+  "root": true,
+  "extends": "eslint-config-es5",
+  "parserOptions": {
+    "ecmaVersion": 6
+  },
+  "plugins": ["eslint-plugin-google-camelcase"]
+}

build-system/config.js

@@ -90,6 +90,11 @@ module.exports = {
   a4aTestPaths: a4aTestPaths,
   unitTestPaths: unitTestPaths,
   integrationTestPaths: integrationTestPaths,
+  buildSystemLintGlobs: [
+    'build-system/tasks/**/*.js',
+    '!build-system/eslint-rules/**/*.*',
+    '!build-system/node_modules/**/*.*',
+  ],
   lintGlobs: [
     '**/*.js',
     '!**/*.extern.js',

build-system/pr-check.js

@@ -255,6 +255,7 @@ function determineBuildTargets(filePaths) {
 const command = {
   testBuildSystem: function() {
     timedExecOrDie(`${gulp} ava`);
+    timedExec(`${gulp} lint --build_system`);  // Run in warning mode initially.
   },
   testDocumentLinks: function(files) {
     timedExecOrDie(`${gulp} check-links`);

build-system/tasks/lint.js

@@ -19,9 +19,11 @@
 var argv = require('minimist')(process.argv.slice(2));
 var config = require('../config');
 var eslint = require('gulp-eslint');
+var getStdout = require('../exec.js').getStdout;
 var gulp = require('gulp-help')(require('gulp'));
 var gulpIf = require('gulp-if');
 var lazypipe = require('lazypipe');
+var path = require('path');
 var util = require('gulp-util');
 var watch = require('gulp-watch');
 
@@ -32,8 +34,28 @@ var options = {
   rulePaths: ['build-system/eslint-rules/'],
   plugins: ['eslint-plugin-google-camelcase'],
 };
+var buildSystemOptions = {
+  fix: false,
+  plugins: ['eslint-plugin-google-camelcase'],
+};
 
-var watcher = lazypipe().pipe(watch, config.lintGlobs);
+/**
+ * On travis, we'll start by linting just the build-system files that are being
+ * changed in the current PR. For local runs, we lint all build-system files.
+ *
+ * @return {!Array<string>}
+ */
+function getBuildSystemFiles() {
+  if (!!process.env.TRAVIS_PULL_REQUEST_SHA) {
+    var filesInPr =
+        getStdout(`git diff --name-only master...HEAD`).trim().split('\n');
+    return filesInPr.filter(function(file) {
+      return file.startsWith('build-system') && path.extname(file) == '.js'
+    });
+  } else {
+    return config.buildSystemLintGlobs;
+  }
+}
 
 /**
  * Checks if current Vinyl file has been fixed by eslint.
@@ -46,41 +68,79 @@ function isFixed(file) {
 }
 
 /**
- * Run the eslinter on the src javascript and log the output
- * @return {!Stream} Readable stream
+ * Initializes the linter stream based on globs
+ * @param {!object} globs
+ * @param {!object} streamOptions
+ * @return {!ReadableStream}
  */
-function lint() {
-  var errorsFound = false;
-  var stream = gulp.src(config.lintGlobs);
-
+function initializeStream(globs, streamOptions) {
+  var stream = gulp.src(globs, streamOptions);
   if (isWatching) {
+    var watcher = lazypipe().pipe(watch, globs);
     stream = stream.pipe(watcher());
   }
+  return stream;
+}
 
-  if (argv.fix) {
-    options.fix = true;
-  }
-
+/**
+ * Runs the linter on the given stream using the given options.
+ * @param {!string} path
+ * @param {!ReadableStream} stream
+ * @param {!object} options
+ * @return {boolean}
+ */
+function runLinter(path, stream, options) {
+  var errorsFound = false;
   return stream.pipe(eslint(options))
     .pipe(eslint.formatEach('stylish', function(msg) {
       errorsFound = true;
       util.log(util.colors.red(msg));
     }))
-    .pipe(gulpIf(isFixed, gulp.dest('.')))
+    .pipe(gulpIf(isFixed, gulp.dest(path)))
     .pipe(eslint.failAfterError())
-    .on('end', function() {
+    .on('error', function() {
       if (errorsFound && !options.fix) {
-        util.log(util.colors.blue('Run `gulp lint --fix` to automatically ' +
-            'fix some of these lint warnings/errors. This is a destructive ' +
-            'operation (operates on the file system) so please make sure ' +
-            'you commit before running.'));
+        if (!!process.env.TRAVIS_PULL_REQUEST_SHA) {
+          util.log(util.colors.yellow('NOTE:'),
+              'The linter is currently running in warning mode.',
+              'The errors found above must eventually be fixed.');
+        } else {
+          util.log(util.colors.yellow('NOTE:'),
+              'You can use', util.colors.cyan('--fix'), 'with your',
+              util.colors.cyan('gulp lint'),
+              'command to automatically fix some of these lint errors.');
+          util.log(util.colors.yellow('WARNING:'),
+              'Since this is a destructive operation (operates on the file',
+              'system), make sure you commit before running the command.');
+        }
       }
     });
 }
 
+/**
+ * Run the eslinter on the src javascript and log the output
+ * @return {!Stream} Readable stream
+ */
+function lint() {
+  if (argv.fix) {
+    options.fix = true;
+    buildSystemOptions.fix = true;
+  }
+  if (argv.build_system) {
+    var stream =
+        initializeStream(getBuildSystemFiles(), { base: 'build-system' });
+    return runLinter('./build-system/', stream, buildSystemOptions);
+  } else {
+    var stream = initializeStream(config.lintGlobs, {});
+    return runLinter('.', stream, options);
+  }
+}
+
+
 gulp.task('lint', 'Validates against Google Closure Linter', lint,
 {
   options: {
+    'build_system': '  Runs the linter against the build system directory',
     'watch': '  Watches for changes in files, validates against the linter',
     'fix': '  Fixes simple lint errors (spacing etc).'
   }

package.json

@@ -51,6 +51,7 @@
     "cssnano": "3.3.2",
     "del": "2.2.0",
     "eslint": "3.18.0",
+    "eslint-config-es5": "0.5.0",
     "eslint-plugin-google-camelcase": "0.0.2",
     "esprima": "3.1.3",
     "express": "4.14.0",