[no-unnecessary-condition] Add option to ignore expressions as an arguments of certain functions.

[IvanGoncharov]

We working on converting graphql and express-graphql package to TS and want to use no-unnecessary-condition since we had this lint rule in Flow.
The problem is that both libraries are used in a lot of JS projects so we need to check arguments of public API functions for popular mistakes.
So we created devCheck function for this purpose:

function devAssert(condition: unknown, message: string): asserts condition {
  const booleanCondition = Boolean(condition);
  if (!booleanCondition) {
    throw new Error(message);
  }
}

Problem is that we get this error:

  190:13  error  Unnecessary conditional, the types have no overlap  @typescript-eslint/no-unnecessary-condition

for this code

export function graphqlHTTP(options: Options): Middleware {
  devAssert(options != null, 'GraphQL middleware requires options.');

It would be ideal if we can pass devAssert as an option for the no-unnecessary-condition rule and expressions inside arguments would be ignored.

[bradzacher]

since we had this lint rule in Flow.

What fllint rule did this?
There's the sketchy-null rules which is akin to our strict-boolean-expressions rule.

---

Why do you need to pass != null? Why not just drop that always true condition, and just use options. It'll do the same thing.

This rule has no understanding of assertion functions. Adding support for them isn't going to be overly easy.
Happy to accept a PR that adds an option to check the parent call expression to see if it's a function with an asserts return type.

[IvanGoncharov]

What fllint rule did this?

@bradzacher These two rules from Flow core:
https://flow.org/en/docs/linting/rule-reference/#toc-unnecessary-invariant
https://flow.org/en/docs/linting/rule-reference/#toc-unnecessary-optional-chain

Why do you need to pass != null?

It will work for objects but not for strings, numbers, etc. so I prefer to just use != null everywhere.

Happy to accept a PR that adds an option to check the parent call expression to see if it's a function with an asserts return type.

Actually, we have two different kinds of asserts invariant and devAssert.
So we want to apply this rule for invariant to mimic Flow unnecessary-invariant but not apply on devAssert.
What will work for us is a simple option that takes a list of function names and disables this check for all its arguments.

This rule has no understanding of assertion functions. Adding support for them isn't going to be overly easy.

Can it be done by calling skip on all function calls where the name of a function matches one of the names specified through the rule option?

[stenzengel]

@IvanGoncharov For your use case, we use an additional assert function. E.g.:

export function devAssertDefined<T>(value: T, message?: string): asserts value is NonNullable<T> {
  devAssert(value !== null, message ?? 'Assertion error: Value is null');
  devAssert(value !== undefined, message ?? 'Assertion error: Value is undefined');
}

[bradzacher]

What will work for us is a simple option that takes a list of function names and disables this check for all its arguments.

Not going to happen. String based options lack any context. For type-aware lint rules this creates the potential for huge holes in lint rule checks.

Example: you add the function foo to the ignore list. Someone defines a foo function locally (unaware of the eslint config). That function is now ignored by the lint rule and will false negative, even though the intention was only the foo imported from this other file is ignored.

The only solution I'd accept here is a type-aware one which specifically checks for functions that return assertion predicates.