danielnixon/eslint-plugin-total-functions

An ESLint plugin to enforce the use of total functions (and prevent the use of partial functions) in TypeScript.

eslint-plugin-total-functions's Issues

New rule: ban index signatures entirely

Index signatures lead to all sorts of unsoundness and you're better off using a real Map in almost every case.

New rule: force mutable function parameters to be invariant

https://stackblitz.com/edit/unsound-typescript

• unsoundArray - could be fixed by forcing mutable types to be invariant
• unsoundBivarianceCo - fixed by --strictFunctionTypes
• unsoundBivarianceContra - could be fixed by forcing mutable types to be invariant

Add option to no-unsafe-type-assertion rule to ban `as` entirely...

...except for as const and as unknown.

false positive? assign readonly tuple to (compatible) readonly array

See e.g. oaf-project/oaf-side-effects@9fa851f

Document rules

[no-unsafe-subscript] doesn't catch some cases due to definite assignment analysis

Observe this annoying case:

const arr: readonly number[] = [];
const foo: number | undefined = arr[0]; // NOT flagged by no-unsafe-subscript
foo.toString(); // compiles; explodes at runtime

This is caused by definite assignment analysis. Even though foo has an explicit type annotation of number | undefined, TypeScript determines that foo cannot be undefined because it is "definitely assigned". toString() then explodes at runtime.

Our no-unsafe-subscript rule currently doesn't flag const foo: number | undefined = arr[0] because the contextual type of the expression arr[0] is number | undefined (the type of the value it's being assigned to).

This demonstrates that we can't assume the array access is safe even when the contextual type contains undefined -- at least for assignment expressions, which as far as I can tell are the only place that definite assignment analysis applies.

We will have to update no-unsafe-subscript to either ignore the contextual type in all cases (which would cause lots of pain and what amount to false positives) or only in assignment expressions (if possible).

🤦

no-unsafe-subscript doesn't catch non-computed property access

See e.g.
https://www.reddit.com/r/typescript/comments/ibbl0b/dictionary_object_does_not_give_undefined_as/

snyk protect

https://support.snyk.io/hc/en-us/articles/360003812558-Protect-your-code-with-patches

Leverage type information in no-array-subscript rule

The no-array-subscript rule currently forbids all array index access.

Leveraging type information could allow us to ignore certain usages that are known at compile time to be safe.

https://github.com/typescript-eslint/typescript-eslint#can-we-write-rules-which-leverage-type-information

Allow non-literal (but const) keys

This shouldn't be flagged:

const arr = [0, 1, 2] as const; const key = 0 as const; const foo = arr[key];

Allow array destructuring if array type includes `undefined`

Optional Array Access - Should be Valid

The primary way I would like to access an array value is with the ?. (optional chain operator) to access a value from an array:

        const arr: ReadonlyArray<number> = [];
        const foo = arr?.[0]; 
        // Type is number | undefined;

This is essentially the same as:

        const foo = arr[0] as number | undefined;

So this should be a valid pattern, not an invalid pattern.

From: https://github.com/danielnixon/eslint-plugin-total-functions/blob/master/src/rules/no-array-subscript.test.ts

In the invalid patterns:

// optional member array access
    {
      filename: "file.ts",
      code: `
        const arr: ReadonlyArray<number> = [];
        const foo = arr?.[0];
      `,
      errors: [
        {
          messageId: "errorStringGeneric",
          type: AST_NODE_TYPES.OptionalMemberExpression,
        },
      ],
    },

Am I missing something?

[require-strict-mode] enforce individual strict flags are not false

It's not enough to check that strict is true, we need to make sure the individual strict mode flags (strictNullChecks and the rest) are either undefined (in which case strict applies) or true but never false, even if strict is true.

Prevent infinite recursion in no-unsafe-assignment rule

Replace the hack from 8d36f41 with a real solution that keeps track of seen types

Don't flag numeric properties on objects

declare const grey: {
  50: '#fafafa';
  100: '#f5f5f5';
}

// Shouldn't be flagged.
const foo = grey[200];

Assess other typesystem holes

e.g. https://medium.com/free-code-camp/typescript-and-its-types-f509d799947d

And create rules (or tickets to create rules) as appropriate

TSX test cases

... and any relaxation of the rules to allow safe usages required in response

[no-unsafe-assignment] doesn't handle interface implementation

From microsoft/TypeScript#18770 (comment)

type I = {
  readonly a: string;
};

