Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add prefer-replace-all rule #488

Merged
merged 20 commits into from Feb 1, 2020
Merged

Add prefer-replace-all rule #488

Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
900e16e
Added files for prefer-replace-all rule
jmoore914 Dec 24, 2019
aa0a5c4
Removed unnecessary escape characters from regex
jmoore914 Dec 24, 2019
b1b18ea
Use quote-string util; renamed some functions; added tests for quotes
jmoore914 Dec 30, 2019
0baf816
Linting
jmoore914 Dec 31, 2019
1db2fbe
Use selector instead of function check
fisker Dec 31, 2019
31401c0
Fixed fixer alterning nodes it doesn't need to
jmoore914 Dec 31, 2019
dd1767d
Linting
jmoore914 Dec 31, 2019
5142086
Improve readability
fisker Jan 2, 2020
9d51cc7
fix fixture code style
fisker Jan 2, 2020
715eb67
Simplify tests
fisker Jan 3, 2020
f446a4a
Update rules/prefer-replace-all.js
jmoore914 Jan 4, 2020
5748d2c
Simplified isLiteralCharactersOnly test
jmoore914 Jan 11, 2020
3a872ed
Merge upstream
jmoore914 Jan 20, 2020
cb080f0
Merge remote-tracking branch 'upstream/master' into prefer-replace-all
jmoore914 Jan 20, 2020
93433b1
Remove escape characters from serach string in fixer
jmoore914 Jan 24, 2020
1242021
Remove extra flag
fisker Jan 26, 2020
df6d636
Update prefer-replace-all.md
sindresorhus Feb 1, 2020
e10b581
Update readme.md
sindresorhus Feb 1, 2020
6824b24
Update prefer-replace-all.js
sindresorhus Feb 1, 2020
fadd1b2
Update prefer-replace-all.js
sindresorhus Feb 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
87 changes: 50 additions & 37 deletions test/prefer-replace-all.js
View file
Open in desktop
Expand Up @@ -19,56 +19,69 @@ const error = {

ruleTester.run('prefer-replace-all', rule, {
valid: [
'foo.replace(/No global flag/, "123")',
'foo.replace(/[abc]/g, "123")',
'foo.replace(/abc?/g, "123")',
'foo.replace(/Non-literal characters .*/g, \'something\');',
'foo.replace(/Other non-literal \\W/g, \'something\');',

'foo.replace(/Extra flags/gi, \'something\');',
'foo.replace("Not a regex expression", \'something\')',
'foo.methodNotReplace(/Wrong method name/g, \'something\');'
// No global flag
'foo.replace(/a/, bar)',
// Special characters
'foo.replace(/[a]/g, bar)',
'foo.replace(/a?/g, bar)',
'foo.replace(/.*/g, bar)',
'foo.replace(/\\W/g, bar)',
// Extra flag
'foo.replace(/a/gi, bar)',
// Not regex literal
'foo.replace(\'string\', bar)',
// Not 2 arguments
'foo.replace(/a/g)',
// New
'new foo.replace(/a/g, bar)',
// Function call
'replace(/a/g, bar)',
// Not call
'foo.replace',
// Not replace
'foo.methodNotReplace(/a/g, bar);',
// `replace` is not Identifier
'foo[\'replace\'](/a/gi, bar)'
],
invalid: [
{
code: 'foo.replace(/This has no special regex symbols/g, \'something\')',
output: 'foo.replaceAll(\'This has no special regex symbols\', \'something\')',
errors: [error]
},
{
code: 'foo.replace(/\\(It also checks for escaped regex symbols\\)/g, \'something\')',
output: 'foo.replaceAll(\'\\(It also checks for escaped regex symbols\\)\', \'something\')',
errors: [error]
},
{
code: 'foo.replace(/a\\\\bc\\?/g, \'123\')',
output: 'foo.replaceAll(\'a\\\\bc\\?\', \'123\')',
errors: [error]
},
{
code: 'console.log(foo.replace(/a\\\\bc\\?/g, \'123\'))',
output: 'console.log(foo.replaceAll(\'a\\\\bc\\?\', \'123\'))',
code: 'foo.replace(/a/g, bar)',
output: 'foo.replaceAll(\'a\', bar)',
errors: [error]
},
// Comments
{
code: 'foo.replace(/"doubleQuotes"/g, \'1"2"3\')',
output: 'foo.replaceAll(\'"doubleQuotes"\', \'1"2"3\')',
code: `
foo/* comment 1 */
.replace/* comment 2 */(
/* comment 3 */
/a/g // comment 4
,
bar
)
`,
output: `
foo/* comment 1 */
.replaceAll/* comment 2 */(
/* comment 3 */
'a' // comment 4
,
bar
)
`,
errors: [error]
},
// Quotes
{
code: 'foo.replace(/\'singleQuotes\'/g, "1\'2\'3")',
output: 'foo.replaceAll(\'\\\'singleQuotes\\\'\', "1\'2\'3")',
code: 'foo.replace(/"\'/g, \'\\\'\')',
output: 'foo.replaceAll(\'"\\\'\', \'\\\'\')',
errors: [error]
},
// Escaped symbols
{
code: 'foo.replace(/searchPattern/g, \'\\\'escapedQuotes\\\'\')',
output: 'foo.replaceAll(\'searchPattern\', \'\\\'escapedQuotes\\\'\')',
code: 'foo.replace(/\\./g, bar)',
output: 'foo.replaceAll(\'.\', bar)',
errors: [error]
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmoore914 I think I found a broken case, we still need replace \

Copy link
Collaborator

@fisker fisker Jan 3, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's not that simple at all, I think we should test all these cases

/\./g 
/\\./g 
/\\\./g

{
code: 'foo().replace(/searchPattern/g, bar())',
output: 'foo().replaceAll(\'searchPattern\', bar())',
errors: [error]
}
]
});