Writing linter rules:

why, how and when

Jeroen Engels

@jfmengels

Jeroen Engels

@jfmengels everywhere

jfmengels.net

Falcon LogScale

Fast, scalable, and affordable log management

Elm Radio podcast

elm-review

Not a tutorial

Aim: Provide the instinct

when to use custom linter rules

Linter

Code

Errors

Unused variables

Simplifiable code

Code style

Common bugs

Customizable

Configure rules

Custom rules

Tools and techniques

Guarantees

bugs 🐛

to prevent

function shout(sentence) {
    return sentence.toUpperCase();
}

shout(100);
// Uncaught TypeError: sentence.toUpperCase is not a function

function shout(sentence) {
    return sentence.toUpperCase();
}

shout(100);

function shout(sentence: string) {
    return sentence.toUpperCase();
}

shout(100);
// Argument of type 'number' is not assignable to parameter of type 'string'.

Static type checker

function canDeleteUsers(userRole: "user" | "admin"): bool {
    if (userRole === "admin") {
        return true;
    } else {
        return true;
    }
}

test("canDeleteUsers returns false when role is user", () => {
    assert.equals(canDeleteUsers("user"), false);
});

function canDeleteUsers(userRole: "user" | "admin"): bool {
    if (userRole === "admin") {
        return true;
    } else {
        return false;
    }
}

Tests: checking

specific values in specific scenarios

Constraints = guarantees

Constraint	Guarantee
Can't call shout with non-strings	shout() won't crash the application
canDeleteUsers should return false when called with "user"	Regular users can't delete other users

import http from "http"

function createUser(authToken, newUserData) {
    return http.post({
        url: "our-company.com/api/users",
        headers: {
          auth: authToken
        },
        body: newUserData
    });
}

function deleteUser(authToken, userId) {
    return http.delete({
        url: "our-company.com/api/users/" + userId,
        // forgot to pass in the authentication token!
    });
}

// custom-http.ts
import coreHttp from "http";

export default {
  post: post,
  delete: delete,
}

function post({path: string, auth: string, body: string}) {
  return coreHttp.post({...});
}

function delete({path: string, auth: string}) {
  return coreHttp.delete({...});
}

import http from "./custom-http";

function createUser(authToken, newUserData) {
    return http.post({
        path: "/users",
        auth: authToken,
        body: newUserData
    });
}

function deleteUser(authToken, userId) {
    return http.delete({
        path: "/users/" + userId,
        auth: authToken
    });
}

import http from "http"

function editUser(authToken, userId, userData) {
    return http.put({
        url: "our-company.com/api/users/" + userId,
        headers: {
          auth: authToken
        },
        body: userData
    });
}

Type checker	Checks whether a value is in a set of possible values
Tests	Checks that given specific scenarios, specific variables have specific values
Linter	Checks that code was written in a way that follows a list of rules

Different angles for creating guarantees

import http from "http"
            ^^^^^^^^^^^

using grep

grep -e 'from "http"' src/**/*.ts

Building a linter

#!/bin/bash
MATCH=$(
    grep -e 'from "http"' src/**/*.ts \
      --with-filename --line-number --color=always --exclude=src/custom-http.ts
)
if [ -n "$MATCH" ]; then
    echo "Found linting problems:"
    echo ""
    echo "Error: Do not use the http module directly"
    echo $MATCH
    exit 1
fi

Building a linter

using grep

// Single-quotes
import http from 'http'

// Too many spaces
import http from  "http"

// On several lines
import
  {request}
  from
  "http"

False negatives

grep -e 'from "http"' src/**/*.ts

/*
Don't ever import the core HTTP module 

    import http from "http"

Instead use our awesome custom wrapper.

    import http from "./custom-http"
*/

False positives

Abstract Syntax Trees

(a + b) / 2

Integer 2

Binary expression
    using "+"

Reference to "a"

Binary expression
    using "/"

Reference to "b"

Parenthesized

right

left

value

Pattern matching on the AST

a / 0

Integer 0

Reference to "a"

Binary expression
    using "/"

Rule: No division by 0

If I see a binary expression using "/" where on the right side there's an integer 0 , then report a problem.

