L✔ ◾ Enhancing linting (#537)

## Summary This change adds additional linting rules, to create a more prescriptive and less ad hoc design. This is designed to help facilitate external contributions to maintain the existing style. These changes have been automatically or manually applied but are typically fairly mechanical. Therefore, there should be little risk of regression. ## Detailed Summary This includes several changes to improve code quality, update dependencies, and enhance error handling. The most important changes include updating the ESLint configuration, modifying the `package.json` file, and refactoring the `GitInvoker` class. ### ESLint Configuration Updates: * Changed ESLint configuration to use recommended and strict type-checked settings, and replaced single quotes with double quotes for consistency. (`eslint.config.mjs`, [eslint.config.mjsL6-R354](diffhunk://#diff-9601a8f6c734c2001be34a2361f76946d19a39a709b5e8c624a2a5a0aade05f2L6-R354)) * Updated rules to enforce stricter linting, including rules such as `@typescript-eslint/explicit-function-return-type` and `@typescript-eslint/no-unused-expressions`. (`eslint.config.mjs`, [eslint.config.mjsL6-R354](diffhunk://#diff-9601a8f6c734c2001be34a2361f76946d19a39a709b5e8c624a2a5a0aade05f2L6-R354)) ### `package.json` Modifications: * Updated the lint script to target TypeScript files. (`package.json`, [package.jsonL17-R17](diffhunk://#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L17-R17)) * Added the `http-status-codes` dependency. (`package.json`, [package.jsonR55](diffhunk://#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R55)) ### Code Refactoring: * Improved error handling in `index.ts` by adding a catch block to exit with a failure code. (`src/task/index.ts`, [src/task/index.tsR9-R19](diffhunk://#diff-445f33c5199a2e71fde9062a5f8c6b5237e3230e83e8be8fcfbf06de48abd5abR9-R19)) * Refactored `GitInvoker` class to remove redundant methods and use more concise syntax. (`src/task/src/git/gitInvoker.ts`, [[1]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L7-R10) [[2]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L22-R25) [[3]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L37-L80) [[4]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L93-L124) [[5]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L142-R101) [[6]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L182-R113) [[7]](diffhunk://#diff-393c3008fe54c65d56363117b91baeb2bfacc051055e6fbffc71df7ea919cc28L206-R137) ## Testing ### Test Types - [X] Unit tests - [X] Manual tests ### Unit Test Coverage 100% --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
microsoft · Sep 6, 2024 · 7bfc023 · 7bfc023
1 parent 4bd567b
commit 7bfc023
Show file tree

Hide file tree

Showing 71 changed files with 3,110 additions and 2,597 deletions.
diff --git a/dist/index.js b/dist/index.js
diff --git a/eslint.config.mjs b/eslint.config.mjs
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -14,7 +14,7 @@
     "build": "npm run build:release && npm run build:package && npm run build:docs",
     "clean": "rimraf debug && rimraf release",
     "deploy": "npm run build:release && exitzero tfx build tasks delete --task-id 907d3b28-6b37-4ac7-ac75-9631ee53e512 --no-prompt && tfx build tasks upload --task-path release/task --no-prompt",
-    "lint": "eslint --fix",
+    "lint": "eslint --fix **/*.ts",
     "test": "npm run build:debug && cd debug/task && c8 --reporter=text --reporter=text-summary mocha tests/**/*.spec.js",
     "test:fast": "cross-env-shell \"mkdirp debug && ncp src debug\" && cd debug/task && tsc --sourceMap && c8 --reporter=text --reporter=text-summary mocha tests/**/*.spec.js",
     "update:package": "npm update"
@@ -52,6 +52,7 @@
     "axios": "1.7.4",
     "azure-devops-node-api": "14.0.2",
     "azure-pipelines-task-lib": "4.16.0",
+    "http-status-codes": "2.3.0",
     "isomorphic-fetch": "3.0.0",
     "octokit": "3.2.1",
     "parse-git-diff": "0.0.15",

diff --git a/src/task/index.ts b/src/task/index.ts
@@ -6,11 +6,14 @@
 import "reflect-metadata";
 import PullRequestMetrics from "./src/pullRequestMetrics";
 import { container } from "tsyringe";
+import { exitCodeForFailure } from "./src/utilities/constants";
 
 const run = async (): Promise<void> => {
   const pullRequestMetrics: PullRequestMetrics =
     container.resolve(PullRequestMetrics);
   await pullRequestMetrics.run(__dirname);
 };
 
-run().finally((): void => {});
+run().catch((): void => {
+  process.exit(exitCodeForFailure);
+});
diff --git a/src/task/src/git/gitInvoker.ts b/src/task/src/git/gitInvoker.ts
@@ -4,10 +4,10 @@
  */
 
 import * as Validator from "../utilities/validator";
-import { DecimalRadix } from "../utilities/constants";
 import ExecOutput from "../runners/execOutput";
 import Logger from "../utilities/logger";
 import RunnerInvoker from "../runners/runnerInvoker";
+import { decimalRadix } from "../utilities/constants";
 import { singleton } from "tsyringe";
 
 /**
@@ -19,10 +19,10 @@ export default class GitInvoker {
   private readonly _logger: Logger;
   private readonly _runnerInvoker: RunnerInvoker;
 
-  private _isInitialized: boolean = false;
-  private _targetBranch: string = "";
-  private _pullRequestId: number = 0;
-  private _pullRequestIdInternal: string = "";
+  private _isInitialized = false;
+  private _targetBranch = "";
+  private _pullRequestId = 0;
+  private _pullRequestIdInternal = "";
 
   /**
    * Initializes a new instance of the `GitInvoker` class.
@@ -34,50 +34,6 @@ export default class GitInvoker {
     this._runnerInvoker = runnerInvoker;
   }
 
-  /**
-   * Gets a value indicating whether the current folder corresponds to a Git repo.
-   * @returns A promise containing a value indicating whether the current folder corresponds to a Git repo.
-   */
-  public async isGitRepo(): Promise<boolean> {
-    this._logger.logDebug("* GitInvoker.isGitRepo()");
-
-    try {
-      await this.invokeGit("rev-parse --is-inside-work-tree");
-      return true;
-    } catch {
-      return false;
-    }
-  }
-
-  /**
-   * Gets a value indicating whether the pull request ID is available.
-   * @returns A value indicating whether the pull request ID is available.
-   */
-  public isPullRequestIdAvailable(): boolean {
-    this._logger.logDebug("* GitInvoker.isPullRequestIdAvailable()");
-
-    return !isNaN(parseInt(this.pullRequestIdInternal, DecimalRadix));
-  }
-
-  /**
-   * Gets a value indicating whether sufficient Git history is available to generate the PR metrics.
-   * @returns A promise containing a value indicating whether sufficient Git history is available to generate the PR metrics.
-   */
-  public async isGitHistoryAvailable(): Promise<boolean> {
-    this._logger.logDebug("* GitInvoker.isGitHistoryAvailable()");
-
-    this.initialize();
-
-    try {
-      await this.invokeGit(
-        `rev-parse --branch origin/${this._targetBranch}...pull/${this._pullRequestIdInternal}/merge`,
-      );
-      return true;
-    } catch {
-      return false;
-    }
-  }
-
   /**
    * Gets the ID of the pull request.
    * @returns The ID of the pull request.
@@ -90,38 +46,13 @@ export default class GitInvoker {
     }
 
     this._pullRequestId = Validator.validateNumber(
-      parseInt(this.pullRequestIdInternal, DecimalRadix),
+      parseInt(this.pullRequestIdInternal, decimalRadix),
       "Pull Request ID",
       "GitInvoker.pullRequestId",
     );
     return this._pullRequestId;
   }
 
-  /**
-   * Gets a diff summary related to the changes in the current branch.
-   * @returns A promise containing the diff summary.
-   */
-  public async getDiffSummary(): Promise<string> {
-    this._logger.logDebug("* GitInvoker.getDiffSummary()");
-
-    this.initialize();
-    return this.invokeGit(
-      `diff --numstat --ignore-all-space origin/${this._targetBranch}...pull/${this._pullRequestIdInternal}/merge`,
-    );
-  }
-
-  private initialize(): void {
-    this._logger.logDebug("* GitInvoker.initialize()");
-
-    if (this._isInitialized) {
-      return;
-    }
-
-    this._targetBranch = this.targetBranch;
-    this._pullRequestIdInternal = this.pullRequestIdInternal;
-    this._isInitialized = true;
-  }
-
   private get pullRequestIdInternal(): string {
     this._logger.logDebug("* GitInvoker.pullRequestIdInternal");
 
@@ -139,35 +70,35 @@ export default class GitInvoker {
     this._logger.logDebug("* GitInvoker.pullRequestIdForGitHub");
 
     const gitHubReference: string | undefined = process.env.GITHUB_REF;
-    if (!gitHubReference) {
+    if (typeof gitHubReference === "undefined") {
       this._logger.logWarning("'GITHUB_REF' is undefined.");
       return "";
     }
 
     const gitHubReferenceElements: string[] = gitHubReference.split("/");
-    if (gitHubReferenceElements.length < 3) {
+    if (typeof gitHubReferenceElements[2] === "undefined") {
       this._logger.logWarning(
         `'GITHUB_REF' is in an incorrect format '${gitHubReference}'.`,
       );
       return "";
     }
 
-    return gitHubReferenceElements[2]!;
+    return gitHubReferenceElements[2];
   }
 
   private get pullRequestIdForAzurePipelines(): string {
     this._logger.logDebug("* GitInvoker.pullRequestIdForAzurePipelines");
 
     const variable: string | undefined = process.env.BUILD_REPOSITORY_PROVIDER;
-    if (!variable) {
+    if (typeof variable === "undefined") {
       this._logger.logWarning("'BUILD_REPOSITORY_PROVIDER' is undefined.");
       return "";
     }
 
     if (variable === "GitHub" || variable === "GitHubEnterprise") {
       const result: string | undefined =
         process.env.SYSTEM_PULLREQUEST_PULLREQUESTNUMBER;
-      if (!result) {
+      if (typeof result === "undefined") {
         this._logger.logWarning(
           "'SYSTEM_PULLREQUEST_PULLREQUESTNUMBER' is undefined.",
         );
@@ -179,7 +110,7 @@ export default class GitInvoker {
 
     const result: string | undefined =
       process.env.SYSTEM_PULLREQUEST_PULLREQUESTID;
-    if (!result) {
+    if (typeof result === "undefined") {
       this._logger.logWarning(
         "'SYSTEM_PULLREQUEST_PULLREQUESTID' is undefined.",
       );
@@ -203,7 +134,7 @@ export default class GitInvoker {
       "SYSTEM_PULLREQUEST_TARGETBRANCH",
       "GitInvoker.targetBranch",
     );
-    const expectedStart: string = "refs/heads/";
+    const expectedStart = "refs/heads/";
     if (variable.startsWith(expectedStart)) {
       const startIndex: number = expectedStart.length;
       return variable.substring(startIndex);
@@ -212,6 +143,75 @@ export default class GitInvoker {
     return variable;
   }
 
+  /**
+   * Gets a value indicating whether the current folder corresponds to a Git repo.
+   * @returns A promise containing a value indicating whether the current folder corresponds to a Git repo.
+   */
+  public async isGitRepo(): Promise<boolean> {
+    this._logger.logDebug("* GitInvoker.isGitRepo()");
+
+    try {
+      await this.invokeGit("rev-parse --is-inside-work-tree");
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  /**
+   * Gets a value indicating whether the pull request ID is available.
+   * @returns A value indicating whether the pull request ID is available.
+   */
+  public isPullRequestIdAvailable(): boolean {
+    this._logger.logDebug("* GitInvoker.isPullRequestIdAvailable()");
+
+    return !isNaN(parseInt(this.pullRequestIdInternal, decimalRadix));
+  }
+
+  /**
+   * Gets a value indicating whether sufficient Git history is available to generate the PR metrics.
+   * @returns A promise containing a value indicating whether sufficient Git history is available to generate the PR metrics.
+   */
+  public async isGitHistoryAvailable(): Promise<boolean> {
+    this._logger.logDebug("* GitInvoker.isGitHistoryAvailable()");
+
+    this.initialize();
+
+    try {
+      await this.invokeGit(
+        `rev-parse --branch origin/${this._targetBranch}...pull/${this._pullRequestIdInternal}/merge`,
+      );
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  /**
+   * Gets a diff summary related to the changes in the current branch.
+   * @returns A promise containing the diff summary.
+   */
+  public async getDiffSummary(): Promise<string> {
+    this._logger.logDebug("* GitInvoker.getDiffSummary()");
+
+    this.initialize();
+    return this.invokeGit(
+      `diff --numstat --ignore-all-space origin/${this._targetBranch}...pull/${this._pullRequestIdInternal}/merge`,
+    );
+  }
+
+  private initialize(): void {
+    this._logger.logDebug("* GitInvoker.initialize()");
+
+    if (this._isInitialized) {
+      return;
+    }
+
+    this._targetBranch = this.targetBranch;
+    this._pullRequestIdInternal = this.pullRequestIdInternal;
+    this._isInitialized = true;
+  }
+
   private async invokeGit(parameters: string): Promise<string> {
     this._logger.logDebug("* GitInvoker.invokeGit()");