How not to lint your code?

This post is taken from my blog so be sure to check it out for more up-to-date content 😉

As a programmer, I think you have high expectation for the code you write. It should be easily readable and understandable by those you'll interact with it in the near future. That includes the writer himself, e.g. 1 year later. When you take a look at your old code and don't know what given fragment does, then it clearly wasn't well-written. That's why every programmer has a goal of perfect, scalable architecture and strict code style definition for every LOC they write. And while the architecture and structure of the project are extremely important aspects, there isn't one, single way to make it fit for everyone. That's why here, in this article I won't talk about these things. Let's talk about code style instead, or more specifically about code linting.

Linting is a process of analyzing your code for potential programming errors, bugs, styling errors etc. But, I guess that you already know that (maybe except strict definition which nobody cares about 😉). In modern code, editors and IDEs linters provide the ability to write better code with the help of live checking. In the world of JS development, there are some incredibly useful tools for that, including ESLint, JSLint, and JSHint. With deep customization and extension options, they surely provide enough room to create the best configuration matching your code style for any given project. With that said, creating these configs may not be such an easy task, especially when you don't really know if the specific rule is important or not. 🤔

What it's all about?

I'm glad you asked. 🙃 I would like to tell you a story of mine. A story about configuring my linter and how it ended up. To be fair, the point of all this is not to say that linters are bad or complex. Instead, I'm just sharing my a little funny story.

To start, as you might know, if you've read my previous posts, I'm a big fan of TypeScript. I use it for almost all of my to-be JS projects. This time was no exception too. So, I swap in TypeScript and set up my working directory. As this project was meant to be public, I decided that code style ** is an important factor here. That's why I used Prettier. If you don't know already, this is a tool for **formatting/prettifying your code, no linting, and stuff - just formatting. So, it's fine for details like strings, line's length and etc. But, of course, that's not where we end.

Then, I turned my attention to the TypeScript configuration file - tsconfig.json. I wanted to have the strictest possible rules set and so I turned on some important options.

• noImplicitReturns - ensures that your functions return (value or nothing at all) in all possible scenarios

• strict - this one is a little bit trickier. It combines 5 other options into one, simple package:

  • noImplicitAny - ensures that there's no any type in your code
  • noImplicitThis - doesn't allow referencing this that has any type
  • alwaysStrict - parses your code in strict mode and uses 'use strict' flag
  • strictNullChecks - ensures that you'd never access a property with a null value

• noUnusedLocals - checks for unused local variables

And this was a pretty good choice. This way, TypeScript itself assures me better code quality overall. But this wasn't enough for me. 😅

So, next, I installed TSLint, which is basically a linter for TypeScript (has additional type-based rules). And here's where the things start to get interesting...

TSLint setup

TSLint is a pretty good tool as it stands. It has a vast collection of built-in linting rules (with an option to create owns), some default configs (which can be also extended) and more... Aside from that, its support for language service protocol has recently been improved. This basically means better and faster IDE/code editor support for more complex rules, and that's good.

So, I started by downloading the tslint-config-airbnb from NPM. This is an unofficial (not made by Airbnb) TSLint config following the Airbnb's JS style guide. It does a pretty good job of providing you with the best possible JS linting rules. It extends upon i.a. tslint-eslint-rules (providing TSLint with rules present in ESLint) and tslint-microsoft-contrib which adds some more rules, coming straightly from Microsoft (where TS originally came from). Apart from that, I had to use tslint-config-prettier which disables all rules that can possibly conflict with Prettier, which I've been using. Sum it all up and you're left with a pretty valuable setup. And it actually was. Simple, straight forward without any more configuration.

But all these rules appears like nothing in comparison to tslint:all. It is the builtin config turning all reasonable builtin rules on and that's what I turned on! 🙃 Let's talk a bit about how it ended up!

One config to rule 'em all

The fact that I already have some TS code written, made it easy to feel the change at once. And, yeah, there was a lot of red (meaning highlighted errors), a lot! But, it was nothing new to me (done drastic linter's config changes before) and, filled with enthusiasm, I got to work. Believe me, it can be fun to fix/change your code knowing that now it will be cleaner, readable (if you've done your config right) and following some kind of standard.

