feat(build): migrate custom lint rule to Oxlint - #44201
Conversation
…to Oxlint Signed-off-by: hainenber <dotronghai96@gmail.com>
Signed-off-by: hainenber <dotronghai96@gmail.com>
… equivalent Signed-off-by: hainenber <dotronghai96@gmail.com>
github-actions
Bot
added
dependencies:npm
github_actions
| "@apache-superset/core": ["./packages/superset-core/src"], | ||
| "@apache-superset/core/*": ["./packages/superset-core/src/*"], | ||
| "@superset-ui/plugin-chart-*": ["./plugins/plugin-chart-*/src"], | ||
| "@superset-ui/plugin-chart-*": ["./plugins/plugin-chart-*/src"], |
There was a problem hiding this comment.
This is a duplication of above entry. Remove to avoid red underlining in IDEs
|
The requested file |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| const oxlintCustomRuleOutput = execSync( | ||
| 'npx oxlint --config oxlint.custom-lint-rules.mts --format json src', | ||
| { | ||
| encoding: 'utf8', | ||
| maxBuffer: 50 * 1024 * 1024, // 50MB buffer for large outputs | ||
| stdio: ['pipe', 'pipe', 'ignore'], // Ignore stderr | ||
| }, |
There was a problem hiding this comment.
Suggestion: This execSync throws whenever custom rules report errors, so the uploader exits before writing any metrics or backlog rows. [error handling]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/scripts/oxlint-metrics-uploader.js
**Line:** 175:181
**Comment:**
*Error Handling: This `execSync` throws whenever custom rules report errors, so the uploader exits before writing any metrics or backlog rows.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| const { | ||
| metricsByRule: metricsByCustomRule, | ||
| occurrencesData: customRuleOccurrencesData, | ||
| } = parseOxlintResult(results); |
There was a problem hiding this comment.
Suggestion: Custom diagnostics are parsed from results instead of oxlintCustomRuleResults, so custom-rule findings are omitted and standard findings are counted twice. [incorrect variable usage]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/scripts/oxlint-metrics-uploader.js
**Line:** 189:192
**Comment:**
*Incorrect Variable Usage: Custom diagnostics are parsed from `results` instead of `oxlintCustomRuleResults`, so custom-rule findings are omitted and standard findings are counted twice.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| run: | | ||
| docker run --rm $TAG bash -c \ | ||
| "npm i && npm run lint" | ||
| "npm i && npm run lint && node --import tsx --test custom-lint-rules/**/*.test.ts" |
There was a problem hiding this comment.
Suggestion: The workflow runs only the standard Oxlint config and unit tests, so custom-rule violations in frontend source no longer fail CI. [incomplete implementation]
Assessment: 🟠 Major · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .github/workflows/superset-frontend.yml
**Line:** 160:160
**Comment:**
*Incomplete Implementation: The workflow runs only the standard Oxlint config and unit tests, so custom-rule violations in frontend source no longer fail CI.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #75e601
Actionable Suggestions - 1
-
superset-frontend/scripts/oxlint-metrics-uploader.js - 1
- Wrong variable in parse call · Line 190-192
Additional Suggestions - 3
-
superset-frontend/oxlint.custom-lint-rules.mts - 1
-
Disable-directive regression · Line 65-65Setting `respectEslintDisableDirectives: false` makes oxlint ignore all `eslint-disable` comments. The old ESLint config respected them by default, and `src/visualizations/TimeTable/constants.ts` (lines 29, 31) still carries `eslint-disable-next-line theme-colors/no-literal-colors` suppressions that will now be reported as errors, inflating `lint-stats` metrics. Consider leaving it at the default `true`.
-
-
superset-frontend/scripts/oxlint-metrics-uploader.js - 2
-
Inconsistent log counts · Line 241-241The log mixes sources: `Object.keys(metricsByRule)` counts only the first oxlint run, while `finalizedConcurrencesData.length` counts merged occurrences from both runs. Use `Object.keys(mergedMetricsByRule)` so the reported unique-rules count is consistent with the occurrence count.
-
Misspelled variable name · Line 211-211`finalizedConcurrencesData` misspells "Occurrences" as "Concurrences", inconsistent with the sibling `occurrencesData`/`mergedOccurrencesData` variables it derives from. Rename to `finalizedOccurrencesData` for consistency and to avoid confusion when reading the merge/upload flow.
-
Review Details
-
Files reviewed - 5 · Commit Range:
9419f60..f6e9e61- superset-frontend/custom-lint-rules/theme-colors/no-literal-colors.test.ts
- superset-frontend/eslint.config.minimal.js
- superset-frontend/oxlint.custom-lint-rules.mts
- superset-frontend/scripts/check-custom-rules.js
- superset-frontend/scripts/oxlint-metrics-uploader.js
-
Files skipped - 5
- .github/workflows/superset-frontend.yml - Reason: Filter setting
- superset-frontend/oxlint.json - Reason: Filter setting
- superset-frontend/package-lock.json - Reason: Filter setting
- superset-frontend/package.json - Reason: Filter setting
- superset-frontend/tsconfig.json - Reason: Filter setting
-
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers an incremental AI Review. -
/review full- Manually triggers a full AI Review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| metricsByRule: metricsByCustomRule, | ||
| occurrencesData: customRuleOccurrencesData, | ||
| } = parseOxlintResult(results); |
There was a problem hiding this comment.
parseOxlintResult is called with results (the first oxlint run) instead of oxlintCustomRuleResults. The custom-rules run is parsed and logged but never processed, so metricsByCustomRule/customRuleOccurrencesData duplicate the main run's data and the custom rules are double-counted in mergedOccurrencesData. Pass oxlintCustomRuleResults.
Code suggestion
Check the AI-generated fix before applying
| metricsByRule: metricsByCustomRule, | |
| occurrencesData: customRuleOccurrencesData, | |
| } = parseOxlintResult(results); | |
| metricsByRule: metricsByCustomRule, | |
| occurrencesData: customRuleOccurrencesData, | |
| } = parseOxlintResult(oxlintCustomRuleResults); |
Code Review Run #75e601
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44201 +/- ##
==========================================
- Coverage 80.17% 80.15% -0.02%
==========================================
Files 2925 2925
Lines 172670 172533 -137
Branches 40087 40051 -36
==========================================
- Hits 138435 138301 -134
+ Misses 31638 31635 -3
Partials 2597 2597
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: hainenber <dotronghai96@gmail.com>
Code Review Agent Run #2b6c69Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Signed-off-by: hainenber <dotronghai96@gmail.com>
1 participant
feat(build): migrate custom lint rule to Oxlint
SUMMARY
Migrate existing lint rule from ESLint-based to Oxlint-based. Well, not that much of a refactor since Oxlint folks provides excellent compatibility with custom ESLint plugin API. A bit of shame that module-level, Typescript-based custom JS plugin support for Oxlint is not yet mature so I had to use jsdoc to annotate typing for TS-to-JS plugin files but not a dealbreaker, tbh.
I also fixed the typescript rule in existing
oxlint.jsonfrom@typescript-eslinttotypescriptprefix. The ESLint config and script to run and parse Oxlint for metrics uploader is also revamped since we consolidate to Oxlint finally, reducing code paths.npm run check:custom-rulesis still good. The plugin files are still intact with their logicRunning rule tests with native
node:testis super fun and good, ensuring no regressionRunning the lint config only for custom rules is also all fine and dandy. It detected correctly violations
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Green CI
ADDITIONAL INFORMATION