From 40aec939d613873eb6f5d1e04040819bf035b662 Mon Sep 17 00:00:00 2001
From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com>
Date: Mon, 18 Oct 2021 16:45:14 -0700
Subject: [PATCH 1/2] style(lint): use optional properties/fields/params

https://google.github.io/styleguide/tsguide.html#optionals-vs-undefined-type

Closes #1075
---
 apps/bmd/src/AppRoot.tsx                      |   2 +-
 apps/bmd/src/pages/PollWorkerScreen.tsx       |   9 +-
 .../utils/ScreenReader/AriaScreenReader.ts    |   4 +-
 apps/bsd/src/AppRoot.tsx                      |   4 +-
 .../bsd/src/screens/AdvancedOptionsScreen.tsx |  12 +-
 apps/bsd/test/renderInAppContext.tsx          |   2 +-
 .../src/contexts/AppContext.ts                |   3 +-
 .../src/screens/DefinitionContestsScreen.tsx  |   2 +-
 .../test/renderInAppContext.tsx               |   5 +-
 apps/module-scan/src/importer.ts              |   7 +-
 apps/module-scan/src/store.ts                 |   2 +-
 apps/precinct-scanner/src/api/config.ts       |   2 +-
 .../components/ExportResultsModal.test.tsx    |   1 -
 .../docs/rules/gts-use-optionals.md           |  38 +++++
 .../src/configs/recommended.ts                |   1 +
 .../src/rules/gts-use-optionals.ts            | 135 ++++++++++++++++++
 libs/eslint-plugin-vx/src/rules/index.ts      |   2 +
 .../tests/rules/gts-use-optionals.test.ts     | 120 ++++++++++++++++
 libs/hmpb-interpreter/src/utils/defined.ts    |   4 +-
 libs/lsd/src/cli/index.ts                     |   2 +-
 libs/lsd/src/util/geometry.ts                 |   4 +-
 libs/plustek-sdk/src/mocks.ts                 |   4 +-
 libs/utils/src/format.ts                      |   6 +-
 23 files changed, 323 insertions(+), 48 deletions(-)
 create mode 100644 libs/eslint-plugin-vx/docs/rules/gts-use-optionals.md
 create mode 100644 libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
 create mode 100644 libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts

