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.

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

Make regex-shorthand rule use regexp-tree for regex literals #437

Merged
merged 2 commits into from Nov 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -44,6 +44,7 @@
"lodash.topairs": "^4.3.0",
"lodash.upperfirst": "^4.3.1",
"read-pkg-up": "^7.0.0",
"regexp-tree": "^0.1.16",
"regexpp": "^3.0.0",
"reserved-words": "^0.1.2",
"safe-regex": "^2.1.1",
Expand Down
2 changes: 1 addition & 1 deletion rules/custom-error-definition.js
Expand Up @@ -4,7 +4,7 @@ const getDocumentationUrl = require('./utils/get-documentation-url');

const MESSAGE_ID_INVALID_EXPORT = 'invalidExport';

const nameRegexp = /^(?:[A-Z][a-z\d]*)*Error$/;
const nameRegexp = /^(?:[A-Z][\da-z]*)*Error$/;

const getClassName = name => upperfirst(name).replace(/(error|)$/i, 'Error');

Expand Down
4 changes: 2 additions & 2 deletions rules/escape-case.js
Expand Up @@ -6,8 +6,8 @@ const {

const getDocumentationUrl = require('./utils/get-documentation-url');

const escapeWithLowercase = /((?:^|[^\\])(?:\\\\)*)\\(x[a-f\d]{2}|u[a-f\d]{4}|u{(?:[a-f\d]+)})/;
const escapePatternWithLowercase = /((?:^|[^\\])(?:\\\\)*)\\(x[a-f\d]{2}|u[a-f\d]{4}|u{(?:[a-f\d]+)}|c[a-z])/;
const escapeWithLowercase = /((?:^|[^\\])(?:\\\\)*)\\(x[\da-f]{2}|u[\da-f]{4}|u{[\da-f]+})/;
const escapePatternWithLowercase = /((?:^|[^\\])(?:\\\\)*)\\(x[\da-f]{2}|u[\da-f]{4}|u{[\da-f]+}|c[a-z])/;
const hasLowercaseCharacter = /[a-z]+/;
const message = 'Use uppercase characters for the value of the escape sequence.';

Expand Down
8 changes: 4 additions & 4 deletions rules/expiring-todo-comments.js
Expand Up @@ -26,9 +26,9 @@ const packageDependencies = {
...packageJson.devDependencies
};

const DEPENDENCY_INCLUSION_RE = /^[+|-]\s*@?[\S+]\/?\S+/;
const VERSION_COMPARISON_RE = /^(@?[\S+]\/?\S+)@(>|>=)([\d]+(\.\d+){0,2}(-[\da-z-]+(\.[\da-z-]+)*)?(\+[\da-z-]+(\.[\da-z-]+)*)?)/i;
const PKG_VERSION_RE = /^(>|>=)([\d]+(\.\d+){0,2}(-[\da-z-]+(\.[\da-z-]+)*)?(\+[\da-z-]+(\.[\da-z-]+)*)?)\s*$/;
const DEPENDENCY_INCLUSION_RE = /^[+-]\s*@?\S+\/?\S+/;
const VERSION_COMPARISON_RE = /^(@?\S\/?\S+)@(>|>=)(\d+(\.\d+){0,2}(-[\d\-a-z]+(\.[\d\-a-z]+)*)?(\+[\d\-a-z]+(\.[\d\-a-z]+)*)?)/i;
const PKG_VERSION_RE = /^(>|>=)(\d+(\.\d+){0,2}(-[\d-a-z]+(\.[\d-a-z]+)*)?(\+[\d-a-z]+(\.[\d-a-z]+)*)?)\s*$/;
const ISO8601_DATE = /(\d{4})-(\d{2})-(\d{2})/;

function parseTodoWithArguments(string, {terms}) {
Expand All @@ -40,7 +40,7 @@ function parseTodoWithArguments(string, {terms}) {
return false;
}

const TODO_ARGUMENT_RE = /\[([^}]+)\]/i;
const TODO_ARGUMENT_RE = /\[([^}]+)]/i;
const result = TODO_ARGUMENT_RE.exec(string);

