feat/type-safe: Encourage type-safe coding practices with eslint-plugin-typesafe

.eslintrc

@@ -24,7 +24,7 @@
         },
         "project": "./tsconfig.json"
       },
-      "plugins": ["@typescript-eslint", "import", "simple-import-sort"],
+      "plugins": ["@typescript-eslint", "import", "simple-import-sort", "typesafe"],
       "extends": ["plugin:@typescript-eslint/recommended"],
       "rules": {
         // Rules for auto sort of imports
@@ -54,7 +54,8 @@
         "import/newline-after-import": "error",
         "import/no-duplicates": "error",
         "@typescript-eslint/no-floating-promises": 2,
-        "@typescript-eslint/no-unused-vars": 2
+        "@typescript-eslint/no-unused-vars": 2,
+        "typesafe/no-throw-sync-func": "error"
       }
     },
     { "files": ["*.spec.ts"], "extends": ["plugin:jest/recommended"] }

package.json

@@ -215,6 +215,7 @@
     "eslint-plugin-jest": "^24.1.3",
     "eslint-plugin-prettier": "^3.3.0",
     "eslint-plugin-simple-import-sort": "^6.0.1",
+    "eslint-plugin-typesafe": "^0.2.0",
     "form-data": "^3.0.0",
     "google-fonts-plugin": "4.1.0",
     "html-loader": "~0.5.5",

src/app/models/form_statistics_total.server.model.ts

@@ -41,8 +41,8 @@ const compileFormStatisticsTotalModel = (db: Mongoose) => {
   // Hooks
   FormStatisticsTotalSchema.pre<IFormStatisticsTotalSchema>(
     'save',
-    function () {
-      throw new Error('FormStatisticsTotal schema is read only')
+    function (next) {
+      next(new Error('FormStatisticsTotal schema is read only'))
     },
   )
 

src/app/modules/form/admin-form/admin-form.utils.ts

@@ -3,7 +3,6 @@ import { err, ok, Result } from 'neverthrow'
 
 import { createLoggerWithLabel } from '../../../../config/logger'
 import { IPopulatedForm, ResponseMode, Status } from '../../../../types'
-import { assertUnreachable } from '../../../utils/assert-unreachable'
 import {
   ApplicationError,
   DatabaseConflictError,
@@ -205,8 +204,6 @@ export const getAssertPermissionFn = (level: PermissionLevel): AssertFormFn => {
       return assertHasWritePermissions
     case PermissionLevel.Delete:
       return assertHasDeletePermissions
-    default:
-      return assertUnreachable(level)
   }
 }
 

src/app/modules/form/form.service.ts

@@ -4,7 +4,6 @@ import { err, errAsync, ok, okAsync, Result, ResultAsync } from 'neverthrow'
 import { createLoggerWithLabel } from '../../../config/logger'
 import { IFormSchema, IPopulatedForm, Status } from '../../../types'
 import getFormModel from '../../models/form.server.model'
-import { assertUnreachable } from '../../utils/assert-unreachable'
 import { getMongoErrorMessage } from '../../utils/handle-mongo-error'
 import { ApplicationError, DatabaseError } from '../core/core.errors'
 
@@ -116,7 +115,5 @@ export const isFormPublic = (
       return err(new FormDeletedError())
     case Status.Private:
       return err(new PrivateFormError(form.inactiveMessage, form.title))
-    default:
-      return assertUnreachable(form.status)
   }
 }

src/app/modules/submission/__tests__/submission/submission.utils.spec.ts

@@ -44,14 +44,5 @@ describe('submission.utils', () => {
       )
       expect(actual.length).toEqual(2)
     })
-
-    it('should throw error if called with invalid responseMode', async () => {
-      // Arrange
-      const invalidResponseMode = 'something' as ResponseMode
-      // Act + Assert
-      expect(() => getModeFilter(invalidResponseMode)).toThrowError(
-        `This should never be reached in TypeScript: "${invalidResponseMode}"`,
-      )
-    })
   })
 })

src/app/modules/submission/submission.utils.ts

@@ -3,7 +3,6 @@ import { keyBy } from 'lodash'
 import { BasicField, IFieldSchema, ResponseMode } from '../../../types'
 import { isEmailField } from '../../../types/field/utils/guards'
 import { AutoReplyMailData } from '../../services/mail/mail.types'
-import { assertUnreachable } from '../../utils/assert-unreachable'
 import { FIELDS_TO_REJECT } from '../../utils/field-validation/config'
 
 import { ProcessedFieldResponse } from './submission.types'
@@ -20,8 +19,6 @@ export const getModeFilter = (
       return emailModeFilter
     case ResponseMode.Encrypt:
       return encryptModeFilter
-    default:
-      return assertUnreachable(responseMode)
   }
 }
 

src/app/modules/verification/verification.service.ts

