feat: support configurable lint import sources#81
Open
QDyanbing wants to merge 2 commits intoant-design:mainfrom
Open
feat: support configurable lint import sources#81QDyanbing wants to merge 2 commits intoant-design:mainfrom
QDyanbing wants to merge 2 commits intoant-design:mainfrom
Conversation
Allow `antd lint` to recognize additional wrapper import roots via `--import-source` while keeping the default antd-only behavior unchanged.
📝 WalkthroughWalkthrough为 lint 命令增加了可配置的导入源（ Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
|
--import-source 是啥意思？解释一下？ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/__tests__/commands/lint.test.ts (2)
25-30: 为console.logmock 恢复补上finally，避免异常路径污染后续用例。Line 27 的
parseAsync一旦抛错，Line 29 的mockRestore()不会执行，后续测试可能被连带影响。可参考的修改
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); const importArgs = importSources.flatMap((source) => ['--import-source', source]); - await program.parseAsync(['node', 'test', 'lint', ...args, ...importArgs]); - const output = logSpy.mock.calls.map((c) => c[0]).join('\n'); - logSpy.mockRestore(); - return output; + try { + await program.parseAsync(['node', 'test', 'lint', ...args, ...importArgs]); + return logSpy.mock.calls.map((c) => c[0]).join('\n'); + } finally { + logSpy.mockRestore(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/commands/lint.test.ts` around lines 25 - 30, The console.log mock created via vi.spyOn(...) and assigned to logSpy should always be restored even if program.parseAsync(...) throws; wrap the await program.parseAsync(['node','test','lint', ...args, ...importArgs]) call in a try/finally and call logSpy.mockRestore() in the finally block so the mock is cleaned up on both success and error (identify logSpy, vi.spyOn(console, 'log'), program.parseAsync, and mockRestore to locate the code to change).
610-666: 新增用例建议做单测级别临时文件清理。当前这些 case 都会写入临时文件，但没有在每个 case 的
finally或afterEach做清理；建议补上，减少用例间状态耦合。As per coding guidelines "Clean up temporary files/directories created in tests using
finallyblocks orafterEachhooks".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/commands/lint.test.ts` around lines 610 - 666, The tests create temp files via makeTmpFile and read them using runLint and join(tmpDir, ...), but they don't clean up the files which can leak state between tests; add cleanup that removes files under tmpDir after each test (or wrap each case's setup in try/finally) — e.g., add an afterEach hook in src/__tests__/commands/lint.test.ts that deletes tmpDir contents (or calls the existing cleanup utility if present) or modify each spec (like the tests named 'detects deprecated props from configured wrapper root import', 'skips wrapper subpath default import in minimal mode', 'skips wrapper imports by default when no import source is configured', and 'supports multiple configured import sources') to remove files in a finally block after makeTmpFile is used so makeTmpFile and tmpDir are always cleaned up.src/commands/lint.ts (1)
124-126: 预检查命中条件偏宽，建议收窄以减少无效 AST 解析。Line 125 仅用
content.includes(importSource)会命中注释/字符串等非导入场景，导致不必要的parseSync开销。建议改为匹配import/from语句形态。可参考的修改
+function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + function mayContainImportSource(content: string, importSources: string[]): boolean { - return importSources.some((importSource) => content.includes(importSource)); + return importSources.some((importSource) => { + const source = escapeRegExp(importSource); + const pattern = new RegExp( + `(?:from\\s+['"]${source}(?:\\/[^'"]*)?['"]|import\\s+['"]${source}(?:\\/[^'"]*)?['"])`, + ); + return pattern.test(content); + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/lint.ts` around lines 124 - 126, The pre-check in mayContainImportSource is too broad because content.includes(...) matches comments/strings; replace the simple substring check with a tighter pattern match that only detects real import contexts (e.g., ES import/export with from, static import with quotes, CommonJS require(...) and dynamic import(...)) by testing content against a regex per importSource (or building a single regex that escapes the importSource and matches patterns like import ... from 'X' | export ... from 'X' | require('X') | import('X')); update mayContainImportSource to use that pattern test to reduce false positives and unnecessary parseSync calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/__tests__/commands/lint.test.ts`:
- Around line 25-30: The console.log mock created via vi.spyOn(...) and assigned
to logSpy should always be restored even if program.parseAsync(...) throws; wrap
the await program.parseAsync(['node','test','lint', ...args, ...importArgs])
call in a try/finally and call logSpy.mockRestore() in the finally block so the
mock is cleaned up on both success and error (identify logSpy, vi.spyOn(console,
'log'), program.parseAsync, and mockRestore to locate the code to change).
- Around line 610-666: The tests create temp files via makeTmpFile and read them
using runLint and join(tmpDir, ...), but they don't clean up the files which can
leak state between tests; add cleanup that removes files under tmpDir after each
test (or wrap each case's setup in try/finally) — e.g., add an afterEach hook in
src/__tests__/commands/lint.test.ts that deletes tmpDir contents (or calls the
existing cleanup utility if present) or modify each spec (like the tests named
'detects deprecated props from configured wrapper root import', 'skips wrapper
subpath default import in minimal mode', 'skips wrapper imports by default when
no import source is configured', and 'supports multiple configured import
sources') to remove files in a finally block after makeTmpFile is used so
makeTmpFile and tmpDir are always cleaned up.
In `@src/commands/lint.ts`:
- Around line 124-126: The pre-check in mayContainImportSource is too broad
because content.includes(...) matches comments/strings; replace the simple
substring check with a tighter pattern match that only detects real import
contexts (e.g., ES import/export with from, static import with quotes, CommonJS
require(...) and dynamic import(...)) by testing content against a regex per
importSource (or building a single regex that escapes the importSource and
matches patterns like import ... from 'X' | export ... from 'X' | require('X') |
import('X')); update mayContainImportSource to use that pattern test to reduce
false positives and unnecessary parseSync calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1599e4bb-0790-42e6-acbd-4662d0fd860c
📒 Files selected for processing (3)
spec.mdsrc/__tests__/commands/lint.test.tssrc/commands/lint.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
=======================================
Coverage 99.62% 99.62%
=======================================
Files 28 28
Lines 1605 1618 +13
Branches 462 466 +4
=======================================
+ Hits 1599 1612 +13
Misses 5 5
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new --import-source flag to the lint command, allowing users to specify additional entry points (such as component wrappers) that should be treated as antd for linting purposes. The changes include documentation updates, new test cases for configurable sources, and logic to normalize and match these sources during the linting process. Feedback was provided to generalize the performance rule against wildcard imports so that it applies to all configured import sources rather than being restricted to the default antd package.
Author
额外指定一组“也按 antd 组件入口来处理”的导入源 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/lint.ts (1)
170-185:⚠️ Potential issue | 🟡 Minor性能规则提示文案会误导自定义 import source 场景
Line 184 的报错文案写死为
antd。当命中--import-source（例如@corp/ui）时，提示与实际 import 来源不一致，排查体验会变差。💡 建议修复
- report('performance', 'error', lineOf(node), - 'Avoid wildcard import from antd. Use named imports: `import { Button } from \'antd\'`'); + report( + 'performance', + 'error', + lineOf(node), + `Avoid default/namespace import from \`${source}\`. Use named imports from the configured source root.`, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/lint.ts` around lines 170 - 185, The performance rule message hardcodes "antd", which misleads when a custom import source (e.g. --import-source like "@corp/ui") is used; update the report call inside the ImportSpecifier loop (the block that calls report('performance', 'error', lineOf(node), ...)) to interpolate or include the actual import source variable (source) in the message so it reads like "Avoid wildcard import from {source}. Use named imports: `import { Button } from '{source}'`", ensuring proper quoting/escaping of source and preserving the existing conditionals (matchesImportSource, only/performance checks) and message formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/lint.ts`:
- Around line 339-340: The new CLI option definition for '--import-source' uses
a single English description; update it to use bilingual fields and the localize
helper: replace the single string description in the .option call with two
fields description and descriptionZh containing the English and Chinese texts
respectively, and pass the displayed text through localize(en, zh, lang) (import
or reference the localize helper from src/types.ts). Ensure the .option
invocation still uses collectImportSource and the .action signature (target,
cmdOpts) remains unchanged so behavior isn't altered.
---
Outside diff comments:
In `@src/commands/lint.ts`:
- Around line 170-185: The performance rule message hardcodes "antd", which
misleads when a custom import source (e.g. --import-source like "@corp/ui") is
used; update the report call inside the ImportSpecifier loop (the block that
calls report('performance', 'error', lineOf(node), ...)) to interpolate or
include the actual import source variable (source) in the message so it reads
like "Avoid wildcard import from {source}. Use named imports: `import { Button }
from '{source}'`", ensuring proper quoting/escaping of source and preserving the
existing conditionals (matchesImportSource, only/performance checks) and message
formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c918f0c-d8fb-4c10-9e26-fb8868d78be5
📒 Files selected for processing (1)
src/commands/lint.ts
Comment on lines
+339
to
+340
| .option('--import-source <source>', 'Treat additional import source roots as antd component entrypoints', collectImportSource, []) | ||
| .action((target: string | undefined, cmdOpts: { only?: string; importSource?: string[] }) => { |
There was a problem hiding this comment.
新增 CLI 选项描述未按双语本地化规范处理
Line 339 新增用户可见文案为英文单语，未使用 localize(en, zh, lang)。建议补齐中英文并按语言选择输出。
As per coding guidelines: src/**/*.ts: Store bilingual data as paired fields (description / descriptionZh) and use localize(en, zh, lang) helper from src/types.ts to pick the appropriate language.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/lint.ts` around lines 339 - 340, The new CLI option definition
for '--import-source' uses a single English description; update it to use
bilingual fields and the localize helper: replace the single string description
in the .option call with two fields description and descriptionZh containing the
English and Chinese texts respectively, and pass the displayed text through
localize(en, zh, lang) (import or reference the localize helper from
src/types.ts). Ensure the .option invocation still uses collectImportSource and
the .action signature (target, cmdOpts) remains unchanged so behavior isn't
altered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--import-sourcetoantd lintso wrapper package roots can be treated as antd component entrypointsantdimport matching behavior unchanged while allowing multiple configured sourcesspec.mdTest plan
Summary by CodeRabbit
发布说明
新功能
antd lint添加可限制检查类别的--only选项（deprecated、a11y、usage、performance）--import-source选项，支持将额外导入源视为组件入口（默认仍包含antd）文档
deprecated检查、输出 JSON 与--import-source覆盖用法测试