Disallow magic numbers

In order to improve readability in our code, magic numbers are now forbidden by the linter. Current errors are fixed. Many are just ignored and have to be fixed successively. Signed-off-by: Benedikt Richter <[email protected]>
opossum-tool · Mar 2, 2023 · 54962ee · 54962ee
1 parent a544d0c
commit 54962ee
Show file tree

Hide file tree

Showing 36 changed files with 168 additions and 51 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -47,6 +47,10 @@ module.exports = {
     '@typescript-eslint/explicit-function-return-type': 2,
     '@typescript-eslint/await-thenable': 2,
     '@typescript-eslint/require-await': 2,
+    '@typescript-eslint/no-magic-numbers': [
+      'error',
+      { ignore: [-1, 0, 1, 2, 100] },
+    ],
     '@typescript-eslint/no-unnecessary-type-assertion': 2,
     'object-shorthand': 2,
     'react/prop-types': 'off',

diff --git a/src/ElectronBackend/input/__tests__/importFromFile.test.ts b/src/ElectronBackend/input/__tests__/importFromFile.test.ts
@@ -203,7 +203,10 @@ describe('Test of loading function', () => {
       corruptJsonPath
     );
 
-    expect(mainWindow.webContents.send).toHaveBeenCalledTimes(3);
+    const expectedNumberOfCalls = 3;
+    expect(mainWindow.webContents.send).toHaveBeenCalledTimes(
+      expectedNumberOfCalls
+    );
 
     expect(getMessageBoxForParsingError).toHaveBeenCalled();
     expect(getGlobalBackendState()).toEqual(expectedBackendState);

diff --git a/src/ElectronBackend/input/__tests__/parseInputData.test.ts b/src/ElectronBackend/input/__tests__/parseInputData.test.ts
@@ -53,7 +53,8 @@ describe('cleanNonExistentAttributions', () => {
       '/file1': ['attr2', 'attr4'],
       '/file3': ['attr4'],
     });
-    expect(mockCallback.mock.calls.length).toBe(3);
+    const expectedNumberOfCalls = 3;
+    expect(mockCallback.mock.calls.length).toBe(expectedNumberOfCalls);
     expect(mockCallback.mock.calls[0][1]).toContain('/file1');
     expect(mockCallback.mock.calls[1][1]).toContain('/file2');
     expect(mockCallback.mock.calls[2][1]).toContain('/file4');

diff --git a/src/ElectronBackend/main/__tests__/main.test.ts b/src/ElectronBackend/main/__tests__/main.test.ts
@@ -58,7 +58,8 @@ describe('The App backend', () => {
   it('calls ipc handler', async () => {
     await main();
 
-    expect(ipcMain.handle).toHaveBeenCalledTimes(10);
+    const expectedTotalNumberOfCalls = 10;
+    expect(ipcMain.handle).toHaveBeenCalledTimes(expectedTotalNumberOfCalls);
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
       1,
       IpcChannel.ConvertInputFile,
@@ -70,42 +71,49 @@ describe('The App backend', () => {
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       3,
       IpcChannel.OpenDotOpossumFile,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       4,
       IpcChannel.OpenFile,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       5,
       IpcChannel.SaveFile,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       6,
       IpcChannel.DeleteFile,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       7,
       IpcChannel.KeepFile,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       8,
       IpcChannel.SendErrorInformation,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       9,
       IpcChannel.ExportFile,
       expect.any(Function)
     );
     expect(ipcMain.handle).toHaveBeenNthCalledWith(
-      10,
+      expectedTotalNumberOfCalls,
       IpcChannel.OpenLink,
       expect.any(Function)
     );

diff --git a/src/ElectronBackend/output/__tests__/writeCsvToFile.test.ts b/src/ElectronBackend/output/__tests__/writeCsvToFile.test.ts
@@ -147,6 +147,7 @@ describe('writeCsvToFile', () => {
     };
 
     const expectedResources =
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       manyResources.slice(0, 225).join('\n') +
       ' ... (resources shortened, 25 paths are not displayed)';
 
@@ -438,8 +439,9 @@ describe('writeCsvToFile', () => {
       },
     };
 
+    const maxLength = 30000;
     const expectedLicenseText =
-      testLicenseText.substring(0, 30000) + '... (text shortened)';
+      testLicenseText.substring(0, maxLength) + '... (text shortened)';
 
     const temporaryPath: string = createTempFolder();
     const csvPath = path.join(upath.toUnix(temporaryPath), 'test.csv');

diff --git a/src/Frontend/Components/AttributionColumn/__tests__/attribution-column-helpers.test.ts b/src/Frontend/Components/AttributionColumn/__tests__/attribution-column-helpers.test.ts
@@ -11,12 +11,20 @@ import {
 } from '../attribution-column-helpers';
 
 describe('The AttributionColumn helpers', () => {
+  const windowHeight = 1080;
+
   it('getLicenseTextMaxRows in audit view', () => {
-    expect(getLicenseTextMaxRows(1080, View.Audit)).toEqual(29);
+    const expectedLicenseTextMaxRows = 29;
+    expect(getLicenseTextMaxRows(windowHeight, View.Audit)).toEqual(
+      expectedLicenseTextMaxRows
+    );
   });
 
   it('getLicenseTextMaxRows in attribution view', () => {
-    expect(getLicenseTextMaxRows(1080, View.Attribution)).toEqual(31);
+    const expectedLicenseTextMaxRows = 31;
+    expect(getLicenseTextMaxRows(windowHeight, View.Attribution)).toEqual(
+      expectedLicenseTextMaxRows
+    );
   });
 
   it('selectedPackageIsResolved returns true', () => {

diff --git a/src/Frontend/Components/AttributionColumn/attribution-column-helpers.ts b/src/Frontend/Components/AttributionColumn/attribution-column-helpers.ts
@@ -53,8 +53,10 @@ export function getLicenseTextMaxRows(
 ): number {
   const heightOfTextBoxes = 480;
   const heightOfNonLicenseTextComponents =
+    // eslint-disable-next-line @typescript-eslint/no-magic-numbers
     heightOfTextBoxes + (view === View.Audit ? 34 : 0);
   const licenseTextMaxHeight = windowHeight - heightOfNonLicenseTextComponents;
+  // eslint-disable-next-line @typescript-eslint/no-magic-numbers
   return Math.floor(licenseTextMaxHeight / 19);
 }
 
@@ -292,6 +294,7 @@ export function useRows(
   commentRows: number;
 } {
   const [isLicenseTextShown, setIsLicenseTextShown] = useState<boolean>(false);
+  // eslint-disable-next-line @typescript-eslint/no-magic-numbers
   const reduceRowsCount = smallerLicenseTextOrCommentField ? 5 : 1;
   const licenseTextRows =
     getLicenseTextMaxRows(useWindowHeight(), view) - reduceRowsCount;
@@ -300,6 +303,7 @@ export function useRows(
     setIsLicenseTextShown(false);
   }, [resetViewIfThisIdChanges]);
 
+  // eslint-disable-next-line @typescript-eslint/no-magic-numbers
   const copyrightRows = isLicenseTextShown ? 1 : 6;
   const commentRows = isLicenseTextShown
     ? 1

diff --git a/src/Frontend/Components/AttributionView/AttributionView.tsx b/src/Frontend/Components/AttributionView/AttributionView.tsx
@@ -58,6 +58,7 @@ export function AttributionView(): ReactElement {
     setShowMultiselect(!showMultiSelect);
   }
 
+  // eslint-disable-next-line @typescript-eslint/no-magic-numbers
   const countAndSearchAndFilterOffset = showMultiSelect ? 137 : 80;
 
   return (

diff --git a/src/Frontend/Components/AttributionWizardPopup/attribution-wizard-popup-helpers.tsx b/src/Frontend/Components/AttributionWizardPopup/attribution-wizard-popup-helpers.tsx
@@ -102,7 +102,8 @@ export function getAttributionWizardListItems(
 }
 
 function getCountText(count: number, totalAttributeCount: number): string {
-  const percentage = (count / totalAttributeCount) * 100;
+  const percentageFactor = 100;
+  const percentage = (count / totalAttributeCount) * percentageFactor;
   return percentage < 1
     ? `${count} (< 1%)`
     : `${count} (${percentage.toFixed(0)}%)`;

diff --git a/src/Frontend/Components/BackendCommunication/__tests__/BackendCommunication.test.tsx b/src/Frontend/Components/BackendCommunication/__tests__/BackendCommunication.test.tsx
@@ -16,7 +16,8 @@ import { Attributions, ExportType } from '../../../../shared/shared-types';
 describe('BackendCommunication', () => {
   it('renders an Open file icon', () => {
     renderComponentWithStore(<BackendCommunication />);
-    expect(window.electronAPI.on).toHaveBeenCalledTimes(11);
+    const expectedNumberOfCalls = 11;
+    expect(window.electronAPI.on).toHaveBeenCalledTimes(expectedNumberOfCalls);
     expect(window.electronAPI.on).toHaveBeenCalledWith(
       AllowedFrontendChannels.FileLoaded,
       expect.anything()

diff --git a/src/Frontend/Components/ContextMenu/ContextMenu.tsx b/src/Frontend/Components/ContextMenu/ContextMenu.tsx
@@ -128,6 +128,7 @@ export function ContextMenu(props: ContextMenuProps): ReactElement | null {
     }
     setAnchorPosition({
       left: event.clientX - 2,
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       top: event.clientY - 4,
     });
     setAnchorElement(event.currentTarget);

diff --git a/src/Frontend/Components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx b/src/Frontend/Components/ErrorBoundary/__tests__/ErrorBoundary.test.tsx
@@ -44,7 +44,10 @@ describe('ErrorBoundary', () => {
       </ErrorBoundary>
     );
 
-    expect(window.electronAPI.sendErrorInformation).toHaveBeenCalledTimes(3);
+    const expectedNumberOfCalls = 3;
+    expect(window.electronAPI.sendErrorInformation).toHaveBeenCalledTimes(
+      expectedNumberOfCalls
+    );
 
     expect(window.electronAPI.on).toHaveBeenCalledTimes(1);
     expect(window.electronAPI.on).toHaveBeenCalledWith(

diff --git a/...Components/FetchLicenseInformationButton/__tests__/FetchLicenseInformationButton.test.tsx b/...Components/FetchLicenseInformationButton/__tests__/FetchLicenseInformationButton.test.tsx
@@ -6,10 +6,10 @@
 import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 import React, { ReactElement, ReactNode } from 'react';
 import {
-  FetchLicenseInformationButton,
-  FetchStatus,
   FETCH_DATA_BUTTON_DISABLED_TOOLTIP,
   FETCH_DATA_TOOLTIP,
+  FetchLicenseInformationButton,
+  FetchStatus,
   useFetchPackageInfo,
 } from '../FetchLicenseInformationButton';
 import {
@@ -66,8 +66,9 @@ describe('FetchLicenseInformationButton', () => {
 
     it("shows fetching error 'Request failed with status code 404'", () => {
       const MOCK_URL = 'https://github.com/opossum-tool/Oposs';
+      const notFoundStatus = 404;
 
-      axiosMock.onGet(MOCK_URL).replyOnce(404, {});
+      axiosMock.onGet(MOCK_URL).replyOnce(notFoundStatus, {});
 
       renderComponentWithStore(
         <FetchLicenseInformationButton url={MOCK_URL} disabled={false} />
@@ -84,8 +85,9 @@ describe('FetchLicenseInformationButton', () => {
 
     it('shows fetch data tooltip after successful fetch', () => {
       const MOCK_URL = 'https://pypi.org/project/pip';
+      const okStatus = 200;
 
-      axiosMock.onGet(MOCK_URL).replyOnce(200, {
+      axiosMock.onGet(MOCK_URL).replyOnce(okStatus, {
         license: { spdx_id: 'Apache-2.0' },
         content: 'TGljZW5zZSBUZXh0', // "License Text" in base64
         html_url: 'https://github.com/opossum-tool/OpossumUI/blob/main/LICENSE',
@@ -144,7 +146,8 @@ describe('useFetchPackageInfo', () => {
   });
 
   it('fetches data', async () => {
-    axiosMock.onGet(GITHUB_URL).replyOnce(200, {
+    const okStatus = 200;
+    axiosMock.onGet(GITHUB_URL).replyOnce(okStatus, {
       license: { spdx_id: 'Apache-2.0' },
       content: 'TGljZW5zZSBUZXh0', // "License Text" in base64
       html_url: 'https://github.com/opossum-tool/OpossumUI/blob/main/LICENSE',

diff --git a/src/Frontend/Components/FileSearchPopup/FileSearchPopup.tsx b/src/Frontend/Components/FileSearchPopup/FileSearchPopup.tsx
@@ -18,6 +18,7 @@ export function FileSearchPopup(): ReactElement {
   const fileSearchPopupOffset = 260;
   const maxPossibleHeightInPx = useWindowHeight() - fileSearchPopupOffset;
   const resourceListMaxHeightInPx =
+    // eslint-disable-next-line @typescript-eslint/no-magic-numbers
     500 > maxPossibleHeightInPx ? maxPossibleHeightInPx : 500;
 
   function close(): void {

diff --git a/src/Frontend/Components/FileSearchPopup/__tests__/FileSearchPopup.test.tsx b/src/Frontend/Components/FileSearchPopup/__tests__/FileSearchPopup.test.tsx
@@ -43,9 +43,11 @@ describe('FileSearch popup ', () => {
   });
 
   each([
+    // eslint-disable-next-line @typescript-eslint/no-magic-numbers
     ['eagleBlu', 4],
     ['test.js', 2],
     ['non-existing-file', 0],
+    // eslint-disable-next-line @typescript-eslint/no-magic-numbers
     ['eagleblu', 4],
   ]).it(
     'search for %s results in %s results',
@@ -63,6 +65,7 @@ describe('FileSearch popup ', () => {
         jest.advanceTimersByTime(debounceWaitTimeInMs);
       });
 
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       expect(screen.queryAllByText('/', { exact: false })).toHaveLength(9);
 
       fireEvent.change(screen.getByRole('searchbox'), {
@@ -104,6 +107,7 @@ describe('FileSearch popup ', () => {
 
     jest.advanceTimersByTime(smallWaitTimeInMs);
 
+    // eslint-disable-next-line @typescript-eslint/no-magic-numbers
     expect(screen.queryAllByText('/', { exact: false })).toHaveLength(9);
 
     act(() => {
@@ -134,8 +138,10 @@ describe('FileSearch popup ', () => {
       jest.advanceTimersByTime(debounceWaitTimeInMs);
     });
 
+    // eslint-disable-next-line @typescript-eslint/no-magic-numbers
     expect(screen.queryAllByText('/', { exact: false })).toHaveLength(4);
     expect(screen.queryAllByText('/eagleBlu/', { exact: false })).toHaveLength(
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       4
     );
   });

diff --git a/src/Frontend/Components/Filter/FilterMultiSelect.tsx b/src/Frontend/Components/Filter/FilterMultiSelect.tsx
@@ -56,6 +56,7 @@ const classes = {
   },
   dropdownStyle: {
     '& paper': {
+      // eslint-disable-next-line @typescript-eslint/no-magic-numbers
       maxHeight: `${ITEM_HEIGHT * 4.5 + ITEM_PADDING_TOP}px`,
       left: '7px !important',
     },

diff --git a/src/Frontend/Components/List/List.tsx b/src/Frontend/Components/List/List.tsx
@@ -34,7 +34,8 @@ function maxHeightWasGiven(
 }
 
 export function List(props: ListProps): ReactElement {
-  const cardHeight = props.cardVerticalDistance || 24;
+  const defaultCardHeight = 24;
+  const cardHeight = props.cardVerticalDistance || defaultCardHeight;
   const maxHeight = maxHeightWasGiven(props.max)
     ? props.max.height
     : props.max.numberOfDisplayedItems * cardHeight;

diff --git a/src/Frontend/Components/ListCard/ListCard.tsx b/src/Frontend/Components/ListCard/ListCard.tsx
@@ -167,14 +167,16 @@ interface ListCardProps {
 
 export function ListCard(props: ListCardProps): ReactElement | null {
   function getDisplayedCount(): string {
+    const digitsInAThousand = 4;
+    const digitsInAMillion = 7;
     const count = props.count ? props.count.toString() : '';
 
-    if (count.length < 4) {
+    if (count.length < digitsInAThousand) {
       return count;
-    } else if (count.length < 7) {
-      return `${count.slice(0, -3)}k`;
+    } else if (count.length < digitsInAMillion) {
+      return `${count.slice(0, -(digitsInAThousand - 1))}k`;
     } else {
-      return `${count.slice(0, -6)}M`;
+      return `${count.slice(0, -(digitsInAMillion - 1))}M`;
     }
   }
 

diff --git a/src/Frontend/Components/PackageSearchPopup/PackageSearchPopup.tsx b/src/Frontend/Components/PackageSearchPopup/PackageSearchPopup.tsx
@@ -1,4 +1,6 @@
 // SPDX-FileCopyrightText: Nico Carl <[email protected]>
+// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates
+// SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
 //
 // SPDX-License-Identifier: Apache-2.0
 
@@ -83,7 +85,11 @@ export function PackageSearchPopup(): ReactElement {
   const [currentSearchTerm, setCurrentSearchTerm] = useState<string>(
     getInitialSearchTerm(temporaryPackageInfo)
   );
-  const debouncedSearchTerm = useDebounceInput(currentSearchTerm, 500);
+  const debounceDelayInMs = 500;
+  const debouncedSearchTerm = useDebounceInput(
+    currentSearchTerm,
+    debounceDelayInMs
+  );
   const { isLoading, data, isError, error } = useQuery(
     ['clearlyDefinedPackageSearch', debouncedSearchTerm],
     () => searchPackagesOnClearlyDefined(debouncedSearchTerm),

diff --git a/src/Frontend/Components/PackageSearchPopup/__tests__/ClearlyDefinedPackageCard.test.tsx b/src/Frontend/Components/PackageSearchPopup/__tests__/ClearlyDefinedPackageCard.test.tsx
@@ -1,4 +1,6 @@
 // SPDX-FileCopyrightText: Nico Carl <[email protected]>
+// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates
+// SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
 //
 // SPDX-License-Identifier: Apache-2.0
 
@@ -23,9 +25,11 @@ describe('ClearlyDefinedPackageCard', () => {
   });
   const testCoordinate = 'sqlalchemy';
   const definitionEndpoint = `https://api.clearlydefined.io/definitions/${testCoordinate}`;
+  const okStatus = 200;
+  const notFoundStatus = 404;
 
   it('renders after successful fetch', async () => {
-    axiosMock.onGet(definitionEndpoint).replyOnce(200, {
+    axiosMock.onGet(definitionEndpoint).replyOnce(okStatus, {
       licensed: {
         declared: 'MIT',
         facets: {
@@ -82,7 +86,7 @@ describe('ClearlyDefinedPackageCard', () => {
   it('shows error message when fetch fails', async () => {
     // suppress output to console
     jest.spyOn(console, 'error').mockImplementation(() => {});
-    axiosMock.onGet(definitionEndpoint).replyOnce(404);
+    axiosMock.onGet(definitionEndpoint).replyOnce(notFoundStatus);
 
     renderComponentWithStore(
       <QueryClientProvider client={queryClient}>