Skip to content

Commit

Permalink
Fix FP S6594 (sonar-prefer-regexp-exec): Ignore match count use case (
Browse files Browse the repository at this point in the history
#4540)

Co-authored-by: Ilia Kebets <[email protected]>
  • Loading branch information
1 parent 07fa49e commit 9cdfd47
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 21 deletions.
19 changes: 0 additions & 19 deletions its/ruling/src/test/expected/jsts/desktop/typescript-S6594.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
{
"desktop:app/src/crash/crash-app.tsx": [
65
],
"desktop:app/src/lib/actions-log-parser/action-log-parser.ts": [
140,
219,
Expand Down Expand Up @@ -30,35 +27,19 @@
200,
226
],
"desktop:app/src/lib/markdown-filters/emoji-filter.ts": [
64
],
"desktop:app/src/lib/markdown-filters/issue-link-filter.ts": [
112
],
"desktop:app/src/lib/parse-pac-string.ts": [
72
],
"desktop:app/src/lib/progress/lfs.ts": [
54
],
"desktop:app/src/lib/remove-remote-prefix.ts": [
10
],
"desktop:app/src/models/app-menu.ts": [
161
],
"desktop:app/src/models/branch.ts": [
73,
87
],
"desktop:app/src/models/commit-identity.ts": [
23
],
"desktop:app/src/models/git-author.ts": [
3
],
"desktop:script/changelog/parser.ts": [
20
]
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"reddit-mobile:assets/js/trackingEvents.es6.js": [
62,
67
]
}
11 changes: 11 additions & 0 deletions packages/jsts/src/rules/S6594/cb.fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,14 @@ foo.match(/foo/);
// ^^^^^
// fix@qf1 {{Replace with "RegExp.exec()"}}
// edit@qf1 [[ec=19]] {{RegExp(/bar/).exec('foo');}}

const m1 = 'foo'.match(/bar/); // Compliant: we're checking `m1.length` below
if (m1.length > 0) {
console.log(m1[0]);
}

let m2;
m2 = 'foo'.match(/bar/); // Compliant: we're checking `m2?.length` below
if (m2?.length > 0) {
console.log(m2[0]);
}
37 changes: 36 additions & 1 deletion packages/jsts/src/rules/S6594/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@

import { Rule } from 'eslint';
import * as estree from 'estree';
import { isRequiredParserServices, isString } from '../helpers';
import {
getNodeParent,
getVariableFromName,
isMemberWithProperty,
isRequiredParserServices,
isString,
} from '../helpers';
import { getParsedRegex } from '../helpers/regex';

export const rule: Rule.RuleModule = {
Expand All @@ -44,11 +50,22 @@ export const rule: Rule.RuleModule = {
if (!isString(object, services)) {
return;
}

const callExpr = (memberExpr as any).parent as estree.CallExpression;
const regex = getParsedRegex(callExpr.arguments[0], context);
if (regex?.flags.global) {
return;
}

const variable = getLhsVariable(callExpr);
for (const ref of variable?.references ?? []) {
const id = ref.identifier;
const parent = getNodeParent(id);
if (isMemberWithProperty(parent, 'length')) {
return;
}
}

context.report({
node: property,
messageId: 'useExec',
Expand All @@ -66,5 +83,23 @@ export const rule: Rule.RuleModule = {
});
},
};

/**
* Extracts the left-hand side variable of expressions
* like `x` in `const x = <node>` or `x` in `x = <node>`.
*/
function getLhsVariable(node: estree.Node) {
const parent = getNodeParent(node);
let ident: estree.Identifier | undefined;
if (parent.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
ident = parent.id;
} else if (parent.type === 'AssignmentExpression' && parent.left.type === 'Identifier') {
ident = parent.left;
}
if (ident) {
return getVariableFromName(context, ident.name);
}
return null;
}
},
};

0 comments on commit 9cdfd47

Please sign in to comment.