Migrate lint check to disallow innerHTML property. (oppia#13301)

brianrodri · Jul 5, 2021 · c82945e · c82945e
1 parent 901bc8b
commit c82945e
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 59 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -126,6 +126,21 @@
       "rules": {
         "oppia/no-testonly": "off"
       }
+    },
+     {
+      "files": ["*.js", "*.ts"],
+      "excludedFiles": [
+      "Polyfills.ts",
+      "ck-editor-copy-content.service.spec.ts",
+      "unit-test-utils.ajs.ts",
+      "mathjax.directive.ts",
+      "math-expression-content-editor.component.ts",
+      "rte-output-display.component.spec.ts",
+      "core/tests/*"
+      ],
+      "rules": {
+        "oppia/no-inner-html": "error"
+      }
     }
   ],
   "rules": {

diff --git a/scripts/linters/custom_eslint_checks/rules/no-inner-html.js b/scripts/linters/custom_eslint_checks/rules/no-inner-html.js
@@ -0,0 +1,46 @@
+// Copyright 2021 The Oppia Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS-IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @fileoverview Lint check to disallow innerHTML property.
+ */
+
+'use strict';
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Lint check to disallow innerHTML property',
+      category: 'Best Practices',
+      recommended: true,
+    },
+    fixable: null,
+    schema: [],
+    messages: {
+      disallowInnerhtml: 'Please do not use innerHTML property.'
+    }
+  },
+  create: function(context) {
+    var selector = 'MemberExpression[property.name=innerHTML]';
+    return {
+      [selector]: function(node) {
+        context.report({
+          node: node.property,
+          messageId: 'disallowInnerhtml'
+        });
+      }
+    };
+  }
+};
diff --git a/scripts/linters/custom_eslint_checks/rules/no-inner-html.spec.js b/scripts/linters/custom_eslint_checks/rules/no-inner-html.spec.js
@@ -0,0 +1,55 @@
+// Copyright 2021 The Oppia Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS-IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @fileoverview Tests for the no-inner-html.js file.
+ */
+
+'use strict';
+
+var rule = require('./no-inner-html');
+var RuleTester = require('eslint').RuleTester;
+
+var ruleTester = new RuleTester();
+ruleTester.run('no-inner-html', rule, {
+  valid: [
+    `if (!('outerHTML' in SVGElement.prototype)) {
+      Object.defineProperty(SVGElement.prototype, 'outerHTML', {
+        get: function() {
+          $node = this.cloneNode(true);
+          $temp.appendChild($node);
+          return $temp;
+        },
+      });
+    }`
+  ],
+
+  invalid: [
+    {
+      code:
+        `if (!('outerHTML' in SVGElement.prototype)) {
+          Object.defineProperty(SVGElement.prototype, 'outerHTML', {
+            get: function() {
+              $node = this.cloneNode(true);
+              $temp.appendChild($node);
+              return $temp.innerHTML;
+            },
+          });
+        }`,
+      errors: [{
+        message: 'Please do not use innerHTML property.'
+      }]
+    },
+  ]
+});
diff --git a/scripts/linters/general_purpose_linter.py b/scripts/linters/general_purpose_linter.py
@@ -145,21 +145,6 @@
         'excluded_files': (),
         'excluded_dirs': ('core/tests/',)
     },
-    {
-        'regexp': re.compile(r'innerHTML'),
-        'message': 'Please do not use innerHTML property.',
-        'excluded_files': (
-            'core/templates/Polyfills.ts',
-            'core/templates/filters/translate.pipe.spec.ts',
-            'core/templates/components/ck-editor-helpers/' +
-            'ck-editor-copy-content.service.spec.ts',
-            'core/templates/tests/unit-test-utils.ajs.ts',
-            'core/templates/directives/mathjax.directive.ts',
-            'extensions/objects/templates/' +
-            'math-expression-content-editor.component.ts',
-            'rte-output-display.component.spec.ts'),
-        'excluded_dirs': ('core/tests/',)
-    },
     {
         'regexp': re.compile(r'import (\{.*\}|\_) from \'lodash\''),
         'message': (

diff --git a/scripts/linters/general_purpose_linter_test.py b/scripts/linters/general_purpose_linter_test.py
@@ -52,8 +52,6 @@
 FILE_IN_EXCLUDED_PATH = os.path.join(
     'core', 'tests', 'build_sources', 'assets', 'constants.js')
 EXTRA_JS_FILEPATH = os.path.join('core', 'templates', 'demo.js')
-INVALID_INNER_HTML_FILEPATH = os.path.join(
-    LINTER_TESTS_DIR, 'invalid_innerhtml.ts')
 INVALID_RELATIVE_IMPORT_FILEPATH = os.path.join(
     LINTER_TESTS_DIR, 'invalid_relative_import.js')
 INVALID_TEMPLATE_URL_FILEPATH = os.path.join(
@@ -166,16 +164,6 @@ def test_invalid_use_of_relative_import(self):
         self.assertEqual('Bad pattern', lint_task_report.name)
         self.assertTrue(lint_task_report.failed)
 
-    def test_invalid_use_of_inner_html(self):
-        linter = general_purpose_linter.GeneralPurposeLinter(
-            [INVALID_INNER_HTML_FILEPATH], FILE_CACHE)
-        lint_task_report = linter.check_bad_patterns()
-        self.assert_same_list_elements(
-            ['Line 27: Please do not use innerHTML property.'
-            ], lint_task_report.trimmed_messages)
-        self.assertEqual('Bad pattern', lint_task_report.name)
-        self.assertTrue(lint_task_report.failed)
-
     def test_invalid_lodash_general_import(self):
         linter = general_purpose_linter.GeneralPurposeLinter(
             [INVALID_LODASH_GENERAL_IMPORT_FILEPATH], FILE_CACHE)

diff --git a/scripts/linters/test_files/invalid_innerhtml.ts b/scripts/linters/test_files/invalid_innerhtml.ts