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

tools: Add no useless regex char class rule #9591

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ rules:
align-multiline-assignment: 2
assert-fail-single-argument: 2
new-with-error: [2, Error, RangeError, TypeError, SyntaxError, ReferenceError]
no-useless-regex-char-class-escape: [2, { override: ['[', ']'] }]

# Global scoped method and vars
globals:
Expand Down
2 changes: 1 addition & 1 deletion lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const protocolPattern = /^([a-z0-9.+-]+:)/i;
const portPattern = /:[0-9]*$/;

// Special case for a simple path URL
const simplePathPattern = /^(\/\/?(?!\/)[^\?\s]*)(\?[^\s]*)?$/;
const simplePathPattern = /^(\/\/?(?!\/)[^?\s]*)(\?[^\s]*)?$/;

const hostnameMaxLen = 255;
// protocols that can allow "unsafe" and "unwise" chars.
Expand Down
4 changes: 2 additions & 2 deletions tools/doc/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ function loadGtoc(cb) {
function toID(filename) {
return filename
.replace('.html', '')
.replace(/[^\w\-]/g, '-')
.replace(/[^\w-]/g, '-')
.replace(/-+/g, '-');
}

Expand Down Expand Up @@ -284,7 +284,7 @@ function linkJsTypeDocs(text) {
// Handle types, for example the source Markdown might say
// "This argument should be a {Number} or {String}"
for (i = 0; i < parts.length; i += 2) {
typeMatches = parts[i].match(/\{([^\}]+)\}/g);
typeMatches = parts[i].match(/\{([^}]+)\}/g);
if (typeMatches) {
typeMatches.forEach(function(typeMatch) {
parts[i] = parts[i].replace(typeMatch, typeParser.toLink(typeMatch));
Expand Down
8 changes: 4 additions & 4 deletions tools/doc/json.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function doJSON(input, filename, cb) {
// <!-- type = module -->
// This is for cases where the markdown semantic structure is lacking.
if (type === 'paragraph' || type === 'html') {
var metaExpr = /<!--([^=]+)=([^\-]+)-->\n*/g;
var metaExpr = /<!--([^=]+)=([^-]+)-->\n*/g;
text = text.replace(metaExpr, function(_0, k, v) {
current[k.trim()] = v.trim();
return '';
Expand Down Expand Up @@ -371,7 +371,7 @@ function parseListItem(item) {
item.name = 'return';
text = text.replace(retExpr, '');
} else {
var nameExpr = /^['`"]?([^'`": \{]+)['`"]?\s*:?\s*/;
var nameExpr = /^['`"]?([^'`": {]+)['`"]?\s*:?\s*/;
var name = text.match(nameExpr);
if (name) {
item.name = name[1];
Expand All @@ -388,7 +388,7 @@ function parseListItem(item) {
}

text = text.trim();
var typeExpr = /^\{([^\}]+)\}/;
var typeExpr = /^\{([^}]+)\}/;
var type = text.match(typeExpr);
if (type) {
item.type = type[1];
Expand Down Expand Up @@ -551,7 +551,7 @@ var classMethExpr =
/^class\s*method\s*:?[^.]+\.([^ .()]+)\([^)]*\)\s*?$/i;
var methExpr =
/^(?:method:?\s*)?(?:[^.]+\.)?([^ .()]+)\([^)]*\)\s*?$/i;
var newExpr = /^new ([A-Z][a-zA-Z]+)\([^\)]*\)\s*?$/;
var newExpr = /^new ([A-Z][a-zA-Z]+)\([^)]*\)\s*?$/;
var paramExpr = /\((.*)\);?$/;

function newSection(tok) {
Expand Down
190 changes: 190 additions & 0 deletions tools/eslint-rules/no-useless-regex-char-class-escape.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/**
* @fileoverview Disallow useless escape in regex character class
* Based on 'no-useless-escape' rule
*/
'use strict';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const REGEX_CHARCLASS_ESCAPES = new Set('\\bcdDfnrsStvwWxu0123456789]');

/**
* Parses a regular expression into a list of regex character class list.
* @param {string} regExpText raw text used to create the regular expression
* @returns {Object[]} A list of character classes tokens with index and
* escape info
* @example
*
* parseRegExpCharClass('a\\b[cd-]')
*
* returns:
* [
* {
* empty: false,
* start: 4,
* end: 6,
* chars: [
* {text: 'c', index: 4, escaped: false},
* {text: 'd', index: 5, escaped: false},
* {text: '-', index: 6, escaped: false}
* ]
* }
* ]
*/

function parseRegExpCharClass(regExpText) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that this function is very different from the corresponding one in no-useless-escape. This isn't necessarily a problem, but is there a reason to change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it covers only RegExp character class.
(Possible) Useless Escape > (Possible) Useless RegExp Escape > Useless RegExp Character Class Escape (optional override)

const charList = [];
let charListIdx = -1;
const initState = {
escapeNextChar: false,
inCharClass: false,
startingCharClass: false
};

regExpText.split('').reduce((state, char, index) => {
if (!state.escapeNextChar) {
if (char === '\\') {
return Object.assign(state, { escapeNextChar: true });
}
if (char === '[' && !state.inCharClass) {
charListIdx += 1;
charList.push({ start: index + 1, chars: [], end: -1 });
return Object.assign(state, {
inCharClass: true,
startingCharClass: true
});
}
if (char === ']' && state.inCharClass) {
const charClass = charList[charListIdx];
charClass.empty = charClass.chars.length === 0;
if (charClass.empty) {
charClass.start = charClass.end = -1;
} else {
charList[charListIdx].end = index - 1;
}
return Object.assign(state, {
inCharClass: false,
startingCharClass: false
});
}
}
if (state.inCharClass) {
charList[charListIdx].chars.push({
text: char,
index, escaped:
state.escapeNextChar
});
}
return Object.assign(state, {
escapeNextChar: false,
startingCharClass: false
});
}, initState);

return charList;
}

module.exports = {
meta: {
docs: {
description: 'disallow unnecessary regex characer class escape sequences',
category: 'Best Practices',
recommended: false
},
fixable: 'code',
schema: [{
'type': 'object',
'properties': {
'override': {
'type': 'array',
'items': { 'type': 'string' },
'uniqueItems': true
}
},
'additionalProperties': false
}]
},

create(context) {
const overrideSet = new Set(context.options.length
? context.options[0].override || []
: []);

/**
* Reports a node
* @param {ASTNode} node The node to report
* @param {number} startOffset The backslash's offset
* from the start of the node
* @param {string} character The uselessly escaped character
* (not including the backslash)
* @returns {void}
*/
function report(node, startOffset, character) {
context.report({
node,
loc: {
line: node.loc.start.line,
column: node.loc.start.column + startOffset
},
message: 'Unnecessary regex escape in character' +
' class: \\{{character}}',
data: { character },
fix: (fixer) => {
const start = node.range[0] + startOffset;
return fixer.replaceTextRange([start, start + 1], '');
}
});
}

/**
* Checks if a node has superflous escape character
* in regex character class.
*
* @param {ASTNode} node - node to check.
* @returns {void}
*/
function check(node) {
if (node.regex) {
parseRegExpCharClass(node.regex.pattern)
.forEach((charClass) => {
charClass
.chars
// The '-' character is a special case if is not at
// either edge of the character class. To account for this,
// filter out '-' characters that appear in the middle of a
// character class.
.filter((charInfo) => !(charInfo.text === '-' &&
(charInfo.index !== charClass.start &&
charInfo.index !== charClass.end)))

// The '^' character is a special case if it's at the beginning
// of the character class. To account for this, filter out '^'
// characters that appear at the start of a character class.
//
.filter((charInfo) => !(charInfo.text === '^' &&
charInfo.index === charClass.start))

// Filter out characters that aren't escaped.
.filter((charInfo) => charInfo.escaped)

// Filter out characters that are valid to escape, based on
// their position in the regular expression.
.filter((charInfo) => !REGEX_CHARCLASS_ESCAPES.has(charInfo.text))

// Filter out overridden character list.
.filter((charInfo) => !overrideSet.has(charInfo.text))

// Report all the remaining characters.
.forEach((charInfo) =>
report(node, charInfo.index, charInfo.text));
});
}
}

return {
Literal: check
};
}
};
4 changes: 2 additions & 2 deletions tools/license2rtf.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function ParagraphParser() {

// Detect separator "lines" within a block. These mark a paragraph break
// and are stripped from the output.
if (/^\s*[=*\-]{5,}\s*$/.test(line)) {
if (/^\s*[=*-]{5,}\s*$/.test(line)) {
flushParagraph();
return;
}
Expand Down Expand Up @@ -286,7 +286,7 @@ function RtfGenerator() {

function rtfEscape(string) {
return string
.replace(/[\\\{\}]/g, function(m) {
.replace(/[\\{}]/g, function(m) {
return '\\' + m;
})
.replace(/\t/g, function() {
Expand Down