Skip to content

Commit

Permalink
Update: enforceForClassMembers option to accessor-pairs (fixes #12063) (
Browse files Browse the repository at this point in the history
  • Loading branch information
mdjermanovic authored and ilyavolodin committed Sep 14, 2019
1 parent d3c2334 commit 540296f
Show file tree
Hide file tree
Showing 3 changed files with 1,034 additions and 18 deletions.
65 changes: 63 additions & 2 deletions docs/rules/accessor-pairs.md
@@ -1,4 +1,4 @@
# Enforces getter/setter pairs in objects (accessor-pairs)
# Enforces getter/setter pairs in objects and classes (accessor-pairs)

It's a common mistake in JavaScript to create an object with just a setter for a property but never have a corresponding getter defined for it. Without a getter, you cannot read the property, so it ends up not being used.

Expand Down Expand Up @@ -32,10 +32,14 @@ This rule enforces a style where it requires to have a getter for every property

By activating the option `getWithoutSet` it enforces the presence of a setter for every property which has a getter defined.

By default, this rule checks only object literals and property descriptors. If you want this rule
to also check class declarations and class expressions, activate the option `enforceForClassMembers`.

## Options

* `setWithoutGet` set to `true` will warn for setters without getters (Default `true`).
* `getWithoutSet` set to `true` will warn for getters without setters (Default `false`).
* `enforceForClassMembers` set to `true` additionally applies this rule to class getters/setters (Default `false`).

### setWithoutGet

Expand Down Expand Up @@ -143,6 +147,61 @@ Object.defineProperty(o, 'c', {

```

### enforceForClassMembers

By default, this rule does not enforce getter/setter pairs in class declarations and class expressions,
as the default value for `enforceForClassMembers` is `false`.

When `enforceForClassMembers` is set to `true`:

* `"getWithoutSet": true` will also warn for getters without setters in classes.
* `"setWithoutGet": true` will also warn for setters without getters in classes.

Examples of **incorrect** code for `{ "getWithoutSet": true, "enforceForClassMembers": true }`:

```js
/*eslint accessor-pairs: ["error", { "getWithoutSet": true, "enforceForClassMembers": true }]*/

class Foo {
get a() {
return this.val;
}
}

class Bar {
static get a() {
return this.val;
}
}

const Baz = class {
get a() {
return this.val;
}
static set a(value) {
this.val = value;
}
}
```

Examples of **incorrect** code for `{ "setWithoutGet": true, "enforceForClassMembers": true }`:

```js
/*eslint accessor-pairs: ["error", { "setWithoutGet": true, "enforceForClassMembers": true }]*/

class Foo {
set a(value) {
this.val = value;
}
}

const Bar = class {
static set a(value) {
this.val = value;
}
}
```

## Known Limitations

Due to the limits of static analysis, this rule does not account for possible side effects and in certain cases
Expand All @@ -164,7 +223,7 @@ var o = {
};
```

Also, this rule does not disallow duplicate keys in object literals, and in certain cases with duplicate keys
Also, this rule does not disallow duplicate keys in object literals and class definitions, and in certain cases with duplicate keys
might not report a missing pair for a getter/setter, like in the following example:

```js
Expand All @@ -186,6 +245,8 @@ The code above creates an object with just a setter for the property `"a"`.

See [no-dupe-keys](no-dupe-keys.md) if you also want to disallow duplicate keys in object literals.

See [no-dupe-class-members](no-dupe-class-members.md) if you also want to disallow duplicate names in class definitions.

## When Not To Use It

You can turn this rule off if you are not concerned with the simultaneous presence of setters and getters on objects.
Expand Down
62 changes: 51 additions & 11 deletions lib/rules/accessor-pairs.js
Expand Up @@ -152,7 +152,7 @@ module.exports = {
type: "suggestion",

docs: {
description: "enforce getter and setter pairs in objects",
description: "enforce getter and setter pairs in objects and classes",
category: "Best Practices",
recommended: false,
url: "https://eslint.org/docs/rules/accessor-pairs"
Expand All @@ -168,6 +168,10 @@ module.exports = {
setWithoutGet: {
type: "boolean",
default: true
},
enforceForClassMembers: {
type: "boolean",
default: false
}
},
additionalProperties: false
Expand All @@ -177,13 +181,16 @@ module.exports = {
missingGetterInPropertyDescriptor: "Getter is not present in property descriptor.",
missingSetterInPropertyDescriptor: "Setter is not present in property descriptor.",
missingGetterInObjectLiteral: "Getter is not present for {{ name }}.",
missingSetterInObjectLiteral: "Setter is not present for {{ name }}."
missingSetterInObjectLiteral: "Setter is not present for {{ name }}.",
missingGetterInClass: "Getter is not present for class {{ name }}.",
missingSetterInClass: "Setter is not present for class {{ name }}."
}
},
create(context) {
const config = context.options[0] || {};
const checkGetWithoutSet = config.getWithoutSet === true;
const checkSetWithoutGet = config.setWithoutGet !== false;
const enforceForClassMembers = config.enforceForClassMembers === true;
const sourceCode = context.getSourceCode();

/**
Expand All @@ -201,6 +208,13 @@ module.exports = {
loc: astUtils.getFunctionHeadLoc(node.value, sourceCode),
data: { name: astUtils.getFunctionNameWithKind(node.value) }
});
} else if (node.type === "MethodDefinition") {
context.report({
node,
messageId: `${messageKind}InClass`,
loc: astUtils.getFunctionHeadLoc(node.value, sourceCode),
data: { name: astUtils.getFunctionNameWithKind(node.value) }
});
} else {
context.report({
node,
Expand Down Expand Up @@ -313,15 +327,41 @@ module.exports = {
}
}

return {
ObjectExpression(node) {
if (checkSetWithoutGet || checkGetWithoutSet) {
checkObjectLiteral(node);
if (isPropertyDescriptor(node)) {
checkPropertyDescriptor(node);
}
}
/**
* Checks the given object expression as an object literal and as a possible property descriptor.
* @param {ASTNode} node `ObjectExpression` node to check.
* @returns {void}
* @private
*/
function checkObjectExpression(node) {
checkObjectLiteral(node);
if (isPropertyDescriptor(node)) {
checkPropertyDescriptor(node);
}
};
}

/**
* Checks the given class body.
* @param {ASTNode} node `ClassBody` node to check.
* @returns {void}
* @private
*/
function checkClassBody(node) {
const methodDefinitions = node.body.filter(m => m.type === "MethodDefinition");

checkList(methodDefinitions.filter(m => m.static));
checkList(methodDefinitions.filter(m => !m.static));
}

const listeners = {};

if (checkSetWithoutGet || checkGetWithoutSet) {
listeners.ObjectExpression = checkObjectExpression;
if (enforceForClassMembers) {
listeners.ClassBody = checkClassBody;
}
}

return listeners;
}
};

0 comments on commit 540296f

Please sign in to comment.