// eslint-disable-next-line functional/no-class
class C implements I {
  // This should be flagged by no-unsafe-assignment or another (new) rule
  // eslint-disable-next-line functional/prefer-readonly-type
  a = "";
}

const D = new C();

// eslint-disable-next-line functional/no-expression-statement, functional/immutable-data
D.a = "something";

total-functions/no-array-destructuring false positive with string literals

The code that reproduces the error:

interface Response {
  'error-code': number
}

function getError(
  { 'error-code': errorCode }: Response // <-- total-functions/no-array-destructuring
): string {
  return errorCode.toString()
}

There should be no total-functions/no-array-destructuring in this code.
For normal properties (not quoted), everything works fine.

I am using the library version:

• [email protected]
• [email protected]

Eslint config:

extends: [
  ...,
  'plugin:total-functions/recommended',
],
plugins: [
  ...,
  'total-functions'
]

Without rewriting the rules for eslint-plugin-total-functions.

`as` can lead to surprise mutation

Even with both the rules no-unsafe-assignment and no-unsafe-type-assertion enabled, this isn't complained about:

type MutableA = { a: string };
type ReadonlyA = { readonly a: string };

const ro: ReadonlyA = { a: "" };
const mut = ro as MutableA; // This shouldn't be allowed

It's not immediately clear which of those two rules should catch this, but at least one of them should.

Rename rules

no-array-subscript => no-unsafe-subscript
no-array-destructuring => no-unsafe-destructuring

(because they also cover objects and now only ban unsafe cases)

New rule: enforce strict mode

New rule: forbid mutables types and recommend readonly-types as replacement

https://github.com/danielnixon/readonly-types

New rule: Ban functions that throw?