if (!result) {
Expand Down
2 changes: 1 addition & 1 deletion rules/import-index.js
@@ -1,7 +1,7 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');

const regexp = /^(@.*?\/.*?|[./]+?.*?)(?:\/(\.|(?:index(?:\.js)?))?)$/;
const regexp = /^(@.*?\/.*?|[./]+?.*?)\/(\.|(?:index(?:\.js)?))?$/;
const isImportingIndex = value => regexp.test(value);
const normalize = value => value.replace(regexp, '$1');

Expand Down
2 changes: 1 addition & 1 deletion rules/no-abusive-eslint-disable.js
@@ -1,7 +1,7 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');

const disableRegex = /^eslint-disable(-next-line|-line)?($|(\s+(@[\w-]+\/(?:[\w-]+\/)?)?[\w-]+)?)/;
const disableRegex = /^eslint-disable(-next-line|-line)?($|(\s+(@(?:[\w-]+\/){1,2})?[\w-]+)?)/;

const create = context => ({
Program: node => {
Expand Down
2 changes: 1 addition & 1 deletion rules/number-literal-case.js
Expand Up @@ -2,7 +2,7 @@
const getDocumentationUrl = require('./utils/get-documentation-url');

const fix = value => {
if (!/^0[a-zA-Z]/.test(value)) {
if (!/^0[A-Za-z]/.test(value)) {
return value;
}

Expand Down
2 changes: 1 addition & 1 deletion rules/prevent-abbreviations.js
Expand Up @@ -274,7 +274,7 @@ const getNameReplacements = (name, options, limit = 3) => {
}

// Split words
const words = name.split(/(?=[^a-z])|(?<=[^a-zA-Z])/).filter(Boolean);
const words = name.split(/(?=[^a-z])|(?<=[^A-Za-z])/).filter(Boolean);

let hasReplacements = false;
const combinations = words.map(word => {
Expand Down
63 changes: 39 additions & 24 deletions rules/regex-shorthand.js
@@ -1,5 +1,6 @@
'use strict';
const cleanRegexp = require('clean-regexp');
const {generate, optimize, parse} = require('regexp-tree');
const getDocumentationUrl = require('./utils/get-documentation-url');
const quoteString = require('./utils/quote-string');

Expand All @@ -8,23 +9,46 @@ const message = 'Use regex shorthands to improve readability.';
const create = context => {
return {
'Literal[regex]': node => {
const oldPattern = node.regex.pattern;
const {flags} = node.regex;
const {type, value} = context.getSourceCode().getFirstToken(node);

const newPattern = cleanRegexp(oldPattern, flags);

// Handle regex literal inside RegExp constructor in the other handler
if (node.parent.type === 'NewExpression' && node.parent.callee.name === 'RegExp') {
if (type !== 'RegularExpression') {
return;
}

if (oldPattern !== newPattern) {
let parsedSource;
try {
parsedSource = parse(value);
} catch (error) {
context.report({
node,
message,
fix: fixer => fixer.replaceText(node, `/${newPattern}/${flags}`)
message: '{{original}} can\'t be parsed: {{message}}',
data: {
original: value,
message: error.message
}
});

return;
}

const originalRegex = generate(parsedSource).toString();
const optimizedRegex = optimize(value).toString();

if (originalRegex === optimizedRegex) {
return;
}

context.report({
node,
message: '{{original}} can be optimized to {{optimized}}',
data: {
original: value,
optimized: optimizedRegex
},
fix(fixer) {
return fixer.replaceText(node, optimizedRegex);
}
});
},
'NewExpression[callee.name="RegExp"]': node => {
const arguments_ = node.arguments;
Expand All @@ -35,27 +59,18 @@ const create = context => {

const hasRegExp = arguments_[0].regex;

let oldPattern;
let flags;
if (hasRegExp) {
oldPattern = arguments_[0].regex.pattern;
flags = arguments_[1] && arguments_[1].type === 'Literal' ? arguments_[1].value : arguments_[0].regex.flags;
} else {
oldPattern = arguments_[0].value;
flags = arguments_[1] && arguments_[1].type === 'Literal' ? arguments_[1].value : '';
return;
}

const oldPattern = arguments_[0].value;
const flags = arguments_[1] && arguments_[1].type === 'Literal' ? arguments_[1].value : '';

const newPattern = cleanRegexp(oldPattern, flags);

if (oldPattern !== newPattern) {
let fixed;
if (hasRegExp) {
fixed = `/${newPattern}/`;
} else {
// Escape backslash
fixed = (newPattern || '').replace(/\\/g, '\\\\');
fixed = quoteString(fixed);
}
// Escape backslash
const fixed = quoteString((newPattern || '').replace(/\\/g, '\\\\'));

context.report({
node,
Expand Down
2 changes: 1 addition & 1 deletion rules/throw-new-error.js
@@ -1,7 +1,7 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');

const customError = /^(?:[A-Z][a-z\d]*)*Error$/;
const customError = /^(?:[A-Z][\da-z]*)*Error$/;

const create = context => ({
ThrowStatement: node => {
Expand Down
2 changes: 1 addition & 1 deletion rules/utils/is-valid-variable-name.js
@@ -1,3 +1,3 @@
'use strict';

module.exports = name => /^[a-z$_][a-z$_\d]*$/i.test(name);
module.exports = name => /^[$_a-z][\w$]*$/i.test(name);
4 changes: 2 additions & 2 deletions test/filename-case.js
Expand Up @@ -136,13 +136,13 @@ ruleTester.run('filename-case', rule, {
{case: 'camelCase', ignore: ['\\[FOOBAR\\]\\.js']}
]),
testCaseWithOptions('src/foo/[FOOBAR].js', undefined, [
{case: 'camelCase', ignore: [/\[FOOBAR\]\.js/]}
{case: 'camelCase', ignore: [/\[FOOBAR]\.js/]}
]),
testCaseWithOptions('src/foo/{FOOBAR}.js', undefined, [
{case: 'snakeCase', ignore: ['\\{FOOBAR\\}\\.js']}
]),
testCaseWithOptions('src/foo/{FOOBAR}.js', undefined, [
{case: 'snakeCase', ignore: [/\{FOOBAR\}\.js/]}
{case: 'snakeCase', ignore: [/{FOOBAR}\.js/]}
]),
testCaseWithOptions('src/foo/foo.js', undefined, [
{case: 'kebabCase', ignore: ['^(F|f)oo']}
Expand Down
4 changes: 2 additions & 2 deletions test/package.js
Expand Up @@ -55,7 +55,7 @@ test('Every rule is defined in readme.md usage and list of rules in alphabetical
const readme = await pify(fs.readFile)('readme.md', 'utf8');
let usageRules;
try {
const usageRulesMatch = /## Usage.*?"rules": (\{.*?\})/ms.exec(readme);
const usageRulesMatch = /## Usage.*?"rules": ({.*?})/ms.exec(readme);
t.truthy(usageRulesMatch, 'List of rules should be defined in readme.md ## Usage');
usageRules = JSON.parse(usageRulesMatch[1]);
} catch (_) {}
Expand All @@ -65,7 +65,7 @@ test('Every rule is defined in readme.md usage and list of rules in alphabetical
const rulesMatch = /## Rules(.*?)## Recommended config/ms.exec(readme);
t.truthy(rulesMatch, 'List of rules should be defined in readme.md in ## Rules before ## Recommended config');
const rulesText = rulesMatch[1];
const re = /- \[(.*?)\]\((.*?)\) - (.*)\n/gm;
const re = /- \[(.*?)]\((.*?)\) - (.*)\n/gm;
const rules = [];
let match;
do {
Expand Down
4 changes: 2 additions & 2 deletions test/prefer-starts-ends-with.js
Expand Up @@ -24,8 +24,8 @@ const validRegex = [
/^foo$/,
/^foo+/,
/foo+$/,
/^[f,a]/,
/[f,a]$/,
/^[,af]/,
/[,af]$/,
/^\w/,
/\w$/,
/^foo./,
Expand Down