From 658139a1f1f247ac5c215ede279818f8889696f5 Mon Sep 17 00:00:00 2001 From: TingluoHuang Date: Mon, 2 Nov 2020 16:43:44 -0500 Subject: [PATCH 1/9] not print untrusted info to STDOUT. --- dist/index.js | 12 ++++++------ src/IssueProcessor.ts | 16 ++++++---------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/dist/index.js b/dist/index.js index da2fff6a2..14112ea5a 100644 --- a/dist/index.js +++ b/dist/index.js @@ -75,7 +75,7 @@ class IssueProcessor { } for (const issue of issues.values()) { const isPr = !!issue.pull_request; - core.info(`Found issue: issue #${issue.number} - ${issue.title} last updated ${issue.updated_at} (is pr? ${isPr})`); + core.info(`Found issue: issue #${issue.number} last updated ${issue.updated_at} (is pr? ${isPr})`); // calculate string based messages for this issue const staleMessage = isPr ? this.options.stalePrMessage @@ -172,7 +172,7 @@ class IssueProcessor { // find any comments since the date const comments = yield this.listIssueComments(issue.number, sinceDate); const filteredComments = comments.filter(comment => comment.user.type === 'User' && comment.user.login !== github_1.context.actor); - core.info(`Comments not made by ${github_1.context.actor} or another bot: ${filteredComments.length}`); + core.info(`Comments not made by actor or another bot: ${filteredComments.length}`); // if there are any user comments returned return filteredComments.length > 0; }); @@ -222,7 +222,7 @@ class IssueProcessor { // Mark an issue as stale with a comment and a label markStale(issue, staleMessage, staleLabel, skipMessage) { return __awaiter(this, void 0, void 0, function* () { - core.info(`Marking issue #${issue.number} - ${issue.title} as stale`); + core.info(`Marking issue #${issue.number} as stale`); this.staleIssues.push(issue); this.operationsLeft -= 2; // if the issue is being marked stale, the updated date should be changed to right now @@ -261,7 +261,7 @@ class IssueProcessor { // Close an issue based on staleness closeIssue(issue, closeMessage, closeLabel) { return __awaiter(this, void 0, void 0, function* () { - core.info(`Closing issue #${issue.number} - ${issue.title} for being stale`); + core.info(`Closing issue #${issue.number} for being stale`); this.closedIssues.push(issue); this.operationsLeft -= 1; if (this.options.debugOnly) { @@ -309,7 +309,7 @@ class IssueProcessor { // Remove a label from an issue removeLabel(issue, label) { return __awaiter(this, void 0, void 0, function* () { - core.info(`Removing label ${label} from issue #${issue.number} - ${issue.title}`); + core.info(`Removing label from issue #${issue.number}`); this.removedLabelIssues.push(issue); this.operationsLeft -= 1; if (this.options.debugOnly) { @@ -332,7 +332,7 @@ class IssueProcessor { ///see https://developer.github.com/v3/activity/events/ getLabelCreationDate(issue, label) { return __awaiter(this, void 0, void 0, function* () { - core.info(`Checking for label ${label} on issue #${issue.number}`); + core.info(`Checking for label on issue #${issue.number}`); this.operationsLeft -= 1; const options = this.client.issues.listEvents.endpoint.merge({ owner: github_1.context.repo.owner, diff --git a/src/IssueProcessor.ts b/src/IssueProcessor.ts index 9e15ac39f..1016f5a57 100644 --- a/src/IssueProcessor.ts +++ b/src/IssueProcessor.ts @@ -115,7 +115,7 @@ export class IssueProcessor { const isPr = !!issue.pull_request; core.info( - `Found issue: issue #${issue.number} - ${issue.title} last updated ${issue.updated_at} (is pr? ${isPr})` + `Found issue: issue #${issue.number} last updated ${issue.updated_at} (is pr? ${isPr})` ); // calculate string based messages for this issue @@ -277,7 +277,7 @@ export class IssueProcessor { ); core.info( - `Comments not made by ${context.actor} or another bot: ${filteredComments.length}` + `Comments not made by actor or another bot: ${filteredComments.length}` ); // if there are any user comments returned @@ -336,7 +336,7 @@ export class IssueProcessor { staleLabel: string, skipMessage: boolean ): Promise { - core.info(`Marking issue #${issue.number} - ${issue.title} as stale`); + core.info(`Marking issue #${issue.number} as stale`); this.staleIssues.push(issue); @@ -382,9 +382,7 @@ export class IssueProcessor { closeMessage?: string, closeLabel?: string ): Promise { - core.info( - `Closing issue #${issue.number} - ${issue.title} for being stale` - ); + core.info(`Closing issue #${issue.number} for being stale`); this.closedIssues.push(issue); @@ -434,9 +432,7 @@ export class IssueProcessor { // Remove a label from an issue private async removeLabel(issue: Issue, label: string): Promise { - core.info( - `Removing label ${label} from issue #${issue.number} - ${issue.title}` - ); + core.info(`Removing label from issue #${issue.number}`); this.removedLabelIssues.push(issue); @@ -464,7 +460,7 @@ export class IssueProcessor { issue: Issue, label: string ): Promise { - core.info(`Checking for label ${label} on issue #${issue.number}`); + core.info(`Checking for label on issue #${issue.number}`); this.operationsLeft -= 1; From 9b82e8c1efc74af112507c1a9dbd0806b9cde43b Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 19 Nov 2020 20:36:46 -0500 Subject: [PATCH 2/9] Fix missing parenthesis (#197) --- src/IssueProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IssueProcessor.ts b/src/IssueProcessor.ts index 1016f5a57..28faf975c 100644 --- a/src/IssueProcessor.ts +++ b/src/IssueProcessor.ts @@ -250,7 +250,7 @@ export class IssueProcessor { await this.closeIssue(issue, closeMessage, closeLabel); } else { core.info( - `Stale ${issueType} is not old enough to close yet (hasComments? ${issueHasComments}, hasUpdate? ${issueHasUpdate}` + `Stale ${issueType} is not old enough to close yet (hasComments? ${issueHasComments}, hasUpdate? ${issueHasUpdate})` ); } } From 324009e5d028d042dc995e77140cfb983b7f0528 Mon Sep 17 00:00:00 2001 From: Geoffrey Testelin Date: Fri, 20 Nov 2020 12:48:33 +0100 Subject: [PATCH 3/9] fix(label): allow to use spaces inside the labels (#199) * chore(git): ignore .idea folder to avoid adding WebStorm files * test(jest): find all spec files as well * refactor(labels): create a dedicated function to parse the labels at first I thought that the parseCommaSeparatedString method was causing the issue so I move it to a dedicated file to test it since it was private also because I think that this repo could have more clean code and code splitting anyway this was not the root of the #98 issue :/ * fix(label): allow to use spaces inside the labels * docs(isLabeled): add JSDoc * chore(npm): add lint:fix script --- .gitignore | 1 + jest.config.js | 4 +- package-lock.json | 20 +++ package.json | 5 +- src/IssueProcessor.ts | 21 +-- src/functions/is-labeled.spec.ts | 187 +++++++++++++++++++++++++++ src/functions/is-labeled.ts | 24 ++++ src/functions/labels-to-list.spec.ts | 141 ++++++++++++++++++++ src/functions/labels-to-list.ts | 23 ++++ 9 files changed, 407 insertions(+), 19 deletions(-) create mode 100644 src/functions/is-labeled.spec.ts create mode 100644 src/functions/is-labeled.ts create mode 100644 src/functions/labels-to-list.spec.ts create mode 100644 src/functions/labels-to-list.ts diff --git a/.gitignore b/.gitignore index c8b37f362..1ae337ac0 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ node_modules/ lib/ __tests__/runner/* +.idea diff --git a/jest.config.js b/jest.config.js index 563d4ccb8..a27374193 100644 --- a/jest.config.js +++ b/jest.config.js @@ -2,10 +2,10 @@ module.exports = { clearMocks: true, moduleFileExtensions: ['js', 'ts'], testEnvironment: 'node', - testMatch: ['**/*.test.ts'], + testMatch: ['**/*.test.ts', '**/*.spec.ts'], testRunner: 'jest-circus/runner', transform: { '^.+\\.ts$': 'ts-jest' }, verbose: true -} \ No newline at end of file +}; diff --git a/package-lock.json b/package-lock.json index c66d409a3..387ba7e4d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1337,6 +1337,21 @@ "integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=", "dev": true }, + "@types/lodash": { + "version": "4.14.162", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.162.tgz", + "integrity": "sha512-alvcho1kRUnnD1Gcl4J+hK0eencvzq9rmzvFPRmP5rPHx9VVsJj6bKLTATPVf9ktgv4ujzh7T+XWKp+jhuODig==", + "dev": true + }, + "@types/lodash.deburr": { + "version": "4.1.6", + "resolved": "https://registry.npmjs.org/@types/lodash.deburr/-/lodash.deburr-4.1.6.tgz", + "integrity": "sha512-LsdZUIBM/YDQ4+w4/PSq2KTRRuCmBic48vzzKD7PHw1uIsKMWizwDtk0fMdqYmo9/iLMhHiFh2ol0eqhjT0VTg==", + "dev": true, + "requires": { + "@types/lodash": "*" + } + }, "@types/node": { "version": "14.10.0", "resolved": "https://registry.npmjs.org/@types/node/-/node-14.10.0.tgz", @@ -6423,6 +6438,11 @@ "integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==", "dev": true }, + "lodash.deburr": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/lodash.deburr/-/lodash.deburr-4.1.0.tgz", + "integrity": "sha1-3bG7s+8HRYwBd7oH3hRCLLAz/5s=" + }, "lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", diff --git a/package.json b/package.json index 8de99c691..540922087 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "format": "prettier --write **/*.ts", "format-check": "prettier --check **/*.ts", "lint": "eslint src/**/*.ts", + "lint:fix": "eslint src/**/*.ts --fix", "pack": "ncc build", "test": "jest", "all": "npm run build && npm run format && npm run lint && npm run pack && npm test" @@ -28,12 +29,14 @@ "@actions/core": "^1.2.6", "@actions/github": "^4.0.0", "@octokit/rest": "^18.0.4", + "lodash.deburr": "^4.1.0", "semver": "^7.3.2" }, "devDependencies": { - "@types/semver": "^7.3.1", "@types/jest": "^26.0.10", + "@types/lodash.deburr": "^4.1.6", "@types/node": "^14.10.0", + "@types/semver": "^7.3.1", "@typescript-eslint/parser": "^3.10.1", "@vercel/ncc": "^0.24.0", "eslint": "^7.7.0", diff --git a/src/IssueProcessor.ts b/src/IssueProcessor.ts index 28faf975c..77f3b09f0 100644 --- a/src/IssueProcessor.ts +++ b/src/IssueProcessor.ts @@ -1,6 +1,8 @@ import * as core from '@actions/core'; import {context, getOctokit} from '@actions/github'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; +import {isLabeled} from './functions/is-labeled'; +import {labelsToList} from './functions/labels-to-list'; export interface Issue { title: string; @@ -131,7 +133,7 @@ export class IssueProcessor { const closeLabel: string = isPr ? this.options.closePrLabel : this.options.closeIssueLabel; - const exemptLabels = IssueProcessor.parseCommaSeparatedString( + const exemptLabels: string[] = labelsToList( isPr ? this.options.exemptPrLabels : this.options.exemptIssueLabels ); const skipMessage = isPr @@ -157,7 +159,7 @@ export class IssueProcessor { if ( exemptLabels.some((exemptLabel: string) => - IssueProcessor.isLabeled(issue, exemptLabel) + isLabeled(issue, exemptLabel) ) ) { core.info(`Skipping ${issueType} because it has an exempt label`); @@ -165,7 +167,7 @@ export class IssueProcessor { } // does this issue have a stale label? - let isStale = IssueProcessor.isLabeled(issue, staleLabel); + let isStale = isLabeled(issue, staleLabel); // should this issue be marked stale? const shouldBeStale = !IssueProcessor.updatedSince( @@ -486,12 +488,6 @@ export class IssueProcessor { return staleLabeledEvent.created_at; } - private static isLabeled(issue: Issue, label: string): boolean { - const labelComparer: (l: Label) => boolean = l => - label.localeCompare(l.name, undefined, {sensitivity: 'accent'}) === 0; - return issue.labels.filter(labelComparer).length > 0; - } - private static updatedSince(timestamp: string, num_days: number): boolean { const daysInMillis = 1000 * 60 * 60 * 24 * num_days; const millisSinceLastUpdated = @@ -499,11 +495,4 @@ export class IssueProcessor { return millisSinceLastUpdated <= daysInMillis; } - - private static parseCommaSeparatedString(s: string): string[] { - // String.prototype.split defaults to [''] when called on an empty string - // In this case, we'd prefer to just return an empty array indicating no labels - if (!s.length) return []; - return s.split(',').map(l => l.trim()); - } } diff --git a/src/functions/is-labeled.spec.ts b/src/functions/is-labeled.spec.ts new file mode 100644 index 000000000..fbabb3dbf --- /dev/null +++ b/src/functions/is-labeled.spec.ts @@ -0,0 +1,187 @@ +import {Issue} from '../IssueProcessor'; +import {isLabeled} from './is-labeled'; + +describe('isLabeled()', (): void => { + let issue: Issue; + let label: string; + + describe('when the given issue contains no label', (): void => { + beforeEach((): void => { + issue = ({ + labels: [] + } as unknown) as Issue; + }); + + describe('when the given label is a simple label', (): void => { + beforeEach((): void => { + label = 'label'; + }); + + it('should return false', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(false); + }); + }); + }); + + describe('when the given issue contains a simple label', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'label' + } + ] + } as Issue; + }); + + describe('when the given label is a simple label', (): void => { + beforeEach((): void => { + label = 'label'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a kebab case label', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'kebab-case-label' + } + ] + } as Issue; + }); + + describe('when the given label is a kebab case label', (): void => { + beforeEach((): void => { + label = 'kebab-case-label'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a multiple word label', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'label like a sentence' + } + ] + } as Issue; + }); + + describe('when the given label is a multiple word label', (): void => { + beforeEach((): void => { + label = 'label like a sentence'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a multiple word label with %20 spaces', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'label%20like%20a%20sentence' + } + ] + } as Issue; + }); + + describe('when the given label is a multiple word label with %20 spaces', (): void => { + beforeEach((): void => { + label = 'label%20like%20a%20sentence'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a label wih diacritical marks', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'déjà vu' + } + ] + } as Issue; + }); + + describe('when the given issue contains a label', (): void => { + beforeEach((): void => { + label = 'deja vu'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + + describe('when the given issue contains an uppercase label', (): void => { + beforeEach((): void => { + label = 'DEJA VU'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + + describe('when the given issue contains a label wih diacritical marks', (): void => { + beforeEach((): void => { + label = 'déjà vu'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); +}); diff --git a/src/functions/is-labeled.ts b/src/functions/is-labeled.ts new file mode 100644 index 000000000..97ee5e376 --- /dev/null +++ b/src/functions/is-labeled.ts @@ -0,0 +1,24 @@ +import deburr from 'lodash.deburr'; +import {Issue, Label} from '../IssueProcessor'; + +/** + * @description + * Check if the label is listed as a label of the issue + * + * @param {Readonly} issue A GitHub issue containing some labels + * @param {Readonly} label The label to check the presence with + * + * @return {boolean} Return true when the given label is also in the issue labels + */ +export function isLabeled( + issue: Readonly, + label: Readonly +): boolean { + return !!issue.labels.find((issueLabel: Readonly