right

left

Pattern matching on the AST

a / 0

Integer 0

Reference to "a"

Binary expression
    using "/"

function visitor(node) {
  if (
    node.type == "BinaryExpression"
    && node.operator == "/"
    && node.right.type == "Integer"
    && node.right.value == 0
  ) {
    return error("Don't divide by 0");
  }
  return noErrors;
}

right

left

Pattern matching on the AST

import thing from "http"

String literal "http"

Identifier "thing"

Import declaration

source

name

Collect information

Reporting unused variables

"Hello " + name

Reference "name"

var name = ...

Identifier "name"

Variable declaration

name

...

Collect declaration names

Collect used names

Unused variables

Collect information

Reporting unused variables

var declared = [];
var used = [];

function visitor(node) {
  if (node.type == "Variable declaration") {
    declared.push(node.name);
  }
  else if (node.type == "Reference") {
    used.push(node.name);
  }
}

function afterVisitingTheFile() {
  return declared
    .removeAllFrom(used)
    .map(name => error("Unused variable " + name));
}

Application examples

Custom solutions

instead of

built-in solutions

Using consistent utilities

button({ color: "#FF1122" }, "Click me!")
             // ^^^^^^^^^ Don't hardcode colors
// instead

import Colors from "./colors"

button({ color: Colors.primary }, "Click me!")

✨ Guarantees ✨

- Only colors chosen by the designer can be used

- It's possible the change a color all over the app by changing a single variable

Enforce ids are unique

// Ids.ts

export var saveButtonId = "save-button"
export var cancelButtonId = "cancel-button"
export var purchaseDialogId = "purchase-dialog"
export var confirmButtonId = "save-button"
    // oops it's a duplicate ^^^^^^^^^^^^^

✨ Guarantees ✨

- No 2 elements will end up with the same ID

All variants in an array

enum MenuItems {
    Home,
    Dashboards,
    Profile,
    Settings,
    Help,
}

var allMenuItems: MenuItems[] =
  [ Home,
    Dashboards,
    Profile,
    Settings,
    Help,
  ]

Imposing code architecture restrictions

import Something from "./domain/secret-keys/internal"
//                                          ^^^^^^^^
// Only "domain/secret-keys" is allowed to import this internal module

✨ Guarantees ✨

- Encapsulation

Whatever you mention

during code reviews

Enforcing code style

Layout / formatting

Stylistic preferences

Code formatters

module MyModule exposing (Model, Msg, update, view)

import Html exposing (Html, button, div, text)
import Html.Events exposing (onClick)


type alias Model =
    { count : Int }


type Msg
    = Increment
    | Decrement


update : Msg -> Model -> Model
update msg model =
    case msg of
        Increment ->
            { model | count = model.count + 1 }

        Decrement ->
            { model | count = model.count - 1 }


view : Model -> Html Msg
view model =
    div []
        [ button [ onClick Increment ] [ text "+1" ]
        , div [] [ text <| String.fromInt model.count ]
        , button [ onClick Decrement ] [ text "-1" ]
        ]

Consistency is great

Enforcing code style for

the sake of it isn't

What guarantees are gained?

When is a linter rule

not appropriate?

var z = f(m, y)

Lots of exceptions

No short variable names

function or(a, b) {
  return a || b;
}

// is this better?
function or(left, right) {
  return left || right;
}

for (var i = 0, i < array.length; i++) {
    // ...
}

What are the edge cases?

What false positives/negatives

is this rule going to have?

How painful will those make

the experience for the user?

When you need to know

the value of something

a / b

is b == 0?

a / 0

Report it

If you can't tell just by

looking at the code

Guarantees prevent

bugs in the future

Use different tools

to create guarantees

and use them together

Thank you!

Jeroen Engels

@jfmengels

Learn more:

elm-review.com
Talk: Why you don't trust your linter
https://www.youtube.com/watch?v=XjwJeHRa53A
Talk: Static analysis tools love pure FP
https://www.youtube.com/watch?v=_rzoyBq4hJ0
jfmengels.net
Elm Radio podcast

Slides: https://slides.com/jfmengels/writing-linter-rules