Turn on type-aware lint rules (#2259)

david-crespo · web-flow · commit bf97ebc2366c · 2024-05-24T12:18:50.000-05:00
* turn on a couple of type-aware lint rules

* turn on recommended-type-checked except for all the ones I hate

* hm we actually sort of caught a bug and I'm not sure why eslint made this possible
diff --git a/.eslintrc.cjs b/.eslintrc.cjs
@@ -6,9 +6,13 @@ module.exports = {
   parser: '@typescript-eslint/parser',
   parserOptions: {
     warnOnUnsupportedTypeScriptVersion: false,
+    // this config is needed for type aware lint rules
+    project: true,
+    tsconfigRootDir: __dirname,
   },
   extends: [
     'eslint:recommended',
+    'plugin:@typescript-eslint/recommended-type-checked',
     'plugin:@typescript-eslint/strict',
     'plugin:@typescript-eslint/stylistic',
     'plugin:jsx-a11y/recommended',
@@ -45,6 +49,18 @@ module.exports = {
       'error',
       { argsIgnorePattern: '^_', varsIgnorePattern: '^_' },
     ],
+
+    // disabling the type-aware rules we don't like
+    // https://typescript-eslint.io/getting-started/typed-linting/
+    '@typescript-eslint/no-floating-promises': 'off',
+    '@typescript-eslint/no-misused-promises': 'off',
+    '@typescript-eslint/unbound-method': 'off',
+    '@typescript-eslint/no-unsafe-argument': 'off',
+    '@typescript-eslint/no-unsafe-assignment': 'off',
+    '@typescript-eslint/no-unsafe-call': 'off',
+    '@typescript-eslint/no-unsafe-member-access': 'off',
+    '@typescript-eslint/no-unsafe-return': 'off',
+
     eqeqeq: ['error', 'always', { null: 'ignore' }],
     'import/no-default-export': 'error',
     'import/no-unresolved': 'off', // plugin doesn't know anything
diff --git a/app/components/form/SideModalForm.tsx b/app/components/form/SideModalForm.tsx
@@ -7,7 +7,7 @@
  */
 import { useEffect, useId, type ReactNode } from 'react'
 import type { FieldValues, UseFormReturn } from 'react-hook-form'
-import { useNavigationType } from 'react-router-dom'
+import { NavigationType, useNavigationType } from 'react-router-dom'
 
 import type { ApiError } from '@oxide/api'
 
@@ -57,7 +57,7 @@ type SideModalFormProps<TFieldValues extends FieldValues> = {
  * any way to distinguish between fresh pageload and back/forward.
  */
 export function useShouldAnimateModal() {
-  return useNavigationType() === 'PUSH'
+  return useNavigationType() === NavigationType.Push
 }
 
 export function SideModalForm<TFieldValues extends FieldValues>({
diff --git a/app/components/form/fields/DateTimeRangePicker.spec.tsx b/app/components/form/fields/DateTimeRangePicker.spec.tsx
@@ -97,7 +97,7 @@ describe.skip('custom mode', () => {
     expect(screen.getByRole('button', { name: 'Load' })).toHaveClass('visually-disabled')
   })
 
-  it('clicking load after changing date changes range', async () => {
+  it('clicking load after changing date changes range', () => {
     const { setRange } = renderLastDay()
 
     // expect(screen.getByLabelText('Start time')).toHaveValue(dateForInput(subDays(now, 1)))
@@ -125,7 +125,7 @@ describe.skip('custom mode', () => {
     })
   })
 
-  it('clicking reset after changing inputs resets inputs', async () => {
+  it('clicking reset after changing inputs resets inputs', () => {
     const { setRange } = renderLastDay()
 
     // expect(screen.getByLabelText('Start time')).toHaveValue(dateForInput(subDays(now, 1)))
diff --git a/app/components/form/fields/ListboxField.tsx b/app/components/form/fields/ListboxField.tsx
@@ -27,7 +27,7 @@ export type ListboxFieldProps<
   className?: string
   label?: string
   required?: boolean
-  description?: string | React.ReactNode | React.ReactNode
+  description?: string | React.ReactNode
   tooltipText?: string
   control: Control<TFieldValues>
   disabled?: boolean
diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx
@@ -273,7 +273,7 @@ export function CreateImageSideModalForm() {
   // coordinating when to cleanup, we make cleanup idempotent by having it check
   // whether it has already been run, or more concretely before each action,
   // check whether it needs to be done
-  async function closeModal() {
+  function closeModal() {
     if (allDone) {
       backToImages()
       return
diff --git a/app/forms/vpc-edit.tsx b/app/forms/vpc-edit.tsx
@@ -40,7 +40,7 @@ export function EditVpcSideModalForm() {
   const onDismiss = () => navigate(pb.vpcs({ project }))
 
   const editVpc = useApiMutation('vpcUpdate', {
-    async onSuccess(vpc) {
+    onSuccess(vpc) {
       queryClient.invalidateQueries('vpcList')
       queryClient.setQueryData(
         'vpcView',
diff --git a/app/pages/SiloAccessPage.tsx b/app/pages/SiloAccessPage.tsx
@@ -139,7 +139,7 @@ export function SiloAccessPage() {
             doDelete: () =>
               updatePolicy.mutateAsync({
                 // we know policy is there, otherwise there's no row to display
-                body: deleteRole(row.id, siloPolicy!),
+                body: deleteRole(row.id, siloPolicy),
               }),
             label: (
               <span>
diff --git a/app/pages/project/access/ProjectAccessPage.tsx b/app/pages/project/access/ProjectAccessPage.tsx
@@ -169,7 +169,7 @@ export function ProjectAccessPage() {
               updatePolicy.mutateAsync({
                 path: { project },
                 // we know policy is there, otherwise there's no row to display
-                body: deleteRole(row.id, projectPolicy!),
+                body: deleteRole(row.id, projectPolicy),
               }),
             // TODO: explain that this will not affect the role inherited from
             // the silo or roles inherited from group membership. Ideally we'd
diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx
@@ -70,16 +70,10 @@ export const useMakeInstanceActions = (
           onActivate() {
             confirmAction({
               actionType: 'danger',
-              doAction: async () =>
-                stopInstance.mutate(instanceParams, {
+              doAction: () =>
+                stopInstance.mutateAsync(instanceParams, {
                   onSuccess: () =>
                     addToast({ title: `Stopping instance '${instance.name}'` }),
-                  onError: (error) =>
-                    addToast({
-                      variant: 'error',
-                      title: `Error stopping instance '${instance.name}'`,
-                      content: error.message,
-                    }),
                 }),
               modalTitle: 'Confirm stop instance',
               modalContent: (
@@ -89,7 +83,7 @@ export const useMakeInstanceActions = (
                   freed.
                 </p>
               ),
-              errorTitle: `Could not stop ${instance.name}`,
+              errorTitle: `Error stopping ${instance.name}`,
             })
           },
           disabled: !instanceCan.stop(instance) && (
diff --git a/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx b/app/pages/project/vpcs/VpcPage/tabs/VpcFirewallRulesTab.tsx
@@ -72,7 +72,9 @@ const staticColumns = [
     cell: (info) => {
       const { hosts, ports, protocols } = info.getValue()
       const children = [
-        ...(hosts || []).map((tv, i) => <TypeValueCell key={`${tv}-${i}`} {...tv} />),
+        ...(hosts || []).map((tv, i) => (
+          <TypeValueCell key={`host-${tv.type}-${tv.value}-${i}`} {...tv} />
+        )),
         ...(protocols || []).map((p, i) => <Badge key={`${p}-${i}`}>{p}</Badge>),
         ...(ports || []).map((p, i) => (
           <TypeValueCell key={`port-${p}-${i}`} type="Port" value={p} />
diff --git a/app/ui/lib/Button.tsx b/app/ui/lib/Button.tsx
@@ -87,7 +87,7 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
     return (
       <Wrap
         when={isDisabled && disabledReason}
-        with={<Tooltip content={disabledReason!} ref={ref} />}
+        with={<Tooltip content={disabledReason} ref={ref} />}
       >
         <button
           className={cn(buttonStyle({ size, variant }), className, {
diff --git a/app/util/file.spec.ts b/app/util/file.spec.ts
@@ -9,7 +9,7 @@ import { describe, expect, test } from 'vitest'
 
 import { readBlobAsBase64 } from './file'
 
-describe('readBlobAsBase64', async () => {
+describe('readBlobAsBase64', () => {
   test('works with zeros', async () => {
     const blob = new Blob([Buffer.alloc(10)])
     const text = await readBlobAsBase64(blob)
diff --git a/app/util/math.ts b/app/util/math.ts
@@ -43,6 +43,8 @@ export function percentage<T extends number | bigint>(top: T, bottom: T): number
   // be like 10^20 bigger than bottom. In any case, the nice thing is it seems
   // JS runtimes will not overflow when Number is given a huge arg, they just
   // convert to a huge number with reduced precision.
+  // type assertion is necessary according to TS. bug in ts-eslint
+  /* eslint-disable @typescript-eslint/no-unnecessary-type-assertion */
   return Number(((top as bigint) * 10_000n) / (bottom as bigint)) / 100
 }
 
diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts
@@ -362,7 +362,7 @@ export const handlers = makeHandlers({
     const instances = db.instances.filter((i) => i.project_id === project.id)
     return paginated(query, instances)
   },
-  async instanceCreate({ body, query }) {
+  instanceCreate({ body, query }) {
     const project = lookup.project(query)
 
     if (body.name === 'no-default-pool') {
@@ -646,7 +646,7 @@ export const handlers = makeHandlers({
     return json(instance, { status: 202 })
   },
   ipPoolList: ({ query }) => paginated(query, db.ipPools),
-  async ipPoolUtilizationView({ path }) {
+  ipPoolUtilizationView({ path }) {
     const pool = lookup.ipPool(path)
     const ranges = db.ipPoolRanges
       .filter((r) => r.ip_pool_id === pool.id)
diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts
@@ -117,11 +117,16 @@ export const errIfExists = <T extends Record<string, unknown>>(
       Object.entries(match).every(([key, value]) => item[key] === value)
     )
   ) {
-    const name = 'name' in match ? match.name : 'id' in match ? match.id : '<resource>'
+    const name =
+      'name' in match && match.name
+        ? match.name
+        : 'id' in match && match.id
+          ? match.id
+          : '<resource>'
     throw json(
       {
         error_code: 'ObjectAlreadyExists',
-        message: `already exists: ${resourceLabel} "${name}"`,
+        message: `already exists: ${resourceLabel} "${name.toString()}"`,
       },
       { status: 400 }
     )
diff --git a/package.json b/package.json
@@ -20,8 +20,8 @@
     "test": "vitest",
     "e2e": "playwright test",
     "e2ec": "playwright test --project=chrome",
-    "lint": "eslint --ext .js,.ts,.tsx,.json .",
-    "lint-fast": "eslint --cache --fix --ext .js,.ts,.tsx,.json .",
+    "lint": "eslint --ext .js,.ts,.tsx app test mock-api",
+    "lint-fast": "npm run lint -- --cache",
     "fmt": "prettier --cache --write . && npm run lint -- --fix",
     "openapi-gen-ts": "openapi-gen-ts",
     "prettier": "prettier",
@@ -140,6 +140,6 @@
     ]
   },
   "lint-staged": {
-    "*.{js,ts,tsx,json}": "eslint --cache --fix"
+    "*.{js,ts,tsx}": "eslint --cache --fix"
   }
 }
diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts
@@ -83,11 +83,11 @@ test('can copy an image ID to clipboard', async ({ page, browserName }) => {
 
   await page.goto('/images')
   await clickRowAction(page, 'ubuntu-22-04', 'Copy ID')
-  await expect(await clipboardText(page)).toEqual('ae46ddf5-a8d5-40fa-bcda-fcac606e3f9b')
+  expect(await clipboardText(page)).toEqual('ae46ddf5-a8d5-40fa-bcda-fcac606e3f9b')
 
   await page.goto('/projects/mock-project/images')
   await clickRowAction(page, 'image-4', 'Copy ID')
-  await expect(await clipboardText(page)).toEqual('d150b87d-eb20-49d2-8b56-ff5564670e8c')
+  expect(await clipboardText(page)).toEqual('d150b87d-eb20-49d2-8b56-ff5564670e8c')
 })
 
 test('can demote an image from silo', async ({ page }) => {
diff --git a/test/e2e/utils.ts b/test/e2e/utils.ts
@@ -13,7 +13,10 @@ import { MSW_USER_COOKIE } from '../../mock-api/msw/util'
 
 export * from '@playwright/test'
 
-export async function forEach(loc: Locator, fn: (loc0: Locator, i: number) => void) {
+export async function forEach(
+  loc: Locator,
+  fn: (loc0: Locator, i: number) => Promise<void>
+) {
   const count = await loc.count()
   for (let i = 0; i < count; i++) {
     await fn(loc.nth(i), i)
