tslint / codelyzer / ng lint error: "for (... in ...) statements must be filtered with an if statement"

Question

Lint error message:

src/app/detail/edit/edit.component.ts[111, 5]: for (... in ...) statements must be filtered with an if statement

Code snippet (It is a working code. It is also available at angular.io form validation section):

for (const field in this.formErrors) {
      // clear previous error message (if any)
      this.formErrors[field] = '';
      const control = form.get(field);

      if (control && control.dirty && !control.valid) {
        const messages = this.validationMessages[field];
        for (const key in control.errors) {
          this.formErrors[field] += messages[key] + ' ';
        }
      }
    }

Any idea how to fix this lint error?

akrabi · Accepted Answer · 2017-09-06T19:38:44.627

348

To explain the actual problem that tslint is pointing out, a quote from the JavaScript documentation of the for...in statement:

The loop will iterate over all enumerable properties of the object itself and those the object inherits from its constructor's prototype (properties closer to the object in the prototype chain override prototypes' properties).

So, basically this means you'll get properties you might not expect to get (from the object's prototype chain).

To solve this we need to iterate only over the objects own properties. We can do this in two different ways (as suggested by @Maxxx and @Qwertiy).

First solution

for (const field of Object.keys(this.formErrors)) {
    ...
}

Here we utilize the Object.Keys() method which returns an array of a given object's own enumerable properties, in the same order as that provided by a for...in loop (the difference being that a for-in loop enumerates properties in the prototype chain as well).

Second solution

for (var field in this.formErrors) {
    if (this.formErrors.hasOwnProperty(field)) {
        ...
    }
}

In this solution we iterate all of the object's properties including those in it's prototype chain but use the Object.prototype.hasOwnProperty() method, which returns a boolean indicating whether the object has the specified property as own (not inherited) property, to filter the inherited properties out.

edited Sep 06 '17 at 19:38

answered Aug 30 '17 at 11:59

akrabi

4,244
2
18
19

2

I'd like to notice that `Object.keys` is ES5. The only thing from ES6 there is for-of loop. We can iterate array in usual loop from 0 to its length and it would be ES5. – Qwertiy Sep 06 '17 at 16:26
4

once more notice: if somehow `this.formErrors` is null, `for...in` just do nothing, while `for ... of Object.keys()` would throw error. – user3448806 Oct 12 '17 at 06:56
i am following the second solution but still i see the lint message. Disabled lint for time being. – raj240 Feb 02 '19 at 07:37
2

Why don't you recommend `Object.keys(obj).forEach( key => {...}) `? – Ben Carp Feb 24 '19 at 15:06
@BenCarp - it has serious problems when running in an async function, therefore, at least to me, for ... in/of ... are considered more superior. See: https://stackoverflow.com/questions/37576685/using-async-await-with-a-foreach-loop#answer-50874507 – A-S Oct 24 '20 at 19:47
1

note that avoiding null prototypes may be needed in case of using hasOwnProperty prototype method: Object.prototype.hasOwnProperty.call(object, key) – cagcak Jul 13 '21 at 12:08
The two solutions will behave differently if the object has numeric keys. Object.keys will numerically sort them, while the plain for in loop will maintain insertion order. Just something to watch out for as it's a bit unexpected. – Jacek Pakulski Sep 24 '21 at 03:39

score 286 · Answer 2 · answered Mar 29 '17 at 02:49

286

A neater way of applying @Helzgate's reply is possibly to replace your 'for .. in' with

for (const field of Object.keys(this.formErrors)) {

answered Mar 29 '17 at 02:49

Maxxx

3,575
1
19
18

8

This should be the accepted answer as not only it solves the problem, it also reduces the amount of boilerplate code compared to additional conditionals such as `if (this.formErrors.hasOwnProperty(field))`. – Denialos Jul 28 '17 at 08:04
1

Be careful with the answer, it might break your codes. Test after you "fix" it. – ZZZ May 19 '18 at 07:22
3

This doesn't actually remove the tslint error for me. – HammerN'Songs Nov 16 '18 at 15:27
8

@HammerN'Songs check that you changed to for *of* instead of for *in* – Tom Nov 20 '18 at 10:35
same problem here. error is not removed after using this – llamerr Nov 27 '18 at 20:44
remember to change the other line as well to : `for (const key of Object.keys(control.errors)) ` – Jonathan Mar 24 '19 at 05:32

score 74 · Answer 3 · answered Nov 24 '16 at 14:53

74

for (const field in this.formErrors) {
  if (this.formErrors.hasOwnProperty(field)) {

for (const key in control.errors) {
  if (control.errors.hasOwnProperty(key)) {

answered Nov 24 '16 at 14:53

Qwertiy

19,681
15
61
128

score 14 · Answer 4 · answered Jan 15 '17 at 06:33

use Object.keys:

Object.keys(this.formErrors).map(key => {
  this.formErrors[key] = '';
  const control = form.get(key);

  if(control && control.dirty && !control.valid) {
    const messages = this.validationMessages[key];
    Object.keys(control.errors).map(key2 => {
      this.formErrors[key] += messages[key2] + ' ';
    });
  }
});

score 7 · Answer 5 · answered Jan 10 '20 at 00:58

7

If the behavior of for(... in ...) is acceptable/necessary for your purposes, you can tell tslint to allow it.

in tslint.json, add this to the "rules" section.

"forin": false

Otherwise, @Maxxx has the right idea with

for (const field of Object.keys(this.formErrors)) {

answered Jan 10 '20 at 00:58

Nick

742
8
14

Editing tslint.json....quick, easy and works great. Excellent answer! – Daniel Grindstaff Nov 06 '20 at 05:56

score 1 · Answer 6 · answered Nov 19 '19 at 09:56

1

I think this message is not about avoiding to use switch. Instead it wants you to check for hasOwnProperty. The background can be read here: https://stackoverflow.com/a/16735184/1374488

answered Nov 19 '19 at 09:56

lukas_o

3,776
4
34
50

tslint / codelyzer / ng lint error: "for (... in ...) statements must be filtered with an if statement"

6 Answers6

First solution

Second solution

Linked