At the beginning it was good. This strict config ensured me that there were no unchecked undefined values and any other leftovers. There was no option for any any type 😅 to exist. Proper documentation was required for every method or property or function that didn't already have it. Stricter if checks, alphabetical sorting, and class member ordering based on access modifiers clearly provided some additional structure to the code. This assured me that I and any future contributors will have well-defined guidelines to follow. But the reality is that if the configuration turning all rules on was so good, wouldn't everyone just used this one or at least wouldn't it come as the out-of-the-box option? So, after these good, reasonable rules were fulfilled, it was time for those nasty ones to appear...

Conflict counter

With that said, let's take a look at the most notable problems and conflicts that appeared only because of the all-rules-on config.

Array type

In TypeScript you can write your array type in 2 ways - as an array literal (string[]) or as a generic array type ( Array<>). So, what's the problem? Well, here it comes from tslint-microsoft-config additional rule called prefer-array-literal. It's conflicting with the built-in array-type rule. The first one, as the name indicates, recommends you to use the literal syntax. The second recommends syntax based on the complexity of passed type for array items. There's definitely a conflict. I solved it by turning off the array-type rule, thus leaving me with the array literal syntax which I like more.

Magic numbers

Well, this may not be a real issue but can be cumbersome. Have you heard of magic numbers? This is a term used to reference different numbers in your code appearing without any meaning (yup, that's an anti-pattern). Consider the example below:

for(let i = 0; i < 10; i++) {
    // ...
}

Here, the number 10 is magical, as it has appeared from nowhere and not everyone knows what it really does (but it's obvious that it just makes the loop iterate 10 times, right?). So, let's apply a quick fix.

const numOfIterations = 10;
for(let i = 0; i < numOfIterations; i++){
    // ...
}

Know that you can do a little better with variable naming, unlike me. 😅 But, basically, that's all the magic behind magic numbers. 🙃 Now, to be clear, it's not an issue - it's a very good practice to name your numbers so that everybody will know what they're all about. But in situations like the one above, it can seem a little not-so-intuitive, but generally it's definitely useful.

Undefined

Next up, I've got some issues with the undefined value. First comes the strict-boolean-expressions rule. What it does is it forces you to use real booleans where they're expected. What does it mean? Take a look at something like this:

if(possiblyUndefinedVariable){
    // ...
}

That's the way of checking if the variable isn't undefined, that probably many JS/TS developers use. But this rule enforces you to write it in a more strict way, like this:

if(possiblyUndefinedVariable !== undefined){
    // ...
}

So, yeah a bit longer but the more definitive syntax for doing the same thing.

Let's move on to the return-undefined rule. This one ensures that you'll use return undefined instead of return whenever you're function was meant to return any other kind of value. Simple example?

// any type shouldn't be used, but anyway
function returnValue(valueToReturn: any, shouldReturn: boolean){
    if(shouldReturn){
        return valueToReturn;
    }
    return undefined;
}

As you can see, I needed to return undefined even if it wasn't really needed. Also, know that here I could use shouldReturn without strict undefined check because it is of boolean type.

So, these rules might seem a bit unintuitive but they definitely add some structure to your code.

Export default

You know ES modules, right? Well, TSLint has a rule even for them. The no-default-export rule, because we're talking about this one, paired with no-default-import, effectively bans any kind of default exports and imports. This enforces you to export/import only named (assigned to variables) parts of the code, thus improving readability and self-documentation of your code. But, in reality, it can you can use default exports/imports and achieve similar results when using consistent naming.

Increment & decrement

Remember the loop example above? With the increment-decrement rule in mind, it would be considered as one having an issue.

const numOfIterations = 10;
for(let i = 0; i < numOfIterations; i++){
    // ...
}

And it's all because of the ++ (also --)operator. The increment/decrement operator is often seen in the standard for loops. What you may not know, is that it can both follow and precede it's a parameter with both syntaxes having a different meaning. Consider the simple example below:

let a = 1;
let b = a++;
let c = ++a;
console.log(a,b,c); // 3 1 3

By knowing the output, you can deduce the meaning of both syntaxes. By using the operator as the following character, you first assign the variable a to b and then increase a by one. On the other hand, when using the preceding operator syntax, you first increase the value of avariable by one and then assign this to the c variable. It seems logical at first with the same logic applicable to the decrement operator as well.

But, the fact is that these particular differences in syntax can often lead to various, hard-to-discover issues. That's why the rule recommends using the different, more strict syntax:

let a = 1;

let b = a;
a += 1; // a++

a += 1; // ++a
let c = a;

console.log(a,b,c); // 3 1 3

I separated these lines on purpose to show how to achieve the same result with different syntax. I think we both agree that in this way, the thinking behind this code is rather easily understandable compared to the syntax above. However, for those who like the shorter syntax, this rule might seem not really needed.

Inferrable types

Now, let's dive into more TypeScript-specific rules and problems connected with them. Here we again have yet another rule conflict. And this one is a bit more serious. I'm talking about *typedef * vs no-inferrable-types. I think the names can speak for themselves, but let's explain these rules anyway.

const myNumber: number = 1;
const myString = "a";

Here we have two simple variable declarations. What's the difference between them? (I don't care about different types and names 😛) The strict type definition. TypeScript has the ability to infer the type of a variable from its value (at least when assigned during its declaration). You can, of course, write the specific type directly, but who cares about something like that. Writing these types just don't seem very DRY.

So, what's the problem? It's the conflict between these two rules. The first declaration meets the requirements of the typedef rule (which requires all specified constructs to have types strictly defined) but not so much when it comes to no-inferrable-types (which doesn't accept strict definitions where they're not needed). In the other declaration, it's all completely otherwise. The tslint:all config gives you this pleasure of choice by activating both of these rules. 😁

What was my take on this? Well, at first I was thinking about leaving the typedef rule on for just going really strict. But then I thought to myself that this is too much. In addition, I run into something like this:

const myArrowFunction = (arg1: number, arg 2: number) => {
    // ...
}

And the decision about turning the typedef on came easily. Now, what was the problem? In the example above we have the arrow function. As you know, these can only be defined like function expressions, by assigning to a variable (unless you've made IIFE from it). And what typedef wants? For every single variable to have a strict type definition assigned directly. How would it look like?

const myArrowFunction: (arg1: number, arg 2: number) => void 
= (arg1: number, arg 2: number) => {
    // ...
}

I think even the strictest guy wouldn't like the way it looks and feels. The choice is simple. However, if you would like to remain strict, you can always deeply configure the typedef rule to require a strict definition only in specified cases. To be honest, I think that this way of doing things brings some irregularities to your code. But, this is just my opinion.

Interfaces

It's just a quick note. In TypeScript, there is a popular practice to precede all interfaces' names with a capital I letter. And... there's a rule for that too! It's called interface-name and enforces this particular style of naming. While this clearly differentiates interfaces from the rest of constructs, it's not really intuitive at least IMHO. You know, even official TS lib.d.ts does not use this practice (maybe for other JS documentations compatibility reasons, but it's a fact anyway), so it doesn't have to be this way.

For... in & index signature

This is the last thing I would like to grumble about. 🙃 Have you ever been in a situation where you'd need to iterate over an object keys/values/etc.? How did you approach this? I most often use the for... in loop which is probably most popular and arguably the fastest way of doing this.

But, before I introduce you to the problem, let me explain some TypeScript stuff. Firstly, the keyof operator is so-called index type query operator, which basically means that it creates a type for you that includes all known properties of object, interface etc.

const obj = {
    a: 1, 
    b: 2
}
type ObjKeys = keyof typeof obj; // "a" | "b"

I think it's easy to understand. Next, there's an idea of index signature. Basically, it allows you to define that given type is e.g. object which has properties' keys of some type (usually string) allowing only given type of values for them. So, it's something like a general guideline for all properties to follow.

interface Obj {
    [key: string]: number;
}
type ObjKeys = keyof Obj; // string

Also, take a look at what keyof outputs in this example. It should be string, as we've defined earlier.

Now that you know that, let me present the problem. It's connected with keyof type, index signature and also things like for... in loop, Object.keys() and alike. Consider the example below.

interface Obj {
    a: number;
    b: number;
}
const obj: Obj = {
    a: 1,
    b: 2
}
for(const key in obj){
    if(obj.hasOwnProperty(key)){
        console.log(obj[key]) // error
    }
}

The issue is that we can access our obj of type Obj with key because it doesn't have an index signature! And thus it would return any value which isn't allowed by our config! Why's that? Because key is of type string. So, the problem is that for... in loop, as well as any other related method (like Object.keys()) uses string instead of keyof type (which is much, much better option here) for indicating the type of key! How can you solve this problem? By casing the type of key anytime you try to access the value:

// ...
console.log(obj[key as keyof Obj])
// ...

For your knowledge, there have been many GitHub issues opened about this particular problem, but sadly they didn't produce many results (at least that's what I know when using TS 3.2.2).

