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

Breaking: classes default true in accessor-pairs (fixes #12811) #12919

Merged
merged 1 commit into from Feb 27, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
45 changes: 38 additions & 7 deletions docs/rules/accessor-pairs.md
Expand Up @@ -32,14 +32,13 @@ 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`.
This rule always checks object literals and property descriptors. By default, it also checks class declarations and class expressions.

## 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`).
* `enforceForClassMembers` set to `true` additionally applies this rule to class getters/setters (Default `true`). Set `enforceForClassMembers` to `false` if you want this rule to ignore class declarations and class expressions.

### setWithoutGet

Expand Down Expand Up @@ -149,10 +148,7 @@ 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`:
When `enforceForClassMembers` is set to `true` (default):

* `"getWithoutSet": true` will also warn for getters without setters in classes.
* `"setWithoutGet": true` will also warn for setters without getters in classes.
Expand Down Expand Up @@ -202,6 +198,41 @@ const Bar = class {
}
```

When `enforceForClassMembers` is set to `false`, this rule ignores classes.

Examples of **correct** code for `{ "getWithoutSet": true, "setWithoutGet": true, "enforceForClassMembers": false }`:

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

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

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

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

const Quux = class {
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 Down
4 changes: 2 additions & 2 deletions lib/rules/accessor-pairs.js
Expand Up @@ -171,7 +171,7 @@ module.exports = {
},
enforceForClassMembers: {
type: "boolean",
default: false
default: true
}
},
additionalProperties: false
Expand All @@ -190,7 +190,7 @@ module.exports = {
const config = context.options[0] || {};
const checkGetWithoutSet = config.getWithoutSet === true;
const checkSetWithoutGet = config.setWithoutGet !== false;
const enforceForClassMembers = config.enforceForClassMembers === true;
const enforceForClassMembers = config.enforceForClassMembers !== false;
const sourceCode = context.getSourceCode();

/**
Expand Down
89 changes: 50 additions & 39 deletions tests/lib/rules/accessor-pairs.js
Expand Up @@ -330,50 +330,11 @@ ruleTester.run("accessor-pairs", rule, {
//------------------------------------------------------------------------------

// Test default settings
{
code: "class A { set a(foo) {} }",
parserOptions: { ecmaVersion: 6 }
},
{
code: "class A { get a() {} set b(foo) {} }",
options: [{}],
parserOptions: { ecmaVersion: 6 }
},
{
code: "class A { get a() {} }",
options: [{ enforceForClassMembers: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "class A { get a() {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "class A { set a(foo) {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "class A { static get a() {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "class A { static set a(foo) {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "A = class { get a() {} };",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 }
},
{
code: "A = class { get a() {} set b(foo) {} };",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 }
},

// Explicitly disabled option
{
Expand Down Expand Up @@ -1132,6 +1093,56 @@ ruleTester.run("accessor-pairs", rule, {
//------------------------------------------------------------------------------

// Test default settings
{
code: "class A { set a(foo) {} }",
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Getter is not present for class setter 'a'.", type: "MethodDefinition" }]
},
{
code: "class A { get a() {} set b(foo) {} }",
options: [{}],
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Getter is not present for class setter 'b'.", type: "MethodDefinition" }]
},
{
code: "class A { get a() {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Setter is not present for class getter 'a'.", type: "MethodDefinition" }]
},
{
code: "class A { set a(foo) {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Getter is not present for class setter 'a'.", type: "MethodDefinition" }]
},
{
code: "class A { static get a() {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Setter is not present for class static getter 'a'.", type: "MethodDefinition" }]
},
{
code: "class A { static set a(foo) {} }",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Getter is not present for class static setter 'a'.", type: "MethodDefinition" }]
},
{
code: "A = class { get a() {} };",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 },
errors: [{ message: "Setter is not present for class getter 'a'.", type: "MethodDefinition" }]
},
{
code: "A = class { get a() {} set b(foo) {} };",
options: [{ setWithoutGet: true, getWithoutSet: true }],
parserOptions: { ecmaVersion: 6 },
errors: [
{ message: "Setter is not present for class getter 'a'.", type: "MethodDefinition" },
{ message: "Getter is not present for class setter 'b'.", type: "MethodDefinition" }
]
},
{
code: "class A { set a(value) {} }",
options: [{ enforceForClassMembers: true }],
Expand Down