From af939f47b9424ab8c1430f34b94f431156bf03f8 Mon Sep 17 00:00:00 2001
From: Frankly George <54015-franklygeorge@users.noreply.gitgud.io>
Date: Wed, 13 Mar 2024 21:57:41 +0000
Subject: [PATCH] Type checking intergration with sanity checker
---
devTools/scripts/sanityCheck.js | 65 +++++++++++++++++++++-------
devTools/scripts/setup.js | 49 ++++++++++++++++-----
devTools/scripts/typescriptChecks.js | 19 +++++++-
3 files changed, 103 insertions(+), 30 deletions(-)
diff --git a/devTools/scripts/sanityCheck.js b/devTools/scripts/sanityCheck.js
index 9041e505ca0..60a35b859fe 100644
--- a/devTools/scripts/sanityCheck.js
+++ b/devTools/scripts/sanityCheck.js
@@ -5,9 +5,10 @@ import yargs from "yargs";
import {hideBin} from "yargs/helpers";
import {execSync} from 'child_process';
-import extraChecks from "./extraChecks.js";
+import customChecks from "./extraChecks.js";
import spellingChecks from "./spellingChecks.js";
import eslintChecks from "./eslintChecks.js";
+import typescriptChecks from "./typescriptChecks.js";
import parser from './detectChanges.js';
// TODO:@franklygeorge flags to selectively run parts of the sanity check. After you add those change the commands in package.json to use them
@@ -34,11 +35,13 @@ const eslint = new ESLint({
});
/** @type {Array<string>} */
-let sanityProblems = [];
+let customProblems = [];
/** @type {Array<string>} */
let spellingProblems = [];
/** @type {Array<object>} */
let eslintProblems = [];
+/** @type {Array<string>} */
+let typescriptProblems = [];
let stagedFiles = undefined;
@@ -54,7 +57,7 @@ if (args.staged === true) {
}
if (settings.checksEnableExtras === true) {
- sanityProblems = extraChecks(settings.checksOnlyChangedExtras, stagedFiles, parser);
+ customProblems = customChecks(settings.checksOnlyChangedExtras, stagedFiles, parser);
}
if (settings.checksEnableSpelling === true) {
spellingProblems = spellingChecks(settings.checksOnlyChangedSpelling, stagedFiles, parser);
@@ -63,10 +66,14 @@ if (settings.checksEnableESLint === true) {
// @ts-ignore
eslintProblems = await eslintChecks(settings.checksOnlyChangedESLint, stagedFiles, parser);
}
+if (settings.checksEnableTypescript === true) {
+ // @ts-ignore
+ typescriptProblems = typescriptChecks(settings.checksOnlyChangedTypescript, stagedFiles, parser);
+}
-if (sanityProblems.length > 0) {
+if (customProblems.length > 0) {
console.log("");
- console.log(sanityProblems.join("\n"));
+ console.log(customProblems.join("\n"));
}
if (spellingProblems.length > 0) {
console.log("");
@@ -79,36 +86,62 @@ if (eslintProblems.length > 0) {
const reportText = formatter.format(eslintProblems);
console.log(reportText);
}
+if (typescriptProblems.length > 0) {
+ console.log("");
+ console.log(typescriptProblems.join("\n"));
+}
console.log("=".repeat(60));
+let skippedChecks = 0;
+
// TODO:@franklygeorge color the messages below
-if (sanityProblems.length > 0) {
- console.log(`Sanity checks found ${sanityProblems.length} issues.`);
+if (customProblems.length > 0) {
+ console.log(`Custom sanity checks found ${customProblems.length} issues.`);
+} else if (settings.checksEnableExtras === true) {
+ console.log("Custom sanity checks found no issues.");
} else {
- console.log("Sanity checks found no issues.");
+ console.log("Custom sanity checks are currently disabled.");
+ skippedChecks += 1;
}
+
if (spellingProblems.length > 0) {
- console.log(`cSpell found ${spellingProblems.length} issues.`);
+ console.log(`cSpell found ${spellingProblems.length} spelling issues.`);
+} else if (settings.checksEnableSpelling === true) {
+ console.log("cSpell found no spelling issues.");
} else {
- console.log("cSpell found no issues.");
+ console.log("Spelling checks using cSpell are currently disabled.");
+ skippedChecks += 1;
}
+
if (eslintProblems.length > 0) {
- console.log(`ESLint found ${eslintProblems.length} issues.`);
+ console.log(`ESLint found ${eslintProblems.length} linting issues.`);
+} else if (settings.checksEnableESLint === true) {
+ console.log(`ESLint found no linting issues.`);
+} else {
+ console.log("Linting using ESLint is currently disabled.");
+ skippedChecks += 1;
+}
+
+if (typescriptProblems.length > 0) {
+ console.log(`The TypeScript compiler found ${typescriptProblems.length} type issues.`);
+} else if (settings.checksEnableTypescript === true) {
+ console.log(`The Typescript compiler found no type issues.`);
} else {
- console.log(`ESLint found no issues.`);
+ console.log("Type checking using the Typescript compiler is currently disabled.");
+ skippedChecks += 1;
}
-const issueCount = sanityProblems.length + spellingProblems.length + eslintProblems.length;
+const issueCount = customProblems.length + spellingProblems.length + eslintProblems.length + typescriptProblems.length;
console.log("");
-console.log(`${issueCount} issues found.`);
+console.log(`${issueCount} issues found.${(skippedChecks > 0) ? ` ${skippedChecks} sanity checker(s) skipped.`: ""}`);
console.log("=".repeat(60));
-// exclude eslint problems from making git pre-commit hook fail
+// exclude eslint and typescript problems from making git pre-commit hook fail
// we may change this in the future
-if ((issueCount - eslintProblems.length) > 0) {
+if ((issueCount - (eslintProblems.length + typescriptProblems.length)) > 0) {
if (args.staged === true) {
console.log(`You can temporarily disable the pre-commit hook by changing 'Edit Sanity Check Settings' -> 'Running sanity checks before commiting' in "setup.${process.platform === "win32" ? "bat": "sh"}" to 'Sanity checks are temporarily disabled...'`);
}
diff --git a/devTools/scripts/setup.js b/devTools/scripts/setup.js
index 43f5db89a76..b436cd7e13b 100644
--- a/devTools/scripts/setup.js
+++ b/devTools/scripts/setup.js
@@ -41,11 +41,14 @@ const args = yargs(hideBin(process.argv))
* @property {boolean} checksOnlyChangedSpelling If true then we will only check changed lines
* @property {boolean} checksEnableESLint If true then we will run ESLint checks
* @property {boolean} checksOnlyChangedESLint If true then we will only check changed lines
+ * @property {boolean} checksEnableTypescript If true then we will run Typescript checks
+ * @property {boolean} checksOnlyChangedTypescript If true then we will only check changed lines
* @property {-1|0|1} precommitHookEnabled 0 = Disabled, 1 = Enabled, -1 = temporarily disabled
*/
// TODO:@franklygeorge If compilerMinify and compilerSourcemaps are both true, the minified build becomes bigger than a non-minified build. Make these two options exclusive to each other.
// TODO:@franklygeorge Write watcher.[bat,sh,js]
+// TODO:@franklygeorge take all places where extra sanity checks are referenced as `sanity checks` or `extra checks` and reference it as `custom sanity checks`, migrate settings to new variable, and rename the file to customChecks.js
// TODO:@franklygeorge Do we want an extensions.json file for VSCode?
// TODO:@franklygeorge Figure out why setup.[bat,sh] is slow to start (~5 seconds). Probably affects compile.[bat,sh] and sanityCheck.[bat,sh] as well
// TODO:@franklygeorge Search for todo's with @franklygeorge in them and complete them
@@ -71,6 +74,8 @@ const settings = {
checksOnlyChangedSpelling: true,
checksEnableESLint: true,
checksOnlyChangedESLint: true,
+ checksEnableTypescript: false,
+ checksOnlyChangedTypescript: true,
precommitHookEnabled: 1,
};
@@ -132,9 +137,9 @@ async function compilerSettings() {
if (settings.compilerRunSanityChecks === 0) {
choices.push("Not running sanity checks when compiling");
} else if (settings.compilerRunSanityChecks === 1) {
- choices.push("Running sanity checks before compilation");
+ choices.push("Running sanity checks before compiling");
} else {
- choices.push("Running sanity checks after compilation");
+ choices.push("Running sanity checks after compiling");
}
choices.push(settings.compilerFilenameHash
? "Adding the current Git commit hash to the final filename"
@@ -207,8 +212,8 @@ async function compilerSettings() {
settings.compilerAddDebugFiles = !settings.compilerAddDebugFiles;
} else if (
compilerMenuChoice === "Not running sanity checks when compiling" ||
- compilerMenuChoice === "Running sanity checks before compilation" ||
- compilerMenuChoice === "Running sanity checks after compilation"
+ compilerMenuChoice === "Running sanity checks before compiling" ||
+ compilerMenuChoice === "Running sanity checks after compiling"
) {
if (settings.compilerRunSanityChecks === 2) {
settings.compilerRunSanityChecks = 0;
@@ -264,13 +269,13 @@ async function sanityCheckSettings() {
if (settings.compilerRunSanityChecks === 0) {
choices.push("Not running sanity checks when compiling");
} else if (settings.compilerRunSanityChecks === 1) {
- choices.push("Running sanity checks before compilation");
+ choices.push("Running sanity checks before compiling");
} else {
- choices.push("Running sanity checks after compilation");
+ choices.push("Running sanity checks after compiling");
}
choices.push(settings.checksEnableExtras
- ? "Extra sanity checks are enabled"
- : "Extra sanity checks are disabled"
+ ? "Custom sanity checks are enabled"
+ : "Custom sanity checks are disabled"
);
choices.push(settings.checksEnableSpelling
? "Spelling checks are enabled"
@@ -280,6 +285,10 @@ async function sanityCheckSettings() {
? "JavaScript linting is enabled"
: "JavaScript linting is disabled"
);
+ choices.push(settings.checksEnableTypescript
+ ? "JavaScript type checking is enabled"
+ : "JavaScript type checking is disabled"
+ );
if (settings.checksEnableExtras === true) {
choices.push(settings.checksOnlyChangedExtras
? "Sanity checks are only reporting problems on changed lines"
@@ -298,6 +307,12 @@ async function sanityCheckSettings() {
: "JavaScript linting is reporting all problems"
);
}
+ if (settings.checksEnableTypescript === true) {
+ choices.push(settings.checksOnlyChangedTypescript
+ ? "JavaScript type checking is only reporting problems on changed lines"
+ : "JavaScript type checking is reporting all problems"
+ );
+ }
choices.push("Back");
@@ -328,8 +343,8 @@ async function sanityCheckSettings() {
}
} else if (
sanityCheckMenuChoice === "Not running sanity checks when compiling" ||
- sanityCheckMenuChoice === "Running sanity checks before compilation" ||
- sanityCheckMenuChoice === "Running sanity checks after compilation"
+ sanityCheckMenuChoice === "Running sanity checks before compiling" ||
+ sanityCheckMenuChoice === "Running sanity checks after compiling"
) {
if (settings.compilerRunSanityChecks === 2) {
settings.compilerRunSanityChecks = 0;
@@ -337,8 +352,8 @@ async function sanityCheckSettings() {
settings.compilerRunSanityChecks += 1;
}
} else if (
- sanityCheckMenuChoice === "Extra sanity checks are enabled" ||
- sanityCheckMenuChoice === "Extra sanity checks are disabled"
+ sanityCheckMenuChoice === "Custom sanity checks are enabled" ||
+ sanityCheckMenuChoice === "Custom sanity checks are disabled"
) {
settings.checksEnableExtras = !settings.checksEnableExtras;
} else if (
@@ -351,6 +366,11 @@ async function sanityCheckSettings() {
sanityCheckMenuChoice === "JavaScript linting is disabled"
) {
settings.checksEnableESLint = !settings.checksEnableESLint;
+ } else if (
+ sanityCheckMenuChoice === "JavaScript type checking is enabled" ||
+ sanityCheckMenuChoice === "JavaScript type checking is disabled"
+ ) {
+ settings.checksEnableTypescript = !settings.checksEnableTypescript;
} else if (
sanityCheckMenuChoice === "Sanity checks are only reporting problems on changed lines" ||
sanityCheckMenuChoice === "Sanity checks are reporting all problems"
@@ -366,6 +386,11 @@ async function sanityCheckSettings() {
sanityCheckMenuChoice === "JavaScript linting is reporting all problems"
) {
settings.checksOnlyChangedESLint = !settings.checksOnlyChangedESLint;
+ } else if (
+ sanityCheckMenuChoice === "JavaScript type checking is only reporting problems on changed lines" ||
+ sanityCheckMenuChoice === "JavaScript type checking is reporting all problems"
+ ) {
+ settings.checksOnlyChangedTypescript = !settings.checksOnlyChangedTypescript;
} else if (
sanityCheckMenuChoice === "Back"
) {
diff --git a/devTools/scripts/typescriptChecks.js b/devTools/scripts/typescriptChecks.js
index 330850c7a33..95552a205a8 100644
--- a/devTools/scripts/typescriptChecks.js
+++ b/devTools/scripts/typescriptChecks.js
@@ -36,10 +36,18 @@ const args = yargs(hideBin(process.argv))
/**
* Runs the typescript compiler on files
* @param {boolean} changed If true, only check changed files. If false, check all files.
+ * @param {string[]} files If provided we will sent this list of files to the extra checks instead
* @param {detectChanges} parser If provided it will be used instead of the default parser
* @returns {Array<string>}
*/
-function typescriptChecks(changed = false, parser = detectChanges) {
+function typescriptChecks(changed = false, files = undefined, parser = detectChanges) {
+ if (files !== undefined) {
+ // make sure files is an array of strings
+ if (Object.prototype.toString.call(files) !== '[object Array]' || files.every(i => typeof i !== "string")) {
+ throw new Error("files must be an array of strings or undefined");
+ }
+ }
+
/** @type {Array<string>} */
let errors = [];
@@ -83,6 +91,9 @@ function typescriptChecks(changed = false, parser = detectChanges) {
errors = [...new Set(errors)];
if (changed === true) {
+ if (files === undefined) {
+ files = [];
+ }
const changedLines = parser.changedLines();
if (changedLines === null) {
@@ -100,6 +111,10 @@ function typescriptChecks(changed = false, parser = detectChanges) {
if (lineNumber in changedLines[file]) {
return true;
}
+ } else if (file in files) {
+ // I don't think it is actually possible to get here because of how git handles staged and changed files
+ // but better safe then sorry
+ return true;
}
return false;
});
@@ -116,7 +131,7 @@ if (pathToThisFile.includes(pathPassedToNode)) {
const initializedParser = detectChanges;
// @ts-ignore
await initializedParser.init();
- const report = typescriptChecks(args.changed, initializedParser);
+ const report = typescriptChecks(args.changed, undefined, initializedParser);
if (report.length > 0) {
console.log("");
console.log(report.join("\n"));
--
GitLab