Skip to content

Commit

Permalink
Fix: no-dupe-keys false negative (fixes #6801) (#6863)
Browse files Browse the repository at this point in the history 
This commit fixes a false negative between a normal property and a
getter/setter.
In addition, I upgrade code by ES2015.
  • Loading branch information
@mysticatea @nzakas
mysticatea authored and nzakas committed Aug 10, 2016
1 parent 1ecd2a3 commit 11395ca
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 26 deletions.
122 changes: 98 additions & 24 deletions lib/rules/no-dupe-keys.js
View file
Expand Up @@ -5,6 +5,79 @@

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("../ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

const GET_KIND = /^(?:init|get)$/;
const SET_KIND = /^(?:init|set)$/;

/**
* The class which stores properties' information of an object.
*/
class ObjectInfo {

/**
* @param {ObjectInfo|null} upper - The information of the outer object.
* @param {ASTNode} node - The ObjectExpression node of this information.
*/
constructor(upper, node) {
this.upper = upper;
this.node = node;
this.properties = new Map();
}

/**
* Gets the information of the given Property node.
* @param {ASTNode} node - The Property node to get.
* @returns {{get: boolean, set: boolean}} The information of the property.
*/
getPropertyInfo(node) {
const name = astUtils.getStaticPropertyName(node);

if (!this.properties.has(name)) {
this.properties.set(name, {get: false, set: false});
}
return this.properties.get(name);
}

/**
* Checks whether the given property has been defined already or not.
* @param {ASTNode} node - The Property node to check.
* @returns {boolean} `true` if the property has been defined.
*/
isPropertyDefined(node) {
const entry = this.getPropertyInfo(node);

return (
(GET_KIND.test(node.kind) && entry.get) ||
(SET_KIND.test(node.kind) && entry.set)
);
}

/**
* Defines the given property.
* @param {ASTNode} node - The Property node to define.
* @returns {void}
*/
defineProperty(node) {
const entry = this.getPropertyInfo(node);

if (GET_KIND.test(node.kind)) {
entry.get = true;
}
if (SET_KIND.test(node.kind)) {
entry.set = true;
}
}
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -20,37 +93,38 @@ module.exports = {
schema: []
},

create: function(context) {
create(context) {
let info = null;

return {
ObjectExpression(node) {
info = new ObjectInfo(info, node);
},
"ObjectExpression:exit"() {
info = info.upper;
},

ObjectExpression: function(node) {

// Object that will be a map of properties--safe because we will
// prefix all of the keys.
const nodeProps = Object.create(null);
Property(node) {
const name = astUtils.getStaticPropertyName(node);

node.properties.forEach(function(property) {
// Skip if the name is not static.
if (!name) {
return;
}

if (property.type !== "Property") {
return;
}

const keyName = property.key.name || property.key.value,
key = property.kind + "-" + keyName,
checkProperty = (!property.computed || property.key.type === "Literal");

if (checkProperty) {
if (nodeProps[key]) {
context.report(node, property.loc.start, "Duplicate key '{{key}}'.", { key: keyName });
} else {
nodeProps[key] = true;
}
}
});
// Reports if the name is defined already.
if (info.isPropertyDefined(node)) {
context.report({
node: info.node,
loc: node.key.loc,
message: "Duplicate key '{{name}}'.",
data: {name},
});
}

// Update info.
info.defineProperty(node);
}
};

}
};
9 changes: 7 additions & 2 deletions tests/lib/rules/no-dupe-keys.js
View file
Expand Up @@ -24,13 +24,18 @@ ruleTester.run("no-dupe-keys", rule, {
"var x = { foo: 1, bar: 2 };",
"+{ get a() { }, set a(b) { } };",
{ code: "var x = { a: b, [a]: b };", parserOptions: { ecmaVersion: 6 }},
{ code: "var x = { a: b, ...c }", parserOptions: { ecmaVersion: 6, ecmaFeatures: { experimentalObjectRestSpread: true } }}
{ code: "var x = { a: b, ...c }", parserOptions: { ecmaVersion: 6, ecmaFeatures: { experimentalObjectRestSpread: true } }},
{ code: "var x = { get a() {}, set a (value) {} };", parserOptions: { ecmaVersion: 6 }},
{ code: "var x = { a: 1, b: { a: 2 } };", parserOptions: { ecmaVersion: 6 }},
],
invalid: [
{ code: "var x = { a: b, ['a']: b };", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Duplicate key 'a'.", type: "ObjectExpression"}] },
{ code: "var x = { y: 1, y: 2 };", errors: [{ message: "Duplicate key 'y'.", type: "ObjectExpression"}] },
{ code: "var foo = { 0x1: 1, 1: 2};", errors: [{ message: "Duplicate key '1'.", type: "ObjectExpression"}] },
{ code: "var x = { \"z\": 1, z: 2 };", errors: [{ message: "Duplicate key 'z'.", type: "ObjectExpression"}] },
{ code: "var foo = {\n bar: 1,\n bar: 1,\n}", errors: [{ message: "Duplicate key 'bar'.", line: 3, column: 3}]}
{ code: "var foo = {\n bar: 1,\n bar: 1,\n}", errors: [{ message: "Duplicate key 'bar'.", line: 3, column: 3}]},
{ code: "var x = { a: 1, get a() {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Duplicate key 'a'.", type: "ObjectExpression"}] },
{ code: "var x = { a: 1, set a(value) {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Duplicate key 'a'.", type: "ObjectExpression"}] },
{ code: "var x = { a: 1, b: { a: 2 }, get b() {} };", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Duplicate key 'b'.", type: "ObjectExpression"}] },
]
});

0 comments on commit 11395ca

Please sign in to comment.