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

feat: インポート一覧をチェックするように #2513

Merged
merged 10 commits into from
Jan 31, 2025
7 changes: 5 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

"license:generate": "tsx tools/generateLicenses.mts",
"license:merge": "tsx tools/mergeLicenses.mts",
"check-suspicious-imports": "tsx tools/checkSuspiciousImports.mts",

"test:unit": "vitest --run",
"test-watch:unit": "vitest --watch",
Expand All @@ -33,10 +34,10 @@
"test-watch:storybook-vrt": "cross-env TARGET=storybook PWTEST_WATCH=1 playwright test",
"test-ui:storybook-vrt": "cross-env TARGET=storybook playwright test --ui",

"electron:build": "cross-env VITE_TARGET=electron vite build && electron-builder --config electron-builder.config.js --publish never",
"electron:serve": "cross-env VITE_TARGET=electron vite",
"electron:build": "cross-env VITE_TARGET=electron NODE_ENV=production vite build && electron-builder --config electron-builder.config.js --publish never",
"browser:serve": "cross-env VITE_TARGET=browser vite",
"browser:build": "cross-env VITE_TARGET=browser vite build",
"browser:build": "cross-env VITE_TARGET=browser NODE_ENV=production vite build",
"storybook": "storybook dev --port 6006",
"storybook:build": "storybook build",

Expand Down Expand Up @@ -114,6 +115,8 @@
"@vue/eslint-config-prettier": "9.0.0",
"@vue/eslint-config-typescript": "13.0.0",
"@vue/test-utils": "2.4.5",
"acorn": "8.14.0",
"acorn-walk": "8.3.4",
"chromatic": "11.5.4",
"cross-env": "7.0.3",
"dotenv": "16.0.0",
Expand Down
14 changes: 14 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

209 changes: 209 additions & 0 deletions tools/checkSuspiciousImports.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
/**
* 与えられたjsファイルのimport/requireを解析し、以下の条件に当てはまらないものがあれば警告を出力する:
* - Node.js 標準である
* - electron
* - try-catch内での明示的に許可されたパッケージ
*
* また、try-catch内での明示的に許可されたパッケージが使われていない場合も警告を出力する。
*
* pnpm run check-suspicious-imports ./path/to/file.js のように単体でも実行可能。
*
* NOTE:
* electron-builderはpnpmを一緒に使うとnode_modulesが壊れる問題があるが、
* それはビルド後(dist/{main.js,preload.js})でパッケージがimport/requireされなければ問題ないため、
* このスクリプトを使って問題がないかチェックする。
* ref: https://github.com/VOICEVOX/voicevox/issues/2508
*/

import { builtinModules } from "module";
import { styleText } from "util";
sevenc-nanashi marked this conversation as resolved.
Show resolved Hide resolved
import fs from "fs/promises";
import yargs from "yargs";
import { parse } from "acorn";
import { ancestor as visitWithAncestor } from "acorn-walk";
import { hideBin } from "yargs/helpers";

const defaultAllowedModules = [
...builtinModules.map((m) => `node:${m}`),
"electron",
];

type Import = {
path: string;
isInsideTryCatch: boolean;
};
export type CheckSuspiciousImportsOptions = {
allowedModules?: string[];
allowedInTryCatchModules?: string[];
list?: boolean;
};
export function checkSuspiciousImports(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(判断コメント)

このファイルのコードは相当文脈が深く、何やってるか雰囲気はわかるけどぶっちゃけメンテは大変そうな気がしています。
特にacornライブラリを使う周りが専門知識。(どうしようもない)

なのでこのコードのメンテは @sevenc-nanashi さんに頼らせていただけると頼もしいです!!

file: string,
jsContent: string,
options: CheckSuspiciousImportsOptions = {},
): void {
console.log(`Checking suspicious imports in ${file}...`);
const ast = parse(jsContent, {
ecmaVersion: "latest",
sourceType: "module",
});

const allImports: Import[] = [];

const allowedModules = [
...defaultAllowedModules,
...(options.allowedModules ?? []),
];
const allowedInTryCatchModules = options.allowedInTryCatchModules ?? [];

visitWithAncestor(ast, {
ImportDeclaration(node) {
const importPath = node.source.value;
if (typeof importPath === "string") {
allImports.push({
path: importPath,
isInsideTryCatch: false,
});
}
},
CallExpression(node, _, ancestors) {
const isInsideTryCatch = ancestors.some(
(ancestor) => ancestor.type === "TryStatement",
);
if (node.callee.type === "Identifier" && node.callee.name === "require") {
const importPath =
node.arguments[0].type === "Literal" && node.arguments[0].value;
if (typeof importPath === "string") {
allImports.push({
path: importPath,
isInsideTryCatch,
});
}
}
},
});

const normalizedImports: Import[] = [];
for (const importInfo of allImports) {
let path = importInfo.path;
if (builtinModules.includes(path) && !path.startsWith("node:")) {
path = `node:${path}`;
}
const existingImport = normalizedImports.find((i) => i.path === path);
if (existingImport) {
existingImport.isInsideTryCatch &&= importInfo.isInsideTryCatch;
} else {
normalizedImports.push({
...importInfo,
path,
});
}
}

const allowedImports = normalizedImports.filter((i) =>
allowedModules.includes(i.path),
);
const allowedInTryCatchImports = normalizedImports.filter(
(i) =>
!allowedModules.includes(i.path) &&
allowedInTryCatchModules.includes(i.path),
);
const suspiciousImports = normalizedImports.filter(
(i) =>
!allowedModules.includes(i.path) &&
!allowedInTryCatchModules.includes(i.path),
);

for (const [color, title, imports] of [
["green", "Allowed", allowedImports],
["yellow", "Allowed in try-catch", allowedInTryCatchImports],
["red", "Suspicious", suspiciousImports],
] as const) {
if (options.list) {
console.log(styleText(color, ` ${title}:`));
for (const i of imports) {
console.log(styleText(color, ` ${i.path}`));
}
if (imports.length === 0) {
console.log(styleText(color, ` (none)`));
}
} else {
console.log(styleText(color, ` ${title}: ${imports.length}`));
}
}

if (suspiciousImports.length > 0) {
throw new Error(
`Suspicious imports found: ${suspiciousImports
.map((i) => i.path)
.join(", ")}`,
);
}

const unusedAllowedInTryCatchModules = allowedInTryCatchModules.filter(
(m) => !allowedInTryCatchImports.some((i) => i.path === m),
);
if (options.list) {
console.log(styleText("yellow", ` Unused allowed in try-catch modules:`));
for (const m of unusedAllowedInTryCatchModules) {
console.log(styleText("yellow", ` ${m}`));
}
if (unusedAllowedInTryCatchModules.length === 0) {
console.log(styleText("yellow", ` (none)`));
}
} else {
console.log(
styleText(
"yellow",
` Unused allowed in try-catch modules: ${unusedAllowedInTryCatchModules.length}`,
),
);
}
if (unusedAllowedInTryCatchModules.length > 0) {
throw new Error(
`Some allowed in try-catch modules are not used in try-catch block: ${unusedAllowedInTryCatchModules.join(
", ",
)}`,
);
}
}

