Why you don't trust your linter

Jeroen Engels

@jfmengels

Elm Radio

Google Maps analogy

Linter

//nolint:rule
# pylint: disable=rule
# rubocop:disable rule
// NOLINT
// phpcs:disable rule
@SuppressWarnings("rule")
@Suppress("rule")
// eslint-disable rule

You don't trust your linter

elm-review

The reasons

why we don't trust our linters

False positives

🤬

🫶

// linter-disable
// linter-disable
// linter-disable

Causes of false positives

Bugs

Missing information

Presumptions

False positives/negatives

Deceitful linters

False negatives

Providing information

  • Contents of all files in the project

  • Type information

  • Dependencies

  • Project manifest, README

  • External files

What can you deduce from the target language?

Elm is knowable

No dynamic function calls

Functions always return the same type

No side-effects

No mutations

No macros

...

No memory management

No race conditions

...

Elm is knowable

Better understanding of the code

Almost no false positives

Static analysis tools

❤️

pure FP

Video should be released

TODAY! (28-09-2022)

Effective false positives

How is the error communicated?

Is this sufficient to help the

developer fix the issue?

File path

Location

Message

Rule name

Developers need to understand

  • What they did wrong

  • Why it is a problem

  • How to move forward

Not understanding reports

leads to workarounds

view : Model -> Html Msg
view model =
    -- elm-review-disable-next-line
    Html.button
        [ Attr.style "height" "34px"
        , Events.onClick UserClickedOnRemoveButton
        ]
        [ Html.text "Remove" ]

🤬

Context

Problem

Suggestion

Example

Rule name

Source

code extract

Message

File path

and location

Configuring linters

Accidental configuration

Essential configuration

Accidental configuration

Tell the tool how to work with your project

 

  • What language features are available/enabled?

  • What tools does this project work with? Build tools, macros...

False positives

because of the configuration

Accidental configuration

All necessary information

in standard files

Essential configuration

Tell the tool what you want from it

 

  • Which rules do you want to enable?

  • With which rule options?

Configuring elm-review

config : List Rule
config =
    [ NoBadThing.rule
    , NoOtherBadThing.rule { some = "options" }
    -- ...and more rules
    ]

Enabling rules

you agree with

Linter has a lot of rules

Takes time to configure

Use someone else's configuration

Rules you don't agree with

Advice on adding a rule

Understand the problem well

 

Get your team's approval

 

Understand not all rules are good

 

Be ready to disable the rule if it feels painful

Ignoring reports

//nolint:rule
# pylint: disable=rule
# rubocop:disable rule
// NOLINT
// phpcs:disable rule
@SuppressWarnings("rule")
@Suppress("rule")
// eslint-disable rule

Disable comments

Warnings

For rules you don't want to enforce?

How and when to

use disable comments?

Indeterminate rules

Generated/vendored code

Ignoring rules

config : List Rule
config =
    [ NoUnused.Variables.rule
    , NoDebug.Log.rule
    ]
        |> Rule.ignoreErrorsForDirectories [ "src-generated/", "src-vendored/" ]

Allow existing errors

Deprecating a function

{-| Does something.

@deprecated Use someBetterFunction which does it better.

-}
someFunction input =
    -- do something with input

Deprecating a function

Ignoring per file

config : List Rule
config =
    [ NoDeprecated.rule
        |> Rule.ignoreErrorsForFiles
            [ "src/Api.elm"
            , -- ...and other files
            ]
    ]

Suppressing errors

{
  "version": 1,
  "automatically created by": "elm-review suppress",
  "learn more": "elm-review suppress --help",
  "suppressions": [
    { "count": 7, "filePath": "src/Api.elm" }
  ]
}

review/suppressed/NoDeprecated.json

Deprecating code

Deprecating code

Deprecating code

Automatic fixes

Prompt for fixes

False positives and

effective false positives

lead to losing trust

Remove false positives

by providing information

Simpler and knowable

languages make for

better analysis

Communication

  • What is the problem?

  • Why is it a problem?

  • How to solve the problem?

Avoid accidental configuration

as much as possible

Only enable rules

you agree with

Better alternatives than

disable comments and warnings

to ensure rules are enforced

Tools can only gain trust by

correctly and transparently

doing their tasks

over and over again

Thank you!

Jeroen Engels

@jfmengels

Elm Radio

Learn more:

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

  • Static analysis tools love pure FP, Lambda Days

  • jfmengels.net

  • Elm Radio podcast

You don't trust your linter

firm belief in the reliability, truth, or ability of someone or something.

Trust

firm belief in the reliability, truth, or ability of someone or something.

Fix Don'ts

Never provide fixes that lead to compiler errors

 

Don't provide a fix if the user needs to do a follow up

Automatic fixes

🤬

code style

TODO Give example of code fix where style becomes not like what you wished

Automatic fixes

❤️

code formatters

Linting

Linting?

Unnecessary ESLint rules for Elm

92% (56/61) of the recommended rules

87% (228/263) of all the rules

People enable rules they don't agree with

It's hard to configure so many rules

Large use of premade configurations

Linters are about

enforcing code style

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" ]
        ]

Linters are

NOT about

formatting

Linters are

NOT

about code style

Writing a

Static Code Analysis Rule 

Written in Elm, for Elm

  • Every Elm developer knows Elm

  • No runtime errors

Abstract Syntax Trees

(a + b) / 2
Integer 2
OperatorApplication
   operator +
FunctionOrValue a
OperatorApplication
   operator /
FunctionOrValue b
ParenthesizedExpression
rule : Rule
rule =
    Rule.newSchema "NoUsingHtmlButton" ()
        |> Rule.withExpressionVisitor expressionVisitor
        |> Rule.fromSchema


expressionVisitor : Node Expression -> Context -> ( List Error, Context )
expressionVisitor node context =
    case Node.value node of
        Expression.FunctionOrValue [ "Html" ] "button" ->
            ( [ -- Report an error
              ]
            , context
            )

        _ ->
            ( [], context )
expressionVisitor : Node Expression -> Context -> ( List Error, Context )
expressionVisitor node context =
    case Node.value node of
        Expression.FunctionOrValue [ "Html" ] "button" ->
            ( [ Rule.error
                    "Do not use Html.button"
                    (Node.range node)
              ]
            , context
            )

        _ ->
            ( [], context )

Creating guarantees

in a project

We have a great product,

it's working well and it never crashes.

But we have an inconsistent UI.
Let's improve that.

-- Page/Checkout.elm
viewConfirmButton : Html msg
viewConfirmButton =
    Html.button
        [ Attr.style "height" "35px"
        , Events.onClick UserClickedOnConfirmButton
        ]
        [ Html.text "Confirm" ]
-- Page/Payment.elm
viewPayButton : Html msg
viewPayButton =
    Html.button
        [ Attr.style "height" "33px"
        , Events.onClick UserClickedOnPayButton
        ]
        [ Html.text "Pay" ]
module Ui.Button exposing (confirm, cancel, andSomeOtherButtons)
  
confirm : msg -> String -> Html msg

cancel : msg -> String -> Html msg
-- Page/Checkout.elm
import Ui.Button as Button
viewConfirmButton : Html msg
viewConfirmButton =
    Button.confirm UserClickedOnConfirmButton "Confirm"
-- Page/Payment.elm
import Ui.Button as Button
viewPayButton : Html msg
viewPayButton =
    Button.confirm UserClickedOnPayButton "Pay"

"We now have great modules for every UI element.

Now we are sure to have a consistent UI across the application."

New pull request comes in...

view : Model -> Html Msg
view model =
    Html.button
        [ Attr.style "height" "34px"
        , Events.onClick UserClickedOnRemoveButton
        ]
        [ Html.text "Remove" ]

Providing information

  • Type information

  • Dependencies

  • Project manifest, README

  • Collecting data while traversing

False positive in

Ui.Button

type alias Context =
  { isUiButtonModule : Bool }

rule : Rule
rule =
    Rule.newSchema "NoUnusedDeclarations" { isUiButtonModule = False }
    	-- visitors...
        |> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
        |> Rule.fromSchema

moduleDefinitionVisitor : Node Module -> Context -> ( List Error, Context )
moduleDefinitionVisitor (Node _ { moduleName }) context =
    ( [], { isUiButtonModule = (moduleName == "Ui.Button") } )
module Ui.Button exposing (confirm, cancel, andSomeOtherButtons)
expressionVisitor : Node Expression -> Context -> ( List Error, Context )
expressionVisitor node context =
    case Node.value node of
        Expression.FunctionOrValue [ "Html" ] "button" ->
            if not context.isUiButtonModule then
                ( [ Rule.error {- ... -} ]
                , context
                )

            else
                ( [], context )

        _ ->
            ( [], context )

Gathering context from multiple files

module A exposing (used, neverUsed)

used =
    "some thing"

neverUsed =
    "other thing"
module B exposing (thing)

import A

thing =
    A.used ++ " is exposed"

Report unused exports

elm-review

Creating guarantees

Communication

Reducing false positives

Ignoring reports

Trustworthy automatic fixes

Developers need to trust their tools

How do we prevent this from happening?

Types...? Tests...?

view : Model -> Html Msg
view model =
    Html.button
        [ Attr.style "height" "34px"
        , Events.onClick UserClickedOnRemoveButton
        ]
        [ Html.text "Remove" ]

If you can detect the problem just by reading the code, static code analysis can help.

We need a linter

with custom rules

Are we getting the

message across?

File path

Rule name

Location

Message

Are we getting the

message across?

File path

Rule name

Location

Message

Good rule documentation

What is the target problem?

 

What are the consequences of the problem?

 

When should you (not) enable the rule?

Why you don't trust your linter before content cut

By Jeroen Engels

Why you don't trust your linter before content cut

Talk for GOTO Copenhagen 2022

  • 332