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

sindresorhus · Nov 24, 2019 · 3554c17 · 3554c17
1 parent bda7769
commit 3554c17
Show file tree

Hide file tree

Showing 15 changed files with 167 additions and 69 deletions.
diff --git a/package.json b/package.json
@@ -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",

diff --git a/rules/custom-error-definition.js b/rules/custom-error-definition.js
@@ -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');
 

diff --git a/rules/escape-case.js b/rules/escape-case.js
@@ -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.';
 

diff --git a/rules/expiring-todo-comments.js b/rules/expiring-todo-comments.js
@@ -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}) {
@@ -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) {

diff --git a/rules/import-index.js b/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');
 

diff --git a/rules/no-abusive-eslint-disable.js b/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 => {

diff --git a/rules/number-literal-case.js b/rules/number-literal-case.js
@@ -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;
 	}
 

diff --git a/rules/prevent-abbreviations.js b/rules/prevent-abbreviations.js
@@ -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 => {

diff --git a/rules/regex-shorthand.js b/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');
 
@@ -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;
@@ -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,

diff --git a/rules/throw-new-error.js b/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 => {

diff --git a/rules/utils/is-valid-variable-name.js b/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);
diff --git a/test/filename-case.js b/test/filename-case.js
@@ -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']}

diff --git a/test/package.js b/test/package.js
@@ -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 (_) {}
@@ -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 {

diff --git a/test/prefer-starts-ends-with.js b/test/prefer-starts-ends-with.js
@@ -24,8 +24,8 @@ const validRegex = [
 	/^foo$/,
 	/^foo+/,
 	/foo+$/,
-	/^[f,a]/,
-	/[f,a]$/,
+	/^[,af]/,
+	/[,af]$/,
 	/^\w/,
 	/\w$/,
 	/^foo./,