Enable @typescript-eslint/no-unnecessary-condition to ensure no unnecessary null checks

[pokey]

The problem

I recently wrote a bug where I had a function foo() that originally returned a nullable value, and I used it in a conditional foo() == null. I switched foo() to return a boolean, but forgot to update the conditional, which resulted in the conditional always evaluating to false 😳

The solution

We'd like to introduce the @typescript-eslint/no-unnecessary-condition lint rule, which would have flagged an error and helped me to spot this bug.

Unfortunately, when I tried to enable it, I got lots of errors. The ones I saw had to do with the following situations:

• Looking something up in an object of type Record<string, Foo> and then checking if it's null. Not entirely sure what to do about these
• Issues relating to the fact we don't have proper schema validation at any of our boundaries. We generally type them optimistically and then have some ad-hoc checks, which then get flagged by this rule because according to the optimistic types the checks aren't necessary. This one would be solved by Add schema validation to Cursorless api boundary #983
• It's possible there were others; I didn't spend too much time digging because I was just hoping for a quick win

Patch

In case helpful, here's a patch to enable the lint rule

From ec33035f7665be45c8d192dc0e18c251e524c099 Mon Sep 17 00:00:00 2001
From: Pokey Rule <[email protected]>
Date: Tue, 25 Jul 2023 12:03:58 +0200
Subject: [PATCH] Add `@typescript-eslint/no-unnecessary-condition` lint rule

---
 .eslintrc.json | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.eslintrc.json b/.eslintrc.json
index 27b2e8d3a..e459cc397 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -8,7 +8,8 @@
   ],
   "parserOptions": {
     "ecmaVersion": 6,
-    "sourceType": "module"
+    "sourceType": "module",
+    "project": ["./tsconfig.json", "./packages/*/tsconfig.json"]
   },
   "plugins": ["@typescript-eslint", "unused-imports", "import"],
   "rules": {
@@ -23,6 +24,7 @@
     "@typescript-eslint/no-explicit-any": "off",
     "@typescript-eslint/no-inferrable-types": "off",
     "@typescript-eslint/no-non-null-assertion": "off",
+    "@typescript-eslint/no-unnecessary-condition": "error",
     "unused-imports/no-unused-imports": "error",
     "@typescript-eslint/no-unused-vars": [
       "warn",
-- 
2.40.0

[auscompgeek]

I'll leave the same note here that I left in #714:

It'll be good to consider the lint rules that use type information, but we should consider how enabling type checking in linting affects the linter performance.