Skip to content

Draft: Add elm-review rule NoMissingTypeAnnotationInLetIn

Frank Tackitt requested to merge kageurufu/exosphere:elm-review/NoMissingTypeAnnotationInLetIn into master

Overview

Add elm-review rule NoMissingTypeAnnotationInLetIn

This rule forbids declarations without a type annotation inside a let .. in body

-- ELM-REVIEW ERROR --------------------------------------- tests/Tests.elm:42:9

NoMissingTypeAnnotationInLetIn: Missing type annotation for `testData`

41|     let
42|         testData =
            ^^^^^^^^
43|             [ { expectedArticle = "an"

Type annotations help you understand what happens in the code, and it will help
the compiler give better error messages.

We currently have 1331 errors, and with no auto-fix available this would easily take days to manually fix The elm language server does have an action for Add inferred annotation which can sometimes work well. Other times, it just generates fairly useless generic types like a -> b -> (b -> c) -> c

For the above error, it does well adding testData : List { expectedArticle : String, phrase : String, description : String }.

However at src/View/View.elm:518:9, it gives a more vague definition

dropdownTarget : a -> Bool -> Element.Element a
dropdownTarget toggleDropdownMsg dropdownIsShown =

This isn't a "bad" definition, the function is passed into Popover which expects msg -> Bool -> Element.Element msg The naming of a is just less helpful than msg in this case.

Discussion

Adding this will either require a lot of legwork up front manually fixing these, hacking something with the elm language server to auto-fix everywhere we can, or suppressing the 1331 current errors.

I generally like the idea, but it would also raise the barrier to new contributors.

If the consensus is positive, we can discuss how to clear the errors so the rule could be merged.

Merge request reports