Why you don't trust your linter

Jeroen Engels

@jfmengels

Jeroen Engels

@jfmengels everywhere

Elm Radio podcast

elm-review

❤️

(we're hiring)

🤬

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

elm-review.com

False positives

Communication
Automatic fixes

Configuration

Ignoring reports

False positives

when the tool reports issues that it shouldn't have

when the tool reports issues that it shouldn't have

🤬

🫶

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

How should I fix this issue?

Should I ignore this report?

Causes of false positives

Bugs

Missing information

Presumptions

False positives/negatives

(when the tool doesn't report an issue that it should have reported)

Providing information

  • Contents of all files in the project

  • Type information

  • Dependencies

  • Files from different languages

How easy is it to analyze the language?

Compiled languages

Statically typed languages 

Explicit patterns

Dynamic constructs

https://www.youtube.com/watch?v=_rzoyBq4hJ0

Effective false positives

true positives that are considered to be false positives

How is the error communicated?

Not 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

Automatic fixes

Prompt for fixes

Configuring linters

Accidental configuration

Essential configuration

Accidental configuration

  • What language features are available/enabled?

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

False positives

because of the configuration

Tell the tool how to work with your project because it can't figure some information on its own

Accidental configuration

All necessary information

in standard files

// elm.json
{
    "type": "application",
    "source-directories": [
        "src"
    ],
    "elm-version": "0.19.1",
    "dependencies": {
        "...": "..."
    }
}

Essential configuration

Tell the tool what you want from it

 

 

Which rules do you want to enable?

Configuring elm-review

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

Enable rules

you agree with

false positive ≈ error you disagree with

Advice on adding a rule

Be ready to disable the rule if it feels painful

Understand the problem well

Get your team's approval

Understand not all rules are good

Don't use other people's configuration

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...?

Errors

For rules you want to enforce

Line 10: Don't do this thing
Line 11: Don't do this thing
Line 12: Don't do this thing
Line 13: Don't do this thing
Line 14: Don't do this thing
Line 15: Don't do this thing
Line 16: Don't do this thing
Line 17: Don't do this thing
Line 18: Don't do this thing
Line 19: Don't do this thing
Line 20: Don't do this thing
Line 21: Don't do this thing
Line 22: Don't do this thing
Line 23: Don't do this thing
Line 24: Don't do this thing
Line 25: Don't do this thing
Line 26: Don't do this thing
Line 27: Don't do this thing
Line 28: Don't do this thing
Line 29: Don't do this thing
Line 30: Don't do this thing
Line 31: Don't do this thing
Line 32: Don't do this thing
Line 33: Don't do this thing
Line 34: Don't do this thing
Line 35: Don't do this thing
Line 36: Don't do this thing
Line 37: Don't do this thing
Line 38: Don't do this thing
Line 39: Don't do this thing

Line 23: Don't do this thing
Line 38: Don't do this thing

Line 16: Don't do this thing

Line 23: Don't do this thing
Line 38: Don't do this thing

Advice on how to use

// linter-disable
this.code.will.crash();

Not available in elm-review

Warnings

-- elm-review-disable rule

❌

❌

😱

Rules with exceptions

Higher quality rules

Don't write  the rule

Disable comments and warnings

enable bad quality rules

When the rule doesn't make sense in some places

config : List Rule
config =
    [ NoUnused.Variables.rule
    , NoDebug.Log.rule
    ]
        |> Rule.ignoreErrorsForDirectories [ "tests/" ]

Allowing existing errors

gradually adopt a rule

Deprecating a function

{-| Does something.

@deprecated Use someBetterFunction which does it better.

-}
someFunction input =
    -- do something with input

Deprecating a function

//nolint:rule
# pylint: disable=rule
# rubocop:disable rule
// NOLINT
// phpcs:disable rule
@SuppressWarnings("rule")
@Suppress("rule")
// eslint-disable rule
#![deny(clippy::all)]

Line 10: Don't do this thing
Line 11: Don't do this thing
Line 12: Don't do this thing
Line 13: Don't do this thing
Line 14: Don't do this thing
Line 15: Don't do this thing
Line 16: Don't do this thing
Line 17: Don't do this thing
Line 18: Don't do this thing
Line 19: Don't do this thing
Line 20: Don't do this thing
Line 21: Don't do this thing
Line 22: Don't do this thing
Line 23: Don't do this thing
Line 24: Don't do this thing
Line 25: Don't do this thing
Line 26: Don't do this thing
Line 27: Don't do this thing
Line 28: Don't do this thing
Line 29: Don't do this thing
Line 30: Don't do this thing
Line 31: Don't do this thing
Line 32: Don't do this thing
Line 33: Don't do this thing
Line 34: Don't do this thing
Line 35: Don't do this thing
Line 36: Don't do this thing
Line 37: Don't do this thing
Line 38: Don't do this thing
Line 39: Don't do this thing

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

$ elm-review suppress

No new errors

Introducing a new error

Fixing a suppressed error

To summarize...

False positives and

effective false positives

lead to losing trust

Remove false reports

by providing more information

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

Please open issues

to improve your linter

Thank you!

Jeroen Engels

@jfmengels

Learn more:

Slides: https://slides.com/jfmengels/why-you-dont-trust-your-linter

Made with Slides.com