diff --git a/apps/bmd/src/AppRoot.tsx b/apps/bmd/src/AppRoot.tsx
index 279f5c83b..a935b5fa9 100644
--- a/apps/bmd/src/AppRoot.tsx
+++ b/apps/bmd/src/AppRoot.tsx
@@ -92,7 +92,7 @@ interface CardState {
   pauseProcessingUntilNoCardPresent: boolean
   showPostVotingInstructions?: PostVotingInstructions
   voterCardCreatedAt: number
-  tallyOnCard?: Optional<PrecinctScannerCardTally>
+  tallyOnCard?: PrecinctScannerCardTally
 }
 
 interface UserState {
diff --git a/apps/bmd/src/pages/PollWorkerScreen.tsx b/apps/bmd/src/pages/PollWorkerScreen.tsx
index 771039c10..2ff1444a7 100644
--- a/apps/bmd/src/pages/PollWorkerScreen.tsx
+++ b/apps/bmd/src/pages/PollWorkerScreen.tsx
@@ -1,12 +1,7 @@
 import { DateTime } from 'luxon'
 import React, { useState, useEffect, useCallback } from 'react'
 
-import {
-  ElectionDefinition,
-  Optional,
-  Tally,
-  VotingMethod,
-} from '@votingworks/types'
+import { ElectionDefinition, Tally, VotingMethod } from '@votingworks/types'
 import {
   Button,
   ButtonList,
@@ -60,7 +55,7 @@ interface Props {
   machineConfig: MachineConfig
   printer: Printer
   togglePollsOpen: () => void
-  tallyOnCard: Optional<PrecinctScannerCardTally>
+  tallyOnCard?: PrecinctScannerCardTally
   clearTalliesOnCard: () => Promise<void>
 }
 
diff --git a/apps/bmd/src/utils/ScreenReader/AriaScreenReader.ts b/apps/bmd/src/utils/ScreenReader/AriaScreenReader.ts
index 84b9b208c..3b46b9b84 100644
--- a/apps/bmd/src/utils/ScreenReader/AriaScreenReader.ts
+++ b/apps/bmd/src/utils/ScreenReader/AriaScreenReader.ts
@@ -146,9 +146,7 @@ export default class AriaScreenReader implements ScreenReader {
       : this.describeElement(node)
   }
 
-  private cleanDescription(
-    description: string | undefined
-  ): string | undefined {
+  private cleanDescription(description?: string): string | undefined {
     if (!description) {
       return
     }
diff --git a/apps/bsd/src/AppRoot.tsx b/apps/bsd/src/AppRoot.tsx
index c00121080..d53ea7311 100644
--- a/apps/bsd/src/AppRoot.tsx
+++ b/apps/bsd/src/AppRoot.tsx
@@ -123,7 +123,7 @@ const App = ({ card, hardware }: AppRootProps): JSX.Element => {
     setMarkThresholds(await config.getMarkThresholdOverrides())
   }, [])
 
-  const updateElectionDefinition = async (e: Optional<ElectionDefinition>) => {
+  const updateElectionDefinition = async (e?: ElectionDefinition) => {
     setElectionDefinition(e)
     setElectionJustLoaded(true)
   }
@@ -264,7 +264,7 @@ const App = ({ card, hardware }: AppRootProps): JSX.Element => {
   }, [history, isTestMode, refreshConfig])
 
   const setMarkThresholdOverrides = useCallback(
-    async (markThresholdOverrides: Optional<MarkThresholds>) => {
+    async (markThresholdOverrides?: MarkThresholds) => {
       await config.setMarkThresholdOverrides(markThresholdOverrides)
       await refreshConfig()
       history.replace('/')
diff --git a/apps/bsd/src/screens/AdvancedOptionsScreen.tsx b/apps/bsd/src/screens/AdvancedOptionsScreen.tsx
index d2c591341..8ae94f947 100644
--- a/apps/bsd/src/screens/AdvancedOptionsScreen.tsx
+++ b/apps/bsd/src/screens/AdvancedOptionsScreen.tsx
@@ -1,8 +1,4 @@
-import {
-  ElectionDefinition,
-  MarkThresholds,
-  Optional,
-} from '@votingworks/types'
+import { ElectionDefinition, MarkThresholds } from '@votingworks/types'
 import React, { useCallback, useEffect, useState, useContext } from 'react'
 import Button from '../components/Button'
 import LinkButton from '../components/LinkButton'
@@ -23,10 +19,8 @@ interface Props {
   isTestMode: boolean
   isTogglingTestMode: boolean
   toggleTestMode: () => Promise<void>
-  setMarkThresholdOverrides: (
-    markThresholds: Optional<MarkThresholds>
-  ) => Promise<void>
-  markThresholds: Optional<MarkThresholds>
+  setMarkThresholdOverrides: (markThresholds?: MarkThresholds) => Promise<void>
+  markThresholds?: MarkThresholds
   electionDefinition: ElectionDefinition
 }
 
diff --git a/apps/bsd/test/renderInAppContext.tsx b/apps/bsd/test/renderInAppContext.tsx
index b73743279..98a524af5 100644
--- a/apps/bsd/test/renderInAppContext.tsx
+++ b/apps/bsd/test/renderInAppContext.tsx
@@ -8,7 +8,7 @@ import { Router } from 'react-router-dom'
 import AppContext from '../src/contexts/AppContext'
 
 interface RenderInAppContextParams {
-  route?: string | undefined
+  route?: string
   history?: MemoryHistory<any> // eslint-disable-line @typescript-eslint/no-explicit-any
   electionDefinition?: ElectionDefinition
   machineId?: string
diff --git a/apps/election-manager/src/contexts/AppContext.ts b/apps/election-manager/src/contexts/AppContext.ts
index 476e733e8..61050da93 100644
--- a/apps/election-manager/src/contexts/AppContext.ts
+++ b/apps/election-manager/src/contexts/AppContext.ts
@@ -3,7 +3,6 @@ import {
   ElectionDefinition,
   FullElectionTally,
   FullElectionExternalTally,
-  Optional,
   UserSession,
 } from '@votingworks/types'
 import { usbstick, NullPrinter, Printer } from '@votingworks/utils'
@@ -48,7 +47,7 @@ export interface AppContextInterface {
   ) => Promise<void>
   setIsTabulationRunning: React.Dispatch<React.SetStateAction<boolean>>
   generateExportableTallies: () => ExportableTallies
-  currentUserSession: Optional<UserSession>
+  currentUserSession?: UserSession
   attemptToAuthenticateAdminUser: (passcode: string) => boolean
   lockMachine: () => void
   machineConfig: MachineConfig
diff --git a/apps/election-manager/src/screens/DefinitionContestsScreen.tsx b/apps/election-manager/src/screens/DefinitionContestsScreen.tsx
index da67323cd..af4a8331c 100644
--- a/apps/election-manager/src/screens/DefinitionContestsScreen.tsx
+++ b/apps/election-manager/src/screens/DefinitionContestsScreen.tsx
@@ -89,7 +89,7 @@ const TextField = ({
   pattern?: string
   step?: number
   type?: 'text' | 'textarea' | 'number'
-  value: string | number | undefined
+  value?: string | number
 }) => (
   <StyledField>
     <label htmlFor={name}>
diff --git a/apps/election-manager/test/renderInAppContext.tsx b/apps/election-manager/test/renderInAppContext.tsx
index ffdbff011..9b3cb42ea 100644
--- a/apps/election-manager/test/renderInAppContext.tsx
+++ b/apps/election-manager/test/renderInAppContext.tsx
@@ -10,7 +10,6 @@ import {
   ElectionDefinition,
   FullElectionTally,
   FullElectionExternalTally,
-  Optional,
   UserSession,
 } from '@votingworks/types'
 import { usbstick, NullPrinter, Printer } from '@votingworks/utils'
@@ -35,7 +34,7 @@ export const eitherNeitherElectionDefinition = {
 }
 
 interface RenderInAppContextParams {
-  route?: string | undefined
+  route?: string
   history?: MemoryHistory<any> // eslint-disable-line @typescript-eslint/no-explicit-any
   castVoteRecordFiles?: CastVoteRecordFiles
   electionDefinition?: ElectionDefinition
@@ -63,7 +62,7 @@ interface RenderInAppContextParams {
   ) => Promise<void>
   fullElectionExternalTallies?: FullElectionExternalTally[]
   generateExportableTallies?: () => ExportableTallies
-  currentUserSession?: Optional<UserSession>
+  currentUserSession?: UserSession
   attemptToAuthenticateAdminUser?: () => boolean
   lockMachine?: () => undefined
   machineConfig?: MachineConfig
diff --git a/apps/module-scan/src/importer.ts b/apps/module-scan/src/importer.ts
index c085121ae..3dde52bff 100644
--- a/apps/module-scan/src/importer.ts
+++ b/apps/module-scan/src/importer.ts
@@ -5,7 +5,6 @@ import {
   ElectionDefinition,
   MarkAdjudications,
   MarkThresholds,
-  Optional,
   PageInterpretation,
 } from '@votingworks/types'
 import { ScannerStatus, ScanStatus } from '@votingworks/types/api/module-scan'
@@ -69,8 +68,8 @@ export async function saveImages(
 export default class Importer {
   private workspace: Workspace
   private scanner: Scanner
-  private sheetGenerator: BatchControl | undefined
-  private batchId: string | undefined
+  private sheetGenerator?: BatchControl
+  private batchId?: string
   private workerPool?: WorkerPool<workers.Input, workers.Output>
   private workerPoolProvider: () => WorkerPool<workers.Input, workers.Output>
   private interpreterReady = true
@@ -191,7 +190,7 @@ export default class Importer {
   }
 
   async setMarkThresholdOverrides(
-    markThresholds: Optional<MarkThresholds>
+    markThresholds?: MarkThresholds
   ): Promise<void> {
     debug('setting mark thresholds overrides to %s', markThresholds)
     await this.workspace.store.setMarkThresholdOverrides(markThresholds)
diff --git a/apps/module-scan/src/store.ts b/apps/module-scan/src/store.ts
index 8ad7afa63..92622eea9 100644
--- a/apps/module-scan/src/store.ts
+++ b/apps/module-scan/src/store.ts
@@ -413,7 +413,7 @@ export default class Store {
    * in the election definition.
    */
   async setMarkThresholdOverrides(
-    markThresholds: Optional<MarkThresholds>
+    markThresholds?: MarkThresholds
   ): Promise<void> {
     await this.setConfig(ConfigKey.MarkThresholdOverrides, markThresholds)
   }
diff --git a/apps/precinct-scanner/src/api/config.ts b/apps/precinct-scanner/src/api/config.ts
index 5ee2af7fe..aa1bed0db 100644
--- a/apps/precinct-scanner/src/api/config.ts
+++ b/apps/precinct-scanner/src/api/config.ts
@@ -125,7 +125,7 @@ export async function getCurrentPrecinctId(): Promise<
 }
 
 export async function setCurrentPrecinctId(
-  precinctId: Precinct['id'] | undefined
+  precinctId?: Precinct['id']
 ): Promise<void> {
   if (!precinctId) {
     await del('/config/precinct')
diff --git a/apps/precinct-scanner/src/components/ExportResultsModal.test.tsx b/apps/precinct-scanner/src/components/ExportResultsModal.test.tsx
index 15dd7a57f..e17ab5b70 100644
--- a/apps/precinct-scanner/src/components/ExportResultsModal.test.tsx
+++ b/apps/precinct-scanner/src/components/ExportResultsModal.test.tsx
@@ -1,7 +1,6 @@
 import React from 'react'
 
 import { render, fireEvent, waitFor } from '@testing-library/react'
-import fileDownload from 'js-file-download'
 import {
   electionSampleDefinition as electionDefinition,
   electionSampleDefinition,
diff --git a/libs/eslint-plugin-vx/docs/rules/gts-use-optionals.md b/libs/eslint-plugin-vx/docs/rules/gts-use-optionals.md
new file mode 100644
index 000000000..0cfbbf9b9
--- /dev/null
+++ b/libs/eslint-plugin-vx/docs/rules/gts-use-optionals.md
@@ -0,0 +1,38 @@
+# Use optional fields (on interfaces or classes) and parameters rather than a `|undefined` type. (`vx/gts-use-optionals`)
+
+This rule is from
+[Google TypeScript Style Guide section "Optionals vs `|undefined` type"](https://google.github.io/styleguide/tsguide.html#optionals-vs-undefined-type):
+
+> TypeScript supports a special construct for optional parameters and fields,
+> using `?`. Optional parameters implicitly include |undefined in their type.
+> However, they are different in that they can be left out when constructing a
+> value or calling a method. Use optional fields (on interfaces or classes) and
+> parameters rather than a `|undefined` type.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```ts
+interface CoffeeOrder {
+  sugarCubes: number
+  milk: Whole | LowFat | HalfHalf | undefined
+}
+
+function pourCoffee(volume: Milliliter | undefined) {
+  /* … */
+}
+```
+
+Examples of **correct** code for this rule:
+
+```ts
+interface CoffeeOrder {
+  sugarCubes: number
+  milk?: Whole | LowFat | HalfHalf
+}
+
+function pourCoffee(volume?: Milliliter) {
+  /* … */
+}
+```
diff --git a/libs/eslint-plugin-vx/src/configs/recommended.ts b/libs/eslint-plugin-vx/src/configs/recommended.ts
index 7844b9bf3..f86dabfeb 100644
--- a/libs/eslint-plugin-vx/src/configs/recommended.ts
+++ b/libs/eslint-plugin-vx/src/configs/recommended.ts
@@ -43,6 +43,7 @@ export = {
     'vx/gts-no-private-fields': 'error',
     'vx/gts-no-public-modifier': 'error',
     'vx/gts-parameter-properties': 'error',
+    'vx/gts-use-optionals': 'error',
     'vx/no-array-sort-mutation': 'error',
     'vx/no-assert-truthiness': 'error',
     'vx/no-floating-results': ['error', { ignoreVoid: true }],
diff --git a/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts b/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
new file mode 100644
index 000000000..4e7093f2c
--- /dev/null
+++ b/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
@@ -0,0 +1,135 @@
+import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/experimental-utils'
+import { createRule, isBindingName } from '../util'
+
+function isOptionalType(node: TSESTree.TypeNode): boolean {
+  if (node.type === AST_NODE_TYPES.TSUnionType) {
+    return node.types.some(
+      (elementType) =>
+        elementType.type === AST_NODE_TYPES.TSUndefinedKeyword ||
+        isOptionalType(elementType)
+    )
+  }
+
+  if (node.type === AST_NODE_TYPES.TSTypeReference) {
+    return (
+      node.typeName.type === AST_NODE_TYPES.Identifier &&
+      node.typeName.name === 'Optional'
+    )
+  }
+
+  return false
+}
+
+export default createRule({
+  name: 'gts-use-optionals',
+  meta: {
+    docs: {
+      description:
+        'Use optional fields (on interfaces or classes) and parameters rather than a |undefined type.',
+      category: 'Best Practices',
+      recommended: 'error',
+      suggestion: false,
+      requiresTypeChecking: false,
+    },
+    messages: {
+      useOptionalInterfaceProperties: `Use optional properties on interfaces rather than a |undefined type`,
+      useOptionalClassFields: `Use optional fields on classes rather than a |undefined type`,
+      useOptionalParams: `Use optional params rather than a |undefined type`,
+    },
+    schema: [],
+    type: 'problem',
+  },
+  defaultOptions: [],
+
+  create(context) {
+    function checkFunction(node: {
+      params: readonly TSESTree.Parameter[]
+    }): void {
+      const possibleViolations = node.params.map((param) => {
+        if (
+          isBindingName(param) &&
+          param.typeAnnotation &&
+          isOptionalType(param.typeAnnotation.typeAnnotation)
+        ) {
+          return param
+        }
+
+        if (
+          param.type === AST_NODE_TYPES.AssignmentPattern &&
+          param.left.typeAnnotation &&
+          isOptionalType(param.left.typeAnnotation.typeAnnotation)
+        ) {
+          return param
+        }
+
+        return undefined
+      })
+
+      let indexOfLastNonOptionalParam = -1
+      for (const [i, param] of node.params.entries()) {
+        if (isBindingName(param) && !param.optional) {
+          indexOfLastNonOptionalParam = i
+        }
+      }
+
+      for (const [i, param] of possibleViolations.entries()) {
+        if (
+          // do not report params that come before non-optional params
+          i < indexOfLastNonOptionalParam ||
+          !param
+        ) {
+          continue
+        }
+
+        context.report({
+          messageId: 'useOptionalParams',
+          node: param,
+        })
+      }
+    }
+
+    return {
+      ClassProperty(node: TSESTree.ClassProperty): void {
+        if (
+          node.typeAnnotation &&
+          isOptionalType(node.typeAnnotation.typeAnnotation)
+        ) {
+          context.report({
+            messageId: 'useOptionalClassFields',
+            node,
+          })
+        }
+      },
+
+      FunctionDeclaration: checkFunction,
+      FunctionExpression: checkFunction,
+      ArrowFunctionExpression: checkFunction,
+      TSFunctionType: checkFunction,
+      TSMethodSignature: checkFunction,
+
+      TSParameterProperty(node: TSESTree.TSParameterProperty): void {
+        if (
+          node.parameter.typeAnnotation &&
+          isOptionalType(node.parameter.typeAnnotation.typeAnnotation)
+        ) {
+          context.report({
+            messageId: 'useOptionalClassFields',
+            node,
+          })
+        }
+      },
+
+      TSPropertySignature(node: TSESTree.TSPropertySignature): void {
+        if (
+          node.typeAnnotation &&
+          isOptionalType(node.typeAnnotation.typeAnnotation)
+        ) {
+          context.report({
+            messageId: 'useOptionalInterfaceProperties',
+            node,
+          })
+        }
+      },
+    }
+  },
+})
diff --git a/libs/eslint-plugin-vx/src/rules/index.ts b/libs/eslint-plugin-vx/src/rules/index.ts
index 03dd2d324..e1e2186aa 100644
--- a/libs/eslint-plugin-vx/src/rules/index.ts
+++ b/libs/eslint-plugin-vx/src/rules/index.ts
@@ -7,6 +7,7 @@ import gtsNoImportExportType from './gts-no-import-export-type'
 import gtsNoPrivateFields from './gts-no-private-fields'
 import gtsNoPublicModifier from './gts-no-public-modifier'
 import gtsParameterProperties from './gts-parameter-properties'
+import gtsUseOptionals from './gts-use-optionals'
 import noArraySortMutation from './no-array-sort-mutation'
 import noAssertStringOrNumber from './no-assert-truthiness'
 import noFloatingVoids from './no-floating-results'
@@ -20,6 +21,7 @@ export default {
   'gts-no-private-fields': gtsNoPrivateFields,
   'gts-no-public-modifier': gtsNoPublicModifier,
   'gts-parameter-properties': gtsParameterProperties,
+  'gts-use-optionals': gtsUseOptionals,
   'no-array-sort-mutation': noArraySortMutation,
   'no-assert-truthiness': noAssertStringOrNumber,
   'no-floating-results': noFloatingVoids,
diff --git a/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts b/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts
new file mode 100644
index 000000000..28d0478bf
--- /dev/null
+++ b/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts
@@ -0,0 +1,120 @@
+import { ESLintUtils } from '@typescript-eslint/experimental-utils'
+import { join } from 'path'
+import rule from '../../src/rules/gts-use-optionals'
+
+const ruleTester = new ESLintUtils.RuleTester({
+  parserOptions: {
+    ecmaVersion: 2018,
+    tsconfigRootDir: join(__dirname, '../fixtures'),
+    project: './tsconfig.json',
+  },
+  parser: '@typescript-eslint/parser',
+})
+
+ruleTester.run('gts-no-use-optionals', rule, {
+  valid: [
+    {
+      code: 'interface A { a?: boolean }',
+    },
+    {
+      // index signatures do not allow optional properties
+      code: 'interface A { [key: string]: boolean | undefined }',
+    },
+    {
+      code: 'type A = { a?: boolean }',
+    },
+    {
+      code: 'class A { a?: boolean }',
+    },
+    {
+      code: 'function a(a?: boolean) {}',
+    },
+    {
+      code: 'function a(a: boolean | undefined, b: boolean) {}',
+    },
+    {
+      code: 'interface A { a(b?: boolean): void }',
+    },
+    {
+      code: 'interface A { a: (b?: boolean) => void }',
+    },
+  ],
+  invalid: [
+    {
+      code: 'interface A { a: boolean | undefined }',
+      errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
+    },
+    {
+      code: 'interface A { a?: boolean | undefined }',
+      errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
+    },
+    {
+      code: 'interface A { a: Optional<boolean> }',
+      errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
+    },
+    {
+      code: 'type A = { a: boolean | undefined }',
+      errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
+    },
+    {
+      code: 'type A = { a: Optional<boolean> }',
+      errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
+    },
+    {
+      code: 'class A { a: boolean | undefined }',
+      errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
+    },
+    {
+      code: 'class A { a: Optional<boolean> }',
+      errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
+    },
+    {
+      code: 'class A { constructor(private a: boolean | undefined) {} }',
+      errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
+    },
+    {
+      code: 'class A { constructor(private a: Optional<boolean>) {} }',
+      errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
+    },
+    {
+      code: 'function a(a: boolean | undefined) {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'function a(a: Optional<boolean>) {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'function a(a: boolean | undefined = true) {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'const a = function ({}: {} | undefined) {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'const a = function ({}: Optional<{}>) {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'const a = ([]: [0, 1] | undefined) => {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'const a = ([]: Optional<[0, 1]>) => {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'interface A { a(b: boolean | undefined): void }',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'interface A { a: (b: boolean | undefined) => void }',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+    {
+      code: 'function a(a: boolean | undefined, b?: boolean) {}',
+      errors: [{ messageId: 'useOptionalParams', line: 1 }],
+    },
+  ],
+})
diff --git a/libs/hmpb-interpreter/src/utils/defined.ts b/libs/hmpb-interpreter/src/utils/defined.ts
index 0edb8b620..1adb1e00d 100644
--- a/libs/hmpb-interpreter/src/utils/defined.ts
+++ b/libs/hmpb-interpreter/src/utils/defined.ts
@@ -1,10 +1,10 @@
-export default function defined<T>(value: T | undefined | null): T {
+export default function defined<T>(value?: T | null): T {
   if (value === null || value === undefined) {
     throw new Error('expected value to be defined')
   }
   return value
 }
 
-export function isDefined<T>(value: T | undefined | null): value is T {
+export function isDefined<T>(value?: T | null): value is T {
   return value !== null && value !== undefined
 }
diff --git a/libs/lsd/src/cli/index.ts b/libs/lsd/src/cli/index.ts
index 37a965f4e..b9c00d4ed 100644
--- a/libs/lsd/src/cli/index.ts
+++ b/libs/lsd/src/cli/index.ts
@@ -246,7 +246,7 @@ export async function processFile(
 
 export async function main(
   args: readonly string[],
-  stdout: NodeJS.WritableStream | undefined = process.stdout
+  stdout: NodeJS.WritableStream = process.stdout
 ): Promise<void> {
   const options = parseOptions(args)
 
diff --git a/libs/lsd/src/util/geometry.ts b/libs/lsd/src/util/geometry.ts
index 7059e0b7b..33e184f19 100644
--- a/libs/lsd/src/util/geometry.ts
+++ b/libs/lsd/src/util/geometry.ts
@@ -36,9 +36,7 @@ export function angle(x1: number, y1: number, x2: number, y2: number): number {
 export function approximatelyEqual(
   a: number,
   b: number,
-  {
-    relativeErrorFactor = 100,
-  }: { relativeErrorFactor?: number } | undefined = {}
+  { relativeErrorFactor = 100 }: { relativeErrorFactor?: number } = {}
 ): boolean {
   const max = Math.max(a, b)
   const diff = Math.abs(a - b)
diff --git a/libs/plustek-sdk/src/mocks.ts b/libs/plustek-sdk/src/mocks.ts
index 7573c371a..54c0fa2da 100644
--- a/libs/plustek-sdk/src/mocks.ts
+++ b/libs/plustek-sdk/src/mocks.ts
@@ -215,8 +215,8 @@ export class MockScannerClient implements ScannerClient {
     timeout,
   }: {
     status: PaperStatus
-    timeout?: number | undefined
-    interval?: number | undefined
+    timeout?: number
+    interval?: number
   }): Promise<GetPaperStatusResult | undefined> {
     debug('waitForStatus')
 
diff --git a/libs/utils/src/format.ts b/libs/utils/src/format.ts
index 5dc1e3c5b..04506c627 100644
--- a/libs/utils/src/format.ts
+++ b/libs/utils/src/format.ts
@@ -9,9 +9,7 @@ export function count(value: number): string {
 
 export const DEFAULT_LOCALE = 'en-US'
 
-export function localeLongDateAndTime(
-  time?: number | Date | undefined
-): string {
+export function localeLongDateAndTime(time?: number | Date): string {
   return new Intl.DateTimeFormat(DEFAULT_LOCALE, {
     weekday: 'long',
     month: 'long',
@@ -24,7 +22,7 @@ export function localeLongDateAndTime(
   }).format(time)
 }
 
-export function localeWeekdayAndDate(time?: number | Date | undefined): string {
+export function localeWeekdayAndDate(time?: number | Date): string {
   return new Intl.DateTimeFormat(DEFAULT_LOCALE, {
     weekday: 'long',
     month: 'long',

From c4cad2200e4411136b48a990068d8f99a490ad70 Mon Sep 17 00:00:00 2001
From: Brian Donovan <1938+eventualbuddha@users.noreply.github.com>
Date: Tue, 19 Oct 2021 12:05:05 -0700
Subject: [PATCH 2/2] feat(lint): add autofix support to `gts-use-optionals`

---
 .../src/rules/gts-use-optionals.ts            | 163 ++++++++++++++----
 .../tests/rules/gts-use-optionals.test.ts     |  28 +++
 2 files changed, 155 insertions(+), 36 deletions(-)

diff --git a/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts b/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
index 4e7093f2c..722f5f379 100644
--- a/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
+++ b/libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
@@ -1,23 +1,41 @@
 import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/experimental-utils'
+import { ReportFixFunction } from '@typescript-eslint/experimental-utils/dist/ts-eslint'
+import { strict as assert } from 'assert'
 import { createRule, isBindingName } from '../util'
 
-function isOptionalType(node: TSESTree.TypeNode): boolean {
+interface GetUndefinedUnionPartResult {
+  unionType: TSESTree.TSUnionType
+  undefinedType: TSESTree.TSUndefinedKeyword
+}
+
+function getUndefinedUnionPart(
+  node: TSESTree.TypeNode
+): GetUndefinedUnionPartResult | undefined {
   if (node.type === AST_NODE_TYPES.TSUnionType) {
-    return node.types.some(
-      (elementType) =>
-        elementType.type === AST_NODE_TYPES.TSUndefinedKeyword ||
-        isOptionalType(elementType)
-    )
+    for (const elementType of node.types) {
+      if (elementType.type === AST_NODE_TYPES.TSUndefinedKeyword) {
+        return { unionType: node, undefinedType: elementType }
+      }
+    }
   }
+}
 
-  if (node.type === AST_NODE_TYPES.TSTypeReference) {
-    return (
-      node.typeName.type === AST_NODE_TYPES.Identifier &&
-      node.typeName.name === 'Optional'
-    )
-  }
+interface GetOptionalTypeReferenceResult {
+  optionalType: TSESTree.TSTypeReference
+  wrappedType: TSESTree.TypeNode
+}
 
-  return false
+function getOptionalTypeReference(
+  node: TSESTree.TypeNode
+): GetOptionalTypeReferenceResult | undefined {
+  if (
+    node.type === AST_NODE_TYPES.TSTypeReference &&
+    node.typeName.type === AST_NODE_TYPES.Identifier &&
+    node.typeName.name === 'Optional' &&
+    node.typeParameters?.params.length === 1
+  ) {
+    return { optionalType: node, wrappedType: node.typeParameters.params[0] }
+  }
 }
 
 export default createRule({
@@ -31,6 +49,7 @@ export default createRule({
       suggestion: false,
       requiresTypeChecking: false,
     },
+    fixable: 'code',
     messages: {
       useOptionalInterfaceProperties: `Use optional properties on interfaces rather than a |undefined type`,
       useOptionalClassFields: `Use optional fields on classes rather than a |undefined type`,
@@ -42,24 +61,95 @@ export default createRule({
   defaultOptions: [],
 
   create(context) {
+    const sourceCode = context.getSourceCode()
+
+    function getFixFunction(
+      typeAnnotation: TSESTree.TSTypeAnnotation
+    ): ReportFixFunction | undefined {
+      const undefinedUnionResult = getUndefinedUnionPart(
+        typeAnnotation.typeAnnotation
+      )
+
+      if (undefinedUnionResult) {
+        return function* getFixes(fixer) {
+          const pipeBefore = sourceCode.getTokenBefore(
+            undefinedUnionResult.undefinedType
+          )
+          const pipeAfter = sourceCode.getTokenAfter(
+            undefinedUnionResult.undefinedType
+          )
+          /* istanbul ignore else */
+          if (pipeBefore?.value === '|') {
+            yield fixer.remove(pipeBefore)
+          } else if (pipeAfter?.value === '|') {
+            yield fixer.remove(pipeAfter)
+          } else {
+            assert.fail('could not find union type pipe around `undefined`')
+          }
+          yield fixer.remove(undefinedUnionResult.undefinedType)
+
+          const colonToken = sourceCode.getFirstToken(typeAnnotation)
+          assert.equal(colonToken?.value, ':')
+          const questionMarkToken = sourceCode.getTokenBefore(colonToken)
+          if (questionMarkToken?.value !== '?') {
+            yield fixer.insertTextBefore(colonToken, '?')
+          }
+        }
+      }
+
+      const optionalTypeReferenceResult = getOptionalTypeReference(
+        typeAnnotation.typeAnnotation
+      )
+
+      if (optionalTypeReferenceResult) {
+        return function* getFixes(fixer) {
+          yield fixer.remove(optionalTypeReferenceResult.optionalType.typeName)
+
+          const typeParamStartToken = sourceCode.getFirstTokenBetween(
+            optionalTypeReferenceResult.optionalType.typeName,
+            optionalTypeReferenceResult.wrappedType
+          )
+          assert.equal(typeParamStartToken?.value, '<')
+          yield fixer.remove(typeParamStartToken)
+
+          const typeParamEndToken = sourceCode.getLastToken(
+            typeAnnotation.typeAnnotation
+          )
+          assert.equal(typeParamEndToken?.value, '>')
+          yield fixer.remove(typeParamEndToken)
+
+          const colonToken = sourceCode.getFirstToken(typeAnnotation)
+          assert.equal(colonToken?.value, ':')
+          const questionMarkToken = sourceCode.getTokenBefore(colonToken)
+          if (questionMarkToken?.value !== '?') {
+            yield fixer.insertTextBefore(colonToken, '?')
+          }
+        }
+      }
+    }
+
     function checkFunction(node: {
       params: readonly TSESTree.Parameter[]
     }): void {
-      const possibleViolations = node.params.map((param) => {
-        if (
-          isBindingName(param) &&
-          param.typeAnnotation &&
-          isOptionalType(param.typeAnnotation.typeAnnotation)
-        ) {
-          return param
+      const possibleViolations = node.params.map<
+        [TSESTree.BindingName, ReportFixFunction] | undefined
+      >((param) => {
+        if (isBindingName(param) && param.typeAnnotation) {
+          const fix = getFixFunction(param.typeAnnotation)
+          if (fix) {
+            return [param, fix]
+          }
         }
 
         if (
           param.type === AST_NODE_TYPES.AssignmentPattern &&
-          param.left.typeAnnotation &&
-          isOptionalType(param.left.typeAnnotation.typeAnnotation)
+          isBindingName(param.left) &&
+          param.left.typeAnnotation
         ) {
-          return param
+          const fix = getFixFunction(param.left.typeAnnotation)
+          if (fix) {
+            return [param.left, fix]
+          }
         }
 
         return undefined
@@ -72,31 +162,32 @@ export default createRule({
         }
       }
 
-      for (const [i, param] of possibleViolations.entries()) {
+      for (const [i, paramAndFix] of possibleViolations.entries()) {
         if (
           // do not report params that come before non-optional params
           i < indexOfLastNonOptionalParam ||
-          !param
+          !paramAndFix
         ) {
           continue
         }
 
+        const [param, fix] = paramAndFix
         context.report({
           messageId: 'useOptionalParams',
           node: param,
+          fix,
         })
       }
     }
 
     return {
       ClassProperty(node: TSESTree.ClassProperty): void {
-        if (
-          node.typeAnnotation &&
-          isOptionalType(node.typeAnnotation.typeAnnotation)
-        ) {
+        const fix = node.typeAnnotation && getFixFunction(node.typeAnnotation)
+        if (fix) {
           context.report({
             messageId: 'useOptionalClassFields',
             node,
+            fix,
           })
         }
       },
@@ -108,25 +199,25 @@ export default createRule({
       TSMethodSignature: checkFunction,
 
       TSParameterProperty(node: TSESTree.TSParameterProperty): void {
-        if (
+        const fix =
           node.parameter.typeAnnotation &&
-          isOptionalType(node.parameter.typeAnnotation.typeAnnotation)
-        ) {
+          getFixFunction(node.parameter.typeAnnotation)
+        if (fix) {
           context.report({
             messageId: 'useOptionalClassFields',
             node,
+            fix,
           })
         }
       },
 
       TSPropertySignature(node: TSESTree.TSPropertySignature): void {
-        if (
-          node.typeAnnotation &&
-          isOptionalType(node.typeAnnotation.typeAnnotation)
-        ) {
+        const fix = node.typeAnnotation && getFixFunction(node.typeAnnotation)
+        if (fix) {
           context.report({
             messageId: 'useOptionalInterfaceProperties',
             node,
+            fix,
           })
         }
       },
diff --git a/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts b/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts
index 28d0478bf..5efb0113b 100644
--- a/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts
+++ b/libs/eslint-plugin-vx/tests/rules/gts-use-optionals.test.ts
@@ -38,82 +38,110 @@ ruleTester.run('gts-no-use-optionals', rule, {
     {
       code: 'interface A { a: (b?: boolean) => void }',
     },
+    {
+      // ignore this weird `Optional` with two type params, it isn't ours
+      code: 'interface A { a: Optional<boolean, string> }',
+    },
   ],
   invalid: [
     {
       code: 'interface A { a: boolean | undefined }',
+      output: 'interface A { a?: boolean   }',
+      errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
+    },
+    {
+      code: 'interface A { a: undefined | boolean }',
+      output: 'interface A { a?:   boolean }',
       errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
     },
     {
       code: 'interface A { a?: boolean | undefined }',
+      output: 'interface A { a?: boolean   }',
       errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
     },
     {
       code: 'interface A { a: Optional<boolean> }',
+      output: 'interface A { a?: boolean }',
       errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
     },
     {
       code: 'type A = { a: boolean | undefined }',
+      output: 'type A = { a?: boolean   }',
       errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
     },
     {
       code: 'type A = { a: Optional<boolean> }',
+      output: 'type A = { a?: boolean }',
       errors: [{ messageId: 'useOptionalInterfaceProperties', line: 1 }],
     },
     {
       code: 'class A { a: boolean | undefined }',
+      output: 'class A { a?: boolean   }',
       errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
     },
     {
       code: 'class A { a: Optional<boolean> }',
+      output: 'class A { a?: boolean }',
       errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
     },
     {
       code: 'class A { constructor(private a: boolean | undefined) {} }',
+      output: 'class A { constructor(private a?: boolean  ) {} }',
       errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
     },
     {
       code: 'class A { constructor(private a: Optional<boolean>) {} }',
+      output: 'class A { constructor(private a?: boolean) {} }',
       errors: [{ messageId: 'useOptionalClassFields', line: 1 }],
     },
     {
       code: 'function a(a: boolean | undefined) {}',
+      output: 'function a(a?: boolean  ) {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'function a(a: Optional<boolean>) {}',
+      output: 'function a(a?: boolean) {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'function a(a: boolean | undefined = true) {}',
+      output: 'function a(a?: boolean   = true) {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'const a = function ({}: {} | undefined) {}',
+      output: 'const a = function ({}?: {}  ) {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'const a = function ({}: Optional<{}>) {}',
+      output: 'const a = function ({}?: {}) {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'const a = ([]: [0, 1] | undefined) => {}',
+      output: 'const a = ([]?: [0, 1]  ) => {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'const a = ([]: Optional<[0, 1]>) => {}',
+      output: 'const a = ([]?: [0, 1]) => {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'interface A { a(b: boolean | undefined): void }',
+      output: 'interface A { a(b?: boolean  ): void }',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'interface A { a: (b: boolean | undefined) => void }',
+      output: 'interface A { a: (b?: boolean  ) => void }',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
     {
       code: 'function a(a: boolean | undefined, b?: boolean) {}',
+      output: 'function a(a?: boolean  , b?: boolean) {}',
       errors: [{ messageId: 'useOptionalParams', line: 1 }],
     },
   ],