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

Refactor avoidCapture function #1399

Merged
merged 9 commits into from Jul 6, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -37,6 +37,7 @@
"xo"
],
"dependencies": {
"@babel/types": "^7.14.5",
"ci-info": "^3.2.0",
"clean-regexp": "^1.0.0",
"eslint-template-visitor": "^2.3.2",
Expand All @@ -46,7 +47,6 @@
"pluralize": "^8.0.0",
"read-pkg-up": "^7.0.1",
"regexp-tree": "^0.1.23",
"reserved-words": "^0.1.2",
"safe-regex": "^2.1.1",
"semver": "^7.3.5"
},
Expand Down
4 changes: 1 addition & 3 deletions rules/catch-error-name.js
Expand Up @@ -33,8 +33,6 @@ const selector = matches([
]);

const create = context => {
const {ecmaVersion} = context.parserOptions;

const options = {
name: 'error',
ignore: [],
Expand Down Expand Up @@ -79,7 +77,7 @@ const create = context => {
variable.scope,
...variable.references.map(({from}) => from)
];
const fixedName = avoidCapture(expectedName, scopes, ecmaVersion);
const fixedName = avoidCapture(expectedName, scopes);

return {
node,
Expand Down
3 changes: 1 addition & 2 deletions rules/consistent-destructuring.js
Expand Up @@ -52,7 +52,6 @@ const isChildInParentScope = (child, parent) => {
};

const create = context => {
const {ecmaVersion} = context.parserOptions;
const source = context.getSourceCode();
const declarations = new Map();

Expand Down Expand Up @@ -108,7 +107,7 @@ const create = context => {
}

// Destructured member collides with an existing identifier
if (avoidCapture(member, [memberScope], ecmaVersion) !== member) {
if (avoidCapture(member, [memberScope]) !== member) {
return;
}
}
Expand Down
2 changes: 1 addition & 1 deletion rules/no-for-loop.js
Expand Up @@ -355,7 +355,7 @@ const create = context => {

const index = indexIdentifierName;
const element = elementIdentifierName ||
avoidCapture(singular(arrayIdentifierName) || defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion);
avoidCapture(singular(arrayIdentifierName) || defaultElementName, getChildScopesRecursive(bodyScope));
const array = arrayIdentifierName;

let declarationElement = element;
Expand Down
2 changes: 1 addition & 1 deletion rules/prefer-array-find.js
Expand Up @@ -299,7 +299,7 @@ const create = context => {
const singularName = singular(node.id.name);
if (singularName) {
// Rename variable to be singularized now that it refers to a single item in the array instead of the entire array.
const singularizedName = avoidCapture(singularName, getChildScopesRecursive(scope), context.parserOptions.ecmaVersion);
const singularizedName = avoidCapture(singularName, getChildScopesRecursive(scope));
yield * renameVariable(variable, singularizedName, fixer);

// Prevent possible variable conflicts
Expand Down
2 changes: 1 addition & 1 deletion rules/prefer-ternary.js
Expand Up @@ -216,7 +216,7 @@ const create = context => {
let generateNewVariables = false;
if (type === 'ThrowStatement') {
const scopes = getScopes(scope);
const errorName = avoidCapture('error', scopes, context.parserOptions.ecmaVersion, isSafeName);
const errorName = avoidCapture('error', scopes, isSafeName);

for (const scope of scopes) {
if (!scopeToNamesGeneratedByFixer.has(scope)) {
Expand Down
3 changes: 1 addition & 2 deletions rules/prevent-abbreviations.js
Expand Up @@ -313,7 +313,6 @@ const isInternalImport = node => {
};

const create = context => {
const {ecmaVersion} = context.parserOptions;
const options = prepareOptions(context.options[0]);
const filenameWithExtension = context.getPhysicalFilename();

Expand Down Expand Up @@ -416,7 +415,7 @@ const create = context => {
variable.scope
];
variableReplacements.samples = variableReplacements.samples.map(
name => avoidCapture(name, scopes, ecmaVersion, isSafeName)
name => avoidCapture(name, scopes, isSafeName)
);

const problem = {
Expand Down
69 changes: 13 additions & 56 deletions rules/utils/avoid-capture.js
@@ -1,34 +1,7 @@
'use strict';
const reservedWords = require('reserved-words');
const {isValidES3Identifier} = require('@babel/types');
fisker marked this conversation as resolved.
Show resolved Hide resolved
const resolveVariableName = require('./resolve-variable-name.js');

const indexifyName = (name, index) => name + '_'.repeat(index);

const scopeHasArgumentsSpecial = scope => {
while (scope) {
/* istanbul ignore next: `someScopeHasVariableName` seems already handle this */
if (scope.taints.get('arguments')) {
return true;
}

scope = scope.upper;
}

return false;
};

const someScopeHasVariableName = (name, scopes) => scopes.some(scope => resolveVariableName(name, scope));

const someScopeIsStrict = scopes => scopes.some(scope => scope.isStrict);

const nameCollidesWithArgumentsSpecial = (name, scopes, isStrict) => {
if (name !== 'arguments') {
return false;
}

return isStrict || scopes.some(scope => scopeHasArgumentsSpecial(scope));
};

/*
Unresolved reference is probably from the global scope. We should avoid using that name.

Expand All @@ -46,31 +19,24 @@ function unicorn() {
}
```
*/
const isUnresolvedName = (name, scopes) => scopes.some(scope =>
const isUnresolvedName = (name, scope) =>
scope.references.some(reference => reference.identifier && reference.identifier.name === name && !reference.resolved) ||
isUnresolvedName(name, scope.childScopes)
);
scope.childScopes.some(scope => isUnresolvedName(name, scope));

const isSafeName = (name, scopes, ecmaVersion, isStrict) => {
ecmaVersion = Math.min(6, ecmaVersion); // 6 is the latest version understood by `reservedWords`

return (
!someScopeHasVariableName(name, scopes) &&
!reservedWords.check(name, ecmaVersion, isStrict) &&
!nameCollidesWithArgumentsSpecial(name, scopes, isStrict) &&
!isUnresolvedName(name, scopes)
);
};
const isSafeName = (name, scopes) =>
isValidES3Identifier(name) &&
name !== 'arguments' &&
!scopes.some(scope => resolveVariableName(name, scope) || isUnresolvedName(name, scope));

const alwaysTrue = () => true;

/**
Rule-specific name check function.

@callback isSafe
@param {string} indexifiedName - The generated candidate name.
@param {string} name - The generated candidate name.
@param {Scope[]} scopes - The same list of scopes you pass to `avoidCapture`.
@returns {boolean} - `true` if the `indexifiedName` is ok.
@returns {boolean} - `true` if the `name` is ok.
*/

/**
Expand All @@ -84,22 +50,13 @@ Useful when you want to rename a variable (or create a new variable) while being

@param {string} name - The desired name for a new variable.
@param {Scope[]} scopes - The list of scopes the new variable will be referenced in.
@param {number} ecmaVersion - The language version, get it from `context.parserOptions.ecmaVersion`.
@param {isSafe} [isSafe] - Rule-specific name check function.
@returns {string} - Either `name` as is, or a string like `${name}_` suffixed with underscores to make the name unique.
*/
module.exports = (name, scopes, ecmaVersion, isSafe = alwaysTrue) => {
const isStrict = someScopeIsStrict(scopes);

let index = 0;
let indexifiedName = indexifyName(name, index);
while (
!isSafeName(indexifiedName, scopes, ecmaVersion, isStrict) ||
!isSafe(indexifiedName, scopes)
) {
index++;
indexifiedName = indexifyName(name, index);
module.exports = (name, scopes, isSafe = alwaysTrue) => {
while (!isSafeName(name, scopes) || !isSafe(name, scopes)) {
name += '_';
}

return indexifiedName;
return name;
};
2 changes: 1 addition & 1 deletion test/prevent-abbreviations.mjs
Expand Up @@ -1295,7 +1295,7 @@ test({

{
code: 'var y',
output: 'var yield',
output: 'var yield_',
options: customOptions,
errors: createErrors()
},
Expand Down