Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(prlint): lint module with eslint #26486

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions tools/@aws-cdk/prlint/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
module.exports = {
...baseConfig,
rules: {
'no-console': 'off',
'jest/valid-expect': 'off',
},
};
1 change: 1 addition & 0 deletions tools/@aws-cdk/prlint/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
*.js

!decs.d.ts
!.eslintrc.js
4 changes: 2 additions & 2 deletions tools/@aws-cdk/prlint/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as core from '@actions/core';
import * as github from '@actions/github';
import { Octokit } from '@octokit/rest';
import * as linter from './lint';
import { StatusEvent, PullRequestEvent } from '@octokit/webhooks-definitions/schema';
import * as linter from './lint';

async function run() {
const token: string = process.env.GITHUB_TOKEN!;
Expand Down Expand Up @@ -39,4 +39,4 @@ async function run() {
}
}

run()
void run();
76 changes: 38 additions & 38 deletions tools/@aws-cdk/prlint/lint.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { execSync } from 'child_process';
import * as path from 'path';
import { Octokit } from '@octokit/rest';
import { Endpoints } from '@octokit/types';
import { StatusEvent } from '@octokit/webhooks-definitions/schema';
import { breakingModules } from './parser';
import { findModulePath, moduleStability } from './module';
import { Endpoints } from "@octokit/types";
import { execSync } from 'child_process';
import { breakingModules } from './parser';

export type GitHubPr =
Endpoints["GET /repos/{owner}/{repo}/pulls/{pull_number}"]["response"]["data"];
Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}']['response']['data'];

export const CODE_BUILD_CONTEXT = 'AWS CodeBuild us-east-1 (AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv)';

Expand Down Expand Up @@ -237,8 +237,8 @@ export class PullRequestLinter {
await this.client.pulls.dismissReview({
...this.prParams,
review_id: existingReview.id,
message: '✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.'
})
message: '✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.',
});
}
}

Expand All @@ -258,15 +258,15 @@ export class PullRequestLinter {
body: 'The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons.'
+ ' If you believe this pull request should receive an exemption, please comment and provide a justification.'
+ '\n\n\nA comment requesting an exemption should contain the text `Exemption Request`.'
+ ' Additionally, if clarification is needed add `Clarification Request` to a comment.',
+ ' Additionally, if clarification is needed add `Clarification Request` to a comment.',
event: 'REQUEST_CHANGES',
})
});
}

await this.client.issues.createComment({
...this.issueParams,
body,
})
});

