tslint / codelyzer / ng lint error: “for (… in …) statements must be filtered with an if statement”

269
votes

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?

6
angularangular2-formsangular-clitslint
Maybe accept an answer? – Qwertiy

6 Answers

316
votes

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.

277
votes

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

for (const field of Object.keys(this.formErrors)) {
73
votes
for (const field in this.formErrors) {
  if (this.formErrors.hasOwnProperty(field)) {

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

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] + ' ';
    });
  }
});
5
votes

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)) {
1
votes

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