Enable @typescript-eslint/non-nullable-type-assertion-style

[MajorLift]

Motivation

We currently ban non-nullable assertions (!) completely with the ESLint rule @typescript-eslint/no-non-null-assertion.

However, there are arguments for using ! assertions as a tool for improving type safety.

Explanation

The ESLint rule @typescript-eslint/non-nullable-type-assertion-style provides alerts when a as assertion on a nullable type could be replaced with a ! instead.

Enabling this rule wouldn't require disabling @typescript-eslint/no-non-null-assertion. We could still mandate ESLint disable comments when using ! to make it clear that they should be used with caution and as an exception rather than the norm.

In defense of !

1. ! is safer than as

! non-null assertions are generally preferred for requiring less code and being harder to fall out of sync as types change.
Source: https://typescript-eslint.io/rules/non-nullable-type-assertion-style/

as enforces a positive definition that specifies what a type is, while ! applies a negative definition, specifying what a type isn't.

In other words, as prevents any type other than the asserted one from being applied, even if a more accurate one could be inferred due to code changes. ! doesn't share this brittleness, as it allows any other type to be inferred in place of the existing one, only stepping in to strip | undefined, | null from the type.

For this reason, replacing as with ! where applicable may represent a net improvement for type safety.

const example: string[] | undefined = ...
const bad = (example as string[]).pop();
const good = example!.pop();

2. TypeScript inference on nullability is limited

Some examples where the code guarantees the non-nullability of a variable, but TypeScript fails to infer this fact.

1. Optional chaining

while (queue[0]?.origin === originOfCurrentBatch) {
  // `queue` and its head element are defined
  const nextEntry = queue.shift()!;
}

while (queue.length > 0) {
  // `queue` and its head element are defined
  const nextEntry = queue.shift()!;
}

2. Array index access

if (chainIds.length > NETWORK_CACHE_LIMIT.MAX) {
  const oldestChainId = chainIds.sort(
    (c1, c2) =>
      Number(this.state.chainStatus[c2]?.lastVisited) -
      Number(this.state.chainStatus[c1]?.lastVisited),
  )[NETWORK_CACHE_LIMIT.MAX];
  // `oldestChainId` will always be defined, as `chainIds` is guaranteed to have at least `NETWORK_CACHE_LIMIT.MAX` elements
  oldChainIds.push(oldestChainId!);
}

3. Index signature

const oldChainIds = Object.keys(chainIds).filter(
  (chainId) =>
    // `chainId` is of type `keyof typeof chainIds`, meaning `chainIds[chainId]` must be defined
    chainIds[chainId]!.lastVisited < currentTimestamp - NETWORK_CACHE_DURATION,
);

4. Assertions made in a different conditional branch

/*
 * @param options.currentVersion - The current version. Required if
 * `isReleaseCandidate` is set, but optional otherwise.
 */
export async function updateChangelog({
  currentVersion,
  isReleaseCandidate,
  tagPrefixes = ['v'],
}: {
  currentVersion?: string;
  isReleaseCandidate: boolean;
  tagPrefixes: [string, string[]];
}) {
  if (isReleaseCandidate && !currentVersion) {
    throw new Error(
      `A version must be specified if 'isReleaseCandidate' is set.`,
    );
  }
  // Due to the first null check, if `isReleaseCandidate` is truthy, `currentVersion` should always be truthy
  if (
    isReleaseCandidate &&
    await getMostRecentTag({ tagPrefixes }) === `${tagPrefixes[0]}${currentVersion!}` // `currentVersion` is inferred as 'string | undefined'
  ) {
    throw new Error(
      `Current version already has tag, which is unexpected for a release candidate.`,
    );
  }
...