throw new LinterError(body);
}
Expand Down Expand Up @@ -296,7 +296,7 @@ export class PullRequestLinter {
private async communicateResult(result: ValidationCollector): Promise<void> {
const existingReview = await this.findExistingReview();
if (result.isValid()) {
console.log("✅ Success");
console.log('✅ Success');
await this.dismissPRLinterReview(existingReview);
} else {
await this.createOrUpdatePRLinterReview(result.errors, existingReview);
Expand Down Expand Up @@ -342,27 +342,27 @@ export class PullRequestLinter {
* 7. It links to a p2 issue and has an approved community review
*/
private async assessNeedsReview(
pr: Pick<GitHubPr, "mergeable_state" | "draft" | "labels" | "number">,
pr: Pick<GitHubPr, 'mergeable_state' | 'draft' | 'labels' | 'number'>,
): Promise<void> {
const reviews = await this.client.pulls.listReviews(this.prParams);
// NOTE: MEMBER = a member of the organization that owns the repository
// COLLABORATOR = has been invited to collaborate on the repository
const maintainerRequestedChanges = reviews.data.some(
review => review.author_association === 'MEMBER'
&& review.user?.login !== 'aws-cdk-automation'
&& review.state === 'CHANGES_REQUESTED'
&& review.state === 'CHANGES_REQUESTED',
);
const maintainerApproved = reviews.data.some(
review => review.author_association === 'MEMBER'
&& review.state === 'APPROVED'
&& review.state === 'APPROVED',
);
const communityRequestedChanges = reviews.data.some(
review => this.getTrustedCommunityMembers().includes(review.user?.login ?? '')
&& review.state !== 'APPROVED' // community members cannot request changes
)
&& review.state !== 'APPROVED', // community members cannot request changes
);
const communityApproved = reviews.data.some(
review => this.getTrustedCommunityMembers().includes(review.user?.login ?? '')
&& review.state === 'APPROVED'
&& review.state === 'APPROVED',
);

const prLinterFailed = reviews.data.find((review) => review.user?.login === 'aws-cdk-automation' && review.state !== 'DISMISSED') as Review;
Expand Down Expand Up @@ -406,7 +406,7 @@ export class PullRequestLinter {
}
}

private addLabel(label: string, pr: Pick<GitHubPr, "labels" | "number">) {
private addLabel(label: string, pr: Pick<GitHubPr, 'labels' | 'number'>) {
// already has label, so no-op
if (pr.labels.some(l => l.name === label)) { return; }
console.log(`adding ${label} to pr ${pr.number}`);
Expand All @@ -420,7 +420,7 @@ export class PullRequestLinter {
});
}

private removeLabel(label: string, pr: Pick<GitHubPr, "labels" | "number">) {
private removeLabel(label: string, pr: Pick<GitHubPr, 'labels' | 'number'>) {
// does not have label, so no-op
if (!pr.labels.some(l => l.name === label)) { return; }
console.log(`removing ${label} to pr ${pr.number}`);
Expand All @@ -439,7 +439,7 @@ export class PullRequestLinter {
private getTrustedCommunityMembers(): string[] {
if (this.trustedCommunity.length > 0) { return this.trustedCommunity; }

const wiki = execSync('curl https://raw.githubusercontent.com/wiki/aws/aws-cdk/Introducing-CDK-Community-PR-Reviews.md', { encoding: 'utf-8'}).toString();
const wiki = execSync('curl https://raw.githubusercontent.com/wiki/aws/aws-cdk/Introducing-CDK-Community-PR-Reviews.md', { encoding: 'utf-8' }).toString();
const rawMdTable = wiki.split('<!--section-->')[1].split('\n').filter(l => l !== '');
for (let i = 2; i < rawMdTable.length; i++) {
this.trustedCommunity.push(rawMdTable[i].split('|')[1].trim());
Expand All @@ -460,48 +460,48 @@ export class PullRequestLinter {
console.log(`⌛ Fetching files for PR number ${number}`);
const files = await this.client.paginate(this.client.pulls.listFiles, this.prParams);

console.log("⌛ Validating...");
console.log('⌛ Validating...');

const validationCollector = new ValidationCollector(pr, files);

validationCollector.validateRuleSet({
exemption: shouldExemptReadme,
exemptionMessage: `Not validating README changes since the PR is labeled with '${Exemption.README}'`,
testRuleSet: [ { test: featureContainsReadme } ],
testRuleSet: [{ test: featureContainsReadme }],
});

validationCollector.validateRuleSet({
exemption: shouldExemptTest,
exemptionMessage: `Not validating test changes since the PR is labeled with '${Exemption.TEST}'`,
testRuleSet: [ { test: featureContainsTest }, { test: fixContainsTest } ],
testRuleSet: [{ test: featureContainsTest }, { test: fixContainsTest }],
});

validationCollector.validateRuleSet({
exemption: shouldExemptIntegTest,
exemptionMessage: `Not validating integration test changes since the PR is labeled with '${Exemption.INTEG_TEST}'`,
testRuleSet: [ { test: featureContainsIntegTest}, { test: fixContainsIntegTest } ]
testRuleSet: [{ test: featureContainsIntegTest }, { test: fixContainsIntegTest }],
});

validationCollector.validateRuleSet({
testRuleSet: [ { test: validateBreakingChangeFormat } ]
testRuleSet: [{ test: validateBreakingChangeFormat }],
});

validationCollector.validateRuleSet({
testRuleSet: [ { test: validateTitlePrefix } ]
testRuleSet: [{ test: validateTitlePrefix }],
});
validationCollector.validateRuleSet({
testRuleSet: [ { test: validateTitleScope } ]
})
testRuleSet: [{ test: validateTitleScope }],
});

validationCollector.validateRuleSet({
exemption: shouldExemptBreakingChange,
exemptionMessage: `Not validating breaking changes since the PR is labeled with '${Exemption.BREAKING_CHANGE}'`,
testRuleSet: [ { test: assertStability } ]
testRuleSet: [{ test: assertStability }],
});

validationCollector.validateRuleSet({
exemption: shouldExemptCliIntegTested,
testRuleSet: [ { test: noCliChanges } ],
testRuleSet: [{ test: noCliChanges }],
});

await this.deletePRLinterComment();
Expand All @@ -527,31 +527,31 @@ export class PullRequestLinter {
}

function isPkgCfnspec(pr: GitHubPr): boolean {
return pr.title.indexOf("(cfnspec)") > -1;
return pr.title.indexOf('(cfnspec)') > -1;
}

function isFeature(pr: GitHubPr): boolean {
return pr.title.startsWith("feat")
return pr.title.startsWith('feat');
}

function isFix(pr: GitHubPr): boolean {
return pr.title.startsWith("fix")
return pr.title.startsWith('fix');
}

function testChanged(files: GitHubFile[]): boolean {
return files.filter(f => f.filename.toLowerCase().includes("test")).length != 0;
return files.filter(f => f.filename.toLowerCase().includes('test')).length != 0;
}

function integTestChanged(files: GitHubFile[]): boolean {
return files.filter(f => f.filename.toLowerCase().match(/integ.*.ts$/)).length != 0;
}

function integTestSnapshotChanged(files: GitHubFile[]): boolean {
return files.filter(f => f.filename.toLowerCase().includes(".snapshot")).length != 0;
return files.filter(f => f.filename.toLowerCase().includes('.snapshot')).length != 0;
}

function readmeChanged(files: GitHubFile[]): boolean {
return files.filter(f => path.basename(f.filename) == "README.md").length != 0;
return files.filter(f => path.basename(f.filename) == 'README.md').length != 0;
}

function featureContainsReadme(pr: GitHubPr, files: GitHubFile[]): TestResult {
Expand Down Expand Up @@ -627,7 +627,7 @@ function validateBreakingChangeFormat(pr: GitHubPr, _files: GitHubFile[]): TestR
const m = re.exec(body ?? '');
if (m) {
result.assessFailure(!m[0].startsWith('BREAKING CHANGE: '), `Breaking changes should be indicated by starting a line with 'BREAKING CHANGE: ', variations are not allowed. (found: '${m[0]}').`);
result.assessFailure(m[0].slice('BREAKING CHANGE:'.length).trim().length === 0, `The description of the first breaking change should immediately follow the 'BREAKING CHANGE: ' clause.`);
result.assessFailure(m[0].slice('BREAKING CHANGE:'.length).trim().length === 0, 'The description of the first breaking change should immediately follow the \'BREAKING CHANGE: \' clause.');
const titleRe = /^[a-z]+\([0-9a-z-_]+\)/;
result.assessFailure(!titleRe.exec(title), 'The title of this pull request must specify the module name that the first breaking change should be associated to.');
}
Expand All @@ -643,7 +643,7 @@ function validateTitlePrefix(pr: GitHubPr): TestResult {
const m = titleRe.exec(pr.title);
result.assessFailure(
!m,
"The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.");
'The title of this pull request does not follow the Conventional Commits format, see https://www.conventionalcommits.org/.');
return result;
}

Expand All @@ -654,7 +654,7 @@ function validateTitlePrefix(pr: GitHubPr): TestResult {
*/
function validateTitleScope(pr: GitHubPr): TestResult {
const result = new TestResult();
const scopesExemptFromThisRule = [ 'aws-cdk-lib' ];
const scopesExemptFromThisRule = ['aws-cdk-lib'];
// Specific commit types are handled by `validateTitlePrefix`. This just checks whether
// the scope includes an `aws-` prefix or not.
// Group 1: Scope with parens - "(aws-<name>)"
Expand Down
8 changes: 4 additions & 4 deletions tools/@aws-cdk/prlint/module.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as path from 'path';
import * as fs from 'fs-extra';
import * as glob from 'glob';
import * as path from 'path';

let awsCdkLibPath: string;
const modules: string[] = [];
Expand All @@ -9,7 +9,7 @@ const awsCdkLibModules: string[] = [];
export function findModulePath(fuzz: string): string {
discoverModules();

const regex = new RegExp(`[-_/]${fuzz}($|-alpha)`)
const regex = new RegExp(`[-_/]${fuzz}($|-alpha)`);
const matched = [
...modules.filter(m => regex.test(m)),
...(awsCdkLibModules.some(m => regex.test(m)) ? [awsCdkLibPath] : []),
Expand All @@ -32,7 +32,7 @@ function discoverModules() {
throw new Error('env REPO_ROOT must be set');
}
const repoRoot = process.env.REPO_ROOT;
const lernaConfig = require(path.join(repoRoot, 'lerna.json'));
const lernaConfig = require(path.join(repoRoot, 'lerna.json')); // eslint-disable-line @typescript-eslint/no-require-imports
const searchPaths: string[] = lernaConfig.packages;
awsCdkLibPath = path.join(repoRoot, 'packages', 'aws-cdk-lib');
searchPaths.forEach(p => {
Expand Down Expand Up @@ -67,6 +67,6 @@ export function moduleStability(loc: string): 'stable' | 'experimental' | undefi
if (!fs.existsSync(path.join(loc, 'package.json'))) {
throw new Error(`unexpected: no package.json found at location "${loc}"`);
}
const pkgjson = require(path.join(loc, 'package.json'));
const pkgjson = require(path.join(loc, 'package.json')); // eslint-disable-line @typescript-eslint/no-require-imports
return pkgjson.stability;
}
10 changes: 8 additions & 2 deletions tools/@aws-cdk/prlint/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
"@types/jest": "^29.5.3",
"jest": "^29.6.1",
"make-runnable": "^1.4.1",
"typescript": "~5.1.6"
"typescript": "~5.1.6",
"eslint": "^7.32.0",
"eslint-import-resolver-node": "^0.3.7",
"eslint-import-resolver-typescript": "^2.7.1",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-jest": "^24.7.0"
},
"jest": {
"testMatch": [
Expand All @@ -37,6 +42,7 @@
"test": "jest",
"build+test": "npm run build && npm run test",
"build+test+extract": "npm run build+test",
"build+extract": "npm run build"
"build+extract": "npm run build",
"lint": "tsc -b && eslint . --ext=.ts"
}
}
Loading