@@ -332,5 +332,7 @@ const getFieldFromTransaction = (
 const throwError = (message: string, name?: string): never => {
   const error = new Error(message)
   error.name = name || message
+  // TODO(#941) Convert this service to use neverthrow, and re-examine type assertions made
+  // eslint-disable-next-line
   throw error
 }

src/app/services/sms/sms.factory.ts

@@ -31,9 +31,11 @@ export const createSmsFactory = (
     const errorMessage = 'SMS feature must be enabled in Feature Manager first'
     return {
       sendAdminContactOtp: () => {
+        //eslint-disable-next-line
         throw new Error(`sendAdminContactOtp: ${errorMessage}`)
       },
       sendVerificationOtp: () => {
+        //eslint-disable-next-line
         throw new Error(`sendVerificationOtp: ${errorMessage}`)
       },
     }

src/config/feature-manager/util/FeatureManager.class.ts

@@ -14,6 +14,10 @@ const logger = createLoggerWithLabel(module)
 convict.addFormat(validator.url)
 
 export default class FeatureManager {
+  /* eslint-disable typesafe/no-throw-sync-func
+    --------
+    This class bootstraps the application and should
+    cause a crash in case of errors. */
   public states: Partial<Record<FeatureNames, boolean>>
   // Map some feature names to some env vars
   private properties: Partial<IFeatureManager>
@@ -115,4 +119,5 @@ export default class FeatureManager {
       props: this.props<K>(name),
     }
   }
+  /* eslint-enable typesafe/no-throw-sync-func */
 }

src/config/logger.ts

@@ -197,6 +197,7 @@ const createCustomLogger = (logger: Logger) => {
         const { message, meta } = params
         // Not the expected shape, throw to catch block.
         if (!message || isEmpty(meta)) {
+          //eslint-disable-next-line
           throw new Error('Wrong shape')
         }
         return logger.info(message, { meta })
@@ -209,6 +210,7 @@ const createCustomLogger = (logger: Logger) => {
         const { message, meta, error } = params
         // Not the expected shape, throw to catch block.
         if (!message || isEmpty(meta)) {
+          //eslint-disable-next-line
           throw new Error('Wrong shape')
         }
         if (error) {
@@ -224,6 +226,7 @@ const createCustomLogger = (logger: Logger) => {
         const { message, meta, error } = params
         // Not the expected shape, throw to catch block.
         if (!message || isEmpty(meta)) {
+          //eslint-disable-next-line
           throw new Error('Wrong shape')
         }
         if (error) {

src/config/schema.ts

@@ -34,14 +34,17 @@ convict.addFormat({
 /**
  * Verifies that S3 bucket url is a valid url with or without trailing slash
  */
-const validateBucketUrl = (
+const validateS3BucketUrl = (
   val: string,
   {
     isDev,
     hasTrailingSlash,
     region,
   }: { isDev: boolean; hasTrailingSlash: boolean; region: string },
 ) => {
+  /* eslint-disable typesafe/no-throw-sync-func
+    --------
+    The convict package expects format validation functions to throw Errors */
   if (!validator.isURL(val, { require_tld: !isDev })) {
     throw new Error('must be a url')
   }
@@ -59,6 +62,7 @@ const validateBucketUrl = (
   if (!isDev && !isRegionCorrect.test(val)) {
     throw new Error(`region should be ${region}`)
   }
+  /* eslint-enable typesafe/no-throw-sync-func */
 }
 
 // If the default value does not match the format specified, the configuration built from this schema
@@ -352,27 +356,27 @@ export const loadS3BucketUrlSchema = ({
     endPoint: {
       doc: 'Endpoint for S3 buckets',
       format: (val) =>
-        validateBucketUrl(val, { isDev, hasTrailingSlash: false, region }),
+        validateS3BucketUrl(val, { isDev, hasTrailingSlash: false, region }),
       default: 'https://s3.ap-southeast-1.amazonaws.com', // NOTE NO TRAILING / AT THE END OF THIS URL!
       env: 'AWS_ENDPOINT',
     },
     attachmentBucketUrl: {
       doc:
         'Url of attachment S3 bucket derived from S3 endpoint and bucket name',
       format: (val) =>
-        validateBucketUrl(val, { isDev, hasTrailingSlash: true, region }),
+        validateS3BucketUrl(val, { isDev, hasTrailingSlash: true, region }),
       default: null,
     },
     logoBucketUrl: {
       doc: 'Url of logo S3 bucket derived from S3 endpoint and bucket name',
       format: (val) =>
-        validateBucketUrl(val, { isDev, hasTrailingSlash: false, region }),
+        validateS3BucketUrl(val, { isDev, hasTrailingSlash: false, region }),
       default: null,
     },
     imageBucketUrl: {
       doc: 'Url of images S3 bucket derived from S3 endpoint and bucket name',
       format: (val) =>
-        validateBucketUrl(val, { isDev, hasTrailingSlash: false, region }),
+        validateS3BucketUrl(val, { isDev, hasTrailingSlash: false, region }),
       default: null,
     },
   }