sadpandajoe commented on code in PR #44201:
URL: https://github.com/apache/superset/pull/44201#discussion_r4015080405
##########
superset-frontend/custom-lint-rules/theme-colors/index.js:
##########
@@ -85,11 +105,22 @@ const plugin: { rules: Record<string, Rule.RuleModule> } =
{
},
schema: [],
},
- create(context: Rule.RuleContext): Rule.RuleListener {
- const warned: string[] = [];
+ /**
+ * @param {import('oxlint').Rule.RuleContext} context
+ * @returns {import('oxlint').Rule.RuleListener}
+ */
+ createOnce(context) {
Review Comment:
This makes `warned` persist across every file in the Oxlint run, while its
keys contain only line/column spans. Identical-position color violations in
later files are silently suppressed; should this use per-file `create`
semantics or reset the cache in a `before` hook?
##########
superset-frontend/scripts/internal/oxlint-metrics-uploader.js:
##########
@@ -130,102 +163,52 @@ async function runOxlintAndProcess() {
);
const results = JSON.parse(oxlintOutput);
-
- // Process OXC JSON output
- const metricsByRule = {};
- let occurrencesData = [];
-
- // OXC JSON format has diagnostics array
- if (results.diagnostics && Array.isArray(results.diagnostics)) {
- results.diagnostics.forEach(diagnostic => {
- const ruleId = parseRuleId(diagnostic.code);
-
- const file = diagnostic.filename || 'unknown';
- const line = diagnostic.labels?.[0]?.span?.line || 0;
- const column = diagnostic.labels?.[0]?.span?.column || 0;
- const message = diagnostic.message || '';
-
- const ruleData = metricsByRule[ruleId] || { count: 0 };
- ruleData.count += 1;
- metricsByRule[ruleId] = ruleData;
-
- occurrencesData.push({
- rule: ruleId,
- message,
- file,
- line,
- column,
- ts: DATETIME,
- });
- });
- }
-
console.log(
`OXC found ${results.diagnostics?.length || 0} issues across
${results.number_of_files} files`,
);
+ const { metricsByRule, occurrencesData } = parseOxlintResult(results);
+
+ // Also run Oxlint for custom rules and merge results
+ console.log('Running Oxlint for custom rules...');
+ // Run ESLint and capture output directly.
+ // Flat config (oxlint.custom-lint-rules.mts) is explicitly selected via
--config
+ 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
+ },
+ );
- // Also run minimal ESLint for custom rules and merge results
- console.log('Running minimal ESLint for custom rules...');
- let eslintOutput = '[]';
- try {
- // Run ESLint and capture output directly.
- // Flat config (eslint.config.minimal.js) is explicitly selected via
- // --config; ESLint v9+/v10 no longer support eslintrc or --no-eslintrc.
- eslintOutput = execSync(
- 'npx eslint --config eslint.config.minimal.js --no-inline-config
--format json src',
- {
- encoding: 'utf8',
- maxBuffer: 50 * 1024 * 1024,
- stdio: ['pipe', 'pipe', 'ignore'], // Ignore stderr
- },
- );
- } catch (e) {
- // ESLint exits with non-zero when it finds issues, capture the stdout
- if (e.stdout) {
- eslintOutput = e.stdout.toString();
- }
- }
-
- // Parse minimal ESLint output
- try {
- const eslintResults = JSON.parse(eslintOutput);
-
- eslintResults.forEach(result => {
- result.messages.forEach(({ ruleId, line, column, message }) => {
- const ruleData = metricsByRule[ruleId] || { count: 0 };
- ruleData.count += 1;
- metricsByRule[ruleId] = ruleData;
-
- occurrencesData.push({
- rule: ruleId,
- message,
- file: result.filePath,
- line,
- column,
- ts: DATETIME,
- });
- });
- });
-
- console.log(
- `ESLint found ${eslintResults.reduce((sum, r) => sum +
r.messages.length, 0)} custom rule violations`,
- );
- } catch (e) {
- console.log('No ESLint issues found or parsing error:', e.message);
- }
+ // Parse Oxlint output for custom rules
+ const oxlintCustomRuleResults = JSON.parse(oxlintCustomRuleOutput);
+ console.log(
+ `OXC found ${oxlintCustomRuleResults.diagnostics?.length || 0} issues
across ${oxlintCustomRuleResults.number_of_files} files for custom rules`,
+ );
+ const {
+ metricsByRule: metricsByCustomRule,
+ occurrencesData: customRuleOccurrencesData,
+ } = parseOxlintResult(oxlintCustomRuleResults);
+
+ const mergedMetricsByRule = { ...metricsByRule, ...metricsByCustomRule };
Review Comment:
The custom Oxlint pass inherits the default correctness rules, so this
spread replaces whole-repo counts with the `src`-only counts for overlapping
rule IDs while the occurrence rows concatenate both runs. Could the custom
config disable built-in categories, or should this merge sum and deduplicate
overlapping results?
##########
superset-frontend/scripts/oxlint-metrics-uploader.js:
##########
@@ -130,102 +163,52 @@ async function runOxlintAndProcess() {
);
const results = JSON.parse(oxlintOutput);
-
- // Process OXC JSON output
- const metricsByRule = {};
- let occurrencesData = [];
-
- // OXC JSON format has diagnostics array
- if (results.diagnostics && Array.isArray(results.diagnostics)) {
- results.diagnostics.forEach(diagnostic => {
- const ruleId = parseRuleId(diagnostic.code);
-
- const file = diagnostic.filename || 'unknown';
- const line = diagnostic.labels?.[0]?.span?.line || 0;
- const column = diagnostic.labels?.[0]?.span?.column || 0;
- const message = diagnostic.message || '';
-
- const ruleData = metricsByRule[ruleId] || { count: 0 };
- ruleData.count += 1;
- metricsByRule[ruleId] = ruleData;
-
- occurrencesData.push({
- rule: ruleId,
- message,
- file,
- line,
- column,
- ts: DATETIME,
- });
- });
- }
-
console.log(
`OXC found ${results.diagnostics?.length || 0} issues across
${results.number_of_files} files`,
);
+ const { metricsByRule, occurrencesData } = parseOxlintResult(results);
+
+ // Also run Oxlint for custom rules and merge results
+ console.log('Running Oxlint for custom rules...');
+ // Run ESLint and capture output directly.
+ // Flat config (oxlint.custom-lint-rules.mts) is explicitly selected via
--config
+ 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
+ },
Review Comment:
Agreed—the custom rules are errors, so a nonzero Oxlint exit stops
`lint-stats` before either sheet is written. Could this preserve stdout on lint
findings as the previous ESLint path did?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
