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 inputDeprecating 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) / 2Integer 2OperatorApplication
operator +FunctionOrValue aOperatorApplication
operator /FunctionOrValue bParenthesizedExpression
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
