Rule proposal: `proper-object-iterable-methods`

[fisker]

Fail

// Should use `Object.values`
for (const key of Object.keys(foo)) {
	bar(foo[key]);
}

// Should use `Object.entries`
for (const key of Object.keys(foo)) {
	bar(foo[key], key);
}

// Should use `Object.values`
const foo = Object.keys(bar).map(key => baz(bar[key]));

// Should use `Object.entries`
const foo = Object.keys(bar).map(key => baz(bar[key], key));

// Should use `Object.keys`
for (const [key] of Object.entries(foo)) {
	bar(key);
}

Pass

for (const key of Object.keys(foo)) {
	foo[key] = 'new value';
	delete foo[key];
	foo[key]++;
}

[sindresorhus]

Rule name: prefer-object-entries-or-values? Any better suggestions? Or should it be split into two rules?

[fisker]

I think we should also enforce Object.keys over Object.entries in this case

for (const [key] of Object.entries(foo)) {
	bar(key);
}

So better come up a better name cover all them.

[fisker]

proper-object-iterable-methods?

[sindresorhus]

proper-object-iterable-methods?

Yeah. Sounds ok. I cannot come up with something better.

[sindresorhus]

This is now accepted.

[ljosberinn]

a (presumably) more complicated case that could also be detected:

// fail
const foo = Object.keys(myObj).reduce((acc, key) => {
  const value = myObj[key];
  acc[key] = value + 1;
  return acc;
}, {})

// pass
const foo = Object.fromEntries(
  Object.entries(myObj).map(([key, value]) => [key, value + 1])
)

[Pyrolistical]

While I was doing a refactoring to get rid of a for..in. I wondered if this rule should handle this case

// BEFORE
for (const key in object) {
  const value = object[key]
  f(value)
}

// AFTER
for (const [key, value] of Object.entries(object)) {
  f(value)
}

According to MDN,

Object.entries() is the same as iterating with a for...in loop, except that a for...in loop enumerates properties in the prototype chain as well.

The answer is no. This transformation is not generally equivalent. This transformation is only valid for plain objects.

[fregante]

@Pyrolistical presumably you also have the guard-for-in rule so:

• the "before" version should not exist
• the gated version should be equivalent and thus fixable

[Pyrolistical]

@fregante for..in is safe if you know its an object literal. I don't use classes, but you are right this can cause problems to those who do.

> for (const key in {x: 42}) { console.log(key) }
x

[fisker]

for..in is safe if you know its an object literal.

No, it's not.

Object.prototype.map = () => {}
for (const key in {x: 42}) { console.log(key) }

[Pyrolistical]

@fisker that's fair.

installs no-extend-native

[fisker]

<script src="lib-add-method-to-object-prototype-but-forgot-set-non-enumerable.js">

import "lib-add-method-to-object-prototype-but-forgot-set-non-enumerable"