Replace rule S5868 with our implementation providing precise location (…

…#2793)
SonarSource · Sep 7, 2021 · 3f8d0dd · 3f8d0dd
1 parent 571a150
commit 3f8d0dd
Show file tree

Hide file tree

Showing 6 changed files with 200 additions and 7 deletions.
diff --git a/eslint-bridge/src/rules/main.ts b/eslint-bridge/src/rules/main.ts
@@ -172,6 +172,7 @@ import { rule as sonarMaxLinesPerFunction } from './sonar-max-lines-per-function
 import { rule as sonarNoControlRegex } from './sonar-no-control-regex';
 import { rule as sonarNoFallthrough } from './sonar-no-fallthrough';
 import { rule as sonarNoInvalidRegexp } from './sonar-no-invalid-regexp';
+import { rule as sonarNoMisleadingCharacterClass } from './sonar-no-misleading-character-class';
 import { rule as sonarNoRegexSpaces } from './sonar-no-regex-spaces';
 import { rule as sonarNoUnusedVars } from './sonar-no-unused-vars';
 import { rule as sqlQueries } from './sql-queries';
@@ -356,6 +357,7 @@ ruleModules['sonar-max-lines-per-function'] = sonarMaxLinesPerFunction;
 ruleModules['sonar-no-control-regex'] = sonarNoControlRegex;
 ruleModules['sonar-no-fallthrough'] = sonarNoFallthrough;
 ruleModules['sonar-no-invalid-regexp'] = sonarNoInvalidRegexp;
+ruleModules['sonar-no-misleading-character-class'] = sonarNoMisleadingCharacterClass;
 ruleModules['sonar-no-regex-spaces'] = sonarNoRegexSpaces;
 ruleModules['sonar-no-unused-vars'] = sonarNoUnusedVars;
 ruleModules['sql-queries'] = sqlQueries;

diff --git a/eslint-bridge/src/rules/sonar-no-misleading-character-class.ts b/eslint-bridge/src/rules/sonar-no-misleading-character-class.ts
@@ -0,0 +1,68 @@
+/*
+ * SonarQube JavaScript Plugin
+ * Copyright (C) 2011-2021 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+// https://sonarsource.github.io/rspec/#/rspec/S5868
+
+import { Rule } from 'eslint';
+import { createRegExpRule } from './regex-rule-template';
+import { Character, CharacterClassElement } from 'regexpp/ast';
+
+export const rule: Rule.RuleModule = createRegExpRule(context => {
+  function characters(nodes: CharacterClassElement[]): Character[][] {
+    let current: Character[] = [];
+    const sequences: Character[][] = [current];
+    for (const node of nodes) {
+      if (node.type === 'Character') {
+        current.push(node);
+      } else if (node.type === 'CharacterClassRange') {
+        // for following regexp [xa-z] we produce [[xa],[z]]
+        // we would report for example if instead of 'xa' there would be unicode combined class
+        current.push(node.min);
+        current = [node.max];
+        sequences.push(current);
+      } else if (node.type === 'CharacterSet' && current.length > 0) {
+        // CharacterSet is for example [\d], ., or \p{ASCII}
+        // see https://github.com/mysticatea/regexpp/blob/master/src/ast.ts#L222
+        current = [];
+        sequences.push(current);
+      }
+    }
+    return sequences;
+  }
+
+  return {
+    onCharacterClassEnter(ccNode) {
+      for (const chars of characters(ccNode.elements)) {
+        const idx = chars.findIndex(
+          (c, i) =>
+            i !== 0 && isCombiningCharacter(c.value) && !isCombiningCharacter(chars[i - 1].value),
+        );
+        if (idx >= 0) {
+          const combinedChar = chars[idx - 1].raw + chars[idx].raw;
+          const message = `Move this Unicode combined character '${combinedChar}' outside of [...]`;
+          context.reportRegExpNode({ regexpNode: chars[idx], node: context.node, message });
+        }
+      }
+    },
+  };
+});
+
+function isCombiningCharacter(codePoint: number) {
+  return /^[\p{Mc}\p{Me}\p{Mn}]$/u.test(String.fromCodePoint(codePoint));
+}
diff --git a/eslint-bridge/src/utils/utils-regex.ts b/eslint-bridge/src/utils/utils-regex.ts
@@ -90,15 +90,26 @@ function getPatternFromNode(
   return null;
 }
 
-export function isRegExpConstructor(node: estree.Node): node is estree.CallExpression {
+function isRegExpWithGlobalThis(node: estree.Node) {
   return (
-    (node.type === 'CallExpression' || node.type === 'NewExpression') &&
-    node.callee.type === 'Identifier' &&
-    node.callee.name === 'RegExp' &&
+    node.type === 'NewExpression' &&
+    node.callee.type === 'MemberExpression' &&
+    isIdentifier(node.callee.object, 'globalThis') &&
+    isIdentifier(node.callee.property, 'RegExp') &&
     node.arguments.length > 0
   );
 }
 
+export function isRegExpConstructor(node: estree.Node): node is estree.CallExpression {
+  return (
+    ((node.type === 'CallExpression' || node.type === 'NewExpression') &&
+      node.callee.type === 'Identifier' &&
+      node.callee.name === 'RegExp' &&
+      node.arguments.length > 0) ||
+    isRegExpWithGlobalThis(node)
+  );
+}
+
 export function getFlags(callExpr: estree.CallExpression): string | null {
   if (callExpr.arguments.length < 2) {
     return '';

diff --git a/eslint-bridge/tests/rules/sonar-no-misleading-character-class.test.ts b/eslint-bridge/tests/rules/sonar-no-misleading-character-class.test.ts
@@ -0,0 +1,112 @@
+/*
+ * SonarQube JavaScript Plugin
+ * Copyright (C) 2011-2021 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+import { RuleTester } from 'eslint';
+import { rule } from 'rules/sonar-no-misleading-character-class';
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018, sourceType: 'module' } });
+
+const combiningClass = c => `Move this Unicode combined character '${c}' outside of [...]`;
+
+ruleTester.run('', rule, {
+  valid: [
+    'var r = /[\\uD83D\\d\\uDC4D]/',
+    'var r = /[\\uD83D-\\uDC4D]/',
+    'var r = /[👍]/u',
+    'var r = /[\\uD83D\\uDC4D]/u',
+    'var r = /[\\u{1F44D}]/u',
+    'var r = /❇️/',
+    'var r = /Á/',
+    'var r = /[❇]/',
+    'var r = /👶🏻/',
+    'var r = /[👶]/u',
+    'var r = /🇯🇵/',
+    'var r = /[JP]/',
+    'var r = /👨‍👩‍👦/',
+
+    // Ignore solo lead/tail surrogate.
+    'var r = /[\\uD83D]/',
+    'var r = /[\\uDC4D]/',
+    'var r = /[\\uD83D]/u',
+    'var r = /[\\uDC4D]/u',
+
+    // Ignore solo combining char.
+    'var r = /[\\u0301]/',
+    'var r = /[\\uFE0F]/',
+    'var r = /[\\u0301]/u',
+    'var r = /[\\uFE0F]/u',
+
+    // Coverage
+    'var r = /[x\\S]/u',
+    'var r = /[xa-z]/u',
+  ],
+  invalid: [
+    {
+      code: 'var r = /[\\u0041\\u0301-\\u0301]/',
+      errors: [{ column: 17, endColumn: 23, message: combiningClass('\\u0041\\u0301') }],
+    },
+    {
+      code: 'var r = /[Á]/',
+      errors: [{ message: combiningClass('Á') }],
+    },
+    {
+      code: 'var r = /[Á]/u',
+      errors: [{ message: combiningClass('Á') }],
+    },
+    {
+      code: 'var r = /[\\u0041\\u0301]/',
+      errors: [{ message: combiningClass('\\u0041\\u0301') }],
+    },
+    {
+      code: 'var r = /[\\u0041\\u0301]/u',
+      errors: [{ message: combiningClass('\\u0041\\u0301') }],
+    },
+    {
+      code: 'var r = /[\\u{41}\\u{301}]/u',
+      errors: [{ message: combiningClass('\\u{41}\\u{301}') }],
+    },
+    {
+      code: 'var r = /[❇️]/',
+      errors: [{ message: combiningClass('❇️') }],
+    },
+    {
+      code: 'var r = /[❇️]/u',
+      errors: [{ message: combiningClass('❇️') }],
+    },
+    {
+      code: 'var r = /[\\u2747\\uFE0F]/',
+      errors: [{ message: combiningClass('\\u2747\\uFE0F') }],
+    },
+    {
+      code: 'var r = /[\\u2747\\uFE0F]/u',
+      errors: [{ message: combiningClass('\\u2747\\uFE0F') }],
+    },
+    {
+      code: 'var r = /[\\u{2747}\\u{FE0F}]/u',
+      errors: [{ message: combiningClass('\\u{2747}\\u{FE0F}') }],
+    },
+    {
+      code: String.raw`var r = new globalThis.RegExp("[❇️]", "")`,
+      errors: [{ message: combiningClass('❇️') }],
+    },
+    {
+      code: String.raw`"cc̈d̈d".replaceAll(RegExp("[c̈d̈]"), "X")`,
+      errors: [{ message: combiningClass('c̈') }],
+    },
+  ],
+});
diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java
@@ -212,7 +212,6 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
       NoMagicNumbersCheck.class,
       NoMimeSniffCheck.class,
       NoMisleadingArrayReverseCheck.class,
-      NoMisleadingCharactersCheck.class,
       NoMisusedNewCheck.class,
       NoMixedContentCheck.class,
       NoNestedSwitchCheck.class,
@@ -276,6 +275,7 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
       SingleCharacterAlternativeCheck.class,
       SocketsCheck.class,
       SonarNoInvalidRegexCheck.class,
+      SonarNoMisleadingCharacterClassCheck.class,
       SqlQueriesCheck.class,
       StandardInputCheck.class,
       StatefulRegexCheck.class,

diff --git a/...t/checks/NoMisleadingCharactersCheck.java → ...SonarNoMisleadingCharacterClassCheck.java b/...t/checks/NoMisleadingCharactersCheck.java → ...SonarNoMisleadingCharacterClassCheck.java
@@ -27,11 +27,11 @@
 @TypeScriptRule
 @JavaScriptRule
 @Rule(key = "S5868")
-public class NoMisleadingCharactersCheck implements EslintBasedCheck {
+public class SonarNoMisleadingCharacterClassCheck implements EslintBasedCheck {
 
   @Override
   public String eslintKey() {
-    return "no-misleading-character-class";
+    return "sonar-no-misleading-character-class";
   }
 
 }