Skip to content

Commit

Permalink
NONE Enabling more eslint rules and fixing errors (#2413)
Browse files Browse the repository at this point in the history
* NONE Enabling more eslint rules and fixing errors

* NONE fix unit tests

* NONE fix more unit tests

* NONE fix lint errors from merge

* Clean up jest expect rule

* NONE refactor eslint file

* Fix lint errors from merge

* Addressing initial feedback

* Addressing feedback

* More careful with toString

* Address feedback

* Addressing feedback

* minor fixes

* Fix some typing
  • Loading branch information
atraversatlassian authored Sep 15, 2023
1 parent 3167e8c commit 608370c
Show file tree
Hide file tree
Showing 111 changed files with 399 additions and 272 deletions.
100 changes: 60 additions & 40 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@
}
},
"rules": {
"@typescript-eslint/explicit-module-boundary-types": "off",
"@typescript-eslint/no-var-requires": "error",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
"no-unused-vars": "off",
"no-console": "error",
"indent": ["error", "tab", {
"SwitchCase": 1
}],
"@typescript-eslint/indent": ["error", "tab", {
"SwitchCase": 1
}],
"quotes": [
"error",
"double",
Expand All @@ -48,48 +49,25 @@
"allowTemplateLiterals": true
}
],
"indent": ["error", "tab", {
"SwitchCase": 1
}],
"@typescript-eslint/indent": ["error", "tab", {
"SwitchCase": 1
}],
"@import/no-unresolved": "off",
//extra rules
"object-curly-spacing": ["error", "always"],
"semi": ["error", "always"],
"no-trailing-spaces": "error",
"comma-dangle": "error",
"keyword-spacing": "error",
"space-in-parens": ["error", "never"],
"func-style" : ["error", "expression"],
"require-await": "off",
"@typescript-eslint/no-floating-promises": ["error"],
"jest/expect-expect": [
"error",
{
"assertFunctionNames": [
"expect",
"supertest.**.expect",
"*.expect"
]
}
],
// To be removed later
"@typescript-eslint/no-non-null-assertion": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/prefer-nullish-coalescing": "off",
"@typescript-eslint/restrict-template-expressions": "off",
"@typescript-eslint/no-unsafe-assignment": "off",
"@typescript-eslint/no-unsafe-argument": "off",
"@typescript-eslint/no-unsafe-member-access": "off",
"@typescript-eslint/no-unsafe-return": "off",
"@typescript-eslint/no-unsafe-call": "off",
"@typescript-eslint/unbound-method": "off",
"@typescript-eslint/require-await": "off",
"@typescript-eslint/await-thenable": "off",
"@typescript-eslint/restrict-plus-operands": "off",
"@typescript-eslint/no-misused-promises": "off"

