ESLint: no-restricted-syntax (#10889)

* Enable no-restricted syntax rule

* Fix webpack.config.js

* Remove unused function from utils/common.js

* Refactor triple nested for loop

* Fix loops in src/explore components

* Fix loops in SqlLab components

* Fix loops in AlteredSliceTag

* Fix loops in FilterableTable

* Add fixtures and uinit tests for findControlItem

* Add license

superset-frontend/.eslintrc.js

@@ -101,7 +101,6 @@ module.exports = {
         'no-multi-spaces': 0,
         'no-prototype-builtins': 0,
         'no-restricted-properties': 0,
-        'no-restricted-syntax': 0,
         'no-restricted-imports': [
           'error',
           {
@@ -215,7 +214,6 @@ module.exports = {
     'no-multi-spaces': 0,
     'no-prototype-builtins': 0,
     'no-restricted-properties': 0,
-    'no-restricted-syntax': 0,
     'no-restricted-imports': [
       'error',
       {

superset-frontend/spec/javascripts/explore/controlUtils_spec.jsx

@@ -17,19 +17,20 @@
  * under the License.
  */
 
-import React from 'react';
+import { getChartControlPanelRegistry, t } from '@superset-ui/core';
-import {
-  getChartControlPanelRegistry,
-  ColumnOption,
-  t,
-} from '@superset-ui/core';
 import {
   getControlConfig,
   getControlState,
   getFormDataFromControls,
   applyMapStateToPropsToControl,
   getAllControlsState,
+  findControlItem,
 } from 'src/explore/controlUtils';
+import {
+  controlPanelSectionsChartOptions,
+  controlPanelSectionsChartOptionsOnlyColorScheme,
+  controlPanelSectionsChartOptionsTable,
+} from 'spec/javascripts/explore/fixtures';
 
 describe('controlUtils', () => {
   const state = {
@@ -43,56 +44,10 @@ describe('controlUtils', () => {
   beforeAll(() => {
     getChartControlPanelRegistry()
       .registerValue('test-chart', {
-        controlPanelSections: [
+        controlPanelSections: controlPanelSectionsChartOptions,
-          {
-            label: t('Chart Options'),
-            expanded: true,
-            controlSetRows: [
-              [
-                'color_scheme',
-                {
-                  name: 'rose_area_proportion',
-                  config: {
-                    type: 'CheckboxControl',
-                    label: t('Use Area Proportions'),
-                    description: t(
-                      'Check if the Rose Chart should use segment area instead of ' +
-                        'segment radius for proportioning',
-                    ),
-                    default: false,
-                    renderTrigger: true,
-                  },
-                },
-              ],
-              [
-                {
-                  name: 'stacked_style',
-                  config: {
-                    type: 'SelectControl',
-                    label: t('Stacked Style'),
-                    renderTrigger: true,
-                    choices: [
-                      ['stack', 'stack'],
-                      ['stream', 'stream'],
-                      ['expand', 'expand'],
-                    ],
-                    default: 'stack',
-                    description: '',
-                  },
-                },
-              ],
-            ],
-          },
-        ],
       })
       .registerValue('test-chart-override', {
-        controlPanelSections: [
+        controlPanelSections: controlPanelSectionsChartOptionsOnlyColorScheme,
-          {
-            label: t('Chart Options'),
-            expanded: true,
-            controlSetRows: [['color_scheme']],
-          },
-        ],
         controlOverrides: {
           color_scheme: {
             label: t('My beautiful colors'),
@@ -100,40 +55,7 @@ describe('controlUtils', () => {
         },
       })
       .registerValue('table', {
-        controlPanelSections: [
+        controlPanelSections: controlPanelSectionsChartOptionsTable,
-          {
-            label: t('Chart Options'),
-            expanded: true,
-            controlSetRows: [
-              [
-                'metric',
-                'metrics',
-                {
-                  name: 'all_columns',
-                  config: {
-                    type: 'SelectControl',
-                    queryField: 'columns',
-                    multi: true,
-                    label: t('Columns'),
-                    default: [],
-                    description: t('Columns to display'),
-                    optionRenderer: c => <ColumnOption column={c} showType />,
-                    valueRenderer: c => <ColumnOption column={c} />,
-                    valueKey: 'column_name',
-                    allowAll: true,
-                    mapStateToProps: stateRef => ({
-                      options: stateRef.datasource
-                        ? stateRef.datasource.columns
-                        : [],
-                    }),
-                    commaChoosesOption: false,
-                    freeForm: true,
-                  },
-                },
-              ],
-            ],
-          },
-        ],
       });
   });
 
@@ -287,4 +209,38 @@ describe('controlUtils', () => {
       });
     });
   });
+
+  describe('findControlItem', () => {
+    it('find control as a string', () => {
+      const controlItem = findControlItem(
+        controlPanelSectionsChartOptions,
+        'color_scheme',
+      );
+      expect(controlItem).toEqual('color_scheme');
+    });
+
+    it('find control as a control object', () => {
+      let controlItem = findControlItem(
+        controlPanelSectionsChartOptions,
+        'rose_area_proportion',
+      );
+      expect(controlItem.name).toEqual('rose_area_proportion');
+      expect(controlItem).toHaveProperty('config');
+
+      controlItem = findControlItem(
+        controlPanelSectionsChartOptions,
+        'stacked_style',
+      );
+      expect(controlItem.name).toEqual('stacked_style');
+      expect(controlItem).toHaveProperty('config');
+    });
+
+    it('returns null when key is not found', () => {
+      const controlItem = findControlItem(
+        controlPanelSectionsChartOptions,
+        'non_existing_key',
+      );
+      expect(controlItem).toBeNull();
+    });
+  });
 });

superset-frontend/spec/javascripts/explore/fixtures.jsx

@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React from 'react';
+import { ColumnOption, t } from '@superset-ui/core';
+
+export const controlPanelSectionsChartOptions = [
+  {
+    label: t('Chart Options'),
+    expanded: true,
+    controlSetRows: [
+      [
+        'color_scheme',
+        {
+          name: 'rose_area_proportion',
+          config: {
+            type: 'CheckboxControl',
+            label: t('Use Area Proportions'),
+            description: t(
+              'Check if the Rose Chart should use segment area instead of ' +
+                'segment radius for proportioning',
+            ),
+            default: false,
+            renderTrigger: true,
+          },
+        },
+      ],
+      [
+        {
+          name: 'stacked_style',
+          config: {
+            type: 'SelectControl',
+            label: t('Stacked Style'),
+            renderTrigger: true,
+            choices: [
+              ['stack', 'stack'],
+              ['stream', 'stream'],
+              ['expand', 'expand'],
+            ],
+            default: 'stack',
+            description: '',
+          },
+        },
+      ],
+    ],
+  },
+];
+
+export const controlPanelSectionsChartOptionsOnlyColorScheme = [
+  {
+    label: t('Chart Options'),
+    expanded: true,
+    controlSetRows: [['color_scheme']],
+  },
+];
+
+export const controlPanelSectionsChartOptionsTable = [
+  {
+    label: t('Chart Options'),
+    expanded: true,
+    controlSetRows: [
+      [
+        'metric',
+        'metrics',
+        {
+          name: 'all_columns',
+          config: {
+            type: 'SelectControl',
+            queryField: 'columns',
+            multi: true,
+            label: t('Columns'),
+            default: [],
+            description: t('Columns to display'),
+            optionRenderer: c => <ColumnOption column={c} showType />,
+            valueRenderer: c => <ColumnOption column={c} />,
+            valueKey: 'column_name',
+            allowAll: true,
+            mapStateToProps: stateRef => ({
+              options: stateRef.datasource ? stateRef.datasource.columns : [],
+            }),
+            commaChoosesOption: false,
+            freeForm: true,
+          },
+        },
+      ],
+    ],
+  },
+];

superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx

@@ -173,12 +173,9 @@ class TabbedSqlEditors extends React.PureComponent {
   UNSAFE_componentWillReceiveProps(nextProps) {
     const nextActiveQeId =
       nextProps.tabHistory[nextProps.tabHistory.length - 1];
-    const queriesArray = [];
+    const queriesArray = Object.values(nextProps.queries).filter(
-    for (const id in nextProps.queries) {
+      query => query.sqlEditorId === nextActiveQeId,
-      if (nextProps.queries[id].sqlEditorId === nextActiveQeId) {
+    );
-        queriesArray.push(nextProps.queries[id]);
-      }
-    }
     if (!areArraysShallowEqual(queriesArray, this.state.queriesArray)) {
       this.setState({ queriesArray });
     }

superset-frontend/src/SqlLab/components/TableElement.jsx

@@ -110,10 +110,9 @@ class TableElement extends React.PureComponent {
           />
         );
       }
-      let latest = [];
+      let latest = Object.entries(table.partitions.latest).map(
-      for (const k in table.partitions.latest) {
+        ([key, value]) => `${key}=${value}`,
-        latest.push(`${k}=${table.partitions.latest[k]}`);
+      );
-      }
       latest = latest.join('/');
       header = (
         <Well bsSize="small">

superset-frontend/src/SqlLab/reducers/sqlLab.js

@@ -493,8 +493,7 @@ export default function sqlLabReducer(state = {}, action) {
       // Fetch the updates to the queries present in the store.
       let change = false;
       let { queriesLastUpdate } = state;
-      for (const id in action.alteredQueries) {
+      Object.entries(action.alteredQueries).forEach(([id, changedQuery]) => {
-        const changedQuery = action.alteredQueries[id];
         if (
           !state.queries.hasOwnProperty(id) ||
           (state.queries[id].state !== 'stopped' &&
@@ -506,7 +505,7 @@ export default function sqlLabReducer(state = {}, action) {
           newQueries[id] = { ...state.queries[id], ...changedQuery };
           change = true;
         }
-      }
+      });
       if (!change) {
         newQueries = state.queries;
       }

superset-frontend/src/components/AlteredSliceTag.jsx

@@ -76,19 +76,17 @@ export default class AlteredSliceTag extends React.Component {
 
     const fdKeys = Object.keys(cfd);
     const diffs = {};
-    for (const fdKey of fdKeys) {
+    fdKeys.forEach(fdKey => {
-      // Ignore values that are undefined/nonexisting in either
       if (!ofd[fdKey] && !cfd[fdKey]) {
-        continue;
+        return;
       }
-      // Ignore obsolete legacy filters
       if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
-        continue;
+        return;
       }
       if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) {
         diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
       }
-    }
+    });
     return diffs;
   }
 
@@ -149,7 +147,7 @@ export default class AlteredSliceTag extends React.Component {
   renderRows() {
     const { diffs } = this.state;
     const rows = [];
-    for (const key in diffs) {
+    Object.entries(diffs).forEach(([key, diff]) => {
       rows.push(
         <Tr key={key}>
           <Td
@@ -160,11 +158,11 @@ export default class AlteredSliceTag extends React.Component {
               key
             }
           />
-          <Td column="before">{this.formatValue(diffs[key].before, key)}</Td>
+          <Td column="before">{this.formatValue(diff.before, key)}</Td>
-          <Td column="after">{this.formatValue(diffs[key].after, key)}</Td>
+          <Td column="after">{this.formatValue(diff.after, key)}</Td>
         </Tr>,
       );
-    }
+    });
     return rows;
   }
 

superset-frontend/src/components/FilterableTable/FilterableTable.tsx

@@ -26,11 +26,11 @@ import {
   Grid,
   ScrollSync,
   SortDirection,
+  SortDirectionType,
   SortIndicator,
   Table,
-  SortDirectionType,
 } from 'react-virtualized';
-import { t, getMultipleTextDimensions } from '@superset-ui/core';
+import { getMultipleTextDimensions, t } from '@superset-ui/core';
 
 import Button from '../Button';
 import CopyToClipboard from '../CopyToClipboard';
@@ -241,24 +241,22 @@ export default class FilterableTable extends PureComponent<
   }
 
   formatTableData(data: Record<string, unknown>[]): Datum[] {
-    const formattedData = data.map(row => {
+    return data.map(row => {
       const newRow = {};
-      for (const k in row) {
+      Object.entries(row).forEach(([key, val]) => {
-        const val = row[k];
         if (['string', 'number'].indexOf(typeof val) >= 0) {
-          newRow[k] = val;
+          newRow[key] = val;
         } else {
-          newRow[k] = val === null ? null : JSONbig.stringify(val);
+          newRow[key] = val === null ? null : JSONbig.stringify(val);
         }
-      }
+      });
       return newRow;
     });
-    return formattedData;
   }
 
   hasMatch(text: string, row: Datum) {
-    const values = [];
+    const values: string[] = [];
-    for (const key in row) {
+    Object.keys(row).forEach(key => {
       if (row.hasOwnProperty(key)) {
         const cellValue = row[key];
         if (typeof cellValue === 'string') {
@@ -270,7 +268,7 @@ export default class FilterableTable extends PureComponent<
           values.push(cellValue.toString());
         }
       }
-    }
+    });
     const lowerCaseText = text.toLowerCase();
     return values.some(v => v.includes(lowerCaseText));
   }

superset-frontend/src/explore/components/ExploreViewContainer.jsx

@@ -293,20 +293,18 @@ class ExploreViewContainer extends React.Component {
 
   renderErrorMessage() {
     // Returns an error message as a node if any errors are in the store
-    const errors = [];
+    const errors = Object.entries(this.props.controls)
-    const ctrls = this.props.controls;
+      .filter(
-    for (const controlName in this.props.controls) {
+        ([, control]) =>
-      const control = this.props.controls[controlName];
+          control.validationErrors && control.validationErrors.length > 0,
-      if (control.validationErrors && control.validationErrors.length > 0) {
+      )
-        errors.push(
+      .map(([key, control]) => (
-          <div key={controlName}>
+        <div key={key}>
-            {t('Control labeled ')}
+          {t('Control labeled ')}
-            <strong>{` "${control.label}" `}</strong>
+          <strong>{` "${control.label}" `}</strong>
-            {control.validationErrors.join('. ')}
+          {control.validationErrors.join('. ')}
-          </div>,
+        </div>
-        );
+      ));
-      }
-    }
     let errorMessage;
     if (errors.length > 0) {
       errorMessage = <div style={{ textAlign: 'left' }}>{errors}</div>;

superset-frontend/src/explore/components/controls/SelectControl.jsx

@@ -103,7 +103,7 @@ export default class SelectControl extends React.PureComponent {
     if (opt) {
       if (this.props.multi) {
         optionValue = [];
-        for (const o of opt) {
+        opt.forEach(o => {
           // select all options
           if (o.meta === true) {
             this.props.onChange(
@@ -114,7 +114,7 @@ export default class SelectControl extends React.PureComponent {
             return;
           }
           optionValue.push(o[this.props.valueKey] || o);
-        }
+        });
       } else if (opt.meta === true) {
         return;
       } else {

superset-frontend/src/explore/controlUtils.js

@@ -51,22 +51,19 @@ export function validateControl(control, processedState) {
 /**
  * Find control item from control panel config.
  */
-function findControlItem(controlPanelSections, controlKey) {
+export function findControlItem(controlPanelSections, controlKey) {
-  for (const section of controlPanelSections) {
+  return (
-    for (const controlArr of section.controlSetRows) {
+    controlPanelSections
-      for (const control of controlArr) {
+      .map(section => section.controlSetRows)
-        if (controlKey === control) return control;
+      .flat(2)
-        if (
+      .find(
-          control !== null &&
+        control =>
-          typeof control === 'object' &&
+          controlKey === control ||
-          control.name === controlKey
+          (control !== null &&
-        ) {
+            typeof control === 'object' &&
-          return control;
+            control.name === controlKey),
-        }
+      ) ?? null
-      }
+  );
-    }
-  }
-  return null;
 }
 
 export const getControlConfig = memoizeOne(function getControlConfig(

superset-frontend/src/utils/common.js

@@ -82,14 +82,6 @@ export function getShortUrl(longUrl) {
     );
 }
 
-export function supersetURL(rootUrl, getParams = {}) {
-  const url = new URL(rootUrl, window.location.origin);
-  for (const k in getParams) {
-    url.searchParams.set(k, getParams[k]);
-  }
-  return url.href;
-}
-
 export function optionLabel(opt) {
   if (opt === null) {
     return NULL_STRING;

superset-frontend/webpack.config.js

@@ -78,7 +78,7 @@ const plugins = [
       //   }
       // }
       const entryFiles = {};
-      for (const [entry, chunks] of Object.entries(entrypoints)) {
+      Object.entries(entrypoints).forEach(([entry, chunks]) => {
         entryFiles[entry] = {
           css: chunks
             .filter(x => x.endsWith('.css'))
@@ -87,7 +87,8 @@ const plugins = [
             .filter(x => x.endsWith('.js'))
             .map(x => path.join(output.publicPath, x)),
         };
-      }
+      });
+
       return {
         ...seed,
         entrypoints: entryFiles,
@@ -430,7 +431,7 @@ if (isDevMode) {
 
   // find all the symlinked plugins and use their source code for imports
   let hasSymlink = false;
-  for (const [pkg, version] of Object.entries(packageConfig.dependencies)) {
+  Object.entries(packageConfig.dependencies).forEach(([pkg, version]) => {
     const srcPath = `./node_modules/${pkg}/src`;
     if (/superset-ui/.test(pkg) && fs.existsSync(srcPath)) {
       console.log(
@@ -441,7 +442,7 @@ if (isDevMode) {
       config.resolve.alias[`${pkg}$`] = `${pkg}/src`;
       hasSymlink = true;
     }
-  }
+  });
   if (hasSymlink) {
     console.log(''); // pure cosmetic new line
   }