Exclude internal files from autofix commits

.changeset/red-llamas-explain.md

@@ -0,0 +1,10 @@
+---
+'skuba': patch
+---
+
+lint: Exclude internal files from autofix commits
+
+`skuba lint` now avoids committing the following internal files in a [GitHub autofix](https://seek-oss.github.io/skuba/docs/deep-dives/github.html#github-autofixes):
+
+- `.npmrc`
+- `Dockerfile-incunabulum`

src/api/git/commitAllChanges.ts

@@ -3,13 +3,21 @@ import git from 'isomorphic-git';
 
 import { commit } from './commit';
 import type { Identity } from './commit';
+import type { ChangedFile } from './getChangedFiles';
 import { getChangedFiles } from './getChangedFiles';
 
 interface CommitAllParameters {
   dir: string;
   message: string;
   author?: Identity;
   committer?: Identity;
+
+  /**
+   * File changes to exclude from the commit.
+   *
+   * Defaults to `[]` (no exclusions).
+   */
+  ignore?: ChangedFile[];
 }
 
 /**
@@ -18,10 +26,12 @@ interface CommitAllParameters {
 export const commitAllChanges = async ({
   dir,
   message,
+
   author,
   committer,
+  ignore,
 }: CommitAllParameters): Promise<string | undefined> => {
-  const changedFiles = await getChangedFiles({ dir });
+  const changedFiles = await getChangedFiles({ dir, ignore });
 
   if (!changedFiles.length) {
     return;

src/api/git/getChangedFiles.int.test.ts

@@ -62,6 +62,54 @@ it('should return files which were deleted', async () => {
   expect(files).toStrictEqual([{ path: newFileName, state: 'deleted' }]);
 });
 
+it('should exclude file changes based on ignore parameter', async () => {
+  await Promise.all([
+    fs.promises.writeFile('a', '1'),
+    fs.promises.writeFile('b', '1'),
+  ]);
+
+  await git.add({ fs, dir, filepath: ['a', 'b'] });
+  await git.commit({
+    fs,
+    dir,
+    message: 'initial commit',
+    author,
+  });
+
+  await Promise.all([
+    fs.promises.rm('a'),
+    fs.promises.writeFile('b', '2'),
+    fs.promises.writeFile('c', '2'),
+  ]);
+
+  const files = await getChangedFiles({
+    dir,
+    ignore: [
+      {
+        path: 'c',
+        // c file change is not matched
+        state: 'modified',
+      },
+      {
+        path: 'a',
+        // a file change is not matched
+        state: 'added',
+      },
+      {
+        // b file change is matched and therefore ignored
+        path: 'b',
+        state: 'modified',
+      },
+    ],
+  });
+
+  expect(files).toStrictEqual([
+    // b file change is matched and therefore ignored
+    { path: 'a', state: 'deleted' },
+    { path: 'c', state: 'added' },
+  ]);
+});
+
 it('should return an empty array if no files were changed', async () => {
   await fs.promises.writeFile(newFileName, '');
   await git.add({ fs, dir, filepath: newFileName });

src/api/git/getChangedFiles.ts

@@ -18,6 +18,13 @@ export interface ChangedFile {
 }
 interface ChangedFilesParameters {
   dir: string;
+
+  /**
+   * File changes to exclude from the result.
+   *
+   * Defaults to `[]` (no exclusions).
+   */
+  ignore?: ChangedFile[];
 }
 
 const mapState = (
@@ -40,6 +47,8 @@ const mapState = (
  */
 export const getChangedFiles = async ({
   dir,
+
+  ignore = [],
 }: ChangedFilesParameters): Promise<ChangedFile[]> => {
   const allFiles = await git.statusMatrix({ fs, dir });
   return allFiles
@@ -49,5 +58,11 @@ export const getChangedFiles = async ({
         row[WORKDIR] !== UNMODIFIED ||
         row[STAGE] !== UNMODIFIED,
     )
-    .map((row) => ({ path: row[FILEPATH], state: mapState(row) }));
+    .map((row) => ({ path: row[FILEPATH], state: mapState(row) }))
+    .filter(
+      (changedFile) =>
+        !ignore.some(
+          (i) => i.path === changedFile.path && i.state === changedFile.state,
+        ),
+    );
 };

src/api/git/index.ts

@@ -1,6 +1,7 @@
 export { commit } from './commit';
 export { commitAllChanges } from './commitAllChanges';
 export { currentBranch } from './currentBranch';
+export type { ChangedFile } from './getChangedFiles';
 export { getChangedFiles } from './getChangedFiles';
 export { getHeadCommitId, getHeadCommitMessage } from './log';
 export { getOwnerAndRepo } from './remote';

src/api/github/push.ts

@@ -7,7 +7,6 @@ import type {
 import fs from 'fs-extra';
 
 import * as Git from '../git';
-import type { ChangedFile } from '../git/getChangedFiles';
 
 import { apiTokenFromEnvironment } from './environment';
 
@@ -29,6 +28,13 @@ interface UploadAllFileChangesParams {
    * The headline of the commit message
    */
   messageHeadline: string;
+
+  /**
+   * File changes to exclude from the upload.
+   *
+   * Defaults to `[]` (no exclusions).
+   */
+  ignore?: Git.ChangedFile[];
   /**
    * The body of the commit message
    */
@@ -52,15 +58,18 @@ interface UploadAllFileChangesParams {
  * specified.
  */
 export const uploadAllFileChanges = async ({
-  dir,
   branch,
+  dir,
   messageHeadline,
+
+  ignore,
   messageBody,
   updateLocal = false,
 }: UploadAllFileChangesParams): Promise<string | undefined> => {
-  const changedFiles = await Git.getChangedFiles({ dir });
+  const changedFiles = await Git.getChangedFiles({ dir, ignore });
+
   if (!changedFiles.length) {
-    return undefined;
+    return;
   }
 
   const fileChanges = await readFileChanges(changedFiles);
@@ -102,7 +111,7 @@ export interface FileChanges {
  * https://docs.github.com/en/graphql/reference/input-objects#filechanges
  */
 export const readFileChanges = async (
-  changedFiles: ChangedFile[],
+  changedFiles: Git.ChangedFile[],
 ): Promise<FileChanges> => {
   const { added, deleted } = changedFiles.reduce<{
     added: string[];

src/cli/lint/autofix.test.ts

@@ -5,7 +5,7 @@ import * as GitHub from '../../api/github';
 import { runESLint } from '../../cli/adapter/eslint';
 import { runPrettier } from '../../cli/adapter/prettier';
 
-import { autofix } from './autofix';
+import { AUTOFIX_IGNORE_FILES, autofix } from './autofix';
 
 jest.mock('simple-git');
 jest.mock('../../api/git');
@@ -155,6 +155,13 @@ describe('autofix', () => {
 
       expectAutofixCommit();
 
+      expect(Git.commitAllChanges).toHaveBeenNthCalledWith(1, {
+        dir: expect.any(String),
+        message: 'Run `skuba format`',
+
+        ignore: AUTOFIX_IGNORE_FILES,
+      });
+
       expect(push).toHaveBeenNthCalledWith(1);
 
       expect(stdout()).toMatchInlineSnapshot(`
@@ -197,6 +204,14 @@ describe('autofix', () => {
       ).resolves.toBeUndefined();
 
       expectAutofixCommit({ eslint: false });
+
+      expect(Git.commitAllChanges).toHaveBeenNthCalledWith(1, {
+        dir: expect.any(String),
+        message: 'Run `skuba format`',
+
+        ignore: AUTOFIX_IGNORE_FILES,
+      });
+
       expect(push).toHaveBeenNthCalledWith(1);
 
       // We should only run Prettier
@@ -357,9 +372,11 @@ describe('autofix', () => {
 
       expectAutofixCommit();
       expect(GitHub.uploadAllFileChanges).toHaveBeenNthCalledWith(1, {
-        dir: expect.any(String),
         branch: 'dev',
+        dir: expect.any(String),
         messageHeadline: 'Run `skuba format`',
+
+        ignore: AUTOFIX_IGNORE_FILES,
       });
 
       // We should run both ESLint and Prettier
@@ -382,9 +399,11 @@ describe('autofix', () => {
 
       expectAutofixCommit({ eslint: false });
       expect(GitHub.uploadAllFileChanges).toHaveBeenNthCalledWith(1, {
-        dir: expect.any(String),
         branch: 'dev',
+        dir: expect.any(String),
         messageHeadline: 'Run `skuba format`',
+
+        ignore: AUTOFIX_IGNORE_FILES,
       });
 
       // We should only run Prettier

src/cli/lint/autofix.ts

@@ -14,6 +14,17 @@ import type { Input } from './types';
 
 const AUTOFIX_COMMIT_MESSAGE = 'Run `skuba format`';
 
+export const AUTOFIX_IGNORE_FILES: Git.ChangedFile[] = [
+  {
+    path: '.npmrc',
+    state: 'added',
+  },
+  {
+    path: 'Dockerfile-incunabulum',
+    state: 'added',
+  },
+];
+
 const shouldPush = async ({
   currentBranch,
   dir,
@@ -100,6 +111,8 @@ export const autofix = async (params: AutofixParameters): Promise<void> => {
       const ref = await Git.commitAllChanges({
         dir,
         message: AUTOFIX_COMMIT_MESSAGE,
+
+        ignore: AUTOFIX_IGNORE_FILES,
       });
 
       if (!ref) {
@@ -122,9 +135,11 @@ export const autofix = async (params: AutofixParameters): Promise<void> => {
 
     const ref = await throwOnTimeout(
       GitHub.uploadAllFileChanges({
-        dir,
         branch: currentBranch,
+        dir,
         messageHeadline: AUTOFIX_COMMIT_MESSAGE,
+
+        ignore: AUTOFIX_IGNORE_FILES,
       }),
       { s: 30 },
     );