Lint git index content on commit (#113300)

This fixes two fundamental issues:
* the list of files to be linted is built looking at the diffs of the ref
  specified with `--ref` (or otherwise the current index) with those on
  the filesystem
* the content to be linted is read from the filesystem instead of the
  specified `ref` or the one in the index

src/dev/eslint/lint_files.ts

@@ -12,6 +12,45 @@ import { REPO_ROOT } from '@kbn/utils';
 import { createFailError, ToolingLog } from '@kbn/dev-utils';
 import { File } from '../file';
 
+// For files living on the filesystem
+function lintFilesOnFS(cli: CLIEngine, files: File[]) {
+  const paths = files.map((file) => file.getRelativePath());
+  return cli.executeOnFiles(paths);
+}
+
+// For files living somewhere else (ie. git object)
+async function lintFilesOnContent(cli: CLIEngine, files: File[]) {
+  const report: {
+    results: any[];
+    errorCount: number;
+    warningCount: number;
+    fixableErrorCount: number;
+    fixableWarningCount: number;
+  } = {
+    results: [],
+    errorCount: 0,
+    warningCount: 0,
+    fixableErrorCount: 0,
+    fixableWarningCount: 0,
+  };
+
+  for (let i = 0; i < files.length; i++) {
+    const r = cli.executeOnText(await files[i].getContent(), files[i].getRelativePath());
+    // Despite a relative path was given, the result would contain an absolute one. Work around it.
+    r.results[0].filePath = r.results[0].filePath.replace(
+      files[i].getAbsolutePath(),
+      files[i].getRelativePath()
+    );
+    report.results.push(...r.results);
+    report.errorCount += r.errorCount;
+    report.warningCount += r.warningCount;
+    report.fixableErrorCount += r.fixableErrorCount;
+    report.fixableWarningCount += r.fixableWarningCount;
+  }
+
+  return report;
+}
+
 /**
  * Lints a list of files with eslint. eslint reports are written to the log
  * and a FailError is thrown when linting errors occur.
@@ -20,15 +59,16 @@ import { File } from '../file';
  * @param  {Array<File>} files
  * @return {undefined}
  */
-export function lintFiles(log: ToolingLog, files: File[], { fix }: { fix?: boolean } = {}) {
+export async function lintFiles(log: ToolingLog, files: File[], { fix }: { fix?: boolean } = {}) {
   const cli = new CLIEngine({
     cache: true,
     cwd: REPO_ROOT,
     fix,
   });
 
-  const paths = files.map((file) => file.getRelativePath());
-  const report = cli.executeOnFiles(paths);
+  const virtualFilesCount = files.filter((file) => file.isVirtual()).length;
+  const report =
+    virtualFilesCount && !fix ? await lintFilesOnContent(cli, files) : lintFilesOnFS(cli, files);
 
   if (fix) {
     CLIEngine.outputFixes(report);

src/dev/file.ts

@@ -7,11 +7,13 @@
  */
 
 import { dirname, extname, join, relative, resolve, sep, basename } from 'path';
+import { createFailError } from '@kbn/dev-utils';
 
 export class File {
   private path: string;
   private relativePath: string;
   private ext: string;
+  private fileReader: undefined | (() => Promise<string>);
 
   constructor(path: string) {
     this.path = resolve(path);
@@ -55,6 +57,11 @@ export class File {
     );
   }
 
+  // Virtual files cannot be read as usual, an helper is needed
+  public isVirtual() {
+    return this.fileReader !== undefined;
+  }
+
   public getRelativeParentDirs() {
     const parents: string[] = [];
 
@@ -81,4 +88,15 @@ export class File {
   public toJSON() {
     return this.relativePath;
   }
+
+  public setFileReader(fileReader: () => Promise<string>) {
+    this.fileReader = fileReader;
+  }
+
+  public getContent() {
+    if (this.fileReader) {
+      return this.fileReader();
+    }
+    throw createFailError('getContent() was invoked on a non-virtual File');
+  }
 }

src/dev/precommit_hook/get_files_for_commit.js

@@ -6,12 +6,65 @@
  * Side Public License, v 1.
  */
 
+import { format } from 'util';
 import SimpleGit from 'simple-git';
 import { fromNode as fcb } from 'bluebird';
 
 import { REPO_ROOT } from '@kbn/utils';
 import { File } from '../file';
 
+/**
+ * Return the `git diff` argument used for building the list of files
+ *
+ * @param  {String} gitRef
+ * @return {String}
+ *
+ *   gitRef               return
+ *   ''                   '--cached'
+ *   '<ref>'              '<ref>~1..<ref>'
+ *   '<ref>..'            '<ref>..'
+ *   '<ref>...'           '<ref>...'
+ *   '..<ref>'            '..<ref>'
+ *   '...<ref>'           '...<ref>'
+ *   '<ref_A>..<ref_B>'   '<ref_A>..<ref_B>'
+ *   '<ref_A>...<ref_B>'  '<ref_A>...<ref_B>'
+ */
+function getRefForDiff(gitRef) {
+  if (!gitRef) {
+    return '--cached';
+  } else if (gitRef.includes('..')) {
+    return gitRef;
+  } else {
+    return format('%s~1..%s', gitRef, gitRef);
+  }
+}
+
+/**
+ * Return the <ref> used for reading files content
+ *
+ * @param  {String} gitRef
+ * @return {String}
+ *
+ *   gitRef               return
+ *   ''                   ''
+ *   '<ref>'              '<ref>'
+ *   '<ref>..'            'HEAD'
+ *   '<ref>...'           'HEAD'
+ *   '..<ref>'            '<ref>'
+ *   '...<ref>'           '<ref>'
+ *   '<ref_A>..<ref_B>'   '<ref_B>'
+ *   '<ref_A>...<ref_B>'  '<ref_B>'
+ */
+function getRefForCat(gitRef) {
+  if (!gitRef) {
+    return '';
+  } else if (gitRef.includes('..')) {
+    return gitRef.endsWith('..') ? 'HEAD' : gitRef.slice(gitRef.lastIndexOf('..') + 2);
+  } else {
+    return gitRef;
+  }
+}
+
 /**
  * Get the files that are staged for commit (excluding deleted files)
  * as `File` objects that are aware of their commit status.
@@ -21,29 +74,23 @@ import { File } from '../file';
  */
 export async function getFilesForCommit(gitRef) {
   const simpleGit = new SimpleGit(REPO_ROOT);
-  const gitRefForDiff = gitRef ? gitRef : '--cached';
-  const output = await fcb((cb) => simpleGit.diff(['--name-status', gitRefForDiff], cb));
+  const gitRefForDiff = getRefForDiff(gitRef);
+  const gitRefForCat = getRefForCat(gitRef);
+
+  const output = await fcb((cb) => {
+    simpleGit.diff(['--diff-filter=d', '--name-only', gitRefForDiff], cb);
+  });
 
   return (
     output
       .split('\n')
       // Ignore blank lines
       .filter((line) => line.trim().length > 0)
-      // git diff --name-status outputs lines with two OR three parts
-      // separated by a tab character
-      .map((line) => line.trim().split('\t'))
-      .map(([status, ...paths]) => {
-        // ignore deleted files
-        if (status === 'D') {
-          return undefined;
-        }
-
-        // the status is always in the first column
-        // .. If the file is edited the line will only have two columns
-        // .. If the file is renamed it will have three columns
-        // .. In any case, the last column is the CURRENT path to the file
-        return new File(paths[paths.length - 1]);
+      .map((path) => {
+        const file = new File(path);
+        const object = format('%s:%s', gitRefForCat, path);
+        file.setFileReader(() => fcb((cb) => simpleGit.catFile(['-p', object], cb)));
+        return file;
       })
-      .filter(Boolean)
   );
 }

src/dev/run_precommit_hook.js

@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import { run, combineErrors, createFlagError } from '@kbn/dev-utils';
+import { run, combineErrors, createFlagError, createFailError } from '@kbn/dev-utils';
 import * as Eslint from './eslint';
 import * as Stylelint from './stylelint';
 import { getFilesForCommit, checkFileCasing } from './precommit_hook';
@@ -23,6 +23,11 @@ run(
       throw createFlagError('expected --max-files to be a number greater than 0');
     }
 
+    const virtualFilesCount = files.filter((file) => file.isVirtual()).length;
+    if (virtualFilesCount > 0 && virtualFilesCount < files.length) {
+      throw createFailError('Mixing of virtual and on-filesystem files is unsupported');
+    }
+
     if (maxFilesCount && files.length > maxFilesCount) {
       log.warning(
         `--max-files is set to ${maxFilesCount} and ${files.length} were discovered. The current script execution will be skipped.`
@@ -66,7 +71,11 @@ run(
       help: `
       --fix              Execute eslint in --fix mode
       --max-files        Max files number to check against. If exceeded the script will skip the execution
-      --ref              Run checks against any git ref files (example HEAD or <commit_sha>) instead of running against staged ones
+      --ref              Run checks against git ref files instead of running against staged ones
+                         Examples:
+                           HEAD~1..HEAD   files changed in the commit at HEAD
+                           HEAD           equivalent to HEAD~1..HEAD
+                           main...        files changed in current branch since the common ancestor with main
     `,
     },
   }

src/dev/stylelint/lint_files.js

@@ -16,15 +16,8 @@ import { createFailError } from '@kbn/dev-utils';
 const stylelintPath = path.resolve(__dirname, '..', '..', '..', '.stylelintrc');
 const styleLintConfig = safeLoad(fs.readFileSync(stylelintPath));
 
-/**
- * Lints a list of files with eslint. eslint reports are written to the log
- * and a FailError is thrown when linting errors occur.
- *
- * @param  {ToolingLog} log
- * @param  {Array<File>} files
- * @return {undefined}
- */
-export async function lintFiles(log, files) {
+// For files living on the filesystem
+function lintFilesOnFS(files) {
   const paths = files.map((file) => file.getRelativePath());
 
   const options = {
@@ -34,7 +27,52 @@ export async function lintFiles(log, files) {
     ignorePath: path.resolve(__dirname, '..', '..', '..', '.stylelintignore'),
   };
 
-  const report = await stylelint.lint(options);
+  return stylelint.lint(options);
+}
+
+// For files living somewhere else (ie. git object)
+async function lintFilesOnContent(files) {
+  const report = {
+    errored: false,
+    output: '',
+    postcssResults: [],
+    results: [],
+    maxWarningsExceeded: {
+      maxWarnings: 0,
+      foundWarnings: 0,
+    },
+  };
+
+  for (let i = 0; i < files.length; i++) {
+    const options = {
+      code: await files[i].getContent(),
+      config: styleLintConfig,
+      formatter: 'string',
+      ignorePath: path.resolve(__dirname, '..', '..', '..', '.stylelintignore'),
+    };
+    const r = await stylelint.lint(options);
+    report.errored = report.errored || r.errored;
+    report.output += r.output.replace(/<input css \d+>/, files[i].getRelativePath()).slice(0, -1);
+    report.postcssResults.push(...(r.postcssResults || []));
+    report.maxWarnings = r.maxWarnings;
+    report.foundWarnings += r.foundWarnings;
+  }
+
+  return report;
+}
+
+/**
+ * Lints a list of files with eslint. eslint reports are written to the log
+ * and a FailError is thrown when linting errors occur.
+ *
+ * @param  {ToolingLog} log
+ * @param  {Array<File>} files
+ * @return {undefined}
+ */
+export async function lintFiles(log, files) {
+  const virtualFilesCount = files.filter((file) => file.isVirtual()).length;
+  const report = virtualFilesCount ? await lintFilesOnContent(files) : await lintFilesOnFS(files);
+
   if (report.errored) {
     log.error(report.output);
     throw createFailError('[stylelint] errors');