Skip to content

Commit

Permalink
prefer-number-properties: Fix false positives (#2050)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker authored Mar 30, 2023
1 parent e1c50ac commit 124bfa7
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 24 deletions.
8 changes: 6 additions & 2 deletions rules/consistent-destructuring.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const avoidCapture = require('./utils/avoid-capture.js');
const {not, notLeftHandSideSelector} = require('./selectors/index.js');
const {not} = require('./selectors/index.js');
const isLeftHandSide = require('./utils/is-left-hand-side.js');

const MESSAGE_ID = 'consistentDestructuring';
const MESSAGE_ID_SUGGEST = 'consistentDestructuringSuggest';
Expand All @@ -15,7 +16,6 @@ const declaratorSelector = [
const memberSelector = [
'MemberExpression',
'[computed!=true]',
notLeftHandSideSelector(),
not([
'CallExpression > .callee',
'NewExpression> .callee',
Expand Down Expand Up @@ -70,6 +70,10 @@ const create = context => {
});
},
[memberSelector](node) {
if (isLeftHandSide(node)) {
return;
}

const declaration = declarations.get(source.getText(node.object));

if (!declaration) {
Expand Down
7 changes: 5 additions & 2 deletions rules/prefer-array-find.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const {isParenthesized, findVariable} = require('@eslint-community/eslint-utils'
const {
not,
methodCallSelector,
notLeftHandSideSelector,
} = require('./selectors/index.js');
const getVariableIdentifiers = require('./utils/get-variable-identifiers.js');
const avoidCapture = require('./utils/avoid-capture.js');
Expand All @@ -15,6 +14,7 @@ const {
removeMethodCall,
renameVariable,
} = require('./fix/index.js');
const isLeftHandSide = require('./utils/is-left-hand-side.js');

const ERROR_ZERO_INDEX = 'error-zero-index';
const ERROR_SHIFT = 'error-shift';
Expand Down Expand Up @@ -62,7 +62,6 @@ const zeroIndexSelector = [
'[computed!=false]',
'[property.type="Literal"]',
'[property.raw="0"]',
notLeftHandSideSelector(),
methodCallSelector({
...filterMethodSelectorOptions,
path: 'object',
Expand Down Expand Up @@ -267,6 +266,10 @@ const create = context => {

const listeners = {
[zeroIndexSelector](node) {
if (isLeftHandSide(node)) {
return;
}

return {
node: node.object.callee.property,
messageId: ERROR_ZERO_INDEX,
Expand Down
7 changes: 5 additions & 2 deletions rules/prefer-at.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const {
getNegativeIndexLengthNode,
removeLengthNode,
} = require('./shared/negative-index.js');
const {methodCallSelector, callExpressionSelector, notLeftHandSideSelector} = require('./selectors/index.js');
const {methodCallSelector, callExpressionSelector} = require('./selectors/index.js');
const {removeMemberExpressionProperty, removeMethodCall} = require('./fix/index.js');
const {isLiteral} = require('./ast/index.js');

Expand All @@ -38,7 +38,6 @@ const indexAccess = [
'MemberExpression',
'[optional!=true]',
'[computed!=false]',
notLeftHandSideSelector(),
].join('');
const sliceCall = methodCallSelector({method: 'slice', minimumArguments: 1, maximumArguments: 2});
const stringCharAt = methodCallSelector({method: 'charAt', argumentsLength: 1});
Expand Down Expand Up @@ -150,6 +149,10 @@ function create(context) {

return {
[indexAccess](node) {
if (isLeftHandSide(node)) {
return;
}

const indexNode = node.property;
const lengthNode = getNegativeIndexLengthNode(indexNode, node.object);

Expand Down
2 changes: 2 additions & 0 deletions rules/prefer-number-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
const {GlobalReferenceTracker} = require('./utils/global-reference-tracker.js');
const {replaceReferenceIdentifier} = require('./fix/index.js');
const {fixSpaceAroundKeyword} = require('./fix/index.js');
const isLeftHandSide = require('./utils/is-left-hand-side.js');

const MESSAGE_ID_ERROR = 'error';
const MESSAGE_ID_SUGGESTION = 'suggestion';
Expand Down Expand Up @@ -90,6 +91,7 @@ const create = context => {
const tracker = new GlobalReferenceTracker({
objects,
handle: reference => checkProperty(reference, sourceCode),
filter: ({node}) => !isLeftHandSide(node),
});

return tracker.createListeners(context);
Expand Down
1 change: 0 additions & 1 deletion rules/selectors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ module.exports = {
callOrNewExpressionSelector: require('./call-or-new-expression-selector.js').callOrNewExpressionSelector,
STATIC_REQUIRE_SELECTOR: require('./require-selector.js').STATIC_REQUIRE_SELECTOR,
STATIC_REQUIRE_SOURCE_SELECTOR: require('./require-selector.js').STATIC_REQUIRE_SOURCE_SELECTOR,
notLeftHandSideSelector: require('./not-left-hand-side.js'),
};
15 changes: 0 additions & 15 deletions rules/selectors/not-left-hand-side.js

This file was deleted.

13 changes: 11 additions & 2 deletions rules/utils/is-left-hand-side.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
'use strict';

// Keep logic sync with `../selector/not-left-hand-side.js`
const isLeftHandSide = node =>
(node.parent.type === 'AssignmentExpression' && node.parent.left === node)
(
(node.parent.type === 'AssignmentExpression' || node.parent.type === 'AssignmentPattern')
&& node.parent.left === node
)
|| (node.parent.type === 'UpdateExpression' && node.parent.argument === node)
|| (node.parent.type === 'ArrayPattern' && node.parent.elements.includes(node))
|| (
node.parent.type === 'Property'
&& node.parent.value === node
&& node.parent.parent.type === 'ObjectPattern'
&& node.parent.parent.properties.includes(node.parent)
)
|| (
node.parent.type === 'UnaryExpression'
&& node.parent.operator === 'delete'
Expand Down
1 change: 1 addition & 0 deletions test/prefer-array-find.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ test({
'++ array.filter(foo)[0]',
'array.filter(foo)[0]--',
'delete array.filter(foo)[0]',
'[array.filter(foo)[0] = 1] = []',
],
invalid: [
{
Expand Down
2 changes: 2 additions & 0 deletions test/prefer-at.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ test.snapshot({
'array[array.length - 1] --',
'delete array[array.length - 1]',
'class Foo {bar; #bar; baz() {return this.#bar[this.bar.length - 1]}}',
'([array[array.length - 1]] = [])',
'({foo: array[array.length - 1] = 9} = {})',
],
invalid: [
'array[array.length - 1];',
Expand Down
18 changes: 18 additions & 0 deletions test/prefer-number-properties.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,24 @@ test({
return ${code}
}
`),

// Not read
'global.isFinite = Number.isFinite;',
'global.isFinite ??= 1;',
'isFinite ||= 1;',
'[global.isFinite] = [];',
'[global.isFinite = 1] = [];',
'[[global.isFinite = 1]] = [];',
'[isFinite] = [];',
'[isFinite = 1] = [];',
'[[isFinite = 1]] = [];',
'({foo: global.isFinite} = {});',
'({foo: global.isFinite = 1} = {});',
'({foo: {bar: global.isFinite = 1}} = {});',
'({foo: isFinite} = {});',
'({foo: isFinite = 1} = {});',
'({foo: {bar: isFinite = 1}} = {});',
'delete global.isFinite;',
],

invalid: [
Expand Down

0 comments on commit 124bfa7

Please sign in to comment.