You could think of typecasting the key just once and saving it to a variable at the beginning of the loop, but it isn't very good to have two variables to hold the same value, and most likely similar names. So yeah, that's definitely a big issue.

And... that's probably the last one of the most important ones. Of course, there were some other, smaller issues but they were mostly regarding my coding style, so I didn't include them here. 😁

What's the point again?

As I said earlier, the point of this article is not to discourage you from using linters. Just to give you a warning about how important it is to have a good configuration in place. And also to have fun and learn something new (TSLint rules, TypeScript stuff, etc.). You always can use the tslint:all config (or something similar for your linter-of-choice) and then disable rules that aren't needed. Just be sure to use linter when doing any kind of project (especially big one). 😉

That's all for now. If you like this article, consider checking out my blog for latest blog posts. Also, follow me on Twitter 🐦 and on my Facebook page for more. ✌

Comments

[Dian Fay]

I've used ESLint for years; I keep telling myself I'll switch to prettier the next time I find myself writing an // eslint-disable-line or // eslint-disable-next-line but it never seems to take....

One thing to note, naming your magic numbers is all well and good but numOfIterations doesn't tell you anything that 10 didn't. If you're going to go to the effort, it should explain why, not just what (like comments!).

[Arek Nawo]

I said about it in the article.

Know that you can do a little better with variable naming, unlike me. 😅

