Skip to content

Commit

Permalink
chore: replace existing field if present
Browse files Browse the repository at this point in the history
  • Loading branch information
FRSgit committed Dec 20, 2024
1 parent 0e6afb3 commit a7edb37
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 100 deletions.
124 changes: 75 additions & 49 deletions rules/prefer-class-fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,6 @@ const isThisAssignmentExpression = node => {
return true;
};

/**
@param {import('estree').Expression | import('estree').PrivateIdentifier} node
*/
const getPropertyName = node => {
if (node.type === 'Identifier') {
return node.name;
}

if (node.type === 'Literal') {
return `[${node.raw}]`;
}

if (
node.type === 'TemplateLiteral'
&& node.expressions.length === 0
&& node.quasis.length === 1
) {
return `[\`${node.quasis[0].value.raw}\`]`;
}
};

/**
@param {import('eslint').Rule.Node} node
@param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode
Expand All @@ -67,6 +46,58 @@ const removeFieldAssignment = (node, sourceCode, fixer) => {
: fixer.remove(node);
};

/**
@param {string} propertyName
@param {import('estree').ClassBody} classBody
*/
const findClassFieldNamed = (propertyName, classBody) => {
for (const classBodyChild of classBody.body) {
if (
classBodyChild.type === 'PropertyDefinition'
&& classBodyChild.key.type === 'Identifier'
&& classBodyChild.key.name === propertyName
) {
return classBodyChild;
}
}
};

/**
@param {string} propertyName
@param {string} propertyValue
@param {import('estree').ClassBody} classBody
@param {import('estree').MethodDefinition} constructor
@param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode
@param {import('eslint').Rule.RuleFixer} fixer
*/
const addOrReplaceClassFieldDeclaration = (
propertyName,
propertyValue,
classBody,
constructor,
sourceCode,
fixer,
) => {
const alreadyExistingDeclaration = findClassFieldNamed(
propertyName,
classBody,
);

if (alreadyExistingDeclaration) {
return fixer.replaceText(
alreadyExistingDeclaration,
`${propertyName} = ${propertyValue}`,
);
}

const classBodyStartRange = [classBody.range[0], classBody.range[0] + 1];
const indent = getIndentString(constructor, sourceCode);
return fixer.insertTextAfterRange(
classBodyStartRange,
`\n${indent}${propertyName} = ${propertyValue};`,
);
};

/**
@type {import('eslint').Rule.RuleModule['create']}
*/
Expand All @@ -87,39 +118,34 @@ const create = context => {
return;
}

const classBodyStartRange = [classBody.range[0], classBody.range[0] + 1];
const indent = getIndentString(constructor, sourceCode);

for (let i = constructorBody.length - 1; i >= 0; i--) {

Check failure on line 121 in rules/prefer-class-fields.js

View workflow job for this annotation

GitHub Actions / run-rules-on-codebase

The variable `i` should be named `index`. A more descriptive name will do too
const node = constructorBody[i];
if (
!isThisAssignmentExpression(node)
|| node.expression.right?.type !== 'Literal'
|| node.expression.operator !== '='
isThisAssignmentExpression(node)
&& node.expression.right?.type === 'Literal'
&& node.expression.operator === '='
&& node.expression.left.property.type === 'Identifier'
) {
continue;
return {
node,
messageId: MESSAGE_ID,

/**
@param {import('eslint').Rule.RuleFixer} fixer
*/
* fix(fixer) {
yield removeFieldAssignment(node, sourceCode, fixer);
yield addOrReplaceClassFieldDeclaration(
node.expression.left.property.name,
node.expression.right.raw,
classBody,
constructor,
sourceCode,
fixer,
);
},
};
}

const propertyName = getPropertyName(node.expression.left.property);
if (!propertyName) {
continue;
}

return {
node,
messageId: MESSAGE_ID,

/**
@param {import('eslint').Rule.RuleFixer} fixer
*/
* fix(fixer) {
yield removeFieldAssignment(node, sourceCode, fixer);
yield fixer.insertTextAfterRange(
classBodyStartRange,
`\n${indent}${propertyName} = ${node.expression.right.raw};`,
);
},
};
}
},
};
Expand Down
10 changes: 2 additions & 8 deletions test/prefer-class-fields.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,9 @@ test.snapshot({
`,
outdent`
class Foo {
foo = 'test';
constructor() {
this['foo'] = 'foo';
}
}
`,
outdent`
class Foo {
constructor() {
this[\`foo\`] = 'foo';
this.foo = 'foo';
}
}
`,
Expand Down
55 changes: 12 additions & 43 deletions test/snapshots/prefer-class-fields.mjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,56 +148,24 @@ Generated by [AVA](https://avajs.dev).
6 | }␊
`

## invalid(5): class Foo { constructor() { this['foo'] = 'foo'; } }
## invalid(5): class Foo { foo = 'test'; constructor() { this.foo = 'foo'; } }

> Input
`␊
1 | class Foo {␊
2 | constructor() {␊
3 | this['foo'] = 'foo';␊
4 | }␊
5 | }␊
`

> Output
`␊
1 | class Foo {␊
2 | ['foo'] = 'foo';␊
2 | foo = 'test';␊
3 | constructor() {␊
4 | }␊
5 | }␊
`

> Error 1/1
`␊
1 | class Foo {␊
2 | constructor() {␊
> 3 | this['foo'] = 'foo';␊
| ^^^^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊
4 | }␊
5 | }␊
`

## invalid(6): class Foo { constructor() { this[`foo`] = 'foo'; } }

> Input
`␊
1 | class Foo {␊
2 | constructor() {␊
3 | this[\`foo\`] = 'foo';␊
4 | }␊
5 | }␊
4 | this.foo = 'foo';␊
5 | }␊
6 | }␊
`

> Output
`␊
1 | class Foo {␊
2 | [\`foo\`] = 'foo';
2 | foo = 'foo'␊
3 | constructor() {␊
4 | }␊
5 | }␊
Expand All @@ -207,9 +175,10 @@ Generated by [AVA](https://avajs.dev).
`␊
1 | class Foo {␊
2 | constructor() {␊
> 3 | this[\`foo\`] = 'foo';␊
| ^^^^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊
4 | }␊
5 | }␊
2 | foo = 'test';␊
3 | constructor() {␊
> 4 | this.foo = 'foo';␊
| ^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊
5 | }␊
6 | }␊
`
Binary file modified test/snapshots/prefer-class-fields.mjs.snap
Binary file not shown.

0 comments on commit a7edb37

Please sign in to comment.