sparksp / elm-review-rules-to-avoid

Provides some elm-analyse rules for elm-review that we don't want to publish. These are written as an exercise to confirm they're possible.

Hi there 👋

elm-review-rules-to-avoid's Issues

NoSingleFieldRecord

See https://stil4m.github.io/elm-analyse/#/messages/SingleFieldRecord

What the rule should do:

Forbid records and extensible records with only one field.

What problems does it solve:

Using a record is obsolete if you only plan to store a single field in it. You can use the field's type or introduce a custom Type instead.

Example of things the rule would report:

• Records in type aliases
• Records in function signature (argument or return)
• Records in custom types
• Records in records, tuples or sub-types

Example of things the rule would not report:

• Any records with more than one field
• Pattern matching a single field

When (not) to enable this rule:

This rule could be really handy, except for a few situational occasions when you really wouldn't want it. Including:

• Named arguments

NoNothingToNothing

See https://stil4m.github.io/elm-analyse/#/messages/MapNothingToNothing

What the rule should do:

Forbid case expressions where Nothing is matched and then Nothing is returned.

What problems does it solve:

It makes the code more complicated than necessary. Use May.andThen or Maybe.map instead.

Example of things the rule would report:

module Greet exposing (greet)

greet : Maybe String -> Maybe String
greet maybeName =
    case maybeName of
        Nothing ->
            Nothing

        Just name ->
            Just ("Hello " ++ name)

Example of things the rule would not report:

module Greet exposing (greet)

greet : Maybe String -> Maybe String
greet maybeName =
    case maybeName of
        Nothing ->
           Just "Hello"

        Just name ->
            Just ("Hello " ++ name)

When (not) to enable this rule:

You don't need this rule. If you're writing code like this then there may be some wider code restructures to consider.

I am looking for:

• Reasons you would / would not want to use this rule.

NoSingleFieldRecord should report records in custom types

Related #2.

Examples of things the rule should report:

type Foo
    = Foo { bar : String }

type Bish a
    = Bish { a | bosh : String }

Examples of things the rule should not report:

type Point
    = Point { x : Int, y : Int }

type Bish a
    = Bish { a | bosh : String, bash : Int }

NoFunctionInLet

See: https://stil4m.github.io/elm-analyse/#/messages/FunctionInLet

What the rule should do:

Forbid the declaration of functions in let expressions.

What problems does it solve:

In a let statement you can define variables and functions in their own scope. But you are already in the scope of a module. Just define the functions you want on a top-level. There is no not much need to define functions in let statements.

This will also stop you from mixing values into the local function from the scope (which can cause unexpected performance reductions).

Example of things the rule would report:

foo : Int -> Int
foo x =
    let
        somethingIShouldDefineOnTopLevel : Int -> Int
        somethingIShouldDefineOnTopLevel y =
            y + 1
    in
        somethingIShouldDefineOnTopLevel x

Example of things the rule would not report:

• Anything without a function signature type annotation or an argument (where it's not clear whether it's a constant or a function).

When (not) to enable this rule:

Just don't enable this - whilst it's a good basic rule of thumb, there often needs to be much more to this review than this simple rule can account for.

I am looking for:

• Good reasons not to use this rule.

Maintain docs.json with CI

We should use GitHub Actions to ensure that docs.json is kept up to date so that the doc preview is always correct.

NoSingleFiledRecord should report records in other records

Related #2.

Examples of things that should be reported:

type alias Vector =
    { start : { point : Point }
    , end : { point : Point }
    }

Examples of things that should not be reported:

type alias Vector =
    { start : { x : Int, y : Int }
    , end : { x : Int, y : Int }
    }

NoSingleLineRecords

See https://stil4m.github.io/elm-analyse/#/messages/MultiLineRecordFormatting

What the rule should do:

Forbid record definitions in type aliases that are written on a single line.

What problems does it solve:

Records are easier to read and give more meaningful source control diff when they're written one field per line.

Example of things the rule would report:

module Foo exposing (Person)

type alias Person =
    { name : String, age : Int, address : Address }

Example of things the rule would not report:

module Foo exposing (Person)

type alias Person =
    { name : String
    , age : Int
    , address : Address
    }

It would also ignore record definitions used in function signatures and all record construction.

When (not) to enable this rule:

You don't ever need to enable this rule. This is a code style that can easily be reviewed manually as appropriate.

I am looking for:

• Reasons why you would want to use this rule.
• Reasons why you would not want to use this rule.