Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup: add no-unused-vars and imports eslint rules and fix the codebase #790

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
'plugin:@typescript-eslint/recommended',
'plugin:prettier/recommended',
],
plugins: ['unused-imports'],
parserOptions: {
ecmaVersion: 2018,
sourceType: 'module',
Expand All @@ -24,6 +25,9 @@ module.exports = {
'@typescript-eslint/no-namespace': 'off',
'@typescript-eslint/explicit-module-boundary-types': 'off',
'@typescript-eslint/no-unused-vars': 'off',
'no-unused-vars': 'off',
'unused-imports/no-unused-imports': 'error',
'unused-imports/no-unused-vars': ['error', { ignoreRestSiblings: true }],
'@typescript-eslint/ban-types': 'off',
'@typescript-eslint/no-use-before-define': [
'error',
Expand Down
31 changes: 31 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@blueprintjs/select": "5.1.1",
"@improbable-eng/grpc-web": "^0.15.0",
"@react-hook/resize-observer": "1.2.6",
"@react-spring/web": "9.7.3",
"@use-gesture/react": "10.3.0",
"classnames": "2.5.1",
"core-js": "3.35.1",
Expand All @@ -51,7 +52,6 @@
"react": "18.2.0",
"react-dom": "18.2.0",
"react-router-dom": "6.22.0",
"@react-spring/web": "9.7.3",
"react-window": "1.8.10",
"url-parse": "1.5.10"
},
Expand Down Expand Up @@ -91,6 +91,7 @@
"eslint-config-prettier": "9.1.0",
"eslint-plugin-prettier": "5.1.3",
"eslint-plugin-react": "7.33.2",
"eslint-plugin-unused-imports": "^3.1.0",
"fs-extra": "11.2.0",
"html-webpack-plugin": "5.6.0",
"husky": "9.0.10",
Expand Down
2 changes: 1 addition & 1 deletion scripts/assets-transformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
const path = require('path');

module.exports = {
process(src, filename, config, options) {
process(_, filename) {
return 'module.exports = ' + JSON.stringify(path.basename(filename)) + ';';
},
};
7 changes: 2 additions & 5 deletions scripts/install-grpc-deps/protoc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import decompress from 'decompress';

const VERSION = '3.11.4';

const DL_PREFIX =
'https://github.com/protocolbuffers/protobuf/releases/download';
const DL_PREFIX = 'https://github.com/protocolbuffers/protobuf/releases/download';

const PLATFORM =
{
Expand Down Expand Up @@ -53,9 +52,7 @@ async function download(url, targetFile) {
if (code > 300 && code < 400 && !!response.headers.location) {
return resolve(download(response.headers.location, targetFile));
}
const fileWriter = fs
.createWriteStream(targetFile)
.on('finish', () => resolve());
const fileWriter = fs.createWriteStream(targetFile).on('finish', () => resolve());
response.pipe(fileWriter);
})
.on('error', error => reject(error));
Expand Down
2 changes: 2 additions & 0 deletions src/api/customprotocol-core/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,8 @@ export class Stream<H extends HandlerTypes = {}> extends EventEmitter<Union<Hand
return false;
}

// TODO: fix inheritance - _isFirst is used in Stream implementation so can't be removed here
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like its not an issue of inheritance, more like an issue of linter. Would that work if you replaced _isFirst with _ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can _ but then the next person landing on this code will ask himself why there is an unused variable, will likely remove it, and broke again the inheritance. So we should keep the comment to explain why there is an unused variable here until we fix the inheritance, a.k.a we don't add unused variable because parents might use them.

// eslint-disable-next-line unused-imports/no-unused-vars
protected messageBuilder(msg: Message, _isFirst: boolean): Message {
return msg;
}
Expand Down
2 changes: 1 addition & 1 deletion src/api/grpc/error.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { GrpcError, GrpcStatus, StatusCode, GrpcMetadata } from './common';
import { GrpcError, GrpcStatus, StatusCode } from './common';

// TODO: provide additional getters if needed
export class GrpcWrappedError implements Error {
Expand Down
2 changes: 0 additions & 2 deletions src/api/http-client/result.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import _ from 'lodash';

import { camelCasify } from '~/domain/misc';
import { Result } from '~/utils';

Expand Down
1 change: 0 additions & 1 deletion src/components/ArrowsRenderer/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import _ from 'lodash';
import { observer } from 'mobx-react';
import React, { useRef } from 'react';

Expand Down
1 change: 0 additions & 1 deletion src/components/Card/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import _ from 'lodash';
import classnames from 'classnames';
import React, { useLayoutEffect, useRef } from 'react';
import { observer } from 'mobx-react';
Expand Down
2 changes: 1 addition & 1 deletion src/components/DetailsPanel/hooks/usePanelResize.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react';
import { useCallback, useEffect, useMemo, useState, useRef } from 'react';
import { sizes } from '~/ui/vars';

import * as storage from '~/storage/local';
Expand Down
2 changes: 1 addition & 1 deletion src/components/DetailsPanel/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { observer } from 'mobx-react';
import React, { FunctionComponent, useCallback, useEffect } from 'react';
import React, { useCallback, useEffect } from 'react';

import { DragPanel } from '~/components/DragPanel';
import {
Expand Down
37 changes: 0 additions & 37 deletions src/components/EndpointCardHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,40 +35,3 @@ export const EndpointCardHeader = memo(function EndpointCardHeader(props: Props)
</div>
);
});

interface PolicyProps {
ingress: boolean;
egress: boolean;
}

const PolicyInfo = memo(function PolicyProps(props: PolicyProps) {
const inLocked = props.ingress ? 'locked' : 'unlocked';
const eLocked = props.egress ? 'locked' : 'unlocked';

const ingressIcon = `icons/misc/ingress-${inLocked}.svg`;
const egressIcon = `icons/misc/ingress-${eLocked}.svg`;

const ingressCls = classnames({
[css.left]: true,
[css.active]: props.ingress,
});

const egressCls = classnames({
[css.right]: true,
[css.active]: props.egress,
});

return (
<div className={css.connectors}>
<div className={ingressCls}>
<img src={ingressIcon} />
<span>Ingress</span>
</div>

<div className={egressCls}>
<span>Egress</span>
<img src={egressIcon} />
</div>
</div>
);
});
2 changes: 1 addition & 1 deletion src/components/FlowsTable/ColumnsSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { memo, useCallback } from 'react';

import { usePopover } from '~/ui/hooks/usePopover';

import { CommonProps, Column, columnKeys, getColumnLabel } from './general';
import { CommonProps, Column, getColumnLabel } from './general';

export interface Props extends CommonProps {
visibleColumns: Set<Column>;
Expand Down
4 changes: 2 additions & 2 deletions src/components/FlowsTable/Sidebar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Icon } from '@blueprintjs/core';
import React, { memo, useCallback, useMemo, useEffect, useState } from 'react';
import React, { memo, useCallback, useEffect, useState } from 'react';

import { Flow, Verdict } from '~/domain/flows';
import { Filters, FilterEntry, FilterDirection } from '~/domain/filtering';
Expand Down Expand Up @@ -51,7 +51,7 @@ export const FlowsTableSidebar = memo<Props>(function FlowsTableSidebar(props) {
const [podSelection, setPodSelection] = useState<Set<string>>(new Set());
const [identitySelection, setIdSelection] = useState<Set<number>>(new Set());
const [ipSelection, setIpSelection] = useState<Set<string>>(new Set());
const [tcpFlagSelection, setTCPSelection] = useState<Set<TCPFlagName>>(new Set());
const [tcpFlagSelection] = useState<Set<TCPFlagName>>(new Set());

const [isDnsSelected, setDnsSelected] = useState(false);

Expand Down
4 changes: 0 additions & 4 deletions src/components/FlowsTable/SidebarComponents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,6 @@ export const VerdictEntry = memo<VerdictEntryProps>(function FlowsTableSidebarVe
[css.auditVerdict]: props.verdict === Verdict.Audit,
});

const onClick = useCallback(() => {
props.onClick?.();
}, [props.onClick]);

return (
<span className={className} onClick={props.onClick}>
{verdictHelpers.toString(props.verdict)}
Expand Down
8 changes: 3 additions & 5 deletions src/components/FlowsTable/__tests__/Row.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import { Column } from '~/components/FlowsTable';
import { Flow } from '~/domain/flows';
import { HubbleFlow } from '~/domain/hubble';

const tsUpdateDelay = 5000;

jest.useFakeTimers();

interface Expectations {
Expand Down Expand Up @@ -63,7 +61,7 @@ const runAppearanceTests = (row: HTMLElement, exps: Expectations, selected: bool
}
};

const runTemporalTests = (row: HTMLElement, flow: Flow) => {
const runTemporalTests = (row: HTMLElement) => {
jest.clearAllTimers();
const tsLabel = row.querySelector('.cell:nth-child(12)')!;

Expand All @@ -77,7 +75,7 @@ const runTest = (ntest: number, hf: HubbleFlow, exps: Expectations) => {
const isSelected = [false, true];

isSelected.forEach(selected => {
const onSelect = jest.fn((flow: Flow | null) => void 0);
const onSelect = jest.fn(() => void 0);
const selectedStr = selected ? 'selected' : 'not-selected';

describe(`FlowsTable: Row (${selectedStr}) / test ${ntest}`, () => {
Expand All @@ -99,7 +97,7 @@ const runTest = (ntest: number, hf: HubbleFlow, exps: Expectations) => {
});

test(`temporal`, () => {
runTemporalTests(row, flow);
runTemporalTests(row);
});

test(`interactions`, () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/FlowsTable/hooks/useScroll.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useRef, useEffect, MutableRefObject } from 'react';
import { useRef, useEffect, MutableRefObject } from 'react';
import { FixedSizeListProps } from 'react-window';

import { sizes } from '~/ui';
Expand Down
1 change: 0 additions & 1 deletion src/components/FlowsTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { FixedSizeList } from 'react-window';

import { Flow } from '~/domain/flows';
import { sizes } from '~/ui';
import { Ticker } from '~/utils/ticker';

import { CommonProps, TickerEvents } from './general';
import { useScroll, OnFlowsDiffCount } from './hooks/useScroll';
Expand Down
1 change: 0 additions & 1 deletion src/components/Map/NamespaceBackplate.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { memo } from 'react';

import { XYWH } from '~/domain/geometry';
import { sizes } from '~/ui/vars';

import css from './styles.scss';

Expand Down
2 changes: 1 addition & 1 deletion src/components/ServiceMapApp/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export const ServiceMapApp = observer(function ServiceMapApp() {
visibleHeight={mapVisibleHeight ?? 0}
wasDragged={mapWasDragged}
onMapDrag={onMapDrag}
onCardMutated={muts => ui.serviceMap.cardsMutationsObserved(muts)}
onCardMutated={() => ui.serviceMap.cardsMutationsObserved()}
/>
) : (
<LoadingOverlay
Expand Down
14 changes: 2 additions & 12 deletions src/components/ServiceMapArrowRenderer/ServiceMapArrowBody.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import React, { useCallback, useEffect, useRef } from 'react';
import { observer } from 'mobx-react-lite';
import classnames from 'classnames';
import * as mobx from 'mobx';
import * as d3 from 'd3';

import { XY } from '~/domain/geometry';
import { link } from '~/domain/helpers';

import { ArrowRendererProps } from '~/components/ArrowsRenderer';
import { ServiceMapArrow, AccessPointArrow } from '~/ui-layer/service-map/coordinates/arrow';
import { ServiceMapArrow } from '~/ui-layer/service-map/coordinates/arrow';
import { colors, sizes } from '~/ui/vars';

import * as helpers from './helpers';
Expand All @@ -22,14 +20,6 @@ export const ServiceMapArrowBody = observer(function ServiceMapArrowBody(props:
const endingFigures = useRef<SVGGElement | null>(null);
const innerArrows = useRef<SVGGElement | null>(null);

const arrow = mobx
.computed(() => {
if (!(props.arrow instanceof ServiceMapArrow)) return null;

return props.arrow;
})
.get();

const renderArrow = useCallback((arrow: ServiceMapArrow) => {
// NOTE: Here we use data bind with one element in array just to have
// NOTE: an access to enter/update sets (see d3 General Update Pattern).
Expand Down Expand Up @@ -85,7 +75,7 @@ export const ServiceMapArrowBody = observer(function ServiceMapArrowBody(props:
const startPlate = d3
.select(endingFigures.current!)
.selectAll<SVGPathElement, XY>('path.start-plate')
.data([arrow.start], _ => `${arrow.id}-start-plate`);
.data([arrow.start], () => `${arrow.id}-start-plate`);

startPlate.attr('d', helpers.svg.startPlatePath);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useEffect, useRef } from 'react';
import React, { useCallback } from 'react';
import { observer } from 'mobx-react';
import classnames from 'classnames';
import * as d3 from 'd3';
Expand Down Expand Up @@ -40,7 +40,7 @@ export const ServiceMapArrowDuckFeet = observer(function ServiceMapArrowDuckFeet
const endingConnector = d3
.select(target)
.selectAll<SVGCircleElement, XY>('circle.ending-connector')
.data([connectorPosition], _ => `${connectorId}-ending-connector`);
.data([connectorPosition], () => `${connectorId}-ending-connector`);

endingConnector.attr('cx', d => d.x).attr('cy', d => d.y);

Expand Down
1 change: 0 additions & 1 deletion src/components/ServiceMapCard/http-groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export class HTTPEndpointGroup {
public static createSorted(endpoints?: PartialConnections<L7Endpoint>): HTTPEndpointGroup[] {
if (endpoints == null) return [];

const pathGroups: Map<string, L7Endpoint[]> = new Map();
const sortedEndpoints: L7Endpoint[] = [];

endpoints?.get(L7Kind.HTTP)?.forEach(ep => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Teleport/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import _ from 'lodash';
import React, { useEffect, useLayoutEffect, useMemo, useState } from 'react';
import React, { useEffect, useState } from 'react';
import ReactDOM from 'react-dom';

export type Props = {
Expand Down
5 changes: 2 additions & 3 deletions src/components/TopBar/FlowsFilterInput.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { Button, Classes, MenuItem } from '@blueprintjs/core';
import { ItemRenderer, MultiSelect2 } from '@blueprintjs/select';
import { trim } from 'lodash';
import React, { useCallback, useState, memo } from 'react';
import classnames from 'classnames';
import React, { useCallback, useState } from 'react';

import { TagDirection } from './TagDirection';

import { FilterDirection, FilterEntry, FilterKind } from '~/domain/filtering';
import { FilterEntry, FilterKind } from '~/domain/filtering';
import { Labels } from '~/domain/labels';

import css from './FlowsFilterInput.scss';
Expand Down
2 changes: 1 addition & 1 deletion src/components/TopBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { observer } from 'mobx-react';
import { Verdict } from '~/domain/hubble';
import { FilterEntry } from '~/domain/filtering';
import { Status } from '~/domain/status';
import { DataMode, TransferState } from '~/domain/interactions';
import { TransferState } from '~/domain/interactions';
import { NamespaceDescriptor } from '~/domain/namespaces';

import { FlowsFilterInput } from './FlowsFilterInput';
Expand Down
Loading
Loading