tslint complaining "statements must be filtered with an if statement" when using switch

Question

Lets say I have the following method:

getErrorMessage(state: any, thingName?: string) {
    const thing: string = state.path || thingName;
    const messages: string[] = [];
    if (state.errors) {
        for (const errorName in state.errors) {
            switch (errorName) {
                case 'required':
                    messages.push(`You must enter a ${thing}`);
                    break;
                case 'minlength':
                    messages.push(`A ${thing} must be at least ${state.errors['minlength'].requiredLength}characters`);
                    break;
                case 'pattern':
                    messages.push(`The ${thing} contains illegal characters`);
                    break;
                case 'validateCardNumberWithAlgo':
                    messages.push(`Card doesnt pass algo`);
                    break;
            }
        }
    }
    return messages;
}

when I run

ng lint

I get the following error :

for (... in ...) statements must be filtered with an if statement

Having a look at similar question I don't think that answer would be applicable to my situation. After all switch statement sits in the category of if-else-if ladder.

tslint should consider switch statement as form of if statement but it doesn't?!

Would you suggest the same thing if I had 100 conditions to check? — MHOOS, May 30 '17 at 16:12
I would actually suggest you refactor the inner for loop into something else. :) A method call, or maybe map over the errors array to create the list of error messages. :) — toskv, May 30 '17 at 16:13
Possible duplicate of [What does the JSLint error 'body of a for in should be wrapped in an if statement' mean?](https://stackoverflow.com/questions/1963102/what-does-the-jslint-error-body-of-a-for-in-should-be-wrapped-in-an-if-statemen) — toskv, May 30 '17 at 16:36
check out that question, it's for JS but the same applies. It has nothing to do with the switch statement, but with the for .. in :) — toskv, May 30 '17 at 16:36
@toskv I think the point is that the rule says that an `if` statement should come after the `for...in`, and a switch statement can be seen as a kind of `if` statement, yet it doesn't resolve the error. — Frank Modica, May 30 '17 at 16:49
I believe the point of the rule is to filter out any properties that come from the prototype chain of the object accidentally. i.e. it expects `if (obj.hasOwnProperty(propName))`. Right now you cannot be sure that `required` exists directly on `state.errors` object or from a prototype of it. — Saravana, May 30 '17 at 16:53
@Saravana you are correct - that is the intention of the rule. But you can literally have any `if` statement and the error will go away - it doesn't check specifically for `hasOwnProperty`. So in that case, a `switch` statement could be seen as similar to any `if` statement. The `case` statements could even be targeting only the property names known not to be on the prototype. — Frank Modica, May 30 '17 at 17:06
@FrankModica you are right, maybe the switch statement is not really used for checking if an object has a property. :-/ Anyway I added an answer that gets by this rule by not using for .. in at all. :) — toskv, May 30 '17 at 17:47

Frank Modica · Accepted Answer · 2017-05-30T17:51:14.140

This made me curious, so I checked out the TSlint source code for this rule. It has a function called isFiltered which seems to only check for ts.SyntaxKind.IfStatement, not for ts.SyntaxKind.SwitchStatement.

function isFiltered({statements}: ts.Block): boolean {
    switch (statements.length) {
        case 0: return true;
        case 1: return statements[0].kind === ts.SyntaxKind.IfStatement;
        default:
            return statements[0].kind === ts.SyntaxKind.IfStatement && nodeIsContinue((statements[0] as ts.IfStatement).thenStatement);
    }

}

So unless you want to convert your object to an array, you'll need to use a fix from the link you provided. Either Object.keys, or an if statement:

    for (const errorName in state.errors) {
      if (state.errors.hasOwnProperty(errorName)) {
        switch (errorName) {

The interesting thing is that you can have any kind of if statement and the error will go away. There is no check to see if you are calling hasOwnProperty.

score 15 · Answer 2 · answered May 30 '17 at 17:46

The rule is meant to prevent you from accessing properties defined on the object prototype when using for .. in.

You can however refactor the code to just not use that, and make it easier to maintain and develop as well.

An example would be this:

interface ErrorMessageFactory {
  (thing: string, state?): string
}

type Errors = 'required' | 'minlength' | 'pattern' | 'validateCardNumberWithAlgo'

let errorFactory: {[e in Errors]: ErrorMessageFactory} = {
  required: (thing) => `You must enter a ${thing}`,
  minlength: (thing, state) => `A ${thing} must be at least ${state.errors['minlength'].requiredLength}characters`,
  pattern: (thing) => `The ${thing} contains illegal characters`,
  validateCardNumberWithAlgo: (thing) => `Card doesnt pass algo`
}



function getErrorMessage(state: any, thingName?: string) {
  if (state.errors) {
    return state.errors.map((error) => errorFactory[error](thingName, state));
  }
  return [];
}

You can see a working snippet in the playground here.

tslint complaining "statements must be filtered with an if statement" when using switch

2 Answers2