chore(prlint): check PR size (#33288)

Add a new prlint rule that checks whether the size of the PR exceeds the predefined threshold. The PR size is measured by how many lines of code were added or removed (as reported by Git). If either the number of additions or deletions exceeds 1000 lines, the rule reports an error.

Only two packages are subject to this check: `aws-cdk` and `@aws-cdk-testing/cli-integ`. Markdown files and `THIRD_PARTY_LICENSES` are also excluded.

Prior art: [amplify-backend](https://github.com/aws-amplify/amplify-backend/blob/main/scripts/check_pr_size.ts).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: otaviomacedo

tools/@aws-cdk/prlint/README.md

@@ -2,29 +2,31 @@
 
 This package includes a GitHub Action that does the following:
 - Checks pull requests around PR titles, description and other metadata.
-- Assesses whether or not the PR is ready for review and attaches the correct label to the PR.
+- Assesses whether the PR is ready for review and attaches the correct label to the PR.
 
 # Rules
 
-The first part of the GitHub Action validates whether or not the pull request adheres
+The first part of the GitHub Action validates whether the pull request adheres
 (or has been exempted from) to the following rules:
 
 1. `feat` requires a change to a `README.md` (exemption is the label `pr-linter/exempt-readme`).
 2. Both `feat` and `fix` PRs require a change to a unit test file and integration test files (exemption is the label `pr-linter/exempt-unit-test` or `pr-linter/exempt-integ-test`).
-4. `BREAKING CHANGE` section is formatted correctly, per the [conventional commits] spec.
-5. No breaking changes announced for stable modules.
-6. Title prefix and scope is formatted correctly.
-7. The PR is not opened from the main branch of the author's fork.
-8. Changes to the cli have been run through the test pipeline where cli integ tests are run (indicated by the label `pr-linter/cli-integ-tested`).
-9. No manual changes to `packages/aws-cdk-lib/region-info/build-tools/metadata.ts` file.
+3. `BREAKING CHANGE` section is formatted correctly, per the [conventional commits] spec.
+4. No breaking changes announced for stable modules.
+5. Title prefix and scope is formatted correctly.
+6. The PR is not opened from the main branch of the author's fork.
+7. Changes to the cli have been run through the test pipeline where cli integ tests are run (indicated by the label `pr-linter/cli-integ-tested`).
+8. No manual changes to `packages/aws-cdk-lib/region-info/build-tools/metadata.ts` file.
+9. The size of the PR (number of lines added or removed) does not exceed the 
+   pre-defined threshold of 1000.
 
 > These rules are currently hard coded, in the future, we should consider using [danger.js](https://danger.systems/js/).
 
 [conventional commits]: https://www.conventionalcommits.org
 
 # Evaluation and Assigning Labels
 
-The GitHub Action also handles whether or not the PR is ready for review, and by whom.
+The GitHub Action also handles whether the PR is ready for review, and by whom.
 A PR is ready for review when:
 
 1. It is not a draft
@@ -33,7 +35,7 @@ A PR is ready for review when:
 4. A maintainer has not requested changes
 5. A maintainer has not approved
 
-If the PR is ready for review, we also differentiate whether or not it is ready for a
+If the PR is ready for review, we also differentiate whether it is ready for a
 maintainer review (`pr/needs-maintainer-review`) or if we are soliciting help from our
 pool of trusted reviewers (`pr/needs-community-review`).
 
@@ -54,7 +56,7 @@ yarn install
 
 # Usage
 
-The steps for your Github action would look something like this -
+The steps for your GitHub action would look something like this -
 
 ```yaml
 steps:

tools/@aws-cdk/prlint/constants.ts

@@ -16,6 +16,7 @@ export enum Exemption {
   REQUEST_CLARIFICATION = 'pr/reviewer-clarification-requested',
   REQUEST_EXEMPTION = 'pr-linter/exemption-requested',
   CODECOV = "pr-linter/exempt-codecov",
+  SIZE_CHECK = "pr-linter/exempt-size-check",
 }
 
 const CODECOV_PREFIX = 'codecov/';

tools/@aws-cdk/prlint/github.ts

@@ -30,10 +30,31 @@ export interface GitHubLabel {
   readonly name: string;
 }
 
-export interface GitHubFile {
+export interface Delta {
+  readonly additions: number;
+  readonly deletions: number;
+}
+
+export interface GitHubFile extends Delta {
   readonly filename: string;
 }
 
+export function sumChanges(files: GitHubFile[]) {
+  function add(d1: Delta, d2: Delta): Delta {
+    return {
+      additions: d1.additions + d2.additions,
+      deletions: d1.deletions + d2.deletions,
+    };
+  }
+
+  const identity = {
+    additions: 0,
+    deletions: 0
+  };
+
+  return files.reduce(add, identity);
+}
+
 
 /**
  * Combine all check run conclusions

tools/@aws-cdk/prlint/lint.ts

@@ -1,7 +1,7 @@
 import * as path from 'path';
 import { findModulePath, moduleStability } from './module';
 import { breakingModules } from './parser';
-import { CheckRun, GitHubFile, GitHubPr, Review, summarizeRunConclusions } from "./github";
+import { CheckRun, GitHubFile, GitHubPr, Review, sumChanges, summarizeRunConclusions } from "./github";
 import { TestResult, ValidationCollector } from './results';
 import { CODE_BUILD_CONTEXT, CODECOV_CHECKS, Exemption } from './constants';
 import { StatusEvent } from '@octokit/webhooks-definitions/schema';
@@ -257,6 +257,13 @@ export class PullRequestLinter extends PullRequestLinterBase {
       ],
     });
 
+    validationCollector.validateRuleSet({
+      exemption: shouldExemptSizeCheck,
+      testRuleSet: [
+        { test: prIsSmall },
+      ],
+    })
+
     if (pr.base.ref === 'main') {
       // Only check CodeCov for PRs targeting 'main'
       const runs = await this.checkRuns(sha);
@@ -421,6 +428,10 @@ function shouldExemptCliIntegTested(pr: GitHubPr): boolean {
   return (hasLabel(pr, Exemption.CLI_INTEG_TESTED) || pr.user?.login === 'aws-cdk-automation');
 }
 
+function shouldExemptSizeCheck(pr: GitHubPr): boolean {
+  return hasLabel(pr, Exemption.SIZE_CHECK);
+}
+
 function shouldExemptAnalyticsMetadataChange(pr: GitHubPr): boolean {
   return (hasLabel(pr, Exemption.ANALYTICS_METADATA_CHANGE) || pr.user?.login === 'aws-cdk-automation');
 }
@@ -570,6 +581,31 @@ function noAnalyticsEnumsChanges(_pr: GitHubPr, files: GitHubFile[]): TestResult
   return result;
 }
 
+function prIsSmall(_pr: GitHubPr, files: GitHubFile[]): TestResult {
+  const folders = ['packages/aws-cdk/', 'packages/@aws-cdk-testing/cli-integ/'];
+  const exclude = [/THIRD_PARTY_LICENSES/, /.*\.md/, /.*\.test\.ts/];
+  const maxLinesAdded = 1000;
+  const maxLinesRemoved = 1000;
+
+  const filesToCheck: GitHubFile[] = files
+    .filter(r => folders.some(folder => r.filename.startsWith(folder)))
+    .filter(r => exclude.every(re => !re.test(r.filename)));
+
+  const sum = sumChanges(filesToCheck);
+
+  const result = new TestResult();
+  result.assessFailure(
+    sum.additions > maxLinesAdded,
+    `The number of lines added (${sum.additions}) is greater than ${maxLinesAdded}`
+  );
+  result.assessFailure(
+    sum.deletions > maxLinesRemoved,
+    `The number of lines removed (${sum.deletions}) is greater than ${maxLinesAdded}`
+  );
+
+  return result;
+}
+
 require('make-runnable/custom')({
   printOutputFrame: false,
 });

tools/@aws-cdk/prlint/test/lint.test.ts

@@ -5,6 +5,8 @@ import { PullRequestLinter } from '../lint';
 import { StatusEvent } from '@octokit/webhooks-definitions/schema';
 import { createOctomock, OctoMock } from './octomock';
 
+type GitHubFileName = Omit<GitHubFile, 'deletions' | 'additions'>;
+
 let mockRemoveLabel = jest.fn();
 let mockAddLabel = jest.fn();
 
@@ -1138,7 +1140,7 @@ describe('integration tests required on features', () => {
   });
 
   describe('metadata file changed', () => {
-    const files: GitHubFile[] = [{
+    const files: GitHubFileName[] = [{
       filename: 'packages/aws-cdk-lib/region-info/build-tools/metadata.ts',
     }];
 
@@ -1170,6 +1172,100 @@ describe('integration tests required on features', () => {
       await expect(legacyValidatePullRequestTarget(prLinter)).rejects.toThrow();
     });
   });
+
+  describe('PR size check', () => {
+    const pr = {
+      title: 'chore: update regions',
+      number: 1234,
+      labels: [],
+      user: {
+        login: 'aws-cdk-automation',
+      },
+    };
+
+    test('PR too large', async () => {
+      const files: GitHubFile[] = [{
+        filename: 'packages/aws-cdk/foo.ts',
+        additions: 1001,
+        deletions: 1002,
+      }];
+
+      const prLinter = configureMock(pr, files);
+
+      await expect(prLinter.validatePullRequestTarget()).resolves.toEqual(expect.objectContaining({
+        requestChanges: {
+          exemptionRequest: false,
+          failures: [
+            'The number of lines added (1001) is greater than 1000',
+            'The number of lines removed (1002) is greater than 1000',
+          ]
+        }
+      }));
+    });
+
+    test('PR size within bounds', async () => {
+      const files: GitHubFile[] = [{
+        filename: 'packages/aws-cdk/foo.ts',
+        additions: 1000,
+        deletions: 1000,
+      }];
+
+      const prLinter = configureMock(pr, files);
+
+      await expect(prLinter.validatePullRequestTarget()).resolves.toEqual(expect.objectContaining({
+        requestChanges: undefined,
+      }));
+    });
+
+    test('PR exempt from size check', async () => {
+      const files: GitHubFile[] = [{
+        filename: 'packages/aws-cdk/foo.ts',
+        additions: 2000,
+        deletions: 111,
+      }];
+
+      const exemptPr = {
+        ...pr,
+        labels: [
+          { name: 'pr-linter/exempt-size-check' },
+        ],
+      }
+
+      const prLinter = configureMock(exemptPr, files);
+
+      await expect(prLinter.validatePullRequestTarget()).resolves.toEqual(expect.objectContaining({
+        requestChanges: undefined,
+      }));
+    });
+
+    test('Only CLI is subject to verification', async () => {
+      const files: GitHubFile[] = [{
+        filename: 'packages/aws-cdk-lib/foo.ts',
+        additions: 1001,
+        deletions: 1002,
+      }];
+
+      const prLinter = configureMock(pr, files);
+
+      await expect(prLinter.validatePullRequestTarget()).resolves.toEqual(expect.objectContaining({
+        requestChanges: undefined
+      }));
+    });
+
+    test('Tests are exempt', async () => {
+      const files: GitHubFile[] = [{
+        filename: 'packages/aws-cdk/test/bootstrap.test.ts',
+        additions: 1001,
+        deletions: 1002,
+      }];
+
+      const prLinter = configureMock(pr, files);
+
+      await expect(prLinter.validatePullRequestTarget()).resolves.toEqual(expect.objectContaining({
+        requestChanges: undefined
+      }));
+    });
+  });
 });
 
 describe('for any PR', () => {
@@ -1182,7 +1278,7 @@ describe('for any PR', () => {
     },
   };
 
-  const ARBITRARY_FILES: GitHubFile[] = [{
+  const ARBITRARY_FILES: GitHubFileName[] = [{
     filename: 'README.md',
   }];
 
@@ -1247,7 +1343,7 @@ describe('for any PR', () => {
   });
 });
 
-function configureMock(pr: Subset<GitHubPr>, prFiles?: GitHubFile[], existingComments?: Array<{ id: number, login: string, body: string }>): PullRequestLinter & { octomock: OctoMock } {
+function configureMock(pr: Subset<GitHubPr>, prFiles?: GitHubFileName[], existingComments?: Array<{ id: number, login: string, body: string }>): PullRequestLinter & { octomock: OctoMock } {
   const octomock = createOctomock();
 
   octomock.pulls.get.mockImplementation((_props: { _owner: string, _repo: string, _pull_number: number, _user: { _login: string} }) => ({