bito-code-review[bot] commented on code in PR #36760:
URL: https://github.com/apache/superset/pull/36760#discussion_r2638290069
##########
superset-frontend/src/explore/controlUtils/standardizedFormData.ts:
##########
@@ -25,7 +25,6 @@ import {
QueryFormMetric,
} from '@superset-ui/core';
import {
- ControlStateMapping,
getStandardizedControls,
isStandardizedFormData,
StandardizedControls,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type Safety Regression</b></div>
<div id="fix">
Removing ControlStateMapping import and switching to any types violates the
critical frontend modernization standards that prohibit any types.
ControlStateMapping is still available from '@superset-ui/chart-controls' and
should be imported there for type safety.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #402208</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/controlUtils/standardizedFormData.ts:
##########
@@ -189,11 +188,10 @@ export class StandardizedFormData {
transform(
targetVizType: string,
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
exploreState: Record<string, any>,
- ): {
- formData: QueryFormData;
- controlsState: ControlStateMapping;
- } {
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ ): any {
/*
* Transform form_data between different viz. Return new form_data and
controlsState.
* 1. get memorized form_data by viz type or get previous form_data
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type Safety Violation</b></div>
<div id="fix">
Changing the transform method return type to any degrades type safety and
violates standards. The method should return a properly typed object with
formData and controlsState.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
</ul>
</details>
<small><i>Code Review Run #402208</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/controlUtils/standardizedFormData.ts:
##########
@@ -243,14 +243,18 @@ export class StandardizedFormData {
getStandardizedControls().clear();
rv = {
formData: transformed,
- controlsState: getControlsState(exploreState, transformed),
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ controlsState: getControlsState(exploreState as any, transformed),
};
}
// refresh validator message
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
rv.controlsState = getControlsState(
- { ...exploreState, controls: rv.controlsState },
- rv.formData,
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ { ...exploreState, controls: rv.controlsState } as any,
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ rv.formData as any,
);
return rv;
}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type Safety Bypass</b></div>
<div id="fix">
Multiple any casts and eslint disables here continue the pattern of avoiding
proper typing. This should be fixed to maintain type safety.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/.cursor/rules/dev-standard.mdc#L35";>dev-standard.mdc:35</a>
</li>
</ul>
</details>
<small><i>Code Review Run #402208</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.tsx:
##########
@@ -344,10 +345,14 @@ function DndColumnMetricSelect(props:
DndColumnMetricSelectProps) {
onChange(multi ? newValues : newValues[0]);
}}
onRemoveMetric={onClickClose}
- columns={columns}
- savedMetrics={savedMetrics}
- savedMetricsOptions={savedMetrics}
- datasource={datasource}
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ columns={columns as any}
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ savedMetrics={savedMetrics as any}
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ savedMetricsOptions={savedMetrics as any}
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ datasource={datasource as any}
onMoveLabel={onShiftOptions}
onDropLabel={() => {}}
type={`${DndItemType.AdhocMetricOption}_${name}_${label}`}
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Type Safety Violation</b></div>
<div id="fix">
The changes add multiple `as any` type assertions and disable the eslint
rule for no-explicit-any, which violates the project's development standards
that explicitly prohibit `any` types in favor of proper TypeScript typing. This
undermines type safety and could lead to runtime errors if the actual types
don't match expectations. Instead of bypassing TypeScript, the types should be
properly aligned or cast to compatible types.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/fcf2d2f/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/fcf2d2f/AGENTS.md#L10";>AGENTS.md:10</a>
</li>
</ul>
</details>
<small><i>Code Review Run #14ed29</i></small>
</div><div>
<div id="suggestion">
<div id="issue"><b>Type Safety Violation</b></div>
<div id="fix">
The changes cast props to 'any' to bypass TypeScript type checks, violating
the codebase's strict 'NO any types' rule from dev-standard.mdc. The
MetricDefinitionValue component expects specific types (e.g., columns with
required 'type' string, savedMetrics as SavedMetricTypeDef[]), but receives
ColumnMeta[] (where 'type' is optional) and Metric[] (with different fields
like 'uuid'). This reduces type safety and risks runtime errors in
metric/column handling.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<div id="suggestion">
<div id="issue"><b>Type Safety Violation</b></div>
<div id="fix">
Similar to the first change, these casts to 'any' violate the 'NO any types'
rule. The props like 'columns' (ColumnMeta[] with optional 'type') and
'savedMetrics' (Metric[] with extra fields like 'uuid') don't match the
expected SavedMetricTypeDef[] and { column_name: string; type: string }[],
leading to unsafe type suppression.
</div>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/.cursor/rules/dev-standard.mdc#L16";>dev-standard.mdc:16</a>
</li>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/6d597ef/AGENTS.md#L57";>AGENTS.md:57</a>
</li>
</ul>
</details>
<small><i>Code Review Run #402208</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
