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.

Sign up for GitHub

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

Jump to bottom

Add getProperties autofixer to no-get rule #1823

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Add getProperties autofixer to no-get rule #1823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b07f275
updates
Apr 2, 2023
6615864
update getProperties implementation
Apr 4, 2023
66f304a
update test
Apr 4, 2023
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
54 changes: 52 additions & 2 deletions lib/rules/no-get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,47 @@ function fixGet({
return fixer.replaceText(node, `${objectTextSafe}${objectPathSeparator}${replacementPath}`);
}

function reportGetProperties({ context, node, objectText, properties }) {
let _properties = properties;
if (properties[0].type === 'ArrayExpression') {
// When properties are in an array(e.g. getProperties(this.obj, [bar, foo]) ), actual properties are under Array.elements.
_properties = properties[0].elements;
}
const propertyNames = _properties.map((arg) => arg.value);

context.report({
node,
message: ERROR_MESSAGE_GET_PROPERTIES,
fix(fixer) {
return fixGetProperties({
fixer,
node,
objectText,
propertyNames,
});
},
});
}

function fixGetProperties({ fixer, node, objectText, propertyNames }) {
if (!propertyNames.every((name) => isValidJSVariableName(name))) {
// Do not autofix if any property is invalid.
return null;
ArtixZ marked this conversation as resolved.
Show resolved Hide resolved
}

if (node.parent.type === 'VariableDeclarator' && node.parent.id.type === 'ObjectPattern') {
// When destructuring assignment is in the left side of "=".
// Example: const { foo, bar } = getProperties(this.obj, "foo", "bar");
// Expectation:
// const const { foo, bar } = this.obj;

return fixer.replaceText(node, `${objectText}`);
}

const newProperties = propertyNames.map((name) => `${name}: ${objectText}.${name}`).join(', ');
return fixer.replaceText(node, `{ ${newProperties} }`);
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
ERROR_MESSAGE_GET,
Expand Down Expand Up @@ -269,7 +310,9 @@ module.exports = {
validateGetPropertiesArguments(node.arguments, ignoreNestedPaths)
) {
// Example: this.getProperties('foo', 'bar');
context.report({ node, message: ERROR_MESSAGE_GET_PROPERTIES });
const objectText = context.getSourceCode().getText(node.callee.object);
const properties = node.arguments;
reportGetProperties({ context, node, objectText, properties });
}

if (
Expand All @@ -279,7 +322,14 @@ module.exports = {
validateGetPropertiesArguments(node.arguments.slice(1), ignoreNestedPaths)
) {
// Example: getProperties(this, 'foo', 'bar');
context.report({ node, message: ERROR_MESSAGE_GET_PROPERTIES });
const objectText = context.getSourceCode().getText(node.arguments[0]);
const properties = node.arguments.slice(1);
reportGetProperties({
context,
node,
objectText,
properties,
});
}
},
};
Expand Down
131 changes: 119 additions & 12 deletions tests/lib/rules/no-get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,18 +321,33 @@ ruleTester.run('no-get', rule, {
// **************************

{
code: "this.getProperties('prop1', 'prop2');",
code: "let obj = this.getProperties('prop1', 'prop2');",
output: 'let obj = { prop1: this.prop1, prop2: this.prop2 };',
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: "const baz = foo.getProperties('prop1', 'prop2');",
output: 'const baz = { prop1: foo.prop1, prop2: foo.prop2 };',
options: [{ catchUnsafeObjects: true }],
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: "const obj = this.getProperties(['prop1', 'prop2']);", // With parameters in array.
output: 'const obj = { prop1: this.prop1, prop2: this.prop2 };',
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: "const obj = this.getProperties('1');", // With invalid JS variable name.
output: null,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: "foo.getProperties('prop1', 'prop2');",
code: "const obj = this.getProperties('a*');", // With invalid JS variable name.
output: null,
options: [{ catchUnsafeObjects: true }],
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: "this.getProperties(['prop1', 'prop2']);", // With parameters in array.
code: "const obj = this.getProperties('obj.foo');", // With invalid JS variable name.
output: null,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
Expand All @@ -341,26 +356,118 @@ ruleTester.run('no-get', rule, {
import { getProperties } from '@ember/object';
import { somethingElse } from '@ember/object';
import { random } from 'random';
getProperties(this, 'prop1', 'prop2');
const obj = getProperties(this, 'prop1', 'prop2');
`,
output: `
import { getProperties } from '@ember/object';
import { somethingElse } from '@ember/object';
import { random } from 'random';
const obj = { prop1: this.prop1, prop2: this.prop2 };
`,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: `
import { getProperties } from '@ember/object';
import { somethingElse } from '@ember/object';
import { random } from 'random';
function foo(){
return getProperties(this, 'prop1', 'prop2');
}
`,
output: `
import { getProperties } from '@ember/object';
import { somethingElse } from '@ember/object';
import { random } from 'random';
function foo(){
return { prop1: this.prop1, prop2: this.prop2 };
}
`,
output: null,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
// Calling the imported function on an unknown object (without `this`).
code: "import { getProperties } from '@ember/object'; getProperties(foo, 'prop1', 'prop2');",
output: null,
code: "import { getProperties } from '@ember/object'; let obj = getProperties(foo, 'prop1', 'prop2');",
output:
"import { getProperties } from '@ember/object'; let obj = { prop1: foo.prop1, prop2: foo.prop2 };",
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
// With renamed import:
code: "import { getProperties as gp } from '@ember/object'; gp(this, 'prop1', 'prop2');",
output: null,
code: "import { getProperties as gp } from '@ember/object'; const obj = gp(this, 'prop1', 'prop2');",
output:
"import { getProperties as gp } from '@ember/object'; const obj = { prop1: this.prop1, prop2: this.prop2 };",
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: "import { getProperties } from '@ember/object'; getProperties(this, ['prop1', 'prop2']);", // With parameters in array.
output: null,
code: "import { getProperties } from '@ember/object'; const obj = getProperties(this, ['prop1', 'prop2']);", // With parameters in array.
output:
"import { getProperties } from '@ember/object'; const obj = { prop1: this.prop1, prop2: this.prop2 };",
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: `
import { getProperties } from '@ember/object';
const { foo, bar } = getProperties(
this.obj,
"foo",
"bar"
);
`,
output: `
import { getProperties } from '@ember/object';
const { foo, bar } = this.obj;
`,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: `
import { getProperties } from '@ember/object';
const { foo: qux, bar } = getProperties(
this.obj,
"bar",
"foo"
);
`,
output: `
import { getProperties } from '@ember/object';
const { foo: qux, bar } = this.obj;
`,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: `
import { getProperties } from '@ember/object';
const { foo, bar, ...qux } = getProperties(
this.obj,
"foo",
"bar",
"baz",
"frex"
);
`,
output: `
import { getProperties } from '@ember/object';
const { foo, bar, ...qux } = this.obj;
`,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},
{
code: `
import { getProperties } from '@ember/object';

const { foo, bar, baz } = getProperties(
get(obj, 't.s'),
'foo',
'bar',
'baz',
);
`,
output: `
import { getProperties } from '@ember/object';

const { foo, bar, baz } = get(obj, 't.s');
`,
errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
},

Expand Down