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?!

Solution 1

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.

Solution 2

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.