From 3f8d0dd7801e5416b02cf9edcec74b47ab4cb80e Mon Sep 17 00:00:00 2001 From: Tibor Blenessy Date: Tue, 7 Sep 2021 15:39:22 +0200 Subject: [PATCH] Replace rule S5868 with our implementation providing precise location (#2793) --- eslint-bridge/src/rules/main.ts | 2 + .../sonar-no-misleading-character-class.ts | 68 +++++++++++ eslint-bridge/src/utils/utils-regex.ts | 19 ++- ...onar-no-misleading-character-class.test.ts | 112 ++++++++++++++++++ .../sonar/javascript/checks/CheckList.java | 2 +- ...SonarNoMisleadingCharacterClassCheck.java} | 4 +- 6 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 eslint-bridge/src/rules/sonar-no-misleading-character-class.ts create mode 100644 eslint-bridge/tests/rules/sonar-no-misleading-character-class.test.ts rename javascript-checks/src/main/java/org/sonar/javascript/checks/{NoMisleadingCharactersCheck.java => SonarNoMisleadingCharacterClassCheck.java} (90%) diff --git a/eslint-bridge/src/rules/main.ts b/eslint-bridge/src/rules/main.ts index dc6697109d9..6e00a8951a6 100644 --- 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 new file mode 100644 index 00000000000..55eaa28adc1 --- /dev/null +++ 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 index 3164d2bab51..a6d754dd732 100644 --- 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 new file mode 100644 index 00000000000..4f6c3809a06 --- /dev/null +++ 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 = /Á/', + '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 = /[Á]/', + errors: [{ message: combiningClass('Á') }], + }, + { + code: 'var r = /[Á]/u', + errors: [{ message: combiningClass('Á') }], + }, + { + 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 index 97e3fa1a002..f00992c9b26 100644 --- 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> getAllChecks() { NoMagicNumbersCheck.class, NoMimeSniffCheck.class, NoMisleadingArrayReverseCheck.class, - NoMisleadingCharactersCheck.class, NoMisusedNewCheck.class, NoMixedContentCheck.class, NoNestedSwitchCheck.class, @@ -276,6 +275,7 @@ public static List> getAllChecks() { SingleCharacterAlternativeCheck.class, SocketsCheck.class, SonarNoInvalidRegexCheck.class, + SonarNoMisleadingCharacterClassCheck.class, SqlQueriesCheck.class, StandardInputCheck.class, StatefulRegexCheck.class, diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/NoMisleadingCharactersCheck.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/SonarNoMisleadingCharacterClassCheck.java similarity index 90% rename from javascript-checks/src/main/java/org/sonar/javascript/checks/NoMisleadingCharactersCheck.java rename to javascript-checks/src/main/java/org/sonar/javascript/checks/SonarNoMisleadingCharacterClassCheck.java index 79794893641..153e7ccd669 100644 --- a/javascript-checks/src/main/java/org/sonar/javascript/checks/NoMisleadingCharactersCheck.java +++ b/javascript-checks/src/main/java/org/sonar/javascript/checks/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"; } }