Skip to content

Commit

Permalink
feat: make no-misleading-character-class report more granular errors (
Browse files Browse the repository at this point in the history 
#18082)

* feat: report granular errors on arbitrary literals

* use npm dependency

* test with unescaped CRLF

* inline `createReportLocationGenerator`

* unit test for templates with expressions

* restore old name `getNodeReportLocations`

* update JSDoc

* `charInfos` → `codeUnits`

* extract char-source to a utility module

* add `read` method to `SourceReader`

* add `advance` method and JSDoc

* fix logic

* `SourceReader` → `TextReader`

* handle `RegExp` calls with regex patterns

* fix for browser test

* fix for Node.js 18

* limit applicability of `getStaticValue` for Node.js 18 compatibility

* fix for `RegExp()` without arguments

* update JSDoc for `getStaticValueOrRegex`
  • Loading branch information
@fasttime
fasttime committed Mar 5, 2024
1 parent 972ef15 commit a451b32
Show file tree
Hide file tree
Showing 4 changed files with 945 additions and 147 deletions.
173 changes: 110 additions & 63 deletions lib/rules/no-misleading-character-class.js
View file
Expand Up @@ -3,11 +3,18 @@
*/
"use strict";

const { CALL, CONSTRUCT, ReferenceTracker, getStringIfConstant } = require("@eslint-community/eslint-utils");
const {
CALL,
CONSTRUCT,
ReferenceTracker,
getStaticValue,
getStringIfConstant
} = require("@eslint-community/eslint-utils");
const { RegExpParser, visitRegExpAST } = require("@eslint-community/regexpp");
const { isCombiningCharacter, isEmojiModifier, isRegionalIndicatorSymbol, isSurrogatePair } = require("./utils/unicode");
const astUtils = require("./utils/ast-utils.js");
const { isValidWithUnicodeFlag } = require("./utils/regular-expressions");
const { parseStringLiteral, parseTemplateToken } = require("./utils/char-source");

//------------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -193,6 +200,33 @@ const findCharacterSequences = {

const kinds = Object.keys(findCharacterSequences);

/**
* Gets the value of the given node if it's a static value other than a regular expression object,
* or the node's `regex` property.
* The purpose of this method is to provide a replacement for `getStaticValue` in environments where certain regular expressions cannot be evaluated.
* A known example is Node.js 18 which does not support the `v` flag.
* Calling `getStaticValue` on a regular expression node with the `v` flag on Node.js 18 always returns `null`.
* A limitation of this method is that it can only detect a regular expression if the specified node is itself a regular expression literal node.
* @param {ASTNode | undefined} node The node to be inspected.
* @param {Scope} initialScope Scope to start finding variables. This function tries to resolve identifier references which are in the given scope.
* @returns {{ value: any } | { regex: { pattern: string, flags: string } } | null} The static value of the node, or `null`.
*/
function getStaticValueOrRegex(node, initialScope) {
if (!node) {
return null;
}
if (node.type === "Literal" && node.regex) {
return { regex: node.regex };
}

const staticValue = getStaticValue(node, initialScope);

if (staticValue?.value instanceof RegExp) {
return null;
}
return staticValue;
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -225,62 +259,7 @@ module.exports = {
create(context) {
const sourceCode = context.sourceCode;
const parser = new RegExpParser();

/**
* Generates a granular loc for context.report, if directly calculable.
* @param {Character[]} chars Individual characters being reported on.
* @param {Node} node Parent string node to report within.
* @returns {Object | null} Granular loc for context.report, if directly calculable.
* @see https://github.com/eslint/eslint/pull/17515
*/
function generateReportLocation(chars, node) {

// Limit to to literals and expression-less templates with raw values === their value.
switch (node.type) {
case "TemplateLiteral":
if (node.expressions.length || sourceCode.getText(node).slice(1, -1) !== node.quasis[0].value.cooked) {
return null;
}
break;

case "Literal":
if (typeof node.value === "string" && node.value !== node.raw.slice(1, -1)) {
return null;
}
break;

default:
return null;
}

return {
start: sourceCode.getLocFromIndex(node.range[0] + 1 + chars[0].start),
end: sourceCode.getLocFromIndex(node.range[0] + 1 + chars.at(-1).end)
};
}

/**
* Finds the report loc(s) for a range of matches.
* @param {Character[][]} matches Characters that should trigger a report.
* @param {Node} node The node to report.
* @returns {Object | null} Node loc(s) for context.report.
*/
function getNodeReportLocations(matches, node) {
const locs = [];

for (const chars of matches) {
const loc = generateReportLocation(chars, node);

// If a report can't match to a range, don't report any others
if (!loc) {
return [node.loc];
}

locs.push(loc);
}

return locs;
}
const checkedPatternNodes = new Set();

/**
* Verify a given regular expression.
Expand Down Expand Up @@ -320,12 +299,58 @@ module.exports = {
} else {
foundKindMatches.set(kind, [...findCharacterSequences[kind](chars)]);
}

}
}
}
});

let codeUnits = null;

/**
* Finds the report loc(s) for a range of matches.
* Only literals and expression-less templates generate granular errors.
* @param {Character[][]} matches Lists of individual characters being reported on.
* @returns {Location[]} locs for context.report.
* @see https://github.com/eslint/eslint/pull/17515
*/
function getNodeReportLocations(matches) {
if (!astUtils.isStaticTemplateLiteral(node) && node.type !== "Literal") {
return matches.length ? [node.loc] : [];
}
return matches.map(chars => {
const firstIndex = chars[0].start;
const lastIndex = chars.at(-1).end - 1;
let start;
let end;

if (node.type === "TemplateLiteral") {
const source = sourceCode.getText(node);
const offset = node.range[0];

codeUnits ??= parseTemplateToken(source);
start = offset + codeUnits[firstIndex].start;
end = offset + codeUnits[lastIndex].end;
} else if (typeof node.value === "string") { // String Literal
const source = node.raw;
const offset = node.range[0];

codeUnits ??= parseStringLiteral(source);
start = offset + codeUnits[firstIndex].start;
end = offset + codeUnits[lastIndex].end;
} else { // RegExp Literal
const offset = node.range[0] + 1; // Add 1 to skip the leading slash.

start = offset + firstIndex;
end = offset + lastIndex + 1;
}

return {
start: sourceCode.getLocFromIndex(start),
end: sourceCode.getLocFromIndex(end)
};
});
}

for (const [kind, matches] of foundKindMatches) {
let suggest;

Expand All @@ -336,7 +361,7 @@ module.exports = {
}];
}

const locs = getNodeReportLocations(matches, node);
const locs = getNodeReportLocations(matches);

for (const loc of locs) {
context.report({
Expand All @@ -351,6 +376,9 @@ module.exports = {

return {
"Literal[regex]"(node) {
if (checkedPatternNodes.has(node)) {
return;
}
verify(node, node.regex.pattern, node.regex.flags, fixer => {
if (!isValidWithUnicodeFlag(context.languageOptions.ecmaVersion, node.regex.pattern)) {
return null;
Expand All @@ -371,12 +399,31 @@ module.exports = {
for (const { node: refNode } of tracker.iterateGlobalReferences({
RegExp: { [CALL]: true, [CONSTRUCT]: true }
})) {
let pattern, flags;
const [patternNode, flagsNode] = refNode.arguments;
const pattern = getStringIfConstant(patternNode, scope);
const flags = getStringIfConstant(flagsNode, scope);
const evaluatedPattern = getStaticValueOrRegex(patternNode, scope);

if (!evaluatedPattern) {
continue;
}
if (flagsNode) {
if (evaluatedPattern.regex) {
pattern = evaluatedPattern.regex.pattern;
checkedPatternNodes.add(patternNode);
} else {
pattern = String(evaluatedPattern.value);
}
flags = getStringIfConstant(flagsNode, scope);
} else {
if (evaluatedPattern.regex) {
continue;
}
pattern = String(evaluatedPattern.value);
flags = "";
}

if (typeof pattern === "string") {
verify(patternNode, pattern, flags || "", fixer => {
if (typeof flags === "string") {
verify(patternNode, pattern, flags, fixer => {

if (!isValidWithUnicodeFlag(context.languageOptions.ecmaVersion, pattern)) {
return null;
Expand Down

0 comments on commit a451b32

Please sign in to comment.