Skip to content

Commit

Permalink
[fix] static-property-placement: fix bug with static property refs …
Browse files Browse the repository at this point in the history 
…inside methods

Fixes #2283
  • Loading branch information
@ljharb
dmason30 authored and ljharb committed May 18, 2019
1 parent bdd6192 commit d443bde
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 38 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Expand Up @@ -143,7 +143,7 @@ Enable the rules that you would like to use.
* [react/sort-comp](docs/rules/sort-comp.md): Enforce component methods order (fixable)
* [react/sort-prop-types](docs/rules/sort-prop-types.md): Enforce propTypes declarations alphabetical sorting
* [react/state-in-constructor](docs/rules/state-in-constructor.md): Enforce the state initialization style to be either in a constructor or with a class property
* [react/static-property-placement](docs/rules/static-property-placement.md): Defines where React component static properties should be positioned.
* [react/static-property-placement](docs/rules/static-property-placement.md): Enforces where React component static properties should be positioned.
* [react/style-prop-object](docs/rules/style-prop-object.md): Enforce style prop value being an object
* [react/void-dom-elements-no-children](docs/rules/void-dom-elements-no-children.md): Prevent void DOM elements (e.g. `<img />`, `<br />`) from receiving children

Expand Down
64 changes: 32 additions & 32 deletions docs/rules/static-property-placement.md
View file
Expand Up @@ -8,19 +8,19 @@ and `propTypes` are declared in an ES6 class.

By default, this rule will check for and warn about declaring any of the above properties outside of the class body.

There are three key options are `static public field`, `static getter`, and `property assignment`.
The three key options are `static public field`, `static getter`, and `property assignment`.

### When `static public field` is enabled (default):

Examples of **incorrect** code for this rule:

```js
class MyComponent extends React.Component {
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get propTypes() { /*...*/ }
}
```
Expand All @@ -39,11 +39,11 @@ Examples of **correct** code for this rule:

```js
class MyComponent extends React.Component {
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static propTypes = { /*...*/ };
}
```
Expand All @@ -54,11 +54,11 @@ Examples of **incorrect** code for this rule:

```js
class MyComponent extends React.Component {
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static propTypes = { /*...*/ };
}
```
Expand All @@ -77,11 +77,11 @@ Examples of **correct** code for this rule:

```js
class MyComponent extends React.Component {
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get propTypes() { /*...*/ }
}
```
Expand All @@ -92,22 +92,22 @@ Examples of **incorrect** code for this rule:

```js
class MyComponent extends React.Component {
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static propTypes = { /*...*/ };
}
```

```js
class MyComponent extends React.Component {
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get propTypes() { /*...*/ }
}
```
Expand Down Expand Up @@ -159,7 +159,7 @@ The `<string>` value must be one these options:
* `static getter`
* `property assignment`

The `options` schema defined above allows you to specify different rules for the different property fields available.
The `options` schema defined above allows you to specify different rules for the different property fields available.

##### Example configuration:
_This is only an example, we do not recommend this as a configuration._
Expand Down
18 changes: 13 additions & 5 deletions lib/rules/static-property-placement.js
View file
Expand Up @@ -83,10 +83,10 @@ module.exports = {
// ----------------------------------------------------------------------

/**
* Checks if we are declaring function in class
* @returns {Boolean} True if we are declaring function in class, false if not.
* Checks if we are declaring context in class
* @returns {Boolean} True if we are declaring context in class, false if not.
*/
function isFunctionInClass () {
function isContextInClass() {
let blockNode;
let scope = context.getScope();
while (scope) {
Expand Down Expand Up @@ -129,9 +129,17 @@ module.exports = {
ClassProperty: node => reportNodeIncorrectlyPositioned(node, STATIC_PUBLIC_FIELD),

MemberExpression: node => {
// If definition type is undefined then it must not be a defining expression or if the definition is inside a
// class body then skip this node.
const right = node.parent.right;
if (!right || right.type === 'undefined' || isContextInClass()) {
return;
}

// Get the related component
const relatedComponent = utils.getRelatedComponent(node);

// Only check es6 components
// If the related component is not an ES6 component then skip this node
if (!relatedComponent || !utils.isES6Component(relatedComponent.node)) {
return;
}
Expand All @@ -142,7 +150,7 @@ module.exports = {

MethodDefinition: node => {
// If the function is inside a class and is static getter then check if correctly positioned
if (isFunctionInClass() && node.static && node.kind === 'get') {
if (isContextInClass() && node.static && node.kind === 'get') {
// Report error if needed
reportNodeIncorrectlyPositioned(node, STATIC_GETTER);
}
Expand Down
29 changes: 29 additions & 0 deletions tests/lib/rules/static-property-placement.js
View file
Expand Up @@ -1072,6 +1072,35 @@ ruleTester.run('static-property-placement', rule, {
}
}
`].join('\n')
},
// ------------------------------------------------------------------------------
// edge cases
// ------------------------------------------------------------------------------
{
// Do not error if property assignment is inside a class function
code: [`
class MyComponent extends React.Component {
static displayName = "Hello";
myMethod() {
console.log(MyComponent.displayName);
}
}
`].join('\n'),
options: [STATIC_PUBLIC_FIELD]
},
{
// Do not error if display name value changed
code: [`
class MyComponent extends React.Component {
static displayName = "Hello";
myMethod() {
MyComponent.displayName = "Bonjour";
}
}
`].join('\n'),
options: [STATIC_PUBLIC_FIELD]
}
],

Expand Down

0 comments on commit d443bde

Please sign in to comment.