Skip to content
This repository was archived by the owner on Oct 3, 2024. It is now read-only.

Commit c7c7885

Browse files
vilchik-elenaandrea-guarino-sonarsource
vilchik-elena
authored and
andrea-guarino-sonarsource
committed
add rule no-duplicate-string (#78)
1 parent 4600fd7 commit c7c7885

File tree

7 files changed

+553
-2
lines changed

7 files changed

+553
-2
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
2424

2525
* Cognitive Complexity of functions should not be too high ([`cognitive-complexity`])
2626
* "switch" statements should not have too many "case" clauses ([`max-switch-cases`])
27+
* String literals should not be duplicated ([`no-duplicate-string`])
2728
* Two branches in a conditional structure should not have exactly the same implementation ([`no-duplicated-branches`])
2829
* Functions should not have identical implementations ([`no-identical-functions`])
2930
* Boolean checks should not be inverted ([`no-inverted-boolean-check`])
@@ -37,6 +38,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
3738
[`cognitive-complexity`]: ./docs/rules/cognitive-complexity.md
3839
[`max-switch-cases`]: ./docs/rules/max-switch-cases.md
3940
[`no-all-duplicated-branches`]: ./docs/rules/no-all-duplicated-branches.md
41+
[`no-duplicate-string`]: ./docs/rules/no-duplicate-string.md
4042
[`no-duplicated-branches`]: ./docs/rules/no-duplicated-branches.md
4143
[`no-element-overwrite`]: ./docs/rules/no-element-overwrite.md
4244
[`no-extra-arguments`]: ./docs/rules/no-extra-arguments.md

‎docs/rules/max-switch-cases.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22

33
When `switch` statements have large sets of `case` clauses, it is usually an attempt to map two sets of data. A real map structure would be more readable and maintainable, and should be used instead.
44

5-
#Options
5+
## Configuration
66

77
This rule has a numeric option (defaulted to 30) to specify the maximum number of switch cases.
88

9-
```
9+
```json
1010
{
11+
"max-switch-cases": "error",
1112
"max-switch-cases": ["error", 10]
1213
}
1314
```

‎docs/rules/no-duplicate-string.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# no-duplicate-string
2+
Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.
3+
On the other hand, constants can be referenced from many places, but only need to be updated in a single place.
4+
5+
## Exceptions
6+
To prevent generating some false-positives, literals having less than 10 characters are excluded as well as literals matching /^\w*$/. String literals inside import/export statements are also ignored.
7+
8+
## Configuration
9+
10+
Number of times a literal must be duplicated to trigger an issue. Default is 3.
11+
12+
```json
13+
{
14+
"no-duplicate-string": "error",
15+
"no-duplicate-string": ["error", 5]
16+
}
17+
```

‎ruling/snapshots/no-duplicate-string‎

Lines changed: 283 additions & 0 deletions
Large diffs are not rendered by default.

‎src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const sonarjsRules: [string, Linter.RuleLevel][] = [
2323
["cognitive-complexity", "error"],
2424
["max-switch-cases", "error"],
2525
["no-all-duplicated-branches", "error"],
26+
["no-duplicate-string", "error"],
2627
["no-duplicated-branches", "error"],
2728
["no-element-overwrite", "error"],
2829
["no-extra-arguments", "error"],

‎src/rules/no-duplicate-string.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* eslint-plugin-sonarjs
3+
* Copyright (C) 2018 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
// https://jira.sonarsource.com/browse/RSPEC-1192
21+
22+
import { Rule } from "eslint";
23+
import { Node, SimpleLiteral } from "estree";
24+
import { getParent } from "../utils/nodes";
25+
26+
// Number of times a literal must be duplicated to trigger an issue
27+
const DEFAULT_THRESHOLD = 3;
28+
const MIN_LENGTH = 10;
29+
const NO_SEPARATOR_REGEXP = /^\w*$/;
30+
const EXCLUDED_CONTEXTS = ["ImportDeclaration", "JSXAttribute", "ExportAllDeclaration", "ExportNamedDeclaration"];
31+
const MESSAGE = "Define a constant instead of duplicating this literal {{times}} times.";
32+
33+
const rule: Rule.RuleModule = {
34+
meta: {
35+
schema: [{ type: "integer", minimum: 2 }],
36+
},
37+
38+
create(context: Rule.RuleContext) {
39+
const literalsByValue: Map<string, SimpleLiteral[]> = new Map();
40+
const threshold: number = context.options[0] !== undefined ? context.options[0] : DEFAULT_THRESHOLD;
41+
42+
return {
43+
Literal: (node: Node) => {
44+
const literal = node as SimpleLiteral;
45+
if (typeof literal.value === "string") {
46+
const stringContent = literal.value.trim();
47+
48+
if (
49+
!isExcludedByUsageContext(context) &&
50+
stringContent.length >= MIN_LENGTH &&
51+
!stringContent.match(NO_SEPARATOR_REGEXP)
52+
) {
53+
const sameStringLiterals = literalsByValue.get(stringContent) || [];
54+
sameStringLiterals.push(literal);
55+
literalsByValue.set(stringContent, sameStringLiterals);
56+
}
57+
}
58+
},
59+
60+
"Program:exit"() {
61+
literalsByValue.forEach(literals => {
62+
if (literals.length >= threshold) {
63+
context.report({
64+
message: MESSAGE,
65+
node: literals[0],
66+
data: { times: literals.length + "" },
67+
});
68+
}
69+
});
70+
},
71+
};
72+
},
73+
};
74+
75+
function isExcludedByUsageContext(context: Rule.RuleContext) {
76+
const parent = getParent(context)!;
77+
const parentType = parent.type;
78+
79+
return EXCLUDED_CONTEXTS.includes(parentType) || isRequireContext(parent, context);
80+
}
81+
82+
function isRequireContext(parent: Node, context: Rule.RuleContext) {
83+
return parent.type === "CallExpression" && context.getSourceCode().getText(parent.callee) === "require";
84+
}
85+
86+
export = rule;

‎tests/rules/no-duplicate-string.test.ts‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* eslint-plugin-sonarjs
3+
* Copyright (C) 2018 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
import { RuleTester } from "eslint";
21+
22+
const ruleTester = new RuleTester({
23+
parserOptions: { ecmaVersion: 2015, sourceType: "module", ecmaFeatures: { jsx: true } },
24+
});
25+
import rule = require("../../src/rules/no-duplicate-string");
26+
27+
ruleTester.run("no-duplicate-string", rule, {
28+
valid: [
29+
{
30+
code: `
31+
console.log("some message");
32+
console.log("some message");
33+
console.log('some message');`,
34+
options: [4],
35+
},
36+
{
37+
code: ` // too small
38+
console.log("a&b");
39+
console.log("a&b");
40+
console.log("a&b");`,
41+
},
42+
{
43+
code: ` // too small when trimmed
44+
// trimming allows to not raise issue for flowtype whitespaces
45+
// which are created as literals for some reason
46+
console.log(" a ");
47+
console.log(" a ");
48+
console.log(" a ");`,
49+
},
50+
{
51+
code: ` // numbers
52+
console.log(12345.67890);
53+
console.log(12345.67890);
54+
console.log(12345.67890);`,
55+
},
56+
{
57+
code: `
58+
console.log("only 2 times");
59+
console.log("only 2 times");`,
60+
},
61+
{
62+
code: `// no separators
63+
console.log("stringstring");
64+
console.log("stringstring");
65+
console.log("stringstring");
66+
console.log("stringstring");`,
67+
},
68+
{
69+
code: `// ImportDeclaration
70+
import defaultExport1 from "module-name-long";
71+
import defaultExport2 from "module-name-long";
72+
import defaultExport3 from "module-name-long";
73+
`,
74+
},
75+
{
76+
code: ` // ImportDeclaration
77+
import * as name1 from "module-name-long";
78+
import * as name2 from "module-name-long";
79+
import * as name3 from "module-name-long";
80+
`,
81+
},
82+
{
83+
code: ` // ImportDeclaration
84+
import "module-name-long";
85+
import "module-name-long";
86+
import "module-name-long";
87+
`,
88+
},
89+
{
90+
code: `// ExportAllDeclaration
91+
export * from "module-name-long";
92+
export * from "module-name-long";
93+
export * from "module-name-long";
94+
`,
95+
},
96+
{
97+
code: `// CallExpression 'require'
98+
const a = require("module-name-long").a;
99+
const b = require("module-name-long").b;
100+
const c = require("module-name-long").c;
101+
`,
102+
},
103+
{
104+
code: `// ExportNamedDeclaration
105+
export { a } from "module-name-long";
106+
export { b } from "module-name-long";
107+
export { c } from "module-name-long";
108+
`,
109+
},
110+
{
111+
code: ` // JSXAttribute
112+
<Foo bar="some string"></Foo>;
113+
<Foo bar="some string"></Foo>;
114+
<Foo bar="some string"></Foo>;
115+
<Foo className="some-string"></Foo>;
116+
`,
117+
},
118+
{
119+
code: `
120+
console.log(\`some message\`);
121+
console.log('some message');
122+
console.log("some message");`,
123+
},
124+
],
125+
invalid: [
126+
{
127+
code: `
128+
console.log("some message");
129+
console.log("some message");
130+
console.log('some message');`,
131+
errors: [
132+
{
133+
message: "Define a constant instead of duplicating this literal 3 times.",
134+
column: 17,
135+
endColumn: 31,
136+
},
137+
],
138+
},
139+
{
140+
code: `
141+
<Foo bar="some string"></Foo>;
142+
<Foo bar="some string"></Foo>;
143+
<Foo bar="some string"></Foo>;
144+
let x = "some-string", y = "some-string", z = "some-string";
145+
`,
146+
errors: [
147+
{
148+
message: "Define a constant instead of duplicating this literal 3 times.",
149+
line: 5,
150+
},
151+
],
152+
},
153+
{
154+
code: `
155+
console.log("some message");
156+
console.log('some message');`,
157+
errors: 1,
158+
options: [2],
159+
},
160+
],
161+
});

0 commit comments

Comments
 (0)