From 7aed1e0d13eba3d8e3d771abe199ca7ffda4b05e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= <gaston@getunleash.ai>
Date: Tue, 14 Feb 2023 15:52:21 +0100
Subject: [PATCH] chore: gradually reduce null-check errors (#3094)

## About the changes

In order to move us towards enabling `strictNullChecks` we'd want to
have a way of gradually enabling this without having to fix all errors
at once, this will force us to start reducing the number of null check
issues.

This new workflow:
1. [Checks out the current branch and main into 2 different
folders](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR15-R23)
2. Uses the **same** script `gradual-strict-null-checks.sh` (from the
current branch) [against each folder in
parallel](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR34-R38)
to count the number of errors if `strictNullChecks` was enabled
3. If the number of potential errors in the current branch is higher
than the number of potential errors in main [it
fails](https://github.com/Unleash/unleash/pull/3094/files#diff-068f2ace1d1d2e773fb5e4240c83ccab251556fd5524fe13847122878e40da3bR41-R46)

As an example, a [new issue was introduced in this
PR](https://github.com/Unleash/unleash/pull/3094/commits/753f57223c2107278dd7ee387444847e5cc4496a)
(and then
[reverted](https://github.com/Unleash/unleash/pull/3094/commits/e4deb62965bdc12b22c2a78a85588b237943483a)),
so we can test the build failure:

https://github.com/Unleash/unleash/actions/runs/4163632636/jobs/7204268519#step:5:10


## Discussion points
This could be a non-mandatory check, just advising, or even adding a
comment in the PR. It might be good to start with a non-strict check,
but at the same time we can decide to make it non-strict if a problem
appears

In some situations, an additional null check error might require us to
fix a bunch of them, increasing the time to deliver. In these cases we
can suppress an individual line with `// @ts-ignore: Object is possibly
'null'.` although might defeat the purpose of this workflow
---
 .../workflows/gradual-strict-null-checks.yml  | 50 +++++++++++++++++++
 scripts/gradual-strict-null-checks.sh         | 13 +++++
 2 files changed, 63 insertions(+)
 create mode 100644 .github/workflows/gradual-strict-null-checks.yml
 create mode 100755 scripts/gradual-strict-null-checks.sh

diff --git a/.github/workflows/gradual-strict-null-checks.yml b/.github/workflows/gradual-strict-null-checks.yml
new file mode 100644
index 0000000000..ed3bc21c07
--- /dev/null
+++ b/.github/workflows/gradual-strict-null-checks.yml
@@ -0,0 +1,50 @@
+name: Lower null checks
+
+on:
+  pull_request:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        node-version: [16.x]
+
+    steps:
+      - name: Checkout current branch
+        uses: actions/checkout@v3
+        with:
+          path: current
+      - name: Checkout main branch
+        uses: actions/checkout@v3
+        with:
+          ref: main
+          path: main
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.node-version }}
+          cache: 'yarn'
+          cache-dependency-path: |
+            current/yarn.lock
+            main/yarn.lock
+      # intentionally use the same script from current branch against both repositories
+      - run: |
+          ./current/scripts/gradual-strict-null-checks.sh ./current > ./current-count &
+          pid1=$!
+          ./current/scripts/gradual-strict-null-checks.sh ./main > ./main-count &
+          pid2=$!
+          wait $pid1 && wait $pid2
+          MAIN=$(cat ./main-count)
+          CURRENT=$(cat ./current-count)
+          if [ $CURRENT -gt $MAIN ]; then
+            echo "The PR is increasing the number of null check errors from ${MAIN} to ${CURRENT}. Check if your branch is up-to-date and consider fixing them before merging"
+            exit 1
+          else
+            echo "The PR has $CURRENT null check errors against $MAIN in main. You're good to go!"
+          fi
\ No newline at end of file
diff --git a/scripts/gradual-strict-null-checks.sh b/scripts/gradual-strict-null-checks.sh
new file mode 100755
index 0000000000..22a1ab839f
--- /dev/null
+++ b/scripts/gradual-strict-null-checks.sh
@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+set -e
+FOLDER="${1:-.}"
+
+cd "${FOLDER}"
+
+# update strictNullChecks
+sed -i 's/\/\/\s*"strictNullChecks":\s*true,/"strictNullChecks": true,/' "./tsconfig.json"
+
+# count errors
+ERRORS=$(yarn 2> /dev/null | grep "Found [0-9]* errors" | sed 's/Found \(.*\) errors in .* files./\1/')
+
+echo ${ERRORS:-0}
\ No newline at end of file