"@typescript-eslint/explicit-module-boundary-types": "off",
"@typescript-eslint/no-var-requires": "error",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
"no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
"no-console": "error",
"@typescript-eslint/restrict-template-expressions": ["error", {
"allowBoolean": true,
"allowNullish": true,
"allowNumber": true
}]
},
"overrides": [{
"files": [ "test/**/*.ts", "src/**/*.test.ts" ],
Expand All @@ -100,7 +78,49 @@
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"jest/no-done-callback": "off",
"jest/expect-expect": "off"
"jest/expect-expect": [
"error",
{
"assertFunctionNames": [
"expect",
"supertest.**.expect",
"*.expect",
"expectDisabledButton",
"expectEnabledButton"
]
}
]
}
},
{
"files": [
"test/**",
"src/*.ts",
"src/config/**",
"src/github/**",
"src/jira/**",
"src/middleware/**",
"src/models/**",
"src/rest/**",
"src/routes/**",
"src/services/**",
"src/sqs/**",
"src/sync/**",
"src/transforms/**",
"src/util/**",
"src/worker/**"
],
"rules": {
// To be removed later
"@typescript-eslint/no-unsafe-assignment": "off",
"@typescript-eslint/no-unsafe-argument": "off",
"@typescript-eslint/no-unsafe-member-access": "off",
"@typescript-eslint/no-unsafe-return": "off",
"@typescript-eslint/no-unsafe-call": "off",
"@typescript-eslint/unbound-method": "off",
"@typescript-eslint/require-await": "off",
"@typescript-eslint/await-thenable": "off",
"@typescript-eslint/no-misused-promises": "off"
}
}]
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"@octokit/auth-app": "^2.6.0",
"@octokit/graphql-schema": "^10.73.0",
"@sentry/node": "^6.8.0",
"@types/safe-json-stringify": "^1.1.2",
"atlassian-jwt": "^2.0.0",
"aws-sdk": "^2.1015.0",
"axios": "^0.26.0",
Expand Down
4 changes: 2 additions & 2 deletions src/app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe("app", () => {
it("please review routes and update snapshot when adding or modifying the routes!", async () => {
const app = getFrontendApp();

const appRoutes: Array<any> = [];
const appRoutes: Array<{method: string, path: string, exec: string[]}> = [];
const collectRoutes = (stack, parentPath = "", parentMiddlewares: Array<any> = []) => {
const ROOT_REGEX = parentPath + sanitizeRegexStr("^\\/?(?=\\/|$)");
const pathMiddlewares = {};
Expand Down Expand Up @@ -51,7 +51,7 @@ describe("app", () => {
collectRoutes(app._router.stack);

const allRoutes = appRoutes.map(route =>
":" + route.method.toUpperCase() + " " + route.path + "\n\t" + route.exec.join(",")
`:${route.method.toUpperCase()} ${route.path}\n\t${route.exec.join(",")}`
).join("\n");

expect(allRoutes).toMatchSnapshot();
Expand Down
2 changes: 1 addition & 1 deletion src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const secureHeaders = (app: Express) => {
defaultSrc: ["'self'"],
// Allow <script> tags hosted by ourselves and from atlassian when inserted into an iframe
scriptSrc: ["'self'", process.env.APP_URL, "https://*.atlassian.net", "https://*.jira.com", "https://connect-cdn.atl-paas.net/",
"'unsafe-inline'", "'strict-dynamic'", (_: Request, res: Response): string => `'nonce-${res.locals.nonce}'`],
"'unsafe-inline'", "'strict-dynamic'", (_: Request, res: Response): string => `'nonce-${res.locals.nonce as string}'`],
// Allow XMLHttpRequest/fetch requests
connectSrc: ["'self'", process.env.APP_URL],
// Allow <style> tags hosted by ourselves as well as style="" attributes
Expand Down
2 changes: 1 addition & 1 deletion src/config/jira-test-site-check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe.each([
["https://site-3.some-test.atlassian.net", true],
["https://real-site-1.non-test.atlassian.net", false]
])("Checking whether it is jira test site", (site, shouldBeTest) => {
it(`should successfully check ${site} to be is-jira-test-site?: ${shouldBeTest}`, () => {
it(`should successfully check ${site ?? "undefined"} to be is-jira-test-site?: ${shouldBeTest}`, () => {
expect(isTestJiraHost(site)).toBe(shouldBeTest);
});
});
6 changes: 3 additions & 3 deletions src/config/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const GIT_REF_URL_REGEX = /^(.*)\/git\/ref\/([^/]+)$/;
const isGitRefUrl = (url: string) => url.match(GIT_REF_URL_REGEX);

const removeGitRefFromUrl = (url: string) =>
url.replace(GIT_REF_URL_REGEX, (_, prefix, gitRef) =>
url.replace(GIT_REF_URL_REGEX, (_, prefix: string, gitRef) =>
`${prefix}/git/ref/${createHashWithSharedSecret(decodeURIComponent(gitRef))}`
);

Expand All @@ -62,7 +62,7 @@ const REST_DEVINFO_BRANCH_URL_REGEX = /^\/rest\/devinfo\/([^/]+)\/repository\/([
const isDevInfoBranchUrl = (url: string) => url.match(REST_DEVINFO_BRANCH_URL_REGEX);

const removeBranchFromDevInfoUrl = (url: string) =>
url.replace(REST_DEVINFO_BRANCH_URL_REGEX, (_, version, repoNo, branchaName, updateSequenceId) =>
url.replace(REST_DEVINFO_BRANCH_URL_REGEX, (_, version, repoNo, branchaName, updateSequenceId: string) =>
[
"/rest/devinfo",
version,
Expand Down Expand Up @@ -101,7 +101,7 @@ const removeOwnersAndReposFromUrl = (url: string) =>
"repos",
createHashWithSharedSecret(decodeURIComponent(repos)),
"branches"
].join("/") + suffix
].join("/") + (suffix as string)
);

const censorUrl = (url) => {
Expand Down
2 changes: 1 addition & 1 deletion src/config/metric-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("metrics helper", () => {
{ repoNum: 1000, bucket: "1000+" },
{ repoNum: 10000, bucket: "1000+" }
])("for each repo num", ({ repoNum, bucket }) => {
it(`should show correct bucket ${bucket} for repo num ${repoNum}`, () => {
it(`should show correct bucket ${bucket} for repo num ${repoNum || "undefined"}`, () => {
expect(repoCountToBucket(repoNum)).toEqual(bucket);
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/config/statsd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const elapsedTimeMetrics = (
res.once("finish", () => {
const elapsedTime = elapsedTimeInMs();
const statusCode = `${res.statusCode}`;
const path = (req.baseUrl || "") + (req.route?.path || "/*");
const path = (req.baseUrl || "") + (req.route?.path as string || "/*");
const tags = { path, method, statusCode };
(req.log || logger).debug(`${method} request executed in ${elapsedTime} with status ${statusCode} path ${path}`);

Expand Down
2 changes: 1 addition & 1 deletion src/github/client/app-token-holder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class AppTokenHolder {
if (!currentToken || currentToken.isAboutToExpire()) {
const key = await keyLocator(ghsaId, jiraHost);
if (!key) {
throw new Error(`No private key found for GitHub app ${appId.toString}`);
throw new Error(`No private key found for GitHub app ${appId.toString()}`);
}
currentToken = AppTokenHolder.createAppJwt(key, appId.appId.toString());
this.appTokenCache.set(appId.toString(), currentToken);
Expand Down
2 changes: 1 addition & 1 deletion src/github/client/github-anonymous-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class GitHubAnonymousClient extends GitHubClient {
// In case of invalid or expired refresh token, GitHub API returns status code 200 with res.data object contains error fields,
// so adding check for presence of access token to make sure that new access token has been generated.
if (!res.data?.access_token) {
throw new Error(`Failed to renew access token ${res.data?.error}`);
throw new Error(`Failed to renew access token ${res.data?.error as string}`);
}
return {
accessToken: res.data.access_token,
Expand Down
2 changes: 1 addition & 1 deletion src/github/client/github-client-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export class GithubClientError extends Error {
this.uiErrorCode = "UNKNOWN";

this.cause = { ...cause };
this.stack = this.stack?.split("\n").slice(0, 2).join("\n") + "\n" + cause.stack;
this.stack = (this.stack?.split("\n").slice(0, 2).join("\n") ?? "Missing Stack") + "\n" + (cause.stack ?? "Missing Stack");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/github/client/github-client-interceptors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const setRequestTimeout = async (config: AxiosRequestConfig): Promise<Axi
//TODO Move to util/axios/common-github-webhook-middleware.ts and use with Jira Client
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const sendResponseMetrics = (metricName: string, gitHubProduct: string, jiraHost: string | undefined, response?: any, status?: string | number, extraTags?: Record<string, string | undefined>) => {
status = `${status || response?.status}`;
status = `${status?.toString() || response?.status as string}`;
const requestDurationMs = Number(
Date.now() - (response?.config?.requestStartTime || 0)
);
Expand Down
4 changes: 2 additions & 2 deletions src/github/client/github-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ export class GitHubClient {
);
this.axios.interceptors.response.use(
instrumentRequest(metricHttpRequest.github, this.restApiUrl, jiraHost, {
withApiKey: "" + (!!gitHubConfig.apiKeyConfig),
withApiKey: (!!gitHubConfig.apiKeyConfig).toString(),
...this.metrics
}),
instrumentFailedRequest(metricHttpRequest.github, this.restApiUrl, jiraHost, {
withApiKey: "" + (!!gitHubConfig.apiKeyConfig),
withApiKey: (!!gitHubConfig.apiKeyConfig).toString(),
...this.metrics
})
);
Expand Down
22 changes: 22 additions & 0 deletions src/github/client/github-client.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,28 @@ export type CodeScanningAlertResponseItem = {
most_recent_instance: CodeScanningAlertResponseItemMostRecentInstance;
};

export interface Branch {
associatedPullRequests: {
nodes: {
title: string;
}[];
};
name: string;
target: {
author: {
avatarUrl: string;
email: string;
name: string;
};
authoredDate: string;
changedFiles: number;
oid: string;
message: string;
url: string;
history: any;
};
}

type CodeScanningAlertResponseItemRule = {
name: string;
description: string;
Expand Down
1 change: 1 addition & 0 deletions src/github/client/github-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ export type DeploymentQueryNode = {
cursor: string,
node: {
createdAt: string,
updatedAt?: string,
repository: Repository,
databaseId: string,
commitOid: string,
Expand Down
63 changes: 58 additions & 5 deletions src/github/code-scanning-alert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { mocked } from "jest-mock";
import { emitWebhookProcessedMetrics } from "utils/webhook-utils";
import Logger from "bunyan";
import { booleanFlag } from "config/feature-flags";
import { JiraClient } from "../jira/client/jira-client";

jest.mock("config/feature-flags");
jest.mock("utils/webhook-utils");
jest.mock("../transforms/transform-code-scanning-alert");
Expand Down Expand Up @@ -44,6 +46,57 @@ const getWebhookContext = <T>(): WebhookContext<T> => {
};
};

const createMockJiraClient = () : JiraClient => {
return {
baseURL: JIRA_BASE_URL,
remoteLink: {
submit: jest.fn()
},
security: {
submitVulnerabilities: jest.fn()
},
issues: {
get: jest.fn(),
getAll: jest.fn(),
parse: jest.fn(),
comments: {
list: jest.fn(),
addForIssue: jest.fn(),
updateForIssue: jest.fn(),
deleteForIssue: jest.fn()
},
transitions: {
getForIssue: jest.fn(),
updateForIssue: jest.fn()
},
worklogs: {
addForIssue: jest.fn()
}
},
devinfo: {
branch: {
delete: jest.fn()
},
installation: {
delete: jest.fn()
},
pullRequest: {
delete: jest.fn()
},
repository: {
delete: jest.fn(),
update: jest.fn()
}
},
workflow: {
submit: jest.fn()
},
deployment: {
submit: jest.fn()
}
};
};

describe("Code Scanning Alert Webhook Handler", () => {
beforeEach(() => {
jest.resetAllMocks();
Expand All @@ -52,11 +105,11 @@ describe("Code Scanning Alert Webhook Handler", () => {

const mockJiraRemoteLinkSubmit = jest.fn();
const mockJiraSecuritySubmit = jest.fn();
const jiraClient = {
baseURL: JIRA_BASE_URL,
remoteLink: { submit: mockJiraRemoteLinkSubmit },
security: { submitVulnerabilities: mockJiraSecuritySubmit }
};
const jiraClient = createMockJiraClient();

jiraClient.baseURL = JIRA_BASE_URL;
jiraClient.remoteLink.submit = mockJiraRemoteLinkSubmit;
jiraClient.security.submitVulnerabilities = mockJiraSecuritySubmit;

it("transform and submit remote link to jira", async () => {
const jiraPayload = { remoteLinks: [] };
Expand Down
Loading

0 comments on commit 608370c

Please sign in to comment.