Merge pull request #1442 from benedikt-richter/warn_for_magic_numbers

Disallow magic numbers
opossum-tool · Mar 2, 2023 · 107d6cc · 107d6cc
2 parents a544d0c + 54962ee
commit 107d6cc
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}>