if (import.meta.filename === process.argv[1]) {
const parser = yargs(hideBin(process.argv))
.option("allowed-modules", {
type: "string",
array: true,
description: "Allowed modules",
alias: "a",
})
.option("allowed-in-try-catch-modules", {
type: "string",
array: true,
description: "Allowed in try-catch modules",
alias: "t",
})
.positional("files", {
type: "string",
description: "Files to check",
array: true,
})
.strictCommands()
.demandCommand(1);
const args = await parser.parse();

for (const rawFile of args._) {
const file = rawFile.toString();
fs.readFile(file, "utf-8")
.then((content) => {
checkSuspiciousImports(file, content, {
allowedModules: args.allowedModules,
allowedInTryCatchModules: args.allowedInTryCatchModules,
list: true,
});
})
.catch((e) => {
console.error(e);
process.exit(1);
});
}
}
46 changes: 41 additions & 5 deletions vite.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,14 @@ import { BuildOptions, defineConfig, loadEnv, Plugin } from "vite";
import { quasar } from "@quasar/vite-plugin";
import { z } from "zod";

import {
checkSuspiciousImports,
CheckSuspiciousImportsOptions,
} from "./tools/checkSuspiciousImports.mjs";

const isElectron = process.env.VITE_TARGET === "electron";
const isBrowser = process.env.VITE_TARGET === "browser";
const isProduction = process.env.NODE_ENV === "production";

export default defineConfig((options) => {
const mode = z
Expand Down Expand Up @@ -48,7 +54,7 @@ export default defineConfig((options) => {
: false;

// ref: electronの起動をスキップしてデバッグ起動を軽くする
const skipLahnchElectron =
const skipLaunchElectron =
mode === "test" || process.env.SKIP_LAUNCH_ELECTRON === "1";

return {
Expand Down Expand Up @@ -83,18 +89,28 @@ export default defineConfig((options) => {
}),
isElectron && [
cleanDistPlugin(),
// TODO: 関数で切り出して共通化できる部分はまとめる
electron([
{
entry: "./backend/electron/main.ts",
// ref: https://github.com/electron-vite/vite-plugin-electron/pull/122
onstart: ({ startup }) => {
console.log("main process build is complete.");
if (!skipLahnchElectron) {
if (!skipLaunchElectron) {
void startup([".", "--no-sandbox"]);
}
},
vite: {
plugins: [tsconfigPaths({ root: import.meta.dirname })],
plugins: [
tsconfigPaths({ root: import.meta.dirname }),
isProduction &&
checkSuspiciousImportsPlugin({
allowedInTryCatchModules: [
// systeminformationのoptionalな依存。try-catch内なので許可。
"osx-temperature-sensor",
],
}),
],
build: {
outDir: path.resolve(import.meta.dirname, "dist"),
sourcemap,
Expand All @@ -105,12 +121,15 @@ export default defineConfig((options) => {
// ref: https://electron-vite.github.io/guide/preload-not-split.html
entry: "./backend/electron/preload.ts",
onstart({ reload }) {
if (!skipLahnchElectron) {
if (!skipLaunchElectron) {
reload();
}
},
vite: {
plugins: [tsconfigPaths({ root: import.meta.dirname })],
plugins: [
tsconfigPaths({ root: import.meta.dirname }),
isProduction && checkSuspiciousImportsPlugin({}),
],
build: {
outDir: path.resolve(import.meta.dirname, "dist"),
sourcemap,
Expand Down Expand Up @@ -153,3 +172,20 @@ const injectBrowserPreloadPlugin = (): Plugin => {
},
};
};

const checkSuspiciousImportsPlugin = (
options: CheckSuspiciousImportsOptions,
): Plugin => {
return {
name: "check-suspicious-imports",
enforce: "post",
apply: "build",
writeBundle(_options, bundle) {
for (const [file, chunk] of Object.entries(bundle)) {
if (chunk.type === "chunk") {
checkSuspiciousImports(file, chunk.code, options);
}
}
},
};
};
Loading