feat(eslint-plugin): add rules for detecting potential issues#3616
feat(eslint-plugin): add rules for detecting potential issues#3616
Conversation
There was a problem hiding this comment.
Pull request overview
Adds new ESLint rules to @kintone/eslint-plugin to flag usage of internal/unsupported kintone APIs and internal UI selectors, along with config/documentation updates.
Changes:
- Added two new rules:
no-cybozu-dataandno-kintone-internal-selector(with tests + docs) - Updated flat configs:
recommended(new rules) andmanifestV2(experimental plugin system) - Updated package/README documentation and root package list
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/eslint-plugin/src/rules/no-kintone-internal-selector.ts | Implements rule to detect internal CSS class usage in selectors and string literals |
| packages/eslint-plugin/src/rules/no-cybozu-data.ts | Implements rule to detect cybozu.data access |
| packages/eslint-plugin/src/rules/index.ts | Exposes the two new rules from the plugin |
| packages/eslint-plugin/src/index.ts | Extends flat configs to include the new rules and adjusts typing |
| packages/eslint-plugin/tests/rules/no-kintone-internal-selector.test.ts | Adds test cases for internal selector detection |
| packages/eslint-plugin/tests/rules/no-cybozu-data.test.ts | Adds test cases for cybozu.data access detection |
| packages/eslint-plugin/docs/rules/no-kintone-internal-selector.md | Documents the selector rule with examples |
| packages/eslint-plugin/docs/rules/no-cybozu-data.md | Documents the cybozu.data rule with examples |
| packages/eslint-plugin/docs/experimental-new-plugin-system.md | Adds guidance for experimental manifest v2 config |
| packages/eslint-plugin/README.md | Updates plugin description and rule list; links experimental doc |
| packages/eslint-plugin/package.json | Updates package description |
| README.md | Adds eslint-plugin to monorepo package list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Matches: | ||
| // cybozu.data | ||
| // cybozu["data"] | ||
| // cybozu?.data | ||
| // cybozu?.["data"] | ||
| 'MemberExpression[object.type="Identifier"][object.name="cybozu"][property.name="data"]': | ||
| (node: TSESTree.Node) => { | ||
| context.report({ | ||
| node, | ||
| messageId: "forbiddenCybozuDataAccess", | ||
| }); | ||
| }, |
There was a problem hiding this comment.
The AST selector only matches non-computed cybozu.data member expressions. It will not match cybozu[\"data\"] (where property is a Literal) and typically won’t match optional chaining forms (cybozu?.data, cybozu?.[\"data\"]) because they are wrapped in a ChainExpression. Update the selectors to also cover computed property access and optional chaining so the implementation matches the documented intent.
| valid: [`const foo = 123;`, `const foo = foo.bar;`], | ||
| invalid: [ | ||
| { | ||
| code: `const foo = cybozu.data`, | ||
| errors: [{ messageId: "forbiddenCybozuDataAccess" }], | ||
| }, | ||
| { | ||
| code: `const foo = cybozu.data.abc`, | ||
| errors: [{ messageId: "forbiddenCybozuDataAccess" }], | ||
| }, | ||
| { | ||
| code: `const foo = cybozu.data['abc']`, | ||
| errors: [{ messageId: "forbiddenCybozuDataAccess" }], | ||
| }, |
There was a problem hiding this comment.
The rule docs claim it covers cybozu[\"data\"] and optional chaining (cybozu?.data, cybozu?.[\"data\"]), but the tests don’t include these patterns. Add explicit invalid cases for those forms to prevent regressions and to validate the intended coverage.
| /[A-Za-z0-9_-]*-gaia/, | ||
| /ocean-[A-Za-z0-9_-]*/, | ||
| /kintone-[A-Za-z0-9_-]*/, |
There was a problem hiding this comment.
The *-gaia pattern (/[A-Za-z0-9_-]*-gaia/) matches foo-gaia as a substring inside longer class names like foo-gaia-bar, even though the docs describe the forbidden form as a suffix (*-gaia). Consider tightening the regex to require a boundary after -gaia (e.g., negative lookahead for [A-Za-z0-9_-]) so foo-gaia-bar doesn’t get flagged as an unintended false positive.
| /[A-Za-z0-9_-]*-gaia/, | |
| /ocean-[A-Za-z0-9_-]*/, | |
| /kintone-[A-Za-z0-9_-]*/, | |
| /[A-Za-z0-9_-]*-gaia(?![A-Za-z0-9_-])/, | |
| /ocean-[A-Za-z0-9_-]*/, | |
| /kintone-[A-Za-z09_-]*/, |
| ruleTester.run("no-kintone-internal-selector", rule, { | ||
| valid: [ | ||
| `element.querySelector('.foo')`, | ||
| `element.querySelectorAll('.foo')`, | ||
| `querySelector('.foo')`, | ||
| `querySelectorAll('.foo')`, | ||
| `$('.foo')`, | ||
| `$(document).on('click', '.foo', handler)`, | ||
| ], |
There was a problem hiding this comment.
The rule implementation also targets matches and closest, but the tests don’t include coverage for those call sites (valid/invalid). Adding a couple of cases (e.g., element.closest('.gaia-argoui-foo'), element.matches('.ocean-foo')) would better validate the supported API surface.
| @@ -0,0 +1,102 @@ | |||
| // import type { CallExpression, Literal } from "estree"; | |||
There was a problem hiding this comment.
There’s a commented-out import at the top that looks like leftover scaffolding, and the inline NOTE is in Japanese while the rest of the rule/docs are in English. Removing the unused commented import and translating the NOTE to English would improve consistency and long-term maintainability.
| callExp: TSESTree.CallExpression, | ||
| ) => reportForbiddenSelectorCall(context, callExp, checked), | ||
|
|
||
| // NOTE: jQuery 等を含む「怪しい文字列リテラル」検知 |
There was a problem hiding this comment.
There’s a commented-out import at the top that looks like leftover scaffolding, and the inline NOTE is in Japanese while the rest of the rule/docs are in English. Removing the unused commented import and translating the NOTE to English would improve consistency and long-term maintainability.
| "no-cybozu-data": noCybozuData, | ||
| "no-kintone-internal-selector": noKintoneInternalSelector, |
There was a problem hiding this comment.
ルールの作り方はこちら
https://eslint.org/docs/latest/extend/custom-rules
ですが、型の支援が効く @typescript-eslint/utils を使って作成してます。
https://typescript-eslint.io/developers/custom-rules/
There was a problem hiding this comment.
Ruleを作るに当たって大きく3つのファイルが必要です。
- ルールの実装
- ルールのドキュメント(.md)
- ルールのテスト
| @@ -0,0 +1,30 @@ | |||
| # Disallow access to the kintone internal API (`no-cybozu-data`) | |||
|
|
|||
There was a problem hiding this comment.
| /** | ||
| * Disallow cybozu.data access | ||
| */ | ||
| export const rule = createRule<Options, MessageIds>({ |
There was a problem hiding this comment.
createRuleというヘルパーを使って作成してます。
(ドキュメント周りのリンクなどをルールに追加したりするのを自動でやってくれるヘルパーです。)
| */ | ||
| export const rule = createRule<Options, MessageIds>({ | ||
| name: "no-cybozu-data", | ||
| meta: { |
There was a problem hiding this comment.
ルールの詳細はESLintおよびtypescript-eslintのドキュメントを見てください
| schema: [], | ||
| }, | ||
| defaultOptions: [], | ||
| create: (context) => ({ |
There was a problem hiding this comment.
このあたりが実装です。
estreeというASTを使っています。
https://sosukesuzuki.dev/advent/2022/06/
| }, | ||
| messages: { | ||
| forbiddenClassname: | ||
| "Using internal kintone UI class name `{{className}}` is not allowed.", |
There was a problem hiding this comment.
context.reportの際に任意のデータをメッセージに含めることができて、今回の場合{{className}}がデータです。
| context.report({ | ||
| node: literal, | ||
| messageId: "suspiciousClassnameLiteral", | ||
| data: { className: match[0] }, |
There was a problem hiding this comment.
メッセージに含めるデータはここで指定しています。
|
|
||
| const configs = { | ||
| recommended: { | ||
| files: ["**/*.{js,cjs,mjs,ts,cts,mts,jsx,tsx}"], |
There was a problem hiding this comment.
ここはプリセットの設定です。
よくeslintのプラグインをインストールしたら、.recommendedみたいな設定をeslint.config.jsで読み込むと思いますがそれです。
もともとはonly-allowed-apiというmanifest_version: 2のときのためのルールが入っていたんですけど、それをmanifestV2というプリセットに待避して、recommendedには今回作成したDOMの非推奨操作に関するルールを入れています。
プリセットは今後ルールが増えてきたら適切な粒度で増やしてあげる良さそうです。
| import { RuleTester } from "@typescript-eslint/rule-tester"; | ||
| import { rule } from "../../src/rules/no-cybozu-data.js"; | ||
|
|
||
| const ruleTester = new RuleTester(); |
There was a problem hiding this comment.
ここで作ったルールのテストをしています。
詳細は https://typescript-eslint.io/packages/rule-tester/ を参照。
| | Name | Description | | ||
| | ---------------------------------------------------------- | ------------------------------------------------------------------------ | | ||
| | [`only-allowed-js-api`](docs/rules/only-allowed-js-api.md) | Only allow the kintone JS APIs listed in permissions.js_api in manifest. | | ||
| See [docs/experimental-new-plugin-system.md](docs/experimental-new-plugin-system.md) for rules supporting plugins with `manifest_version` set to `2`. |
There was a problem hiding this comment.
manifest_version: 2向けのルールは、当面実環境で利用できないので、別ページに移動しました。
| // cybozu["data"] | ||
| // cybozu?.data | ||
| // cybozu?.["data"] | ||
| 'MemberExpression[object.type="Identifier"][object.name="cybozu"][property.name="data"]': |
There was a problem hiding this comment.
#3616 (comment)
copilot の指摘に加えて、window.cybozu.dataも検出できなそう
There was a problem hiding this comment.
(どこまで検出するかですが追加で、)一度cybozuを別の変数に格納されると無理そう
| ) => reportForbiddenSelectorCall(context, callExp, checked), | ||
|
|
||
| // NOTE: jQuery 等を含む「怪しい文字列リテラル」検知 | ||
| Literal: (literal: TSESTree.Literal) => { |
There was a problem hiding this comment.
Literalしか見てないので`.gaia-argoui-foo`みたいなテンプレートリテラルの場合検出できなそう
| languageOptions: { | ||
| parserOptions: { | ||
| projectService: true, | ||
| tsconfigRootDir: dirname(fileURLToPath(import.meta.url)), | ||
| }, | ||
| }, |
There was a problem hiding this comment.
requiresTypeChecking: falseしかないので不要そう?
| languageOptions: { | |
| parserOptions: { | |
| projectService: true, | |
| tsconfigRootDir: dirname(fileURLToPath(import.meta.url)), | |
| }, | |
| }, |
| checked: WeakSet<TSESTree.Literal>, | ||
| ) => { | ||
| for (const arg of node.arguments) { | ||
| if (arg.type === "Literal" && typeof arg.value === "string") { |
There was a problem hiding this comment.
| messageId: "forbiddenClassname", | ||
|
|
||
| data: { className: `gaia-argoui-foo` }, |
There was a problem hiding this comment.
| messageId: "forbiddenClassname", | |
| data: { className: `gaia-argoui-foo` }, | |
| messageId: "forbiddenClassname", | |
| data: { className: `gaia-argoui-foo` }, |
Why
Add new ESLint rules to
@kintone/eslint-pluginto detect usage of internal APIs and CSS classes that should not be used in kintone customization and plugin development.These rules are transferred from kintone/cli-kintone#1228 .
What
New Rules
no-cybozu-data: Prevents access tocybozu.data, an internal and unsupported API that may change without noticeno-kintone-internal-selector: Prevents the use of internal kintone UI class names (gaia-argoui-*,*-gaia,ocean-*,kintone-*)Config Restructuring
recommended: Includes the two new rules (works without manifest file)manifestV2: Includesonly-allowed-js-apirule (for plugins with manifest_version v2, experimental)Documentation
docs/experimental-new-plugin-system.mdHow to test
Checklist
pnpm lintandpnpm teston the root directory.