But it's good advice in general. Variables should be taken care more than they actually are.

[Mihail Malo]

I must say, I wholly support the import/export rules, they make automatic imports, renames, and other tooling work so much better, as well as generally causing less diffs and consistent naming throughout a project.
I also begrudgingly agree with the sorting rules.

I am not convinced about "return-undefined" or even "noImplicitReturns" though.
Implicit returns in a void-returning function are a no-brainer, the body just ends and you're done. And having undefined written is just noise, that is what return means.

Personally I'd forbid return undefined in all positions, and return at the end of a function. Is there a way to do that?

[Arek Nawo]

It's a good idea. Sadly, such a simple rule isn't present in TSLint and even ESLint. The closest thing I found is consistent-return from ESLint, but still, it's not exactly the right thing.

[Mihail Malo]

But my increment :v

  neighborsInfo: (r: BufferWithPointer) => {
    const length = r.uint8()
    const arr: (number | Buffer)[] = new Array(length * 6)
    for (let i = 0, off = 0; i < length; i++) {
      const addr = r.buffer(8)
      arr[off++] = addr
      arr[off++] = r.int16le()
      arr[off++] = r.int16le()
      arr[off++] = r.int16le()
      arr[off++] = r.int8()
      arr[off++] = r.uint8()
    }

    return arr
  }

[Mihail Malo]

Actually, I feel like a lot of people would applaud the typedef rule in this situation (yes, I deleted the types from the implementation):

const myArrowFunction: (arg1: number, arg2: number) => void =
(arg1, arg2) => {
    // ...
}

This way all the type information comes first, and the implementation comes after.
Very haskelly.

[Mihail Malo]

Well, there's the "interface-name": [true, "never-prefix"] option.
Which is what the style I've seen TSc team recommend.

[Basti Ortiz]

I love this article because I can relate to it on so many levels. Don't worry, bro. I know the struggles, too. You aren't the only perfectionist here. 😉

[Arek Nawo]

Well... I definitely feel better now. 😅 As a side-note, I found this. It seems like a nice option if somebody wants to follow strictly defined guidelines with NPM, Express and alike using it. But still, it's not so strict and not so ideal either. That's why for now I disabled my linter (I'll turn it on when I finish writing the basic codebase) as it seemed a bit counter-productive with all this configuration stuff 😁

[Kevin Boosten]

Nice article! Will try out tslint:all tomorrow!

I also standard add this rule from tslint-microsoft-contrib to check my code on any forgotten TODO/FIXME/HACK comments:
“no-suspicious-comment“ 🙂

[Arek Nawo]

Thanks! I think you might be interested in a config of mine that I've recently released. github.com/areknawo/eslint-config-... It aims to solve most of the issues that I've described in this post. Hope you enjoy it! 😊