Add a new config for strict typescript checks

.circleci/config.yml

@@ -117,6 +117,10 @@ jobs:
           name: Run typescript tests
           command: |
             python -m scripts.typescript_checks
+      - run:
+          name: Run typescript tests in strict mode
+          command: |
+            python -m scripts.typescript_checks --strict_checks
       - persist_to_workspace:
           root: /home/circleci/
           paths:

.github/CODEOWNERS

@@ -66,6 +66,7 @@
 /core/templates/services/UpgradedServices.ts @bansalnitish @srijanreddy98
 /typings/ @ankita240796
 /tsconfig.json @ankita240796
+/tsconfig-strict.json @nishantwrp @vojtechjelinek
 
 
 # Answer classification team.

core/templates/pages/admin-page/services/admin-router.service.spec.ts

@@ -20,7 +20,7 @@ import { AdminRouterService } from
   'pages/admin-page/services/admin-router.service.ts';
 
 describe('Admin router service', () => {
-  let ars: AdminRouterService = null;
+  let ars: AdminRouterService;
 
   beforeEach(() => {
     ars = new AdminRouterService();

core/templates/pages/admin-page/services/admin-router.service.ts

@@ -30,8 +30,9 @@ export class AdminRouterService {
     AdminPageConstants.ADMIN_TAB_URLS.ACTIVITIES);
 
   getTabNameByHash(tabHash: string): string | null {
-    for (var tabName in AdminPageConstants.ADMIN_TAB_URLS) {
-      if (AdminPageConstants.ADMIN_TAB_URLS[tabName] === tabHash) {
+    for (const [tabName, tabUrl] of Object.entries(
+      AdminPageConstants.ADMIN_TAB_URLS)) {
+      if (tabUrl === tabHash) {
         return tabName;
       }
     }

scripts/pre_push_hook.py

@@ -67,6 +67,8 @@
 TRAVIS_CI_PROTRACTOR_CHECK_CMDS = [
     PYTHON_CMD, '-m', 'scripts.check_e2e_tests_are_captured_in_ci']
 TYPESCRIPT_CHECKS_CMDS = [PYTHON_CMD, '-m', 'scripts.typescript_checks']
+STRICT_TYPESCRIPT_CHECKS_CMDS = [
+    PYTHON_CMD, '-m', 'scripts.typescript_checks', '--strict_checks']
 GIT_IS_DIRTY_CMD = 'git status --porcelain --untracked-files=no'
 
 
@@ -453,6 +455,16 @@ def main(args=None):
                     'Push aborted due to failing typescript checks.')
                 sys.exit(1)
 
+            strict_typescript_checks_status = 0
+            if does_diff_include_ts_files(files_to_lint):
+                strict_typescript_checks_status = run_script_and_get_returncode(
+                    STRICT_TYPESCRIPT_CHECKS_CMDS)
+            if strict_typescript_checks_status != 0:
+                python_utils.PRINT(
+                    'Push aborted due to failing typescript checks in '
+                    'strict mode.')
+                sys.exit(1)
+
             frontend_status = 0
             travis_ci_check_status = 0
             if does_diff_include_js_or_ts_files(files_to_lint):

scripts/pre_push_hook_test.py

@@ -601,6 +601,26 @@ def mock_run_script_and_get_returncode(unused_script):
         self.assertTrue(
             'Push aborted due to failing typescript checks.' in self.print_arr)
 
+    def test_strict_typescript_check_failiure(self):
+        self.does_diff_include_ts_files = True
+        def mock_run_script_and_get_returncode(script):
+            if script == pre_push_hook.STRICT_TYPESCRIPT_CHECKS_CMDS:
+                return 1
+            return 0
+
+        run_script_and_get_returncode_swap = self.swap(
+            pre_push_hook, 'run_script_and_get_returncode',
+            mock_run_script_and_get_returncode)
+        with self.get_remote_name_swap, self.get_refs_swap, self.print_swap:
+            with self.collect_files_swap, self.uncommitted_files_swap:
+                with self.check_output_swap, self.start_linter_swap:
+                    with self.ts_swap, run_script_and_get_returncode_swap:
+                        with self.assertRaisesRegexp(SystemExit, '1'):
+                            pre_push_hook.main(args=[])
+        self.assertTrue(
+            'Push aborted due to failing typescript checks in '
+            'strict mode.' in self.print_arr)
+
     def test_frontend_test_failure(self):
         self.does_diff_include_js_or_ts_files = True
         def mock_run_script_and_get_returncode(unused_script):

scripts/typescript_checks.py

@@ -17,6 +17,7 @@
 from __future__ import absolute_import  # pylint: disable=import-only-modules
 from __future__ import unicode_literals  # pylint: disable=import-only-modules
 
+import argparse
 import json
 import os
 import shutil
@@ -26,8 +27,21 @@
 import python_utils
 from . import common
 
+_PARSER = argparse.ArgumentParser(
+    description="""
+Run the script from the oppia root folder:
+    python -m scripts.typescript_checks
+Note that the root folder MUST be named 'oppia'.
+""")
+
+_PARSER.add_argument(
+    '--strict_checks',
+    help='optional; if specified, compiles typescript using strict config.',
+    action='store_true')
+
 COMPILED_JS_DIR = os.path.join('local_compiled_js_for_test', '')
 TSCONFIG_FILEPATH = 'tsconfig.json'
+STRICT_TSCONFIG_FILEPATH = 'tsconfig-strict.json'
 
 
 def validate_compiled_js_dir():
@@ -41,8 +55,13 @@ def validate_compiled_js_dir():
             'in %s: %s' % (COMPILED_JS_DIR, TSCONFIG_FILEPATH, out_dir))
 
 
-def compile_and_check_typescript():
-    """Compiles typescript files and checks the compilation errors."""
+def compile_and_check_typescript(config_path):
+    """Compiles typescript files and checks the compilation errors.
+
+    Args:
+        config_path: str. The config that should be used to run the typescript
+            checks.
+    """
     node_path = common.NODE_PATH
     os.environ['PATH'] = '%s/bin:' % node_path + os.environ['PATH']
 
@@ -54,7 +73,7 @@ def compile_and_check_typescript():
     python_utils.PRINT('Compiling and testing typescript...')
     cmd = [
         './node_modules/typescript/bin/tsc', '--project',
-        TSCONFIG_FILEPATH]
+        config_path]
     process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
     error_messages = []
     for line in iter(process.stdout.readline, ''):
@@ -70,7 +89,17 @@ def compile_and_check_typescript():
         python_utils.PRINT('Compilation successful!')
 
 
+def main(args=None):
+    """Run the typescript checks."""
+
+    parsed_args = _PARSER.parse_args(args=args)
+    compile_and_check_typescript(
+        STRICT_TSCONFIG_FILEPATH
+        if parsed_args.strict_checks else
+        TSCONFIG_FILEPATH)
+
+
 # The 'no coverage' pragma is used as this line is un-testable. This is because
 # it will only be called when typescript_checks.py is used as a script.
 if __name__ == '__main__':  # pragma: no cover
-    compile_and_check_typescript()
+    main()

scripts/typescript_checks_test.py

@@ -45,7 +45,8 @@ def test_compiled_js_dir_validation(self):
         with outDir in typescript_checks.TSCONFIG_FILEPATH.
         """
         with self.popen_swap:
-            typescript_checks.compile_and_check_typescript()
+            typescript_checks.compile_and_check_typescript(
+                typescript_checks.TSCONFIG_FILEPATH)
             out_dir = ''
             with python_utils.open_file(
                 typescript_checks.TSCONFIG_FILEPATH, 'r') as f:
@@ -60,7 +61,8 @@ def test_compiled_js_dir_validation(self):
                 'in %s: %s' % (
                     MOCK_COMPILED_JS_DIR, typescript_checks.TSCONFIG_FILEPATH,
                     out_dir)):
-                typescript_checks.compile_and_check_typescript()
+                typescript_checks.compile_and_check_typescript(
+                    typescript_checks.TSCONFIG_FILEPATH)
 
     def test_compiled_js_dir_is_deleted_before_compilation(self):
         """Test that compiled_js_dir is deleted before a fresh compilation."""
@@ -76,7 +78,8 @@ def mock_validate_compiled_js_dir():
             if not os.path.exists(os.path.dirname(MOCK_COMPILED_JS_DIR)):
                 os.mkdir(os.path.dirname(MOCK_COMPILED_JS_DIR))
 
-            typescript_checks.compile_and_check_typescript()
+            typescript_checks.compile_and_check_typescript(
+                typescript_checks.TSCONFIG_FILEPATH)
             self.assertFalse(
                 os.path.exists(os.path.dirname(MOCK_COMPILED_JS_DIR)))
 
@@ -99,14 +102,16 @@ def mock_popen_for_deletion(unused_cmd, stdout):  # pylint: disable=unused-argum
         with popen_swap, compiled_js_dir_swap, validate_swap:
             if not os.path.exists(os.path.dirname(MOCK_COMPILED_JS_DIR)):
                 os.mkdir(os.path.dirname(MOCK_COMPILED_JS_DIR))
-            typescript_checks.compile_and_check_typescript()
+            typescript_checks.compile_and_check_typescript(
+                typescript_checks.TSCONFIG_FILEPATH)
             self.assertFalse(
                 os.path.exists(os.path.dirname(MOCK_COMPILED_JS_DIR)))
 
     def test_no_error_is_produced_for_valid_compilation(self):
         """Test that no error is produced if stdout is empty."""
         with self.popen_swap:
-            typescript_checks.compile_and_check_typescript()
+            typescript_checks.compile_and_check_typescript(
+                typescript_checks.TSCONFIG_FILEPATH)
 
     def test_error_is_produced_for_invalid_compilation(self):
         """Test that error is produced if stdout is not empty."""
@@ -116,4 +121,28 @@ def mock_popen_for_errors(unused_cmd, stdout):  # pylint: disable=unused-argumen
 
         with self.swap(subprocess, 'Popen', mock_popen_for_errors):
             with self.assertRaisesRegexp(SystemExit, '1'):
-                typescript_checks.compile_and_check_typescript()
+                typescript_checks.compile_and_check_typescript(
+                    typescript_checks.TSCONFIG_FILEPATH)
+
+    def test_config_path_when_no_arg_is_used(self):
+        """Test if the config path is correct when no arg is used."""
+        def mock_compile_and_check_typescript(config_path):
+            self.assertEqual(config_path, typescript_checks.TSCONFIG_FILEPATH)
+        compile_and_check_typescript_swap = self.swap(
+            typescript_checks, 'compile_and_check_typescript',
+            mock_compile_and_check_typescript)
+
+        with compile_and_check_typescript_swap:
+            typescript_checks.main(args=[])
+
+    def test_config_path_when_strict_checks_arg_is_used(self):
+        """Test if the config path is correct when strict checks arg is used."""
+        def mock_compile_and_check_typescript(config_path):
+            self.assertEqual(
+                config_path, typescript_checks.STRICT_TSCONFIG_FILEPATH)
+        compile_and_check_typescript_swap = self.swap(
+            typescript_checks, 'compile_and_check_typescript',
+            mock_compile_and_check_typescript)
+
+        with compile_and_check_typescript_swap:
+            typescript_checks.main(args=['--strict_checks'])

tsconfig-strict.json

@@ -0,0 +1,38 @@
+{
+  "compilerOptions": {
+    "allowJs": true,
+    "downlevelIteration": true,
+    "lib": ["es2017", "dom", "webworker"],
+    "outDir": "local_compiled_js_for_test",
+    "rootDir": ".",
+    "skipLibCheck": true,
+    "target": "es5",
+    "sourceMap": true,
+    "strict": true,
+    "typeRoots": ["./node_modules/@types"],
+    "experimentalDecorators": true,
+    "emitDecoratorMetadata": true,
+    "allowSyntheticDefaultImports": true,
+    "esModuleInterop": true,
+    "baseUrl": ".",
+    "paths": {
+      "app.constants": ["core/templates/app.constants"],
+      "app-type.constants": ["core/templates/app-type.constants"],
+      "components/*": ["core/templates/components/*"],
+      "domain/*": ["core/templates/domain/*"],
+      "expressions/*": ["core/templates/expressions/*"],
+      "pages/*": ["core/templates/pages/*"],
+      "services/*": ["core/templates/services/*"],
+      "classifiers/*": ["extensions/classifiers/*"],
+      "interactions/*": ["extensions/interactions/*"],
+      "filters/*": ["core/templates/filters/*"],
+      "static/*": ["third_party/static/*"],
+      "tests/*": ["core/templates/tests/*"]
+    }
+  },
+  "files": [
+    "core/templates/pages/admin-page/services/admin-router.service.ts",
+    "core/templates/pages/admin-page/services/admin-router.service.spec.ts"
+  ],
+  "include": ["typings/*.ts"]
+}