• new URL("asdf") (see https://github.com/danielnixon/readonly-types/blob/master/src/index.ts#L75)
• decodeURIComponent('%')
• "".normalize("asdf")
• others? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects

[no-unsafe-assignment] Add option to catch mutable->readonly assignment

The no-unsafe-assignment rule currently only flags readonly->mutable assignment.

Add an option (off by default) to also catch the reverse: mutable->readonly.

TypeScript 4 support

Need to update no-unsafe-assignment to cover the new short-circuiting assignment operators.

Allow array subscript access if return value is immediately assigned to a type that includes undefined

This would allow something like this without complaining:

const last: <A>(array: ReadonlyArray<A>) => A | undefined = (a) => a[a.length - 1];

These are cases where the partiality isn't really observable, similar to the ignoreImmediateMutation option in https://github.com/jonaskello/eslint-plugin-functional/blob/master/docs/rules/immutable-data.md#options

We probably want an option to disable this too.

Implement fixer for no-array-subscript eslint rule

Replace it with the get function from https://github.com/danielnixon/total-functions

Allow type widening in no-unsafe-type-assertion

Type assertions can be used to widen types. The no-unsafe-type-assertion rule currently flags those usages. It probably shouldn't.

E.g. these should pass the rule:

const foo = "foo" as string;
const bar = 1 as number;

New rule: forbid array destructuring if the rhs is not a tuple

New rule: ban Node.parentElement

microsoft/TypeScript#4689

[no-unsafe-subscript] Rename to no-unsafe-member-access

Since 8c86fed, no-unsafe-subscript now also catches unsafe non-computed (regular, non-subscript) member access (yes, sadly this is a thing with index signatures / records) so it probably deserves a yet broader name (we've expanded it once already).

New rule: Division where denominator cannot be proven non-zero at compile time

Yup...

... but probably not under the recommended config.

Optional chained subscript access is not alerted

The following does not result in an an error:
arr?.[0]

false positive 'total-functions/no-unsafe-type-assertion' rule is disabled but never reported eslint-comments/no-unused-disable

public getMessages(locale?: Locale): IntlMessages {
    if (locale !== undefined) {
      const messages = this.messages.get(locale)

      if (messages !== undefined) {
        return messages
      }
    }

    const messages = this.messages.get(this.locale)

    if (messages !== undefined) {
      return messages
    }

    // eslint-disable-next-line total-functions/no-unsafe-type-assertion
    return {} as IntlMessages
  }

Error "'total-functions/no-unsafe-type-assertion' rule is disabled but never reported" doesn't show up in vscode, but shows up in terminal

Basically I need to return empty object at the end, so it has same type.

Don't flag writing to arrays

(which we shouldn't be doing anyway, but still)

Support noUncheckedIndexedAccess

This is a tracking ticket for microsoft/TypeScript#39560.

When that lands, we will need to:

1. Probably bail out of the no-unsafe-subscript rule early if that compiler flag is on (unless there are any remaining problematic cases). Or deprecate/remove this rule entirely?
2. Ditto for no-unsafe-destructuring.
3. Update the require-strict-mode rule to also check for this flag and complain if it isn't turned on. This assumes we'll have to enable noUncheckedIndexedAccess separately from strict, otherwise this point is just subsumed by #73.

Kill Stryker mutants

https://dashboard.stryker-mutator.io/reports/github.com/danielnixon/eslint-plugin-total-functions/master

New rule: unsafe assignment / declaration / return / call

Prevent this:

type A = { val: string; };
type B = { val: string | number; };

const a: A = { val: "string" };
const b: B = a;

b.val = 3;

console.log(a.val.toUpperCase()); // throws

And this:

interface MutableValue<T> {
    value: T;
}

interface ImmutableValue<T> {
    readonly value: T;
}

let i: ImmutableValue<string> = { value: "hi" };
i.value = "Excellent, I can't change it"; // compile-time error

let m: MutableValue<string> = i;
m.value = "Oh dear, I can change it";

see microsoft/TypeScript#14150 and microsoft/TypeScript#13347

Cannot read property 'isUnion' of undefined

I receive an error from the no-array-destructuring rule at this line:
const { items: [{ phone }] } = await response.json();

The stack trace is:

[Error - 1:43:28 AM] TypeError: Cannot read property 'isUnion' of undefined
    at ArrayPattern (node_modules/eslint-plugin-total-functions/dist/rules/no-array-destructuring.js:44:51)
    at node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (node_modules/eslint/lib/linter/node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode (node_modules/eslint/lib/linter/node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode (node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:635:23)
    at node_modules/eslint/lib/linter/linter.js:949:32
    at Array.forEach (<anonymous>)```

Add mutation score badge

https://stryker-mutator.io/blog/2018-02-08/get-your-mutation-score-badge-now

Allow array subscript access if array type includes `undefined`

Object indexing false positive

I'm wondering why the following example errors:

const periods = {
    oneDay: () => subDays(Date.now(), 1).valueOf(),
    oneWeek: () => subWeeks(Date.now(), 1).valueOf(),
    twoWeeks: () => subWeeks(Date.now(), 2).valueOf(),
    oneMonth: () => subMonths(Date.now(), 1).valueOf(),
    sixMonths: () => subMonths(Date.now(), 6).valueOf()
};
const period: keyof typeof periods = 'oneDay';
periods[period];

Great rule by the way, picking up lots of bugs.

Leverage type information in no-array-destructuring rule

For a tuple with fixed length, array destructuring should be safe.

Decouple object and array cases from no-array-destructuring and no-array-subscript rules

• no-array-destructuring to become no-unsafe-array-destructuring and no-unsafe-object-destructuring
• no-array-subscript to become no-unsafe-array-subscript and no-unsafe-object-subscript

Ban unsafe object destructuring

const obj = { a: "a" } as Record<string, string>;
const { b } = obj;
b.toLocaleUpperCase();

New rule: Optional properties lead to unsoundness

This is unsettling:

type Foo = { readonly foo: string };
type Bar = { readonly foo: string, readonly bar?: () => unknown };
// This won't compile
// const foo: Foo = { foo: "foo", bar: "bar" };
// But this will
const thing = { foo: "foo", bar: "bar" };
const foo: Foo = thing;
// Uh oh...
const bar: Bar = foo;
if (bar.bar !== undefined) {
    // Runtime explosion
    // [ERR]: Executed JavaScript Failed: 
    // [ERR]: bar.bar is not a function 
    bar.bar();
}

It’s hard to point at precisely which line is “wrong”.

We may want a new rule that forbids:

const foo: Foo = thing;

We can think of it as taking the tsc error reported by

const foo: Foo = { foo: "foo", bar: "bar" };

and expanding its scope beyond just object literals.

no-unsafe-assignment: array concat false positive

  const arr: ReadonlyArray<never> = [];
  const foo = arr.concat(arr); // flagged; shouldn't be

New rule: ban unsafe type assertions

These shouldn't be flagged:

const foo = "foo" as unknown; // safe

const foo = "foo" as const; // safe

const arr = ["foo"];
const foo = arr[0] as string | undefined; // safe

But things like this should be:

type Foo = {
  readonly bar: number;
  readonly bas: string;
};

const foo = {} as Foo;

foo.bar.toString(); // Explodes at runtime

Add "all" config