Skip to content

Commit

Permalink
style(lint): use optional properties/fields/params
Browse files Browse the repository at this point in the history
  • Loading branch information
eventualbuddha committed Oct 18, 2021
1 parent 73964df commit 40aec93
Show file tree
Hide file tree
Showing 23 changed files with 323 additions and 48 deletions.
2 changes: 1 addition & 1 deletion apps/bmd/src/AppRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ interface CardState {
pauseProcessingUntilNoCardPresent: boolean
showPostVotingInstructions?: PostVotingInstructions
voterCardCreatedAt: number
tallyOnCard?: Optional<PrecinctScannerCardTally>
tallyOnCard?: PrecinctScannerCardTally
}

interface UserState {
Expand Down
9 changes: 2 additions & 7 deletions apps/bmd/src/pages/PollWorkerScreen.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -60,7 +55,7 @@ interface Props {
machineConfig: MachineConfig
printer: Printer
togglePollsOpen: () => void
tallyOnCard: Optional<PrecinctScannerCardTally>
tallyOnCard?: PrecinctScannerCardTally
clearTalliesOnCard: () => Promise<void>
}

Expand Down
4 changes: 1 addition & 3 deletions apps/bmd/src/utils/ScreenReader/AriaScreenReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions apps/bsd/src/AppRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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('/')
Expand Down
12 changes: 3 additions & 9 deletions apps/bsd/src/screens/AdvancedOptionsScreen.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion apps/bsd/test/renderInAppContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions apps/election-manager/src/contexts/AppContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
ElectionDefinition,
FullElectionTally,
FullElectionExternalTally,
Optional,
UserSession,
} from '@votingworks/types'
import { usbstick, NullPrinter, Printer } from '@votingworks/utils'
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}>
Expand Down
5 changes: 2 additions & 3 deletions apps/election-manager/test/renderInAppContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
ElectionDefinition,
FullElectionTally,
FullElectionExternalTally,
Optional,
UserSession,
} from '@votingworks/types'
import { usbstick, NullPrinter, Printer } from '@votingworks/utils'
Expand All @@ -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
Expand Down Expand Up @@ -63,7 +62,7 @@ interface RenderInAppContextParams {
) => Promise<void>
fullElectionExternalTallies?: FullElectionExternalTally[]
generateExportableTallies?: () => ExportableTallies
currentUserSession?: Optional<UserSession>
currentUserSession?: UserSession
attemptToAuthenticateAdminUser?: () => boolean
lockMachine?: () => undefined
machineConfig?: MachineConfig
Expand Down
7 changes: 3 additions & 4 deletions apps/module-scan/src/importer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
ElectionDefinition,
MarkAdjudications,
MarkThresholds,
Optional,
PageInterpretation,
} from '@votingworks/types'
import { ScannerStatus, ScanStatus } from '@votingworks/types/api/module-scan'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion apps/module-scan/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion apps/precinct-scanner/src/api/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
38 changes: 38 additions & 0 deletions libs/eslint-plugin-vx/docs/rules/gts-use-optionals.md
Original file line number Diff line number Diff line change
@@ -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) {
/**/
}
```
1 change: 1 addition & 0 deletions libs/eslint-plugin-vx/src/configs/recommended.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }],
Expand Down
135 changes: 135 additions & 0 deletions libs/eslint-plugin-vx/src/rules/gts-use-optionals.ts
Original file line number Diff line number Diff line change
@@ -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,
})
}
},
}
},
})
2 changes: 2 additions & 0 deletions libs/eslint-plugin-vx/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 40aec93

Please sign in to comment.