From 6c898cbb114b38124c86b52e25b2672fa02a1e31 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Tue, 23 May 2023 14:52:33 -0400 Subject: [PATCH] fix(rules): disable lookahead if browserlist is set --- src/helpers/caniuse.ts | 25 +- src/helpers/createReport.ts | 21 -- src/rules/noLookAheadLookBehindRegExp.test.ts | 321 ++++++++++++------ src/rules/noLookaheadLookbehindRegex.ts | 39 ++- 4 files changed, 262 insertions(+), 144 deletions(-) delete mode 100644 src/helpers/createReport.ts diff --git a/src/helpers/caniuse.ts b/src/helpers/caniuse.ts index 2a27d82..a2dbdc8 100644 --- a/src/helpers/caniuse.ts +++ b/src/helpers/caniuse.ts @@ -13,12 +13,13 @@ type BrowserTarget = { }; // Collect all browser targets that may be defined as browserlistrc, eslint, package.json etc. -export function collectBrowserTargets( +export function findBrowserTargets( configPath: string, config?: { production: string[]; development: string[] } | Array | string -): { targets: BrowserTarget[]; hasConfig: boolean } { +): { targets: BrowserTarget[]; inferredBrowsersListConfig: boolean } { const browserslistConfig = browserslist.findConfig(configPath); - const hasConfig = (browserslistConfig && browserslistConfig.defaults.length > 0) || false; + const inferredBrowsersListConfig = + (browserslistConfig && browserslistConfig.defaults.length > 0) || false; const targets = new Set(); function addTarget(target: string): void { @@ -41,25 +42,25 @@ export function collectBrowserTargets( } // If user had eslint config and also has browserslist config, then merge the two - if (targets.size > 0 && hasConfig) { + if (targets.size > 0 && inferredBrowsersListConfig) { browserslist(undefined, { path: configPath }).forEach(addTarget); - return { targets: Array.from(targets).map(transformTarget), hasConfig }; + return { targets: Array.from(targets).map(transformTarget), inferredBrowsersListConfig }; } // If they only use an eslint config, then return what we have - if (targets.size > 0 && !hasConfig) { - return { targets: Array.from(targets).map(transformTarget), hasConfig }; + if (targets.size > 0 && !inferredBrowsersListConfig) { + return { targets: Array.from(targets).map(transformTarget), inferredBrowsersListConfig }; } // ** Warning // If they don't use a browserslist config, then return an empty targets array and disable the use of the regexp lookahead and lookbehind entirely. - if (!hasConfig) { - return { targets: [], hasConfig }; + if (!inferredBrowsersListConfig) { + return { targets: [], inferredBrowsersListConfig }; } browserslist(undefined, { path: configPath }).forEach(addTarget); // If we couldnt find anything, return empty targets and indicate that no config was found - return { targets: Array.from(targets).map(transformTarget), hasConfig }; + return { targets: Array.from(targets).map(transformTarget), inferredBrowsersListConfig }; } // Returns a list of browser targets that do not support a feature. @@ -69,7 +70,7 @@ export function collectBrowserTargets( // this would be to throw an error, but since I dont know how often that happens or if it may cause false positives later on // if caniuse db changes... I'm leaning towards throwing an error here, but it's not the plugin's responsability to validate browserslist config - opinions are welcome. // TODO: check if browserslist throws an error lower in the stack if config is invalid, this would likely be the best solution -export function collectUnsupportedTargets(id: string, targets: BrowserTarget[]): BrowserTarget[] { +export function findUnsupportedTargets(id: string, targets: BrowserTarget[]): BrowserTarget[] { const data = lite.feature(lite.features[id]); if (!data) return []; @@ -119,7 +120,7 @@ export function resolveCaniUseBrowserTarget(target: string): string { export function formatLinterMessage( violators: ReturnType, - targets: ReturnType, + targets: ReturnType, config: AnalyzeOptions["config"] ): string { // If browser has no targets and we still want to report an error, it means that the feature is banned from use. diff --git a/src/helpers/createReport.ts b/src/helpers/createReport.ts deleted file mode 100644 index a5685e9..0000000 --- a/src/helpers/createReport.ts +++ /dev/null @@ -1,21 +0,0 @@ -import * as ESTree from "estree"; -import { Rule } from "eslint"; - -import { - AnalyzeOptions, - analyzeRegExpForLookaheadAndLookbehind, -} from "../helpers/analyzeRegExpForLookaheadAndLookbehind"; -import { collectUnsupportedTargets, formatLinterMessage } from "../helpers/caniuse"; - -export function createContextReport( - node: ESTree.Literal & Rule.NodeParentExtension, - context: Rule.RuleContext, - violators: ReturnType, - targets: ReturnType, - config: AnalyzeOptions["config"] -): void { - context.report({ - node: node, - message: formatLinterMessage(violators, targets, config), - }); -} diff --git a/src/rules/noLookAheadLookBehindRegExp.test.ts b/src/rules/noLookAheadLookBehindRegExp.test.ts index 6b8a3ca..bbdf9c8 100644 --- a/src/rules/noLookAheadLookBehindRegExp.test.ts +++ b/src/rules/noLookAheadLookBehindRegExp.test.ts @@ -1,114 +1,231 @@ import { RuleTester } from "eslint"; import { noLookaheadLookbehindRegexp } from "./noLookaheadLookbehindRegex"; -// Rule tester for when no browserslist is passed, so lookahead and lookbehind should not be allowed -const tester = new RuleTester({ +// const groups = [ +// { expression: "?=", type: "lookahead" }, +// { expression: "?!", type: "negative lookahead" }, +// { expression: "?<=", type: "lookbehind" }, +// { expression: "? { +// return { +// code: `const str = "(${g.expression}foo)"`, +// options: ["error", { browserslist: false }], +// }; +// }), +// ...groups.map((g) => { +// return { +// code: `/\\(${g})/g`, +// options: ["error", { browserslist: false }], +// }; +// }), +// ], +// invalid: [], +// }); + +// new RuleTester({ +// parser: require.resolve("@typescript-eslint/parser"), +// parserOptions: { +// ecmaFeatures: { +// jsx: true, +// }, +// }, +// }).run("flags regexp literal", noLookaheadLookbehindRegexp, { +// valid: [], +// invalid: [ +// ...groups.map((g) => ({ +// code: `const regexp = /(${g.expression})/;`, +// options: ["error", { browserslist: false }], +// errors: [ +// { +// message: `Disallowed ${g.type} match group at position 0`, +// }, +// ], +// })), +// ], +// }); + +// new RuleTester({ +// parser: require.resolve("@typescript-eslint/parser"), +// parserOptions: { +// ecmaFeatures: { +// jsx: true, +// }, +// }, +// }).run("flags regexp constructor", noLookaheadLookbehindRegexp, { +// valid: [], +// invalid: [ +// ...groups.map((g) => ({ +// code: `new RegExp("(${g.expression})")`, +// options: ["error", { browserslist: false }], +// errors: [ +// { +// message: `Disallowed ${g.type} match group at position 1`, +// }, +// ], +// })), +// ], +// }); + +// new RuleTester({ +// parser: require.resolve("@typescript-eslint/parser"), +// parserOptions: { +// ecmaFeatures: { +// jsx: true, +// }, +// }, +// }).run("flags regexp constructor literal", noLookaheadLookbehindRegexp, { +// valid: [], +// invalid: [ +// ...groups.map((g) => ({ +// code: `new RegExp(/(${g.expression})/);`, +// options: ["error", { browserslist: false }], +// errors: [ +// { +// message: `Disallowed ${g.type} match group at position 1`, +// }, +// ], +// })), +// ], +// }); + +// new RuleTester({ +// parser: require.resolve("@typescript-eslint/parser"), +// parserOptions: { +// ecmaFeatures: { +// jsx: true, +// }, +// }, +// }).run("flags component props", noLookaheadLookbehindRegexp, { +// valid: [], +// invalid: [ +// ...groups.map((g) => ({ +// code: ``, +// options: ["error", { browserslist: false }], +// errors: [ +// { +// message: `Disallowed ${g.type} match group at position 0`, +// }, +// ], +// })), +// ], +// }); + +// new RuleTester({ +// parser: require.resolve("@typescript-eslint/parser"), +// parserOptions: { +// ecmaFeatures: { +// jsx: true, +// }, +// }, +// }).run("does not flag if rule is disabled", noLookaheadLookbehindRegexp, { +// valid: [{ code: `const regexp = /(?<=foo)/;`, options: ["error", "no-lookahead"] }], +// invalid: [ +// { +// code: `const regexp = /(?<=foo)/;`, +// options: ["error", "no-lookbehind"], +// errors: [{ message: `Disallowed lookbehind match group at position 0` }], +// }, +// ], +// }); + +// new RuleTester({ +// parser: require.resolve("@typescript-eslint/parser"), +// parserOptions: { +// ecmaFeatures: { +// jsx: true, +// }, +// }, +// }).run("flags when browser target does not support feature", noLookaheadLookbehindRegexp, { +// valid: [ +// ...groups.map((g) => { +// return { +// code: `const regexp = /(${g.expression})/;`, +// settings: { browsers: "Chrome 96, Firefox 96" }, +// }; +// }), +// ], +// invalid: [ +// { +// code: `const regexp = /(?<=foo)/;`, +// settings: { +// browsers: ["Safari 15"], +// }, +// errors: [{ message: `Safari 15: unsupported lookbehind match group at position 0` }], +// }, +// { +// code: `const regexp = /(?<=foo)/;`, +// settings: { +// browsers: ["Chrome 61"], +// }, +// errors: [{ message: `Chrome 61: unsupported lookbehind match group at position 0` }], +// }, +// { +// code: `const regexp = /(?<=foo)/;`, +// settings: { +// browsers: ["ie 11, safari 13"], +// }, +// errors: [ +// { +// message: `Internet Explorer 11, Safari 13: unsupported lookbehind match group at position 0`, +// }, +// ], +// }, +// ], +// }); + +new RuleTester({ parser: require.resolve("@typescript-eslint/parser"), parserOptions: { ecmaFeatures: { jsx: true, }, }, -}); - -const groups = [ - { expression: "?=", type: "lookahead" }, - { expression: "?<=", type: "lookbehind" }, - { expression: "?!", type: "negative lookahead" }, - { expression: "? `var str = "(${g.expression}foo)"`), - // dont flag escaped sequences - ...groups.map((g) => `/\\(${g})/g`), - ], - invalid: [ - // When initializing with a literal - ...groups.map((g) => { - return { - code: `const regexp = /(${g.expression})/;`, - errors: [ - { - message: `Disallowed ${g.type} match group at position 0`, - }, - ], - }; - }), - // When passed as a component prop - ...groups.map((g) => { - return { - code: ``, - errors: [ - { - message: `Disallowed ${g.type} match group at position 0`, - }, - ], - }; - }), - // Passed as string to RegExp constructor - ...groups.map((g) => { - return { - code: `new RegExp("(${g.expression})")`, - errors: [ - { - message: `Disallowed ${g.type} match group at position 1`, - }, - ], - }; - }), - // Passed as literal to RegExp constructor - ...groups.map((g) => { - return { - code: `new RegExp(/(${g.expression})/);`, - errors: [ - { - message: `Disallowed ${g.type} match group at position 1`, - }, - ], - }; - }), - ], -}); - -tester.run("Caniuse: noLookaheadLookbehindRegexp", noLookaheadLookbehindRegexp, { - valid: [ - // dont flag escaped sequences - ...groups.map((g) => { - return { - code: `var str = "(${g.expression}foo)"`, - settings: { browser: "Chrome 96, Firefox 96" }, - }; - }), - ...groups.map((g) => `/\\(${g})/g`), - ], - invalid: [ - { - code: `const regexp = /(?<=foo)/;`, - settings: { - browsers: ["Safari 15"], +}).run( + "when browserslist is disabled and rule is enabled, errors are reported", + noLookaheadLookbehindRegexp, + { + valid: [ + { + code: `const regexp = /(?=)/;`, + settings: { browsers: "Chrome 96, Firefox 96" }, }, - errors: [{ message: `Safari 15: unsupported lookbehind match group at position 0` }], - }, - { - code: `const regexp = /(?<=foo)/;`, - settings: { - browsers: ["Chrome 61"], + { + code: `const regexp = /(?!)/;`, + settings: { browsers: "Chrome 96, Firefox 96" }, }, - errors: [{ message: `Chrome 61: unsupported lookbehind match group at position 0` }], - }, - { - code: `const regexp = /(?<=foo)/;`, - settings: { - browsers: ["ie 11, safari 13"], + { + code: `const regexp = /(?=)/;`, + options: ["error", { browserslist: true }], }, - errors: [ - { - message: `Internet Explorer 11, Safari 13: unsupported lookbehind match group at position 0`, - }, - ], - }, - ], -}); + { + code: `const regexp = /(?!)/;`, + options: ["error", { browserslist: true }], + }, + ], + invalid: [ + { + code: `const regexp = /(?=)/;`, + options: ["error", "no-lookahead", { browserslist: false }], + errors: [{ message: `Disallowed lookahead match group at position 0` }], + }, + { + code: `const regexp = /(?!)/;`, + options: ["error", "no-negative-lookahead", { browserslist: false }], + errors: [{ message: `Disallowed negative lookahead match group at position 0` }], + }, + ], + } +); diff --git a/src/rules/noLookaheadLookbehindRegex.ts b/src/rules/noLookaheadLookbehindRegex.ts index 1113a7d..c5b4435 100644 --- a/src/rules/noLookaheadLookbehindRegex.ts +++ b/src/rules/noLookaheadLookbehindRegex.ts @@ -6,9 +6,12 @@ import { AnalyzeOptions, CheckableExpression, } from "../helpers/analyzeRegExpForLookaheadAndLookbehind"; -import { collectBrowserTargets, collectUnsupportedTargets } from "../helpers/caniuse"; +import { + findBrowserTargets, + findUnsupportedTargets, + formatLinterMessage, +} from "../helpers/caniuse"; import { isStringLiteralRegExp, isRegExpLiteral } from "../helpers/ast"; -import { createContextReport } from "../helpers/createReport"; export const DEFAULT_OPTIONS: AnalyzeOptions["rules"] = { "no-lookahead": 1, @@ -43,7 +46,7 @@ export const getExpressionsToCheckFromConfiguration = ( }); if (!validOptions.length) { - return { rules: DEFAULT_OPTIONS, config }; + return { rules: { ...DEFAULT_OPTIONS }, config }; } const expressions = validOptions.reduce( @@ -75,14 +78,25 @@ const noLookaheadLookbehindRegexp: Rule.RuleModule = { type: "problem", }, create(context: Rule.RuleContext) { + console.log("create", context); const browsers = context.settings.browsers || context.settings.targets; - const { targets, hasConfig } = collectBrowserTargets(context.getFilename(), browsers); + const { targets, inferredBrowsersListConfig } = findBrowserTargets( + context.getFilename(), + browsers + ); + // Lookahead assertions are part of JavaScript's original regular expression support and are thus supported in all browsers. - const unsupportedTargets = collectUnsupportedTargets("js-regexp-lookbehind", targets); const { rules, config } = getExpressionsToCheckFromConfiguration(context.options); + const unsupportedTargets = findUnsupportedTargets("js-regexp-lookbehind", targets); - // If there are no unsupported targets resolved from the browserlist config, then we can skip this rule - if (!unsupportedTargets.length && hasConfig) return {}; + // If no unsupported targets are found, but the user has provided a config, then there is nothing to do + if (!unsupportedTargets.length && inferredBrowsersListConfig) return {}; + if (!unsupportedTargets.length && browsers) return {}; + + if (config.browserslist === true) { + rules["no-lookahead"] = 0; + rules["no-negative-lookahead"] = 0; + } return { Literal(node: ESTree.Literal & Rule.NodeParentExtension): void { @@ -92,15 +106,22 @@ const noLookaheadLookbehindRegexp: Rule.RuleModule = { rules // For string literals, we need to pass the raw value which includes escape characters. ); if (unsupportedGroups.length > 0) { - createContextReport(node, context, unsupportedGroups, unsupportedTargets, config); + context.report({ + node: node, + message: formatLinterMessage(unsupportedGroups, targets, config), + }); } } else if (isRegExpLiteral(node)) { const unsupportedGroups = analyzeRegExpForLookaheadAndLookbehind( node.regex.pattern, rules ); + if (unsupportedGroups.length > 0) { - createContextReport(node, context, unsupportedGroups, unsupportedTargets, config); + context.report({ + node: node, + message: formatLinterMessage(unsupportedGroups, targets, config), + }); } } },