Skip to content

Instantly share code, notes, and snippets.

@jfmengels
Last active June 16, 2021 05:23
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save jfmengels/111defdc980926ce472a4a0f8f8b5123 to your computer and use it in GitHub Desktop.
Save jfmengels/111defdc980926ce472a4a0f8f8b5123 to your computer and use it in GitHub Desktop.
elm-review vs ESLint

This document explains a some of the differences between elm-review and ESLint

Documentation for each:

elm-review documentation: https://package.elm-lang.org/packages/jfmengels/elm-review/latest/

ESLint documentation: https://eslint.org/

Configuration

Rules and configuration are written in Elm, not JSON. Configuration looks like the following:

module ReviewConfig exposing (config)

import Review.Rule exposing (Rule)
import Third.Party.Rule
import My.Own.Custom.rule
import Another.Rule

config : List Rule
config =
    [ Third.Party.Rule.rule
    , My.Own.Custom.rule
    , Another.Rule.rule { ruleOptions = [] }
    ]

and rules look like this:

import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.Node as Node exposing (Node)
import Review.Rule as Rule exposing (Error, Rule)

rule : Rule
rule =
    Rule.newModuleRuleSchema "NoDebug" ()
        |> Rule.withSimpleExpressionVisitor expressionVisitor
        |> Rule.fromModuleRuleSchema

expressionVisitor : Node Expression -> List (Error {})
expressionVisitor node =
    case Node.value node of
        Expression.FunctionOrValue moduleName fnName ->
            if List.member "Debug" moduleName then
                [ Rule.error
                    { message = "Remove the use of `Debug` before shipping to production"
                    , details = [ "The `Debug` module is useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing its use before committing and attempting to push to production." ]
                    }
                    (Node.range node)
                ]

            else
                []

        _ ->
            []            

All rules are opt-in. If you want one enabled, you add it the config list. If you don't, leave it out.

Unlike in ESLint, there are no core rules inside elm-review (design explanation). Rules are only in packages published on the package registry. You can also have rules in local files, and there is no need to set up a ESLint plugin (you just import the module).

Rules don't cause crashes because they are written in Elm and Elm helps you avoid runtime errors (potential stack overflow errors being pretty much the only exception).

There are no severity levels, just errors. This raises the bar for when a rule should be enabled (guidelines for adding a rule). A different tool for static analysis proved that this was an acceptable choice to make in Elm. Design decisions on the subject.

There is also no way to disable reported problems using disable comments. Ignoring a rule is done only in the configuration, and rules are configurable to allow disabling for cases you don't want, or to fill gaps in the tool's knowledge. (A full blog post on that coming soon, ask me if you want - June 16th 2021)

Analysis

The target language is much simpler than JS (there are less ways to do the same thing), so writing rules is easier. The language is also side-effect free, so there are less weird things that can happen and that make you abandon the rule idea because "we can't be sure of what this code represents".

You can collect data from other files in the context, so for instance you can find functions that are exported and report them if they are never used in the rest of the project.

Because of that, you can know the type of every expression (at least in theory, in practice it is still a bit hard, but I am working on it).

Reporting

Reports contain a lot more information than for ESLint rules: Example image

The idea is that they look like the reports of the Elm compiler and be as helpful as they are. Hopefully, this makes elm-review more like an assistant rather than a tool that annoys the users (tools like these usually do).

Fixing

You can either run the CLI with --fix or --fix-all. Both will display the proposed changes and prompt the user to accept/refuse them. The former will prompt after every fix, and the later will prompt the result of applying every fix.

Testing

The testing of a rule on a code sample covers everything: messages, position, fixes (if provided), ... I found it frustrating that so many things go un-tested in ESLint, and I find that testing all of these result in much higher-quality rules.

There is a very useful "under" field to test the position easily, that I thing ESLint should incorporate, as a replacement/addition to .

Misc

There is an integrated watch mode, which even re-runs when your configuration changes. Surprisingly, this works really nicely with --fix-all in a separate terminal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment