Designing a Static Analysis Tool for Elm

Jeroen Engels

@jfmengels

Elm Radio

elm-review

Creating guarantees

Communication

Reducing false positives

Ignoring reports

Trustworthy automatic fixes

Static Code Analysis Tools

Linting

Linting?

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

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

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

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 )

Rule name

Source

code extract

Message

File path

Are we getting the

message across?

Developers need to understand

  • What they did wrong

  • Why it is a problem

  • How to move forward

expressionVisitor : Node Expression -> Context -> ( List Error, Context )
expressionVisitor node context =
    case Node.value node of
        Expression.FunctionOrValue [ "Html" ] "button" ->
            ( [ Rule.error
                    "Use Ui.Button instead of the native Html.button"
                    (Node.range node)
              ]
            , context
            )

        _ ->
            ( [], context )
expressionVisitor : Node Expression -> Context -> ( List Error, Context )
expressionVisitor node context =
    case Node.value node of
        Expression.FunctionOrValue [ "Html" ] "button" ->
            ( [ Rule.error
            	  { message = "Use Ui.Button instead of the native Html.button"
                  , details =
                    [ "At fruits.com, we try to have a consistent UI across the application, and one of the ways we do that is by having a single nice module to create buttons, named Ui.Button."
                    , "Here, you defined a button using `Html.button` or `Html.Styled.Button`, which is likely not to have the consistent UI we aim for or some of the guarantees we created around our buttons."
                    , "Instead, you should use the Ui.Button module. I suggest reading the documentation in that module, but here is what it would kind of look like: ..."
                    ]
                  }
                  (Node.range node)
              ]
            , context
            )

        _ ->
            ( [], context )

Context

Problem

Suggestion

Example

Rule name

Source

code extract

Message

File path

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

Developers need to trust their tools

False positives

False negatives

Linters report lots of

false positives

Missing information

Presumptions

False positives/negatives

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

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

...

No disable comments

Elm is knowable

Better understanding of the code

Almost no false positives

Ignoring reports

Indeterminate rules

Generated/vendored code

Ignoring rules

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

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

Automatic fixes

🤬

code style

Automatic fixes

❤️

code formatters

Fixes should be trustworthy

Prompt for fixes

Use static analysis tools to create guarantees

Communication

  • What is the problem?

  • Why is it a problem?

  • How to solve the problem?

Remove false positives

by providing information

Simpler and knowable

languages allow better analysis

Better alternatives than

disable comments

to ensure rules are enforced

Tools can create trust

by reporting real problems and presenting them well

Thank you!

Jeroen Engels

@jfmengels

Elm Radio

Made with Slides.com