Backport rules

tests/List/Extra.elm

@@ -0,0 +1,24 @@
+module List.Extra exposing (find)
+
+{-| Some utilities.
+-}
+
+
+{-| Find the first element that satisfies a predicate and return
+Just that element. If none match, return Nothing.
+
+    find (\num -> num > 5) [ 2, 4, 6, 8 ] == Just 6
+
+-}
+find : (a -> Bool) -> List a -> Maybe a
+find predicate list =
+    case list of
+        [] ->
+            Nothing
+
+        first :: rest ->
+            if predicate first then
+                Just first
+
+            else
+                find predicate rest

tests/NoDebug/Log.elm

@@ -16,6 +16,8 @@ import Review.Rule as Rule exposing (Error, Rule)
 
 {-| Forbid the use of [`Debug.log`](https://package.elm-lang.org/packages/elm/core/latest/Debug) before it goes into production or fails in the CI.
 
+🔧 Running with `--fix` will automatically remove all the reported errors.
+
 `Debug.log` is useful to debug your code, but should not be pushed to production.
 
     config =

tests/NoUnoptimizedRecursion.elm

@@ -0,0 +1,670 @@
+module NoUnoptimizedRecursion exposing
+    ( rule
+    , Configuration, optOutWithComment, optInWithComment
+    )
+
+{-|
+
+@docs rule
+
+Tail-call optimization makes Elm code more performant and helps prevent stack overflows.
+
+Since this optimization is done silently and under specific circumstances, it is unfortunately relatively easy
+to not notice when the optimization is not being applied. You can find the [reasons why a function would not be optimized below](#fail).
+
+I wrote a whole [article about tail-call optimization](https://jfmengels.net/tail-call-optimization/). Some of the information
+are repeated in this rule's documentation, but it's more complete.
+
+
+## Configuration
+
+@docs Configuration, optOutWithComment, optInWithComment
+
+
+## When (not) to enable this rule
+
+This rule is useful for both application maintainers and package authors to detect locations where
+performance could be improved and where stack overflows can happen.
+
+You should not enable this rule if you currently do not want to invest your time into thinking about performance.
+
+
+## Try it out
+
+You can try this rule out by running the following command:
+
+```bash
+elm-review --template jfmengels/elm-review-performance/example --rules NoUnoptimizedRecursion
+```
+
+The rule uses `optOutWithComment "IGNORE TCO"` as its configuration.
+
+
+## Success
+
+This function won't be reported because it is tail-call optimized.
+
+    fun n =
+        if condition n then
+            fun (n - 1)
+
+        else
+            n
+
+This function won't be reported because it has been tagged as ignored.
+
+    -- With opt-out configuration
+    config =
+        [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optOutWithComment "IGNORE TCO")
+        ]
+
+    fun n =
+        -- elm-review: IGNORE TCO
+        fun n * n
+
+This function won't be reported because it has not been tagged.
+
+    -- With opt-in configuration
+    config =
+        [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optInWithComment "CHECK TCO")
+        ]
+
+    fun n =
+        fun n * n
+
+
+## Fail
+
+To understand when a function would not get tail-call optimized, it is important to understand when it would be optimized.
+
+The Elm compiler is able to apply tail-call optimization **only** when a recursive call **(1)** is a simple function application and **(2)** is the last operation that the function does in a branch.
+
+**(1)** means that while `recurse n = recurse (n - 1)` would be optimized, `recurse n = recurse <| n - 1` would not. Even though you may consider `<|` and `|>` as syntactic sugar for function calls, the compiler doesn't (at least with regard to TCO).
+
+As for **(2)**, the locations where a recursive call may happen are:
+
+  - branches of an if expression
+  - branches of a case expression
+  - in the body of a let expression
+  - inside simple parentheses
+
+and only if each of the above appeared at the root of the function or in one of the above locations themselves.
+
+The compiler optimizes every recursive call that adheres to the rules above, and simply doesn't optimize the other
+branches which would call the function naively and add to the stack frame.
+It is therefore possible to have **partially tail-call optimized functions**.
+
+Following is a list of likely situations that will be reported.
+
+
+### An operation is applied on the result of a function call
+
+The result of this recursive call gets multiplied by `n`, making the recursive call not the last thing to happen in this branch.
+
+    factorial : Int -> Int
+    factorial n =
+        if n <= 1 then
+            1
+
+        else
+            factorial (n - 1) * n
+
+Hint: When you need to apply an operation on the result of a recursive call, what you can do is to add an argument holding the result value and apply the operations on it instead.
+
+    factorialHelp : Int -> Int -> Int
+    factorialHelp n result =
+        if n <= 1 then
+            result
+
+        else
+            factorialHelp (result * n)
+
+and split the function into the one that will do recursive calls (above) and an "API-facing" function which will set the initial result value (below).
+
+    factorial : Int -> Int
+    factorial n =
+        factorialHelp n 1
+
+
+### Calls using the |> or <| operators
+
+Even though you may consider these operators as syntactic sugar for function calls, the compiler doesn't and
+the following won't be optimized. The compiler doesn't special-case these functions and considers them as operators just
+like `(*)` in the example above.
+
+    fun n =
+        if condition n then
+            fun <| n - 1
+
+        else
+            n
+    fun n =
+        if condition n then
+            (n - 1)
+                |> fun
+
+        else
+            n
+
+The fix here consists of converting the recursive calls to ones that don't use a pipe operator.
+
+
+### Calls appearing in || or && conditions
+
+The following won't be optimized.
+
+    isPrefixOf : List a -> List a -> Bool
+    isPrefixOf prefix list =
+        case ( prefix, list ) of
+            ( [], _ ) ->
+                True
+
+            ( _ :: _, [] ) ->
+                False
+
+            ( p :: ps, x :: xs ) ->
+                p == x && isPrefixOf ps xs
+
+The fix here is consists of using if expressions instead.
+
+    isPrefixOf : List a -> List a -> Bool
+    isPrefixOf prefix list =
+        case ( prefix, list ) of
+            ( [], _ ) ->
+                True
+
+            ( _ :: _, [] ) ->
+                False
+
+            ( p :: ps, x :: xs ) ->
+                if p == x then
+                    isPrefixOf ps xs
+
+                else
+                    False
+
+
+### Calls from let declarations
+
+Calls from let functions won't be optimized.
+
+    fun n =
+        let
+            funHelp y =
+                fun (y - 1)
+        in
+        funHelp n
+
+Note that recursive let functions can be optimized if they call themselves, but calling the parent function
+will cause the parent to not be optimized.
+
+-}
+
+import Elm.Syntax.Declaration as Declaration exposing (Declaration)
+import Elm.Syntax.Expression as Expression exposing (Expression)
+import Elm.Syntax.Node as Node exposing (Node(..))
+import Elm.Syntax.Range exposing (Range)
+import Review.Rule as Rule exposing (Rule)
+import Set exposing (Set)
+
+
+{-| Reports recursive functions that are not [tail-call optimized](https://functional-programming-in-elm.netlify.app/recursion/tail-call-elimination.html).
+-}
+rule : Configuration -> Rule
+rule configuration =
+    Rule.newModuleRuleSchema "NoUnoptimizedRecursion" initialContext
+        |> Rule.withCommentsVisitor (commentsVisitor configuration)
+        |> Rule.withDeclarationEnterVisitor (declarationVisitor configuration)
+        |> Rule.withExpressionEnterVisitor (expressionEnterVisitor configuration)
+        |> Rule.withExpressionExitVisitor expressionExitVisitor
+        |> Rule.fromModuleRuleSchema
+
+
+
+-- CONFIGURATION
+
+
+{-| Configuration for `NoUnoptimizedRecursion`.
+
+Use [`optOutWithComment`](#optOutWithComment) or [`optInWithComment`](#optInWithComment) to configure this rule.
+
+You can use comments to tag functions as to be checked or ignored, depending on the configuration option you chose.
+This comment has to appear on the line after the `=` that follows the declaration of your function. Note that this
+comment only needs to contain the tag that you're choosing and that it is case-sensitive.
+The same will apply for functions defined in a let expression, since they can be tail-call optimized as well.
+
+-}
+type Configuration
+    = OptOut String
+    | OptIn String
+
+
+{-| Reports recursive functions by default, opt out functions tagged with a comment.
+
+    config =
+        [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optOutWithComment "IGNORE TCO")
+        ]
+
+With the configuration above, the following function would not be reported.
+
+    fun n =
+        -- elm-review: IGNORE TCO
+        if condition n then
+            fun n * n
+
+        else
+            n
+
+The reasons for allowing to opt-out is because sometimes recursive functions are simply not translatable to
+tail-call optimized ones, for instance the ones that need to recurse over multiple elements (`fun left + fun right`).
+
+I recommend to **not** default to ignoring a reported issue, and instead to discuss with your colleagues how to best
+solve the error when you encounter it or when you see them ignore an error.
+
+I recommend to use this configuration option as your permanent configuration once you have fixed or opted-out of every function.
+
+-}
+optOutWithComment : String -> Configuration
+optOutWithComment comment =
+    OptOut comment
+
+
+{-| Reports only the functions tagged with a comment.
+
+    config =
+        [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optInWithComment "CHECK TCO")
+        ]
+
+With the configuration above, the following function would be reported.
+
+    fun n =
+        -- CHECK TCO
+        if condition n then
+            fun n * n
+
+        else
+            n
+
+-}
+optInWithComment : String -> Configuration
+optInWithComment comment =
+    OptIn comment
+
+
+shouldReportFunction : Configuration -> Context -> Range -> Bool
+shouldReportFunction configuration context range =
+    let
+        foundComment : Bool
+        foundComment =
+            Set.member (range.start.row + 1) context.comments
+    in
+    case configuration of
+        OptOut _ ->
+            not foundComment
+
+        OptIn _ ->
+            foundComment
+
+
+
+-- CONTEXT
+
+
+type alias Context =
+    { currentFunctionName : String
+    , tcoLocations : List Range
+    , newScopesForLet : List ( Range, String )
+    , parentScopes : List ( Range, Scope )
+    , parentNames : Set String
+    , comments : Set Int
+    , deOptimizationRange : Maybe Range
+    , deOptimizationReason : List String
+    }
+
+
+type alias Scope =
+    { currentFunctionName : String
+    , tcoLocations : List Range
+    , newScopes : List ( Range, String )
+    }
+
+
+initialContext : Context
+initialContext =
+    { currentFunctionName = ""
+    , tcoLocations = []
+    , newScopesForLet = []
+    , parentScopes = []
+    , parentNames = Set.empty
+    , comments = Set.empty
+    , deOptimizationRange = Nothing
+    , deOptimizationReason = []
+    }
+
+
+
+-- VISITORS
+
+
+commentsVisitor : Configuration -> List (Node String) -> Context -> ( List nothing, Context )
+commentsVisitor configuration comments context =
+    let
+        commentTag : String
+        commentTag =
+            case configuration of
+                OptOut commentTag_ ->
+                    commentTag_
+
+                OptIn commentTag_ ->
+                    commentTag_
+    in
+    ( []
+    , { context
+        | comments =
+            comments
+                |> List.filter (Node.value >> String.contains commentTag)
+                |> List.map (Node.range >> .start >> .row)
+                |> Set.fromList
+      }
+    )
+
+
+declarationVisitor : Configuration -> Node Declaration -> Context -> ( List nothing, Context )
+declarationVisitor configuration node context =
+    case Node.value node of
+        Declaration.FunctionDeclaration function ->
+            ( []
+            , { currentFunctionName =
+                    let
+                        hasArguments : Bool
+                        hasArguments =
+                            function.declaration
+                                |> Node.value
+                                |> .arguments
+                                |> List.isEmpty
+                                |> not
+                    in
+                    if hasArguments && shouldReportFunction configuration context (Node.range function.declaration) then
+                        function.declaration
+                            |> Node.value
+                            |> .name
+                            |> Node.value
+
+                    else
+                        ""
+              , tcoLocations =
+                    [ function.declaration
+                        |> Node.value
+                        |> .expression
+                        |> Node.range
+                    ]
+              , newScopesForLet = []
+              , parentScopes = []
+              , parentNames = Set.empty
+              , comments = context.comments
+              , deOptimizationRange = Nothing
+              , deOptimizationReason = []
+              }
+            )
+
+        _ ->
+            ( [], context )
+
+
+expressionEnterVisitor : Configuration -> Node Expression -> Context -> ( List (Rule.Error {}), Context )
+expressionEnterVisitor configuration node context =
+    let
+        newContext : Context
+        newContext =
+            case context.newScopesForLet of
+                [] ->
+                    context
+
+                ( range, name ) :: restOfNewScopes ->
+                    if range == Node.range node then
+                        { currentFunctionName = name
+                        , tcoLocations = [ range ]
+                        , newScopesForLet = restOfNewScopes
+                        , parentScopes =
+                            ( range
+                            , { currentFunctionName = context.currentFunctionName, tcoLocations = context.tcoLocations, newScopes = restOfNewScopes }
+                            )
+                                :: context.parentScopes
+                        , parentNames = Set.insert context.currentFunctionName context.parentNames
+                        , comments = context.comments
+                        , deOptimizationRange = context.deOptimizationRange
+                        , deOptimizationReason = context.deOptimizationReason
+                        }
+
+                    else
+                        context
+    in
+    if isInTcoLocation newContext (Node.range node) then
+        ( reportReferencesToParentFunctions node newContext, addAllowedLocation configuration node newContext )
+
+    else
+        ( reportRecursiveCallInNonAllowedLocation node newContext, newContext )
+
+
+reportRecursiveCallInNonAllowedLocation : Node Expression -> Context -> List (Rule.Error {})
+reportRecursiveCallInNonAllowedLocation node context =
+    case Node.value node of
+        Expression.FunctionOrValue [] name ->
+            if name == context.currentFunctionName then
+                [ error (Node.range node) context.deOptimizationReason ]
+
+            else
+                []
+
+        _ ->
+            []
+
+
+reportReferencesToParentFunctions : Node Expression -> Context -> List (Rule.Error {})
+reportReferencesToParentFunctions node context =
+    case Node.value node of
+        Expression.Application ((Node funcRange (Expression.FunctionOrValue [] name)) :: _) ->
+            if Set.member name context.parentNames then
+                [ error funcRange [ "Among other possible reasons, the recursive call should not appear inside a let declaration." ] ]
+
+            else
+                []
+
+        _ ->
+            []
+
+
+error : Range -> List String -> Rule.Error {}
+error range additionalDetails =
+    Rule.error
+        { message = "This function call cannot be tail-call optimized"
+        , details =
+            additionalDetails
+                ++ [ "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail" ]
+        }
+        range
+
+
+addAllowedLocation : Configuration -> Node Expression -> Context -> Context
+addAllowedLocation configuration node context =
+    case Node.value node of
+        Expression.Application (function :: _) ->
+            { context
+                | tcoLocations = Node.range function :: context.tcoLocations
+                , deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.IfBlock condition thenBranch elseBranch ->
+            { context
+                | tcoLocations = Node.range thenBranch :: Node.range elseBranch :: context.tcoLocations
+                , deOptimizationRange = Just (Node.range condition)
+                , deOptimizationReason = [ "Among other possible reasons, the recursive call should not appear inside an if condition." ]
+            }
+
+        Expression.LetExpression { declarations, expression } ->
+            let
+                newScopes : List ( Range, String )
+                newScopes =
+                    List.filterMap
+                        (\decl ->
+                            case Node.value decl of
+                                Expression.LetFunction function ->
+                                    let
+                                        functionDeclaration : Expression.FunctionImplementation
+                                        functionDeclaration =
+                                            Node.value function.declaration
+
+                                        hasArguments : Bool
+                                        hasArguments =
+                                            function.declaration
+                                                |> Node.value
+                                                |> .arguments
+                                                |> List.isEmpty
+                                                |> not
+                                    in
+                                    Just
+                                        ( Node.range functionDeclaration.expression
+                                        , if hasArguments && shouldReportFunction configuration context (Node.range function.declaration) then
+                                            Node.value functionDeclaration.name
+
+                                          else
+                                            ""
+                                        )
+
+                                Expression.LetDestructuring _ _ ->
+                                    Nothing
+                        )
+                        declarations
+            in
+            { context
+                | newScopesForLet = newScopes
+
+                {- The following translates to TCO code
+
+                   let
+                       fun x =
+                          fun x
+                   in
+                   fun 1
+                -}
+                , tcoLocations = Node.range expression :: context.tcoLocations
+            }
+
+        Expression.ParenthesizedExpression expr ->
+            {- The following translates to TCO code
+
+               fun x =
+                 (fun x)
+            -}
+            { context | tcoLocations = Node.range expr :: context.tcoLocations }
+
+        Expression.CaseExpression { cases } ->
+            let
+                caseBodies : List Range
+                caseBodies =
+                    List.map (Tuple.second >> Node.range) cases
+            in
+            { context
+                | tcoLocations = caseBodies ++ context.tcoLocations
+                , deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, the recursive call should not appear in the pattern to evaluate for a case expression." ]
+            }
+
+        Expression.OperatorApplication operator _ _ _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason =
+                    List.filterMap identity
+                        [ Just "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                        , if operator == "<|" || operator == "|>" then
+                            Just ("Removing the usage of `" ++ operator ++ "` may fix the issue here.")
+
+                          else
+                            Nothing
+                        ]
+            }
+
+        Expression.Negation _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.TupledExpression _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are storing the result of recursive call inside a tuple. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.ListExpr _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are storing the result of recursive call inside a list. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.RecordExpr _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are storing the result of recursive call inside a record. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.RecordUpdateExpression _ _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are storing the result of recursive call inside a record. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.RecordAccess _ _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, you are accessing a field on the result of recursive call. The recursive call should be the last thing to happen in this branch." ]
+            }
+
+        Expression.LambdaExpression _ ->
+            { context
+                | deOptimizationRange = Just (Node.range node)
+                , deOptimizationReason = [ "Among other possible reasons, the recursive call should not appear inside an anonymous function." ]
+            }
+
+        _ ->
+            context
+
+
+expressionExitVisitor : Node Expression -> Context -> ( List nothing, Context )
+expressionExitVisitor node context =
+    case context.parentScopes of
+        [] ->
+            ( [], removeDeOptimizationRangeIfNeeded node context )
+
+        ( headRange, headScope ) :: restOfParentScopes ->
+            if headRange == Node.range node then
+                ( []
+                , removeDeOptimizationRangeIfNeeded node
+                    { currentFunctionName = headScope.currentFunctionName
+                    , tcoLocations = headScope.tcoLocations
+                    , newScopesForLet = headScope.newScopes
+                    , parentScopes = restOfParentScopes
+                    , parentNames = Set.remove headScope.currentFunctionName context.parentNames
+                    , comments = context.comments
+                    , deOptimizationRange = context.deOptimizationRange
+                    , deOptimizationReason = context.deOptimizationReason
+                    }
+                )
+
+            else
+                ( [], removeDeOptimizationRangeIfNeeded node context )
+
+
+removeDeOptimizationRangeIfNeeded : Node Expression -> Context -> Context
+removeDeOptimizationRangeIfNeeded node context =
+    if Just (Node.range node) == context.deOptimizationRange then
+        { context | deOptimizationRange = Nothing }
+
+    else
+        context
+
+
+isInTcoLocation : Context -> Range -> Bool
+isInTcoLocation context range =
+    List.member range context.tcoLocations

tests/NoUnoptimizedRecursionTest.elm

@@ -0,0 +1,529 @@
+module NoUnoptimizedRecursionTest exposing (all)
+
+import NoUnoptimizedRecursion exposing (optInWithComment, optOutWithComment, rule)
+import Review.Test
+import Test exposing (Test, describe, test)
+
+
+message : String
+message =
+    "This function call cannot be tail-call optimized"
+
+
+all : Test
+all =
+    describe "NoUnoptimizedRecursion"
+        [ test "should not report non-recursive functions" <|
+            \() ->
+                """module A exposing (..)
+a = 1
+fun x = a + x
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report self-referential values without arguments" <|
+            \() ->
+                """module A exposing (..)
+commentDecoder =
+  map2 Comment
+    (field "message" string)
+    (field "responses" (map Responses (list (lazy (\\_ -> commentDecoder)))))
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report self-referential let values without arguments" <|
+            \() ->
+                """module A exposing (..)
+a =
+  let
+    commentDecoder =
+      map2 Comment
+        (field "message" string)
+        (field "responses" (map Responses (list (lazy (\\_ -> commentDecoder)))))
+  in
+  commentDecoder
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should report an error when a function is recursive but applies operations on the result" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  fun x + 1
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 3 }, end = { row = 3, column = 6 } }
+                        ]
+        , test "should not report an error when a function is properly TCO (if then branch)" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  if condition x then
+    fun (x - 1)
+  else
+    x
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error when a function is properly TCO (if else branch)" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  if condition x then
+    x
+  else
+    fun (x - 1)
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should report recursive call in the arguments of a function call" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  other (fun (x - 1))
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 10 }, end = { row = 3, column = 13 } }
+                        ]
+        , test "should report recursive call in the condition of an if block" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  if fun (x - 1) then
+    1
+  else
+    x
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, the recursive call should not appear inside an if condition."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 6 }, end = { row = 3, column = 9 } }
+                        ]
+        , test "should report recursive call in a negation operation" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  -(fun (x - 1))
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 5 }, end = { row = 3, column = 8 } }
+                        ]
+        , test "should report recursive call in a record access operation" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  (fun (x - 1)).field
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are accessing a field on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 4 }, end = { row = 3, column = 7 } }
+                        ]
+        , test "should report recursive call from inside a tuple" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  ( fun (x - 1), 1 )
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are storing the result of recursive call inside a tuple. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 5 }, end = { row = 3, column = 8 } }
+                        ]
+        , test "should report recursive call from inside a list" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  [ fun (x - 1), 1 ]
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are storing the result of recursive call inside a list. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 5 }, end = { row = 3, column = 8 } }
+                        ]
+        , test "should report recursive call from inside a record expression" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  { result = fun (x - 1) }
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are storing the result of recursive call inside a record. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 14 }, end = { row = 3, column = 17 } }
+                        ]
+        , test "should report recursive call from inside a record update expression" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  { y | result = fun (x - 1) }
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are storing the result of recursive call inside a record. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 18 }, end = { row = 3, column = 21 } }
+                        ]
+        , test "should report recursive call in the case of pattern to evaluate" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  case fun (x - 1) of
+    _ -> 1
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, the recursive call should not appear in the pattern to evaluate for a case expression."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 8 }, end = { row = 3, column = 11 } }
+                        ]
+        , test "should not report an error when a function is properly TCO (case branch)" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  case x of
+    _ -> fun (x - 1)
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error when a function is properly TCO (let body)" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  let
+    y = x - 1
+  in
+  fun y
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should report an error when a function is called recursively from inside one of its let functions" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  let
+    fun2 y =
+      fun x
+  in
+  fun2 x
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, the recursive call should not appear inside a let declaration."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 5, column = 7 }, end = { row = 5, column = 10 } }
+                        ]
+        , test "should not report an error when a function is properly TCO (parentheses)" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  (fun (x - 1))
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should report recursive call in a lambda" <|
+            \() ->
+                """module A exposing (..)
+fun n =
+    \\x -> (fun n x)
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, the recursive call should not appear inside an anonymous function."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 12 }, end = { row = 3, column = 15 } }
+                        ]
+        , test "should report recursive call using |>" <|
+            \() ->
+                """module A exposing (..)
+fun x n =
+    if x <= 0 then
+        n
+
+    else
+        n
+            |> fun (x - 1)
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "Removing the usage of `|>` may fix the issue here."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 8, column = 16 }, end = { row = 8, column = 19 } }
+                        ]
+        , test "should report recursive call using |> (simple reference)" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+    n
+        |> fun
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "Removing the usage of `|>` may fix the issue here."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 4, column = 12 }, end = { row = 4, column = 15 } }
+                        ]
+        , test "should report recursive call using <|" <|
+            \() ->
+                """module A exposing (..)
+fun x n =
+    if x <= 0 then
+        n
+
+    else
+        fun (x - 1) <| n
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "Removing the usage of `<|` may fix the issue here."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 7, column = 9 }, end = { row = 7, column = 12 } }
+                        ]
+        , test "should report an error for non-TCO let functions" <|
+            \() ->
+                """module A exposing (..)
+a n =
+  let
+    fun x =
+      fun x + 1
+  in
+  fun 2
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 5, column = 7 }, end = { row = 5, column = 10 } }
+                        ]
+        , test "should not report an error for TCO let functions" <|
+            \() ->
+                """module A exposing (..)
+a n =
+  let
+    fun x =
+      fun x
+  in
+  fun 2
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error for TCO let functions inside a recursive function" <|
+            \() ->
+                """module A exposing (..)
+fun1 n =
+  let
+    fun2 x =
+      fun2 x
+  in
+  if cond then
+    fun1 n
+  else
+    fun2 2
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error when the function body contains has the exact opt out comment" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  -- OPT OUT
+  fun x + 1
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error when the function body contains the opt out comment" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  -- SOME TAG, OPT OUT, OTHER TAG
+  fun x + 1
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error when the function body contains the opt out comment when it has a signature" <|
+            \() ->
+                """module A exposing (..)
+fun : Int -> Int
+fun x =
+  -- OPT OUT
+  fun x + 1
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should not report an error for let functions when the function body contains the opt out comment" <|
+            \() ->
+                """module A exposing (..)
+a n =
+  let
+    fun x =
+      -- OPT OUT
+      fun x + 1
+  in
+  fun x
+"""
+                    |> Review.Test.run (rule (optOutWithComment "OPT OUT"))
+                    |> Review.Test.expectNoErrors
+        , test "should report an error when the function body contains the opt in comment" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  -- OPT IN
+  fun x + 1
+"""
+                    |> Review.Test.run (rule (optInWithComment "OPT IN"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 4, column = 3 }, end = { row = 4, column = 6 } }
+                        ]
+        , test "should report an error for let functions when the function body contains the opt in comment" <|
+            \() ->
+                """module A exposing (..)
+a n =
+  let
+    fun x =
+      -- OPT IN
+      fun x + 1
+  in
+  fun x
+"""
+                    |> Review.Test.run (rule (optInWithComment "OPT IN"))
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = message
+                            , details =
+                                [ "Among other possible reasons, you are applying operations on the result of recursive call. The recursive call should be the last thing to happen in this branch."
+                                , "You can read more about why over at https://package.elm-lang.org/packages/jfmengels/elm-review-performance/latest/NoUnoptimizedRecursion#fail"
+                                ]
+                            , under = "fun"
+                            }
+                            |> Review.Test.atExactly { start = { row = 6, column = 7 }, end = { row = 6, column = 10 } }
+                        ]
+        , test "should not report an error when the function body does not contain the opt in comment" <|
+            \() ->
+                """module A exposing (..)
+fun x =
+  fun x + 1
+"""
+                    |> Review.Test.run (rule (optInWithComment "OPT IN"))
+                    |> Review.Test.expectNoErrors
+        ]

tests/NoUnused/CustomTypeConstructorArgs.elm

@@ -17,7 +17,7 @@ import Elm.Syntax.ModuleName exposing (ModuleName)
 import Elm.Syntax.Node as Node exposing (Node(..))
 import Elm.Syntax.Pattern as Pattern exposing (Pattern)
 import Elm.Syntax.Range exposing (Range)
-import Elm.Syntax.TypeAnnotation as TypeAnnotation
+import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
 import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
 import Review.Rule as Rule exposing (Error, Rule)
 import Set exposing (Set)
@@ -309,36 +309,35 @@ collectCustomType lookupTable node =
                 customTypeConstructors : List ( String, List Range )
                 customTypeConstructors =
                     List.map
-                        (Node.value
+                        (\(Node _ { name, arguments }) ->
-                            >> (\{ name, arguments } ->
+                            ( Node.value name
-                                    ( Node.value name
+                            , arguments
-                                    , arguments
+                                |> List.filter (isNever lookupTable >> not)
-                                        |> List.filter
+                                |> List.map Node.range
-                                            (\arg ->
+                            )
-                                                case Node.value arg of
-                                                    TypeAnnotation.Typed (Node _ ( _, "Never" )) [] ->
-                                                        case ModuleNameLookupTable.moduleNameFor lookupTable arg of
-                                                            Just [ "Basics" ] ->
-                                                                False
-
-                                                            _ ->
-                                                                True
-
-                                                    _ ->
-                                                        True
-                                            )
-                                        |> List.map Node.range
-                                    )
-                               )
                         )
                         typeDeclaration.constructors
             in
-            Just ( Node.value typeDeclaration.name, Dict.fromList customTypeConstructors )
+            if List.isEmpty customTypeConstructors then
+                Nothing
+
+            else
+                Just ( Node.value typeDeclaration.name, Dict.fromList customTypeConstructors )
 
         _ ->
             Nothing
 
 
+isNever : ModuleNameLookupTable -> Node TypeAnnotation -> Bool
+isNever lookupTable node =
+    case Node.value node of
+        TypeAnnotation.Typed (Node neverRange ( _, "Never" )) [] ->
+            ModuleNameLookupTable.moduleNameAt lookupTable neverRange == Just [ "Basics" ]
+
+        _ ->
+            False
+
+
 
 -- DECLARATION VISITOR
 

tests/NoUnused/CustomTypeConstructors.elm

@@ -31,6 +31,8 @@ import Set exposing (Set)
 
 {-| Forbid having unused custom type constructors.
 
+🔧 Running with `--fix` will automatically remove most of the reported errors.
+
     config =
         [ NoUnused.CustomTypeConstructors.rule []
         ]
@@ -41,11 +43,14 @@ anywhere _in the project_.
 If the project is a package and the module that declared the type is exposed and
 the type's constructors are exposed, then the constructors will not be reported.
 
-This does not prevent you from using phantom types: A constructor won't be reported if
+This does not prevent you from using phantom types.
+I highly suggest chaning your phantom types to the following shape: `type TypeName = ConstructorName Never`.
+This shape makes it obvious to tooling and readers that the type can't be created, so if it is used, it must be as a phantom type.
 
-  - It is the only constructor of a type that has no type variable
+**Deprecated configuration for phantom types**
-  - It has no parameters
+
-  - It is used as an argument of a custom type, in the stead of a type variable that is not used in the definition in any of the type's constructors
+_I recommend changing your types like mentioned right above, and to configure the rule like `NoUnused.CustomTypeConstructors.rule []`.
+I'll keep this section and configuration option around until the next major version comes out._
 
 **Note**: At the time of writing, there may be cases where phantom types are not well detected.
 When an opaque type is defined in a dependency, we don't know whether a type variable should be considered as a phantom type.
@@ -70,7 +75,7 @@ by following the definitions of custom types and type aliases, until it finds ou
 variable is not used, or that it hits the limit related to dependencies described above.
 In the meantime, you can configure the rule with all the phantom type exceptions.
 
-I would love help with improving this :)
+**End of deprecated section**
 
 
 ## Fail
@@ -495,54 +500,16 @@ declarationVisitor : Node Declaration -> ModuleContext -> ( List nothing, Module
 declarationVisitor node context =
     case Node.value node of
         Declaration.CustomTypeDeclaration { name, constructors } ->
-            if isPhantomCustomType name constructors then
+            if isPhantomCustomType context.lookupTable (Node.value name) constructors then
                 ( [], context )
 
             else
-                let
-                    constructorsAndNext : List ( Maybe (Node Type.ValueConstructor), Node Type.ValueConstructor )
-                    constructorsAndNext =
-                        List.map2 Tuple.pair
-                            (List.map Just (List.drop 1 constructors) ++ [ Nothing ])
-                            constructors
-
-                    constructorsForCustomType : Dict String ConstructorInformation
-                    constructorsForCustomType =
-                        List.foldl
-                            (\( next, constructor ) ( prev, dict ) ->
-                                let
-                                    nameNode : Node String
-                                    nameNode =
-                                        (Node.value constructor).name
-
-                                    constructorName : String
-                                    constructorName =
-                                        Node.value nameNode
-
-                                    constructorInformation : ConstructorInformation
-                                    constructorInformation =
-                                        { name = constructorName
-                                        , rangeToReport = Node.range nameNode
-                                        , rangeToRemove = findRangeToRemove prev constructor next
-                                        }
-                                in
-                                ( Just constructor
-                                , Dict.insert
-                                    constructorName
-                                    constructorInformation
-                                    dict
-                                )
-                            )
-                            ( Nothing, Dict.empty )
-                            constructorsAndNext
-                            |> Tuple.second
-                in
                 ( []
                 , { context
                     | declaredTypesWithConstructors =
                         Dict.insert
                             (Node.value name)
-                            constructorsForCustomType
+                            (constructorsForCustomType constructors)
                             context.declaredTypesWithConstructors
                   }
                 )
@@ -561,6 +528,45 @@ declarationVisitor node context =
             ( [], context )
 
 
+constructorsForCustomType : List (Node Type.ValueConstructor) -> Dict String ConstructorInformation
+constructorsForCustomType constructors =
+    let
+        constructorsAndNext : List ( Maybe (Node Type.ValueConstructor), Node Type.ValueConstructor )
+        constructorsAndNext =
+            List.map2 Tuple.pair
+                (List.map Just (List.drop 1 constructors) ++ [ Nothing ])
+                constructors
+    in
+    List.foldl
+        (\( next, constructor ) ( prev, dict ) ->
+            let
+                nameNode : Node String
+                nameNode =
+                    (Node.value constructor).name
+
+                constructorName : String
+                constructorName =
+                    Node.value nameNode
+
+                constructorInformation : ConstructorInformation
+                constructorInformation =
+                    { name = constructorName
+                    , rangeToReport = Node.range nameNode
+                    , rangeToRemove = findRangeToRemove prev constructor next
+                    }
+            in
+            ( Just constructor
+            , Dict.insert
+                constructorName
+                constructorInformation
+                dict
+            )
+        )
+        ( Nothing, Dict.empty )
+        constructorsAndNext
+        |> Tuple.second
+
+
 findRangeToRemove : Maybe (Node a) -> Node Type.ValueConstructor -> Maybe (Node c) -> Maybe { start : Elm.Syntax.Range.Location, end : Elm.Syntax.Range.Location }
 findRangeToRemove previousConstructor constructor nextConstructor =
     case previousConstructor of
@@ -582,23 +588,29 @@ findRangeToRemove previousConstructor constructor nextConstructor =
                     Nothing
 
 
-isPhantomCustomType : Node String -> List (Node Type.ValueConstructor) -> Bool
+isPhantomCustomType : ModuleNameLookupTable -> String -> List (Node Type.ValueConstructor) -> Bool
-isPhantomCustomType name constructors =
+isPhantomCustomType lookupTable typeName constructors =
     case constructors of
-        (Node _ constructor) :: [] ->
+        [ Node _ constructor ] ->
-            if Node.value name == Node.value constructor.name then
+            case constructor.arguments of
-                case constructor.arguments of
+                [ arg ] ->
-                    (Node _ (TypeAnnotation.Typed (Node _ ( [], "Never" )) [])) :: [] ->
+                    isNeverOrItself lookupTable typeName arg
-                        True
 
-                    (Node _ (TypeAnnotation.Typed (Node _ ( [ "Basics" ], "Never" )) [])) :: [] ->
+                _ ->
-                        True
+                    False
 
-                    _ ->
+        _ ->
-                        False
+            False
 
-            else
+
-                False
+isNeverOrItself : ModuleNameLookupTable -> String -> Node TypeAnnotation -> Bool
+isNeverOrItself lookupTable typeName node =
+    case Node.value node of
+        TypeAnnotation.Typed (Node neverRange ( _, "Never" )) [] ->
+            ModuleNameLookupTable.moduleNameAt lookupTable neverRange == Just [ "Basics" ]
+
+        TypeAnnotation.Typed (Node _ ( [], argName )) [] ->
+            typeName == argName
 
         _ ->
             False

tests/NoUnused/CustomTypeConstructorsTest.elm

@@ -627,13 +627,37 @@ id = Id
                             }
                             |> Review.Test.atExactly { start = { row = 3, column = 13 }, end = { row = 3, column = 17 } }
                         ]
-        , test "should not report a custom type with one constructor that takes Never" <|
+        , test "should not report a custom type with one constructor that takes Never (constructor named like the type)" <|
             \() ->
                 """
 module MyModule exposing (id)
 type User = User Never
 type Id a = Id a
 
+id : Id User
+id = Id
+"""
+                    |> Review.Test.runWithProjectData project (rule [])
+                    |> Review.Test.expectNoErrors
+        , test "should not report a custom type with one constructor that takes Never (constructor named differently from the type)" <|
+            \() ->
+                """
+module MyModule exposing (id)
+type User = SomeConstructor Never
+type Id a = Id a
+
+id : Id User
+id = Id
+"""
+                    |> Review.Test.runWithProjectData project (rule [])
+                    |> Review.Test.expectNoErrors
+        , test "should not report a custom type with one constructor that takes itself" <|
+            \() ->
+                """
+module MyModule exposing (id)
+type User = SomeConstructor User
+type Id a = Id a
+
 id : Id User
 id = Id
 """

tests/NoUnused/Dependencies.elm

@@ -17,6 +17,7 @@ import Elm.Syntax.Import exposing (Import)
 import Elm.Syntax.Node as Node exposing (Node)
 import Elm.Syntax.Range exposing (Range)
 import Elm.Version
+import List.Extra
 import Review.Project.Dependency as Dependency exposing (Dependency)
 import Review.Rule as Rule exposing (Error, Rule)
 import Set exposing (Set)
@@ -24,6 +25,8 @@ import Set exposing (Set)
 
 {-| Forbid the use of dependencies that are never used in your project.
 
+🔧 Running with `--fix` will automatically remove all the reported errors.
+
 A dependency is considered unused if none of its modules are imported in the project.
 
     config =
@@ -36,7 +39,7 @@ A dependency is considered unused if none of its modules are imported in the pro
 You can try this rule out by running the following command:
 
 ```bash
-elm-review --template jfmengels/elm-elm-review-unused/example --rules NoUnused.Dependencies
+elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.Dependencies
 ```
 
 -}
@@ -250,28 +253,34 @@ finalEvaluationForProject projectContext =
                 depsNotUsedInSrcButUsedInTests : Set String
                 depsNotUsedInSrcButUsedInTests =
                     Set.intersect depsNotUsedInSrc projectContext.usedDependenciesFromTest
-                        |> Set.remove "elm/core"
 
                 depsNotUsedInSrcErrors : List String
                 depsNotUsedInSrcErrors =
-                    Set.diff depsNotUsedInSrc depsNotUsedInSrcButUsedInTests
+                    Set.diff
-                        |> Set.remove "elm/core"
+                        depsNotUsedInSrc
+                        (Set.union packagesNotToReport depsNotUsedInSrcButUsedInTests)
                         |> Set.toList
 
-                testDepsNotUsedInTests : List String
+                testDepsNotUsed : List String
-                testDepsNotUsedInTests =
+                testDepsNotUsed =
-                    Set.diff projectContext.directTestDependencies projectContext.usedDependenciesFromTest
+                    Set.diff
-                        |> Set.remove "elm/core"
+                        projectContext.directTestDependencies
+                        (Set.union projectContext.usedDependenciesFromTest projectContext.usedDependencies)
                         |> Set.toList
             in
             List.map (unusedProjectDependencyError elmJsonKey projectContext.dependencies) depsNotUsedInSrcErrors
-                ++ List.map (unusedTestDependencyError elmJsonKey projectContext.dependencies) testDepsNotUsedInTests
+                ++ List.map (unusedTestDependencyError elmJsonKey projectContext.dependencies) testDepsNotUsed
                 ++ List.map (moveDependencyToTestError elmJsonKey projectContext.dependencies) (Set.toList depsNotUsedInSrcButUsedInTests)
 
         Nothing ->
             []
 
 
+packagesNotToReport : Set String
+packagesNotToReport =
+    Set.fromList [ "elm/core", "lamdera/core", "lamdera/codecs" ]
+
+
 
 -- ERROR FUNCTIONS
 
@@ -412,7 +421,7 @@ fromProject dependenciesDict dependencyLocation packageNameStr project =
                         Nothing ->
                             []
             in
-            case find (isPackageWithName packageNameStr) dependencies of
+            case List.Extra.find (isPackageWithName packageNameStr) dependencies of
                 Just ( packageName, version ) ->
                     Just
                         (ApplicationProject
@@ -437,7 +446,7 @@ fromProject dependenciesDict dependencyLocation packageNameStr project =
                         InTestDeps ->
                             packageInfo.testDeps
             in
-            case find (isPackageWithName packageNameStr) dependencies of
+            case List.Extra.find (isPackageWithName packageNameStr) dependencies of
                 Just ( packageName, constraint ) ->
                     Just (PackageProject { package = packageInfo, name = packageName, constraint = constraint })
 
@@ -592,23 +601,3 @@ removeTestDependency projectAndDependencyIdentifier =
 isPackageWithName : String -> ( Elm.Package.Name, a ) -> Bool
 isPackageWithName packageName ( packageName_, _ ) =
     packageName == Elm.Package.toString packageName_
-
-
-{-| Find the first element that satisfies a predicate and return
-Just that element. If none match, return Nothing.
-
-    find (\num -> num > 5) [ 2, 4, 6, 8 ] == Just 6
-
--}
-find : (a -> Bool) -> List a -> Maybe a
-find predicate list =
-    case list of
-        [] ->
-            Nothing
-
-        first :: rest ->
-            if predicate first then
-                Just first
-
-            else
-                find predicate rest

tests/NoUnused/DependenciesTest.elm

@@ -141,6 +141,33 @@ packageElmJson =
 }"""
 
 
+packageElmJsonWithLamdera : String
+packageElmJsonWithLamdera =
+    """
+{
+    "type": "package",
+    "name": "author/package",
+    "summary": "Summary",
+    "license": "BSD-3-Clause",
+    "version": "1.0.0",
+    "exposed-modules": [
+        "Exposed"
+    ],
+    "elm-version": "0.19.0 <= v < 0.20.0",
+    "dependencies": {
+        "elm/core": "1.0.0 <= v < 2.0.0",
+        "lamdera/core": "1.0.0 <= v < 2.0.0",
+        "lamdera/codecs": "1.0.0 <= v < 2.0.0",
+        "author/package-with-foo": "1.0.0 <= v < 2.0.0",
+        "author/package-with-bar": "1.0.0 <= v < 2.0.0"
+    },
+    "test-dependencies": {
+        "author/package-with-test-foo": "1.0.0 <= v < 2.0.0",
+        "author/package-with-test-bar": "1.0.0 <= v < 2.0.0"
+    }
+}"""
+
+
 packageWithFoo : Dependency
 packageWithFoo =
     let
@@ -754,6 +781,19 @@ a = 1
 }
 """
                         ]
+        , test "should not report test-dependencies used in source-directories" <|
+            \() ->
+                """
+module A exposing (a)
+import Foo
+import Bar
+import TestFoo
+import TestBar
+a = 1
+"""
+                    |> String.replace "\u{000D}" ""
+                    |> Review.Test.runWithProjectData (createProject Nothing packageElmJson) rule
+                    |> Review.Test.expectNoErrors
         , test "should re-organize the indirect dependencies when a dependency gets removed" <|
             \() ->
                 let
@@ -869,6 +909,19 @@ a = 1
                             }
                             |> Review.Test.whenFixed expected
                         ]
+        , test "should not report lamdera/core or lamdera/codecs" <|
+            \() ->
+                """
+module A exposing (a)
+import Foo
+import Bar
+import TestFoo
+import TestBar
+a = 1
+"""
+                    |> String.replace "\u{000D}" ""
+                    |> Review.Test.runWithProjectData (createProject Nothing packageElmJsonWithLamdera) rule
+                    |> Review.Test.expectNoErrors
         ]
 
 

tests/NoUnused/Exports.elm

@@ -16,7 +16,7 @@ import Dict exposing (Dict)
 import Elm.Module
 import Elm.Project
 import Elm.Syntax.Declaration as Declaration exposing (Declaration)
-import Elm.Syntax.Exposing as Exposing
+import Elm.Syntax.Exposing as Exposing exposing (TopLevelExpose)
 import Elm.Syntax.Expression as Expression exposing (Expression)
 import Elm.Syntax.Import exposing (Import)
 import Elm.Syntax.Module as Module exposing (Module)
@@ -24,7 +24,9 @@ import Elm.Syntax.ModuleName exposing (ModuleName)
 import Elm.Syntax.Node as Node exposing (Node(..))
 import Elm.Syntax.Range exposing (Range)
 import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
-import Review.Fix as Fix exposing (Fix)
+import List.Extra
+import NoUnused.LamderaSupport as LamderaSupport
+import Review.Fix as Fix
 import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
 import Review.Rule as Rule exposing (Error, Rule)
 import Set exposing (Set)
@@ -33,6 +35,8 @@ import Set exposing (Set)
 {-| Report functions and types that are exposed from a module but that are never
 used in other modules.
 
+🔧 Running with `--fix` will automatically remove all the reported errors.
+
 If the project is a package and the module that declared the element is exposed,
 then nothing will be reported.
 
@@ -69,6 +73,7 @@ moduleVisitor : Rule.ModuleRuleSchema {} ModuleContext -> Rule.ModuleRuleSchema
 moduleVisitor schema =
     schema
         |> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
+        |> Rule.withCommentsVisitor commentsVisitor
         |> Rule.withImportVisitor importVisitor
         |> Rule.withDeclarationListVisitor declarationListVisitor
         |> Rule.withExpressionEnterVisitor expressionVisitor
@@ -92,16 +97,21 @@ type alias ProjectContext =
 
 type alias ExposedElement =
     { range : Range
-    , rangeToRemove : Maybe Range
+    , rangesToRemove : List Range
     , elementType : ExposedElementType
     }
 
 
 type ProjectType
-    = IsApplication
+    = IsApplication ElmApplicationType
     | IsPackage (Set (List String))
 
 
+type ElmApplicationType
+    = ElmApplication
+    | LamderaApplication
+
+
 type ExposedElementType
     = Function
     | TypeOrTypeAlias
@@ -111,6 +121,7 @@ type ExposedElementType
 type alias ModuleContext =
     { lookupTable : ModuleNameLookupTable
     , exposesEverything : Bool
+    , rawExposed : List (Node TopLevelExpose)
     , exposed : Dict String ExposedElement
     , used : Set ( ModuleName, String )
     , elementsNotToReport : Set String
@@ -119,7 +130,7 @@ type alias ModuleContext =
 
 initialProjectContext : ProjectContext
 initialProjectContext =
-    { projectType = IsApplication
+    { projectType = IsApplication ElmApplication
     , modules = Dict.empty
     , used = Set.empty
     }
@@ -130,6 +141,7 @@ fromProjectToModule lookupTable _ =
     { lookupTable = lookupTable
     , exposesEverything = False
     , exposed = Dict.empty
+    , rawExposed = []
     , used = Set.empty
     , elementsNotToReport = Set.empty
     }
@@ -137,7 +149,7 @@ fromProjectToModule lookupTable _ =
 
 fromModuleToProject : Rule.ModuleKey -> Rule.Metadata -> ModuleContext -> ProjectContext
 fromModuleToProject moduleKey metadata moduleContext =
-    { projectType = IsApplication
+    { projectType = IsApplication ElmApplication
     , modules =
         Dict.singleton
             (Rule.moduleNameFromMetadata metadata)
@@ -197,8 +209,20 @@ elmJsonVisitor maybeProject projectContext =
               }
             )
 
-        _ ->
+        Just (Elm.Project.Application { depsDirect }) ->
-            ( [], { projectContext | projectType = IsApplication } )
+            let
+                elmApplicationType : ElmApplicationType
+                elmApplicationType =
+                    if LamderaSupport.isLamderaApplication depsDirect then
+                        LamderaApplication
+
+                    else
+                        ElmApplication
+            in
+            ( [], { projectContext | projectType = IsApplication elmApplicationType } )
+
+        Nothing ->
+            ( [], { projectContext | projectType = IsApplication ElmApplication } )
 
 
 
@@ -210,52 +234,45 @@ finalEvaluationForProject projectContext =
     projectContext.modules
         |> removeExposedPackages projectContext
         |> Dict.toList
+        |> List.concatMap (errorsForModule projectContext)
+
+
+errorsForModule : ProjectContext -> ( ModuleName, { moduleKey : Rule.ModuleKey, exposed : Dict String ExposedElement } ) -> List (Error scope)
+errorsForModule projectContext ( moduleName, { moduleKey, exposed } ) =
+    exposed
+        |> removeApplicationExceptions projectContext
+        |> removeReviewConfig moduleName
+        |> Dict.filter (\name _ -> not <| Set.member ( moduleName, name ) projectContext.used)
+        |> Dict.toList
         |> List.concatMap
-            (\( moduleName, { moduleKey, exposed } ) ->
+            (\( name, element ) ->
-                exposed
+                let
-                    |> removeApplicationExceptions projectContext
+                    what : String
-                    |> removeReviewConfig moduleName
+                    what =
-                    |> Dict.filter (\name _ -> not <| Set.member ( moduleName, name ) projectContext.used)
+                        case element.elementType of
-                    |> Dict.toList
+                            Function ->
-                    |> List.concatMap
+                                "Exposed function or value"
-                        (\( name, element ) ->
-                            let
-                                what : String
-                                what =
-                                    case element.elementType of
-                                        Function ->
-                                            "Exposed function or value"
 
-                                        TypeOrTypeAlias ->
+                            TypeOrTypeAlias ->
-                                            "Exposed type or type alias"
+                                "Exposed type or type alias"
 
-                                        ExposedType ->
+                            ExposedType ->
-                                            "Exposed type"
+                                "Exposed type"
-
+                in
-                                fixes : List Fix
+                [ Rule.errorForModuleWithFix moduleKey
-                                fixes =
+                    { message = what ++ " `" ++ name ++ "` is never used outside this module."
-                                    case element.rangeToRemove of
+                    , details = [ "This exposed element is never used. You may want to remove it to keep your project clean, and maybe detect some unused code in your project." ]
-                                        Just rangeToRemove ->
+                    }
-                                            [ Fix.removeRange rangeToRemove ]
+                    element.range
-
+                    (List.map Fix.removeRange element.rangesToRemove)
-                                        Nothing ->
+                ]
-                                            []
-                            in
-                            [ Rule.errorForModuleWithFix moduleKey
-                                { message = what ++ " `" ++ name ++ "` is never used outside this module."
-                                , details = [ "This exposed element is never used. You may want to remove it to keep your project clean, and maybe detect some unused code in your project." ]
-                                }
-                                element.range
-                                fixes
-                            ]
-                        )
             )
 
 
 removeExposedPackages : ProjectContext -> Dict ModuleName a -> Dict ModuleName a
 removeExposedPackages projectContext dict =
     case projectContext.projectType of
-        IsApplication ->
+        IsApplication _ ->
             dict
 
         IsPackage exposedModuleNames ->
@@ -265,12 +282,17 @@ removeExposedPackages projectContext dict =
 removeApplicationExceptions : ProjectContext -> Dict String a -> Dict String a
 removeApplicationExceptions projectContext dict =
     case projectContext.projectType of
-        IsApplication ->
-            Dict.remove "main" dict
-
         IsPackage _ ->
             dict
 
+        IsApplication ElmApplication ->
+            Dict.remove "main" dict
+
+        IsApplication LamderaApplication ->
+            dict
+                |> Dict.remove "main"
+                |> Dict.remove "app"
+
 
 removeReviewConfig : ModuleName -> Dict String a -> Dict String a
 removeReviewConfig moduleName dict =
@@ -292,11 +314,39 @@ moduleDefinitionVisitor moduleNode moduleContext =
             ( [], { moduleContext | exposesEverything = True } )
 
         Exposing.Explicit list ->
-            ( [], { moduleContext | exposed = collectExposedElements list } )
+            ( [], { moduleContext | rawExposed = list } )
 
 
-collectExposedElements : List (Node Exposing.TopLevelExpose) -> Dict String ExposedElement
+commentsVisitor : List (Node String) -> ModuleContext -> ( List nothing, ModuleContext )
-collectExposedElements nodes =
+commentsVisitor nodes moduleContext =
+    if List.isEmpty moduleContext.rawExposed then
+        ( [], moduleContext )
+
+    else
+        let
+            comments : List ( Int, String )
+            comments =
+                nodes
+                    |> List.Extra.find (Node.value >> String.startsWith "{-|")
+                    |> Maybe.map
+                        (\(Node range comment) ->
+                            comment
+                                |> String.lines
+                                |> List.drop 1
+                                |> List.indexedMap (\i line -> ( i + range.start.row + 1, line ))
+                                |> List.filter (Tuple.second >> String.startsWith "@docs ")
+                        )
+                    |> Maybe.withDefault []
+        in
+        ( []
+        , { moduleContext
+            | exposed = collectExposedElements comments moduleContext.rawExposed
+          }
+        )
+
+
+collectExposedElements : List ( Int, String ) -> List (Node Exposing.TopLevelExpose) -> Dict String ExposedElement
+collectExposedElements comments nodes =
     let
         listWithPreviousRange : List (Maybe Range)
         listWithPreviousRange =
@@ -318,29 +368,12 @@ collectExposedElements nodes =
         |> List.map3 (\prev next current -> ( prev, current, next )) listWithPreviousRange listWithNextRange
         |> List.indexedMap
             (\index ( maybePreviousRange, Node range value, nextRange ) ->
-                let
-                    rangeToRemove : Maybe Range
-                    rangeToRemove =
-                        if List.length nodes == 1 then
-                            Nothing
-
-                        else if index == 0 then
-                            Just { range | end = nextRange.start }
-
-                        else
-                            case maybePreviousRange of
-                                Nothing ->
-                                    Just range
-
-                                Just previousRange ->
-                                    Just { range | start = previousRange.end }
-                in
                 case value of
                     Exposing.FunctionExpose name ->
                         Just
                             ( name
                             , { range = untilEndOfVariable name range
-                              , rangeToRemove = rangeToRemove
+                              , rangesToRemove = getRangesToRemove comments nodes name index maybePreviousRange range nextRange
                               , elementType = Function
                               }
                             )
@@ -349,7 +382,7 @@ collectExposedElements nodes =
                         Just
                             ( name
                             , { range = untilEndOfVariable name range
-                              , rangeToRemove = rangeToRemove
+                              , rangesToRemove = getRangesToRemove comments nodes name index maybePreviousRange range nextRange
                               , elementType = TypeOrTypeAlias
                               }
                             )
@@ -358,7 +391,7 @@ collectExposedElements nodes =
                         Just
                             ( name
                             , { range = untilEndOfVariable name range
-                              , rangeToRemove = Nothing
+                              , rangesToRemove = []
                               , elementType = ExposedType
                               }
                             )
@@ -370,6 +403,88 @@ collectExposedElements nodes =
         |> Dict.fromList
 
 
+getRangesToRemove : List ( Int, String ) -> List a -> String -> Int -> Maybe Range -> Range -> Range -> List Range
+getRangesToRemove comments nodes name index maybePreviousRange range nextRange =
+    if List.length nodes == 1 then
+        []
+
+    else
+        let
+            exposeRemoval : Range
+            exposeRemoval =
+                if index == 0 then
+                    { range | end = nextRange.start }
+
+                else
+                    case maybePreviousRange of
+                        Nothing ->
+                            range
+
+                        Just previousRange ->
+                            { range | start = previousRange.end }
+        in
+        List.filterMap identity
+            [ Just exposeRemoval
+            , findMap (findDocsRangeToRemove name) comments
+            ]
+
+
+findDocsRangeToRemove : String -> ( Int, String ) -> Maybe Range
+findDocsRangeToRemove name fullComment =
+    case findcommentInMiddle name fullComment of
+        Just range ->
+            Just range
+
+        Nothing ->
+            findCommentAtEnd name fullComment
+
+
+findcommentInMiddle : String -> ( Int, String ) -> Maybe Range
+findcommentInMiddle name ( row, comment ) =
+    String.indexes (" " ++ name ++ ", ") comment
+        |> List.head
+        |> Maybe.map
+            (\index ->
+                { start = { row = row, column = index + 2 }
+                , end = { row = row, column = index + String.length name + 4 }
+                }
+            )
+
+
+findCommentAtEnd : String -> ( Int, String ) -> Maybe Range
+findCommentAtEnd name ( row, comment ) =
+    if comment == "@docs " ++ name then
+        Just
+            { start = { row = row, column = 1 }
+            , end = { row = row + 1, column = 1 }
+            }
+
+    else
+        String.indexes (", " ++ name) comment
+            |> List.head
+            |> Maybe.map
+                (\index ->
+                    { start = { row = row, column = index + 1 }
+                    , end = { row = row, column = index + String.length name + 3 }
+                    }
+                )
+
+
+findMap : (a -> Maybe b) -> List a -> Maybe b
+findMap mapper list =
+    case list of
+        [] ->
+            Nothing
+
+        first :: rest ->
+            case mapper first of
+                Just value ->
+                    Just value
+
+                Nothing ->
+                    findMap mapper rest
+
+
 untilEndOfVariable : String -> Range -> Range
 untilEndOfVariable name range =
     if range.start.row == range.end.row then
@@ -572,8 +687,8 @@ typesUsedInDeclaration moduleContext declaration =
         Declaration.AliasDeclaration alias_ ->
             ( collectTypesFromTypeAnnotation moduleContext alias_.typeAnnotation, False )
 
-        Declaration.PortDeclaration _ ->
+        Declaration.PortDeclaration signature ->
-            ( [], False )
+            ( collectTypesFromTypeAnnotation moduleContext signature.typeAnnotation, False )
 
         Declaration.InfixDeclaration _ ->
             ( [], False )

tests/NoUnused/ExportsTest.elm

@@ -1,86 +1,9 @@
 module NoUnused.ExportsTest exposing (all)
 
-import Elm.Project
-import Elm.Version
-import Json.Decode as Decode
 import NoUnused.Exports exposing (rule)
-import Review.Project as Project exposing (Project)
 import Review.Test
 import Test exposing (Test, describe, test)
-
+import TestProject exposing (application, lamderaApplication, package)
-
-application : Project
-application =
-    Project.new
-        |> Project.addElmJson applicationElmJson
-
-
-applicationElmJson : { path : String, raw : String, project : Elm.Project.Project }
-applicationElmJson =
-    { path = "elm.json"
-    , raw = """{
-    "type": "application",
-    "source-directories": [
-        "src"
-    ],
-    "elm-version": "0.19.1",
-    "dependencies": {
-        "direct": {
-            "elm/core": "1.0.2"
-        },
-        "indirect": {}
-    },
-    "test-dependencies": {
-        "direct": {},
-        "indirect": {}
-    }
-}"""
-    , project =
-        Elm.Project.Application
-            { elm = Elm.Version.one
-            , dirs = []
-            , depsDirect = []
-            , depsIndirect = []
-            , testDepsDirect = []
-            , testDepsIndirect = []
-            }
-    }
-
-
-package : Project
-package =
-    Project.new
-        |> Project.addElmJson (createPackageElmJson ())
-
-
-createPackageElmJson : () -> { path : String, raw : String, project : Elm.Project.Project }
-createPackageElmJson _ =
-    case Decode.decodeString Elm.Project.decoder rawPackageElmJson of
-        Ok elmJson ->
-            { path = "elm.json"
-            , raw = rawPackageElmJson
-            , project = elmJson
-            }
-
-        Err err ->
-            Debug.todo ("Invalid elm.json supplied to test: " ++ Debug.toString err)
-
-
-rawPackageElmJson : String
-rawPackageElmJson =
-    """{
-    "type": "package",
-    "name": "author/package",
-    "summary": "Summary",
-    "license": "BSD-3-Clause",
-    "version": "1.0.0",
-    "exposed-modules": [
-        "Exposed"
-    ],
-    "elm-version": "0.19.0 <= v < 0.20.0",
-    "dependencies": {},
-    "test-dependencies": {}
-}"""
 
 
 details : List String
@@ -97,6 +20,7 @@ all =
         , typeAliasesTests
         , duplicateModuleNameTests
         , importsTests
+        , lamderaTests
 
         -- TODO Add tests that report exposing the type's variants if they are never used.
         ]
@@ -242,6 +166,72 @@ exposed2 = 2
 module A exposing (exposed1)
 exposed1 = 1
 exposed2 = 2
+"""
+                        ]
+        , test "should propose to remove the @docs entry in the module's documentation along with the removed export" <|
+            \() ->
+                """module A exposing (exposed1, exposed2, exposed3)
+{-|
+
+@docs exposed1, exposed2
+@docs exposed3
+-}
+
+exposed1 = 1
+exposed2 = 2
+exposed3 = 3
+"""
+                    |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "Exposed function or value `exposed1` is never used outside this module."
+                            , details = details
+                            , under = "exposed1"
+                            }
+                            |> Review.Test.atExactly { start = { row = 1, column = 20 }, end = { row = 1, column = 28 } }
+                            |> Review.Test.whenFixed """module A exposing (exposed2, exposed3)
+{-|
+
+@docs exposed2
+@docs exposed3
+-}
+
+exposed1 = 1
+exposed2 = 2
+exposed3 = 3
+"""
+                        , Review.Test.error
+                            { message = "Exposed function or value `exposed2` is never used outside this module."
+                            , details = details
+                            , under = "exposed2"
+                            }
+                            |> Review.Test.atExactly { start = { row = 1, column = 30 }, end = { row = 1, column = 38 } }
+                            |> Review.Test.whenFixed """module A exposing (exposed1, exposed3)
+{-|
+
+@docs exposed1
+@docs exposed3
+-}
+
+exposed1 = 1
+exposed2 = 2
+exposed3 = 3
+"""
+                        , Review.Test.error
+                            { message = "Exposed function or value `exposed3` is never used outside this module."
+                            , details = details
+                            , under = "exposed3"
+                            }
+                            |> Review.Test.atExactly { start = { row = 1, column = 40 }, end = { row = 1, column = 48 } }
+                            |> Review.Test.whenFixed """module A exposing (exposed1, exposed2)
+{-|
+
+@docs exposed1, exposed2
+-}
+
+exposed1 = 1
+exposed2 = 2
+exposed3 = 3
 """
                         ]
         , test "should not report anything for modules that expose everything`" <|
@@ -514,6 +504,28 @@ main =
   type1
 """, """module B exposing (Type1)
 type Type1 = Type1
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData application rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an exposed type if it is used in a port (input)" <|
+            \() ->
+                [ """module Main exposing (main)
+import B
+main = somePort
+port somePort : (B.Type1 -> msg) -> Sub msg
+""", """module B exposing (Type1)
+type alias Type1 = { user : String }
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData application rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an exposed type if it is used in a port (output)" <|
+            \() ->
+                [ """module Main exposing (main)
+import B
+main = somePort
+port somePort : B.Type1 -> Cmd msg
+""", """module B exposing (Type1)
+type alias Type1 = { user : String }
 """ ]
                     |> Review.Test.runOnModulesWithProjectData application rule
                     |> Review.Test.expectNoErrors
@@ -806,3 +818,44 @@ b = 2
                     |> Review.Test.runOnModulesWithProjectData application rule
                     |> Review.Test.expectNoErrors
         ]
+
+
+lamderaTests : Test
+lamderaTests =
+    describe "Lamdera support"
+        [ test "should report an exposed `app` function in packages" <|
+            \() ->
+                """module Main exposing (app)
+app = foo
+"""
+                    |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "Exposed function or value `app` is never used outside this module."
+                            , details = details
+                            , under = "app"
+                            }
+                            |> Review.Test.atExactly { start = { row = 1, column = 23 }, end = { row = 1, column = 26 } }
+                        ]
+        , test "should report an exposed `app` function in applications" <|
+            \() ->
+                """module Main exposing (app)
+app = foo
+"""
+                    |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "Exposed function or value `app` is never used outside this module."
+                            , details = details
+                            , under = "app"
+                            }
+                            |> Review.Test.atExactly { start = { row = 1, column = 23 }, end = { row = 1, column = 26 } }
+                        ]
+        , test "should not report an exposed `app` function in Lamdera applications" <|
+            \() ->
+                """module Main exposing (app)
+app = foo
+"""
+                    |> Review.Test.runWithProjectData lamderaApplication rule
+                    |> Review.Test.expectNoErrors
+        ]

tests/NoUnused/LamderaSupport.elm

@@ -0,0 +1,8 @@
+module NoUnused.LamderaSupport exposing (isLamderaApplication)
+
+import Elm.Package
+
+
+isLamderaApplication : List ( Elm.Package.Name, b ) -> Bool
+isLamderaApplication depsDirect =
+    List.any (\( name, _ ) -> Elm.Package.toString name == "lamdera/core") depsDirect

tests/NoUnused/Modules.elm

@@ -17,6 +17,7 @@ import Elm.Syntax.Import exposing (Import)
 import Elm.Syntax.ModuleName exposing (ModuleName)
 import Elm.Syntax.Node as Node exposing (Node)
 import Elm.Syntax.Range exposing (Range)
+import NoUnused.LamderaSupport as LamderaSupport
 import Review.Rule as Rule exposing (Error, Rule)
 import Set exposing (Set)
 
@@ -80,22 +81,32 @@ type alias ProjectContext =
             , moduleNameLocation : Range
             }
     , usedModules : Set ModuleName
-    , isPackage : Bool
+    , projectType : ProjectType
     }
 
 
 type alias ModuleContext =
     { importedModules : Set ModuleName
     , containsMainFunction : Bool
-    , isPackage : Bool
+    , projectType : ProjectType
     }
 
 
+type ProjectType
+    = Package
+    | Application ElmApplicationType
+
+
+type ElmApplicationType
+    = ElmApplication
+    | LamderaApplication
+
+
 initialProjectContext : ProjectContext
 initialProjectContext =
     { modules = Dict.empty
     , usedModules = Set.singleton [ "ReviewConfig" ]
-    , isPackage = False
+    , projectType = Application ElmApplication
     }
 
 
@@ -103,7 +114,7 @@ fromProjectToModule : Rule.ModuleKey -> Node ModuleName -> ProjectContext -> Mod
 fromProjectToModule _ _ projectContext =
     { importedModules = Set.empty
     , containsMainFunction = False
-    , isPackage = projectContext.isPackage
+    , projectType = projectContext.projectType
     }
 
 
@@ -119,7 +130,7 @@ fromModuleToProject moduleKey moduleName moduleContext =
 
         else
             moduleContext.importedModules
-    , isPackage = moduleContext.isPackage
+    , projectType = moduleContext.projectType
     }
 
 
@@ -127,7 +138,7 @@ foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext
 foldProjectContexts newContext previousContext =
     { modules = Dict.union previousContext.modules newContext.modules
     , usedModules = Set.union previousContext.usedModules newContext.usedModules
-    , isPackage = previousContext.isPackage
+    , projectType = previousContext.projectType
     }
 
 
@@ -138,18 +149,30 @@ foldProjectContexts newContext previousContext =
 elmJsonVisitor : Maybe { a | project : Project } -> ProjectContext -> ( List nothing, ProjectContext )
 elmJsonVisitor maybeProject projectContext =
     let
-        ( exposedModules, isPackage ) =
+        ( exposedModules, projectType ) =
             case maybeProject |> Maybe.map .project of
                 Just (Elm.Project.Package { exposed }) ->
                     case exposed of
                         Elm.Project.ExposedList names ->
-                            ( names, True )
+                            ( names, Package )
 
                         Elm.Project.ExposedDict fakeDict ->
-                            ( List.concatMap Tuple.second fakeDict, True )
+                            ( List.concatMap Tuple.second fakeDict, Package )
 
-                _ ->
+                Just (Elm.Project.Application { depsDirect }) ->
-                    ( [], False )
+                    let
+                        elmApplicationType : ElmApplicationType
+                        elmApplicationType =
+                            if LamderaSupport.isLamderaApplication depsDirect then
+                                LamderaApplication
+
+                            else
+                                ElmApplication
+                    in
+                    ( [], Application elmApplicationType )
+
+                Nothing ->
+                    ( [], Application ElmApplication )
     in
     ( []
     , { projectContext
@@ -158,7 +181,7 @@ elmJsonVisitor maybeProject projectContext =
                 |> List.map (Elm.Module.toString >> String.split ".")
                 |> Set.fromList
                 |> Set.union projectContext.usedModules
-        , isPackage = isPackage
+        , projectType = projectType
       }
     )
 
@@ -205,24 +228,39 @@ moduleNameForImport node =
 
 declarationListVisitor : List (Node Declaration) -> ModuleContext -> ( List nothing, ModuleContext )
 declarationListVisitor list context =
-    if context.isPackage then
+    case context.projectType of
-        ( [], context )
+        Package ->
+            ( [], context )
 
-    else
+        Application elmApplicationType ->
-        let
+            let
-            containsMainFunction : Bool
+                mainName : String
-            containsMainFunction =
+                mainName =
-                List.any
+                    mainFunctionName elmApplicationType
-                    (\declaration ->
-                        case Node.value declaration of
-                            Declaration.FunctionDeclaration function ->
-                                (function.declaration |> Node.value |> .name |> Node.value) == "main"
 
-                            _ ->
+                containsMainFunction : Bool
-                                False
+                containsMainFunction =
-                    )
+                    List.any
-                    list
+                        (\declaration ->
-        in
+                            case Node.value declaration of
-        ( []
+                                Declaration.FunctionDeclaration function ->
-        , { context | containsMainFunction = containsMainFunction }
+                                    (function.declaration |> Node.value |> .name |> Node.value) == mainName
-        )
+
+                                _ ->
+                                    False
+                        )
+                        list
+            in
+            ( []
+            , { context | containsMainFunction = containsMainFunction }
+            )
+
+
+mainFunctionName : ElmApplicationType -> String
+mainFunctionName elmApplicationType =
+    case elmApplicationType of
+        ElmApplication ->
+            "main"
+
+        LamderaApplication ->
+            "app"

tests/NoUnused/ModulesTest.elm

@@ -1,86 +1,9 @@
 module NoUnused.ModulesTest exposing (all)
 
-import Elm.Project
-import Elm.Version
-import Json.Decode as Decode
 import NoUnused.Modules exposing (rule)
-import Review.Project as Project exposing (Project)
 import Review.Test
 import Test exposing (Test, describe, test)
-
+import TestProject exposing (application, lamderaApplication, package)
-
-application : Project
-application =
-    Project.new
-        |> Project.addElmJson applicationElmJson
-
-
-applicationElmJson : { path : String, raw : String, project : Elm.Project.Project }
-applicationElmJson =
-    { path = "elm.json"
-    , raw = """{
-    "type": "application",
-    "source-directories": [
-        "src"
-    ],
-    "elm-version": "0.19.1",
-    "dependencies": {
-        "direct": {
-            "elm/core": "1.0.2"
-        },
-        "indirect": {}
-    },
-    "test-dependencies": {
-        "direct": {},
-        "indirect": {}
-    }
-}"""
-    , project =
-        Elm.Project.Application
-            { elm = Elm.Version.one
-            , dirs = []
-            , depsDirect = []
-            , depsIndirect = []
-            , testDepsDirect = []
-            , testDepsIndirect = []
-            }
-    }
-
-
-package : Project
-package =
-    Project.new
-        |> Project.addElmJson (createPackageElmJson ())
-
-
-createPackageElmJson : () -> { path : String, raw : String, project : Elm.Project.Project }
-createPackageElmJson _ =
-    case Decode.decodeString Elm.Project.decoder rawPackageElmJson of
-        Ok elmJson ->
-            { path = "elm.json"
-            , raw = rawPackageElmJson
-            , project = elmJson
-            }
-
-        Err err ->
-            Debug.todo ("Invalid elm.json supplied to test: " ++ Debug.toString err)
-
-
-rawPackageElmJson : String
-rawPackageElmJson =
-    """{
-    "type": "package",
-    "name": "author/package",
-    "summary": "Summary",
-    "license": "BSD-3-Clause",
-    "version": "1.0.0",
-    "exposed-modules": [
-        "Exposed"
-    ],
-    "elm-version": "0.19.0 <= v < 0.20.0",
-    "dependencies": {},
-    "test-dependencies": {}
-}"""
 
 
 details : List String
@@ -89,168 +12,200 @@ details =
     ]
 
 
-tests : List Test
+all : Test
-tests =
+all =
-    [ test "should not report a module when all modules are used" <|
+    describe "NoUnusedModules"
-        \() ->
+        [ test "should not report a module when all modules are used" <|
-            [ """
+            \() ->
+                [ """
 module NotReported exposing (..)
 import OtherModule
 main = text ""
 """
-            , """
+                , """
 module OtherModule exposing (..)
 a = 1
 """
-            ]
+                ]
-                |> Review.Test.runOnModulesWithProjectData application rule
+                    |> Review.Test.runOnModulesWithProjectData application rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should report a module when it is never used" <|
+        , test "should report a module when it is never used" <|
-        \() ->
+            \() ->
-            [ """
+                [ """
 module NotReported exposing (..)
 main = text ""
 """
-            , """
+                , """
 module Reported exposing (..)
 a = 1
 """
-            , """
+                , """
 module Other.Reported exposing (..)
 a = 1
 """
-            ]
+                ]
-                |> Review.Test.runOnModulesWithProjectData application rule
+                    |> Review.Test.runOnModulesWithProjectData application rule
-                |> Review.Test.expectErrorsForModules
+                    |> Review.Test.expectErrorsForModules
-                    [ ( "Reported"
+                        [ ( "Reported"
-                      , [ Review.Test.error
+                          , [ Review.Test.error
+                                { message = "Module `Reported` is never used."
+                                , details = details
+                                , under = "Reported"
+                                }
+                            ]
+                          )
+                        , ( "Other.Reported"
+                          , [ Review.Test.error
+                                { message = "Module `Other.Reported` is never used."
+                                , details = details
+                                , under = "Other.Reported"
+                                }
+                            ]
+                          )
+                        ]
+        , test "should report a module even if it is the only module in the project" <|
+            \() ->
+                """
+module Reported exposing (..)
+import Something
+a = 1
+"""
+                    |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
                             { message = "Module `Reported` is never used."
                             , details = details
                             , under = "Reported"
                             }
                         ]
-                      )
+        , test "should not report an application module if it exposes a main function" <|
-                    , ( "Other.Reported"
+            \() ->
-                      , [ Review.Test.error
+                """
-                            { message = "Module `Other.Reported` is never used."
-                            , details = details
-                            , under = "Other.Reported"
-                            }
-                        ]
-                      )
-                    ]
-    , test "should report a module even if it is the only module in the project" <|
-        \() ->
-            """
-module Reported exposing (..)
-import Something
-a = 1
-"""
-                |> Review.Test.runWithProjectData application rule
-                |> Review.Test.expectErrors
-                    [ Review.Test.error
-                        { message = "Module `Reported` is never used."
-                        , details = details
-                        , under = "Reported"
-                        }
-                    ]
-    , test "should not report an application module if it exposes a main function" <|
-        \() ->
-            """
 module NotReported exposing (..)
 main = text ""
 """
-                |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.runWithProjectData application rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should not report an application module if it contains a main function even if it is not exposed" <|
+        , test "should not report an application module if it contains a main function even if it is not exposed" <|
-        \() ->
+            \() ->
-            """
+                """
 module NotReported exposing (a)
 main = text ""
 a = 1
 """
-                |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.runWithProjectData application rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should not report a module with main function if we don't know the project type" <|
+        , test "should not report a module with main function if we don't know the project type" <|
-        \() ->
+            \() ->
-            """
+                """
 module NotReported exposing (a)
 main = text ""
 a = 1
 """
-                |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.runWithProjectData application rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should not report a module if it imports `Test`" <|
+        , test "should not report a module if it imports `Test`" <|
-        \() ->
+            \() ->
-            """
+                """
 module NotReported exposing (..)
 import Test
 a = 1
 """
-                |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.runWithProjectData application rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should not report the `ReviewConfig` module" <|
+        , test "should not report the `ReviewConfig` module" <|
-        \() ->
+            \() ->
-            """
+                """
 module ReviewConfig exposing (config)
 config = []
 """
-                |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.runWithProjectData application rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should not report modules exposed in a package" <|
+        , test "should not report modules exposed in a package" <|
-        \() ->
+            \() ->
-            """
+                """
 module Exposed exposing (..)
 a = 1
 """
-                |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.runWithProjectData package rule
-                |> Review.Test.expectNoErrors
+                    |> Review.Test.expectNoErrors
-    , test "should report non-exposed and non-used modules from a package" <|
+        , test "should report non-exposed and non-used modules from a package" <|
-        \() ->
+            \() ->
-            """
+                """
 module NotExposed exposing (..)
 a = 1
 """
-                |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.runWithProjectData package rule
-                |> Review.Test.expectErrors
+                    |> Review.Test.expectErrors
-                    [ Review.Test.error
+                        [ Review.Test.error
-                        { message = "Module `NotExposed` is never used."
+                            { message = "Module `NotExposed` is never used."
-                        , details = details
+                            , details = details
-                        , under = "NotExposed"
+                            , under = "NotExposed"
-                        }
+                            }
-                    ]
+                        ]
-    , test "should report non-exposed and non-used package modules that expose a `main` function" <|
+        , test "should report non-exposed and non-used package modules that expose a `main` function" <|
-        \() ->
+            \() ->
-            """
+                """
 module Reported exposing (main)
 main = text ""
 """
-                |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.runWithProjectData package rule
-                |> Review.Test.expectErrors
+                    |> Review.Test.expectErrors
-                    [ Review.Test.error
+                        [ Review.Test.error
-                        { message = "Module `Reported` is never used."
+                            { message = "Module `Reported` is never used."
-                        , details = details
+                            , details = details
-                        , under = "Reported"
+                            , under = "Reported"
-                        }
+                            }
-                    ]
+                        ]
-    , test "should report non-exposed and non-used package modules that define a `main` function" <|
+        , test "should report non-exposed and non-used package modules that define a `main` function" <|
-        \() ->
+            \() ->
-            """
+                """
 module Reported exposing (a)
 main = text ""
 a = 1
 """
-                |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.runWithProjectData package rule
-                |> Review.Test.expectErrors
+                    |> Review.Test.expectErrors
-                    [ Review.Test.error
+                        [ Review.Test.error
-                        { message = "Module `Reported` is never used."
+                            { message = "Module `Reported` is never used."
-                        , details = details
+                            , details = details
-                        , under = "Reported"
+                            , under = "Reported"
-                        }
+                            }
-                    ]
+                        ]
-    ]
+        , test "should report modules that contain a top-level `app` function in packages" <|
-
+            \() ->
-
+                """
-all : Test
+module Reported exposing (app)
-all =
+app = text ""
-    describe "NoUnusedModules" tests
+"""
+                    |> Review.Test.runWithProjectData package rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "Module `Reported` is never used."
+                            , details = details
+                            , under = "Reported"
+                            }
+                        ]
+        , test "should report modules that contain a top-level `app` function in Elm applications" <|
+            \() ->
+                """
+module Reported exposing (app)
+app = text ""
+"""
+                    |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "Module `Reported` is never used."
+                            , details = details
+                            , under = "Reported"
+                            }
+                        ]
+        , test "should report modules that contain a top-level `app` function in Lamdera applications" <|
+            \() ->
+                """
+module Reported exposing (app)
+app = text ""
+"""
+                    |> Review.Test.runWithProjectData lamderaApplication rule
+                    |> Review.Test.expectNoErrors
+        ]

tests/NoUnused/Patterns.elm

@@ -24,6 +24,8 @@ import Set exposing (Set)
 
 {-| Report useless patterns and pattern values that are not used.
 
+🔧 Running with `--fix` will automatically remove all the reported errors.
+
     config =
         [ NoUnused.Patterns.rule
         ]
@@ -201,7 +203,7 @@ report context =
                 errors =
                     List.concat
                         [ singleErrors
-                        , recordErrors
+                        , List.concatMap (recordErrors context) records
                         , simplifiablePatterns
                         ]
 
@@ -217,51 +219,6 @@ report context =
                                     pattern.range
                                     pattern.fix
                             )
-
-                recordErrors : List (Rule.Error {})
-                recordErrors =
-                    records
-                        |> List.concatMap
-                            (\{ fields, recordRange } ->
-                                let
-                                    ( unused, used ) =
-                                        List.partition (isNodeInContext context) fields
-                                in
-                                case unused of
-                                    [] ->
-                                        []
-
-                                    firstNode :: restNodes ->
-                                        let
-                                            first : String
-                                            first =
-                                                Node.value firstNode
-
-                                            rest : List String
-                                            rest =
-                                                List.map Node.value restNodes
-
-                                            ( errorRange, fix ) =
-                                                case used of
-                                                    [] ->
-                                                        ( recordRange, Fix.replaceRangeBy recordRange "_" )
-
-                                                    _ ->
-                                                        ( Range.combine (List.map Node.range unused)
-                                                        , Node Range.emptyRange (Pattern.RecordPattern used)
-                                                            |> Writer.writePattern
-                                                            |> Writer.write
-                                                            |> Fix.replaceRangeBy recordRange
-                                                        )
-                                        in
-                                        [ Rule.errorWithFix
-                                            { message = listToMessage first rest
-                                            , details = listToDetails first rest
-                                            }
-                                            errorRange
-                                            [ fix ]
-                                        ]
-                            )
             in
             ( errors
             , List.foldl
@@ -274,6 +231,58 @@ report context =
             ( [], context )
 
 
+recordErrors : Context -> { fields : List (Node String), recordRange : Range } -> List (Rule.Error {})
+recordErrors context { fields, recordRange } =
+    if List.isEmpty fields then
+        [ Rule.errorWithFix
+            { message = "Record pattern is not needed"
+            , details = [ "This pattern is redundant and should be replaced with '_'." ]
+            }
+            recordRange
+            [ Fix.replaceRangeBy recordRange "_" ]
+        ]
+
+    else
+        let
+            ( unused, used ) =
+                List.partition (isNodeInContext context) fields
+        in
+        case unused of
+            [] ->
+                []
+
+            firstNode :: restNodes ->
+                let
+                    first : String
+                    first =
+                        Node.value firstNode
+
+                    rest : List String
+                    rest =
+                        List.map Node.value restNodes
+
+                    ( errorRange, fix ) =
+                        case used of
+                            [] ->
+                                ( recordRange, Fix.replaceRangeBy recordRange "_" )
+
+                            _ ->
+                                ( Range.combine (List.map Node.range unused)
+                                , Node Range.emptyRange (Pattern.RecordPattern used)
+                                    |> Writer.writePattern
+                                    |> Writer.write
+                                    |> Fix.replaceRangeBy recordRange
+                                )
+                in
+                [ Rule.errorWithFix
+                    { message = listToMessage first rest
+                    , details = listToDetails first rest
+                    }
+                    errorRange
+                    [ fix ]
+                ]
+
+
 findDeclaredPatterns :
     Scope
     ->
@@ -328,7 +337,7 @@ findPatterns use (Node range pattern) =
         Pattern.TuplePattern [ Node _ Pattern.AllPattern, Node _ Pattern.AllPattern ] ->
             [ SimplifiablePattern
                 (Rule.errorWithFix
-                    { message = "Tuple pattern is not needed."
+                    { message = "Tuple pattern is not needed"
                     , details = redundantDetails
                     }
                     range
@@ -339,7 +348,7 @@ findPatterns use (Node range pattern) =
         Pattern.TuplePattern [ Node _ Pattern.AllPattern, Node _ Pattern.AllPattern, Node _ Pattern.AllPattern ] ->
             [ SimplifiablePattern
                 (Rule.errorWithFix
-                    { message = "Tuple pattern is not needed."
+                    { message = "Tuple pattern is not needed"
                     , details = redundantDetails
                     }
                     range
@@ -367,7 +376,7 @@ findPatterns use (Node range pattern) =
             if use == Destructuring && List.all isAllPattern patterns then
                 [ SimplifiablePattern
                     (Rule.errorWithFix
-                        { message = "Named pattern is not needed."
+                        { message = "Named pattern is not needed"
                         , details = redundantDetails
                         }
                         range
@@ -489,7 +498,7 @@ errorsForPattern use (Node range pattern) context =
 errorsForUselessNamePattern : Range -> Context -> ( List (Rule.Error {}), Context )
 errorsForUselessNamePattern range context =
     ( [ Rule.errorWithFix
-            { message = "Named pattern is not needed."
+            { message = "Named pattern is not needed"
             , details = redundantDetails
             }
             range
@@ -502,7 +511,7 @@ errorsForUselessNamePattern range context =
 errorsForUselessTuple : Range -> Context -> ( List (Rule.Error {}), Context )
 errorsForUselessTuple range context =
     ( [ Rule.errorWithFix
-            { message = "Tuple pattern is not needed."
+            { message = "Tuple pattern is not needed"
             , details = redundantDetails
             }
             range
@@ -600,7 +609,7 @@ errorsForAsPattern patternRange inner (Node range name) context =
 
     else if isAllPattern inner then
         ( [ Rule.errorWithFix
-                { message = "Pattern `_` is not needed."
+                { message = "Pattern `_` is not needed"
                 , details = removeDetails
                 }
                 (Node.range inner)
@@ -618,7 +627,7 @@ findPatternForAsPattern patternRange inner (Node range name) =
     if isAllPattern inner then
         SimplifiablePattern
             (Rule.errorWithFix
-                { message = "Pattern `_` is not needed."
+                { message = "Pattern `_` is not needed"
                 , details = removeDetails
                 }
                 (Node.range inner)

tests/NoUnused/PatternsTests.elm

@@ -409,7 +409,7 @@ foo =
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "Pattern `_` is not needed."
+                        { message = "Pattern `_` is not needed"
                         , details = [ "This pattern is redundant and should be removed." ]
                         , under = "_"
                         }
@@ -610,7 +610,7 @@ foo =
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "Pattern `_` is not needed."
+                        { message = "Pattern `_` is not needed"
                         , details = [ "This pattern is redundant and should be removed." ]
                         , under = "_"
                         }
@@ -786,7 +786,7 @@ foo =
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "Named pattern is not needed."
+                        { message = "Named pattern is not needed"
                         , details = redundantReplaceDetails
                         , under = "Singular _"
                         }
@@ -801,7 +801,7 @@ foo =
     bosh
 """
                     , Review.Test.error
-                        { message = "Named pattern is not needed."
+                        { message = "Named pattern is not needed"
                         , details = redundantReplaceDetails
                         , under = "Pair _ _"
                         }
@@ -829,7 +829,7 @@ foo =
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "Named pattern is not needed."
+                        { message = "Named pattern is not needed"
                         , details = redundantReplaceDetails
                         , under = "Singular _"
                         }
@@ -843,7 +843,7 @@ foo =
     bosh
 """
                     , Review.Test.error
-                        { message = "Named pattern is not needed."
+                        { message = "Named pattern is not needed"
                         , details = redundantReplaceDetails
                         , under = "Pair _ _"
                         }
@@ -918,6 +918,35 @@ foo =
             bar
     in
     bash
+"""
+                    ]
+    , test "should report empty record values" <|
+        \() ->
+            """
+module A exposing (..)
+foo =
+    let
+        {} =
+            bar
+    in
+    bash
+"""
+                |> Review.Test.run rule
+                |> Review.Test.expectErrors
+                    [ Review.Test.error
+                        { message = "Record pattern is not needed"
+                        , details = redundantReplaceDetails
+                        , under = "{}"
+                        }
+                        |> Review.Test.whenFixed
+                            """
+module A exposing (..)
+foo =
+    let
+        _ =
+            bar
+    in
+    bash
 """
                     ]
     , test "should report highlight the least amount of values possible" <|
@@ -1012,7 +1041,7 @@ foo =
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "Tuple pattern is not needed."
+                        { message = "Tuple pattern is not needed"
                         , details = redundantReplaceDetails
                         , under = "( _, _ )"
                         }
@@ -1041,7 +1070,7 @@ foo =
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "Tuple pattern is not needed."
+                        { message = "Tuple pattern is not needed"
                         , details = redundantReplaceDetails
                         , under = "( _, _, _ )"
                         }

tests/NoUnused/RangeDict.elm

@@ -1,4 +1,4 @@
-module NoUnused.RangeDict exposing (RangeDict, empty, fromList, get, insert, member)
+module NoUnused.RangeDict exposing (RangeDict, empty, fromList, get, insert, insertAll, member, singleton)
 
 import Dict exposing (Dict)
 import Elm.Syntax.Range exposing (Range)
@@ -18,6 +18,19 @@ insert range =
     Dict.insert (rangeAsString range)
 
 
+insertAll : List ( Range, v ) -> RangeDict v -> RangeDict v
+insertAll list dict =
+    List.foldl
+        (\( range, v ) -> insert range v)
+        dict
+        list
+
+
+singleton : Range -> v -> RangeDict v
+singleton range value =
+    Dict.singleton (rangeAsString range) value
+
+
 fromList : List ( Range, v ) -> RangeDict v
 fromList values =
     values

tests/NoUnused/Variables.elm

@@ -20,6 +20,8 @@ import Elm.Syntax.ModuleName exposing (ModuleName)
 import Elm.Syntax.Node as Node exposing (Node(..))
 import Elm.Syntax.Pattern as Pattern exposing (Pattern)
 import Elm.Syntax.Range exposing (Range)
+import Elm.Syntax.Type
+import Elm.Syntax.TypeAlias exposing (TypeAlias)
 import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
 import NoUnused.NonemptyList as NonemptyList exposing (Nonempty)
 import NoUnused.RangeDict as RangeDict exposing (RangeDict)
@@ -32,6 +34,8 @@ import Set exposing (Set)
 
 {-| Report variables or types that are declared or imported but never used.
 
+🔧 Running with `--fix` will automatically remove all the reported errors.
+
     config =
         [ NoUnused.Variables.rule
         ]
@@ -697,7 +701,18 @@ expressionEnterVisitorHelp (Node range value) context =
                                         namesUsedInPattern =
                                             getUsedVariablesFromPattern context pattern
                                     in
-                                    ( []
+                                    ( if not (introducesVariable pattern) then
+                                        Rule.errorWithFix
+                                            { message = "Pattern doesn't introduce any variables"
+                                            , details =
+                                                [ "This value has been computed but isn't assigned to any variable, which makes the value unusable. You should remove it at the location I pointed at." ]
+                                            }
+                                            (Node.range pattern)
+                                            [ Fix.removeRange (letDeclarationToRemoveRange letBlockContext (Node.range declaration)) ]
+                                            :: errors
+
+                                      else
+                                        errors
                                     , List.foldl markValueAsUsed foldContext namesUsedInPattern.types
                                         |> markAllModulesAsUsed namesUsedInPattern.modules
                                     )
@@ -913,72 +928,138 @@ getUsedModulesFromPattern lookupTable patternNode =
             getUsedModulesFromPattern lookupTable pattern
 
 
+introducesVariable : Node Pattern -> Bool
+introducesVariable patternNode =
+    case Node.value patternNode of
+        Pattern.VarPattern _ ->
+            True
+
+        Pattern.AsPattern _ _ ->
+            True
+
+        Pattern.RecordPattern fields ->
+            not (List.isEmpty fields)
+
+        Pattern.TuplePattern patterns ->
+            List.any introducesVariable patterns
+
+        Pattern.UnConsPattern pattern1 pattern2 ->
+            List.any introducesVariable [ pattern1, pattern2 ]
+
+        Pattern.ListPattern patterns ->
+            List.any introducesVariable patterns
+
+        Pattern.NamedPattern _ patterns ->
+            List.any introducesVariable patterns
+
+        Pattern.ParenthesizedPattern pattern ->
+            introducesVariable pattern
+
+        _ ->
+            False
+
+
 
 -- DECLARATION LIST VISITOR
 
 
 declarationListVisitor : List (Node Declaration) -> ModuleContext -> ( List (Error {}), ModuleContext )
 declarationListVisitor nodes context =
-    List.foldl
+    ( []
-        (\node ( errors, ctx ) ->
+    , List.foldl registerTypes context nodes
-            case Node.value node of
+    )
-                Declaration.CustomTypeDeclaration { name, constructors, documentation } ->
-                    let
-                        typeName : String
-                        typeName =
-                            Node.value name
 
-                        constructorsForType : Dict String String
-                        constructorsForType =
-                            constructors
-                                |> List.map (Node.value >> .name >> Node.value)
-                                |> List.map (\constructorName -> ( constructorName, typeName ))
-                                |> Dict.fromList
 
-                        customType : CustomTypeData
+registerTypes : Node Declaration -> ModuleContext -> ModuleContext
-                        customType =
+registerTypes node context =
-                            { under = Node.range name
+    case Node.value node of
-                            , rangeToRemove = rangeToRemoveForNodeWithDocumentation node documentation
+        Declaration.CustomTypeDeclaration customType ->
-                            , variants = List.map (Node.value >> .name >> Node.value) constructors
+            registerCustomType (Node.range node) customType context
-                            }
-                    in
-                    ( errors
-                    , { ctx
-                        | localCustomTypes =
-                            Dict.insert
-                                (Node.value name)
-                                customType
-                                ctx.localCustomTypes
-                        , constructorNameToTypeName = Dict.union constructorsForType ctx.constructorNameToTypeName
-                      }
-                    )
 
-                Declaration.AliasDeclaration { name, documentation } ->
+        Declaration.AliasDeclaration typeAliasDeclaration ->
-                    let
+            registerTypeAlias (Node.range node) typeAliasDeclaration context
-                        contextWithRemovedShadowedImports : ModuleContext
-                        contextWithRemovedShadowedImports =
-                            { context | importedCustomTypeLookup = Dict.remove (Node.value name) context.importedCustomTypeLookup }
-                    in
-                    ( []
-                    , if context.exposesEverything then
-                        contextWithRemovedShadowedImports
 
-                      else
+        _ ->
-                        registerVariable
+            context
-                            { typeName = "Type"
+
-                            , under = Node.range name
+
-                            , rangeToRemove = Just (rangeToRemoveForNodeWithDocumentation node documentation)
+registerTypeAlias : Range -> TypeAlias -> ModuleContext -> ModuleContext
-                            , warning = ""
+registerTypeAlias range { name, typeAnnotation } context =
-                            }
+    let
-                            (Node.value name)
+        contextWithRemovedShadowedImports : ModuleContext
-                            contextWithRemovedShadowedImports
+        contextWithRemovedShadowedImports =
-                    )
+            case Node.value typeAnnotation of
+                TypeAnnotation.Record _ ->
+                    { context | importedCustomTypeLookup = Dict.remove (Node.value name) context.importedCustomTypeLookup }
 
                 _ ->
-                    ( errors, ctx )
+                    context
-        )
+
-        ( [], context )
+        -- TODO Rename
-        nodes
+        typeAlias : CustomTypeData
+        typeAlias =
+            { under = Node.range name
+            , rangeToRemove = untilStartOfNextLine range
+            , variants = []
+            }
+    in
+    case Node.value typeAnnotation of
+        TypeAnnotation.Record _ ->
+            if context.exposesEverything then
+                contextWithRemovedShadowedImports
+
+            else
+                registerVariable
+                    { typeName = "Type"
+                    , under = Node.range name
+                    , rangeToRemove = Just (untilStartOfNextLine range)
+                    , warning = ""
+                    }
+                    (Node.value name)
+                    contextWithRemovedShadowedImports
+
+        _ ->
+            { contextWithRemovedShadowedImports
+                | localCustomTypes =
+                    Dict.insert
+                        (Node.value name)
+                        typeAlias
+                        contextWithRemovedShadowedImports.localCustomTypes
+            }
+
+
+registerCustomType : Range -> Elm.Syntax.Type.Type -> ModuleContext -> ModuleContext
+registerCustomType range { name, constructors } context =
+    let
+        typeName : String
+        typeName =
+            Node.value name
+
+        constructorNames : List String
+        constructorNames =
+            List.map (Node.value >> .name >> Node.value) constructors
+
+        constructorsForType : Dict String String
+        constructorsForType =
+            constructorNames
+                |> List.map (\constructorName -> ( constructorName, typeName ))
+                |> Dict.fromList
+
+        customType : CustomTypeData
+        customType =
+            { under = Node.range name
+            , rangeToRemove = untilStartOfNextLine range
+            , variants = constructorNames
+            }
+    in
+    { context
+        | localCustomTypes =
+            Dict.insert
+                (Node.value name)
+                customType
+                context.localCustomTypes
+        , constructorNameToTypeName = Dict.union constructorsForType context.constructorNameToTypeName
+    }
 
 
 
@@ -1025,7 +1106,7 @@ declarationVisitor node context =
                         registerVariable
                             { typeName = "Top-level variable"
                             , under = Node.range functionImplementation.name
-                            , rangeToRemove = Just (rangeToRemoveForNodeWithDocumentation node function.documentation)
+                            , rangeToRemove = Just (untilStartOfNextLine (Node.range node))
                             , warning = ""
                             }
                             functionName
@@ -1034,8 +1115,8 @@ declarationVisitor node context =
                 newContext : ModuleContext
                 newContext =
                     { newContextWhereFunctionIsRegistered | inTheDeclarationOf = [ functionName ], declarations = Dict.empty }
-                        |> (\ctx -> List.foldl markAsUsed ctx namesUsedInSignature.types)
                         |> (\ctx -> List.foldl markValueAsUsed ctx namesUsedInArgumentPatterns.types)
+                        |> markAllAsUsed namesUsedInSignature.types
                         |> markAllModulesAsUsed namesUsedInSignature.modules
                         |> markAllModulesAsUsed namesUsedInArgumentPatterns.modules
 
@@ -1128,19 +1209,6 @@ foldUsedTypesAndModules =
     List.foldl (\a b -> { types = a.types ++ b.types, modules = a.modules ++ b.modules }) { types = [], modules = [] }
 
 
-rangeToRemoveForNodeWithDocumentation : Node Declaration -> Maybe (Node a) -> Range
-rangeToRemoveForNodeWithDocumentation (Node nodeRange _) documentation =
-    case documentation of
-        Nothing ->
-            untilStartOfNextLine nodeRange
-
-        Just (Node documentationRange _) ->
-            untilStartOfNextLine
-                { start = documentationRange.start
-                , end = nodeRange.end
-                }
-
-
 finalEvaluation : ModuleContext -> List (Error {})
 finalEvaluation context =
     let
@@ -1335,24 +1403,13 @@ registerFunction letBlockContext function context =
 
                 Nothing ->
                     { types = [], modules = [] }
-
-        functionRange : Range
-        functionRange =
-            case function.signature of
-                Just signature ->
-                    mergeRanges
-                        (Node.range function.declaration)
-                        (Node.range signature)
-
-                Nothing ->
-                    Node.range function.declaration
     in
     List.foldl markAsUsed context namesUsedInSignature.types
         |> markAllModulesAsUsed namesUsedInSignature.modules
         |> registerVariable
             { typeName = "`let in` variable"
             , under = Node.range declaration.name
-            , rangeToRemove = Just (letDeclarationToRemoveRange letBlockContext functionRange)
+            , rangeToRemove = Just (letDeclarationToRemoveRange letBlockContext (Node.range function.declaration))
             , warning = ""
             }
             (Node.value declaration.name)
@@ -1592,47 +1649,6 @@ untilStartOfNextLine range =
         { range | end = { row = range.end.row + 1, column = 1 } }
 
 
-{-| Create a new range that starts at the start of the range that starts first,
-and ends at the end of the range that starts last. If the two ranges are distinct
-and there is code in between, that code will be included in the resulting range.
-
-    range : Range
-    range =
-        Fix.mergeRanges
-            (Node.range node1)
-            (Node.range node2)
-
--}
-mergeRanges : Range -> Range -> Range
-mergeRanges a b =
-    let
-        start : { row : Int, column : Int }
-        start =
-            case comparePosition a.start b.start of
-                LT ->
-                    a.start
-
-                EQ ->
-                    a.start
-
-                GT ->
-                    b.start
-
-        end : { row : Int, column : Int }
-        end =
-            case comparePosition a.end b.end of
-                LT ->
-                    b.end
-
-                EQ ->
-                    b.end
-
-                GT ->
-                    a.end
-    in
-    { start = start, end = end }
-
-
 {-| Make a range stop at a position. If the position is not inside the range,
 then the range won't change.
 
@@ -1664,13 +1680,3 @@ positionAsInt { row, column } =
     -- 1.000.000 characters long. Then, as long as ranges don't overlap,
     -- this should work fine.
     row * 1000000 + column
-
-
-comparePosition : { row : Int, column : Int } -> { row : Int, column : Int } -> Order
-comparePosition a b =
-    case compare a.row b.row of
-        EQ ->
-            compare a.column b.column
-
-        order ->
-            order

tests/NoUnused/VariablesTest.elm

@@ -353,15 +353,18 @@ letInTests =
     [ test "should report unused variables from let declarations" <|
         \() ->
             """module SomeModule exposing (a)
-a = let b = 1
+a = let
+        unused : number
+        unused = 1
     in 2"""
                 |> Review.Test.run rule
                 |> Review.Test.expectErrors
                     [ Review.Test.error
-                        { message = "`let in` variable `b` is not used"
+                        { message = "`let in` variable `unused` is not used"
                         , details = details
-                        , under = "b"
+                        , under = "unused"
                         }
+                        |> Review.Test.atExactly { start = { row = 4, column = 9 }, end = { row = 4, column = 15 } }
                         |> Review.Test.whenFixed """module SomeModule exposing (a)
 a = 2"""
                     ]
@@ -468,6 +471,26 @@ a = let () = b
                         |> Review.Test.whenFixed """module SomeModule exposing (a)
 a = 2"""
                     ]
+    , test "should report () destructuring even if something comes afterwards" <|
+        \() ->
+            """module SomeModule exposing (a)
+a = let () = b
+        {c} = 1
+    in c"""
+                |> Review.Test.run rule
+                |> Review.Test.expectErrors
+                    [ Review.Test.error
+                        { message = "Unit value is unused"
+                        , details =
+                            [ "This value has no data, which makes the value unusable. You should remove it at the location I pointed at."
+                            ]
+                        , under = "()"
+                        }
+                        |> Review.Test.whenFixed ("""module SomeModule exposing (a)
+a = let$
+        {c} = 1
+    in c""" |> String.replace "$" " ")
+                    ]
     , test "should report parenthesized wildcard assignments" <|
         \() ->
             """module SomeModule exposing (a)
@@ -483,6 +506,45 @@ a = let (_) = 1
                         , under = "_"
                         }
                         |> Review.Test.whenFixed """module SomeModule exposing (a)
+a = 2"""
+                    ]
+    , test "should report pattern match of data-less constructor" <|
+        \() ->
+            """module SomeModule exposing (a)
+type Foo = Foo
+a = let Foo = Foo
+    in 2"""
+                |> Review.Test.run rule
+                |> Review.Test.expectErrors
+                    [ Review.Test.error
+                        { message = "Pattern doesn't introduce any variables"
+                        , details =
+                            [ "This value has been computed but isn't assigned to any variable, which makes the value unusable. You should remove it at the location I pointed at."
+                            ]
+                        , under = "Foo"
+                        }
+                        |> Review.Test.atExactly { start = { row = 3, column = 9 }, end = { row = 3, column = 12 } }
+                        |> Review.Test.whenFixed """module SomeModule exposing (a)
+type Foo = Foo
+a = 2"""
+                    ]
+    , test "should report pattern match that doesn't introduce any variables" <|
+        \() ->
+            """module SomeModule exposing (a)
+type Foo = Foo
+a = let ( Foo, (Bar _), _ ) = x
+    in 2"""
+                |> Review.Test.run rule
+                |> Review.Test.expectErrors
+                    [ Review.Test.error
+                        { message = "Pattern doesn't introduce any variables"
+                        , details =
+                            [ "This value has been computed but isn't assigned to any variable, which makes the value unusable. You should remove it at the location I pointed at."
+                            ]
+                        , under = "( Foo, (Bar _), _ )"
+                        }
+                        |> Review.Test.whenFixed """module SomeModule exposing (a)
+type Foo = Foo
 a = 2"""
                     ]
     ]
@@ -584,15 +646,6 @@ a =
             """module SomeModule exposing (a)
 type Baz = Baz String
 
-a =
-  \\(Baz value) -> []"""
-                |> Review.Test.run rule
-                |> Review.Test.expectNoErrors
-    , test "should not report type alias when it is deconstructed in a function call" <|
-        \() ->
-            """module SomeModule exposing (a)
-type alias Baz = { a : String}
-
 a =
   \\(Baz value) -> []"""
                 |> Review.Test.run rule
@@ -776,8 +829,8 @@ a = Html.Styled.Attributes.href"""
         \() ->
             [ """module SomeModule exposing (a)
 import B
-a = let (B.B ()) = x
+a = let (B.B y) = x
-    in 1
+    in y
 """
             , """module B exposing (B)
 type B = B ()"""
@@ -914,7 +967,7 @@ type C = C_Value
             ]
                 |> Review.Test.runOnModules rule
                 |> Review.Test.expectNoErrors
-    , test "should report open type import when the exposed constructor is shadowed by a local type alias" <|
+    , test "should report open type import when the exposed constructor is shadowed by a local type alias when it is a record" <|
         \() ->
             [ """module A exposing (a)
 import B exposing (C(..))
@@ -939,6 +992,71 @@ a = C_Value""" |> String.replace "$" " ")
                         ]
                       )
                     ]
+    , test "should report open type import when the exposed constructor is NOT shadowed by a local type alias when it is not a record" <|
+        \() ->
+            [ """module A exposing (a)
+import B exposing (C(..))
+type alias C = B.C
+a = C"""
+            , """module B exposing (C(..))
+type C = C
+"""
+            ]
+                |> Review.Test.runOnModules rule
+                |> Review.Test.expectErrorsForModules
+                    [ ( "A"
+                      , [ Review.Test.error
+                            { message = "Type `C` is not used"
+                            , details = details
+                            , under = "C"
+                            }
+                            |> Review.Test.atExactly { start = { row = 3, column = 12 }, end = { row = 3, column = 13 } }
+                            |> Review.Test.whenFixed """module A exposing (a)
+import B exposing (C(..))
+a = C"""
+                        ]
+                      )
+                    ]
+    , test "should report open type import when the exposed constructor is shadowed by a custom type constructor" <|
+        \() ->
+            [ """module A exposing (a)
+import B exposing (C(..))
+type Type = C
+a = C"""
+            , """module B exposing (C(..))
+type C = C
+"""
+            ]
+                |> Review.Test.runOnModules rule
+                |> Review.Test.expectErrorsForModules
+                    [ ( "A"
+                      , [ Review.Test.error
+                            { message = "Imported type `C` is not used"
+                            , details = details
+                            , under = "C(..)"
+                            }
+                            |> Review.Test.whenFixed ("""module A exposing (a)
+import B$
+type Type = C
+a = C""" |> String.replace "$" " ")
+                        ]
+                      )
+                    ]
+    , test "should not report open type import when a constructor is used but the type is locally shadowed" <|
+        \() ->
+            [ """module A exposing (a)
+import B exposing (C(..))
+type alias C = A.C
+
+a : C -> String
+a (C s) = s
+"""
+            , """module B exposing (C(..))
+type C = C String
+"""
+            ]
+                |> Review.Test.runOnModules rule
+                |> Review.Test.expectNoErrors
     , test "should report open type import when none of its constructors is used (imported dependency)" <|
         \() ->
             """module A exposing (a)
@@ -969,7 +1087,7 @@ import UnknownDependency exposing (C(..))
 a = 1"""
                 |> Review.Test.runWithProjectData packageProject rule
                 |> Review.Test.expectNoErrors
-    , test "should not report open type import when at least one of the exposed constructors are used in a pattern" <|
+    , test "should not report open type import when at least one of the exposed constructors is used in a pattern" <|
         \() ->
             [ """module A exposing (a)
 import B exposing (C(..))
@@ -1570,7 +1688,7 @@ outer arg =
             []
     in
     inner arg
-    """
+"""
                 |> Review.Test.run rule
                 |> Review.Test.expectNoErrors
     ]
@@ -1940,7 +2058,7 @@ outer arg =
             []
     in
     inner arg
-    """
+"""
                 |> Review.Test.run rule
                 |> Review.Test.expectNoErrors
     , test "should not report unused type alias when it is used in a function call in a let expression" <|
@@ -1952,9 +2070,26 @@ outer arg =
         inner = Baz range
     in
     inner arg
-    """
+"""
                 |> Review.Test.run rule
                 |> Review.Test.expectNoErrors
+    , test "should report unused type alias when it aliases something else than a record" <|
+        \() ->
+            """module SomeModule exposing (a)
+type alias UnusedType = String
+a = 1
+"""
+                |> Review.Test.run rule
+                |> Review.Test.expectErrors
+                    [ Review.Test.error
+                        { message = "Type `UnusedType` is not used"
+                        , details = details
+                        , under = "UnusedType"
+                        }
+                        |> Review.Test.whenFixed """module SomeModule exposing (a)
+a = 1
+"""
+                    ]
     ]
 
 

tests/Simplify/Normalize.elm

@@ -0,0 +1,530 @@
+module Simplify.Normalize exposing (Comparison(..), areAllTheSame, compare, getNumberValue)
+
+import Dict
+import Elm.Syntax.Expression as Expression exposing (Expression)
+import Elm.Syntax.ModuleName exposing (ModuleName)
+import Elm.Syntax.Node as Node exposing (Node(..))
+import Elm.Syntax.Pattern as Pattern exposing (Pattern)
+import Elm.Syntax.Range as Range exposing (Range)
+import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
+
+
+areTheSame : ModuleNameLookupTable -> Node Expression -> Node Expression -> Bool
+areTheSame lookupTable left right =
+    normalize lookupTable left == normalize lookupTable right
+
+
+areAllTheSame : ModuleNameLookupTable -> Node Expression -> List (Node Expression) -> Bool
+areAllTheSame lookupTable first rest =
+    let
+        normalizedFirst : Node Expression
+        normalizedFirst =
+            normalize lookupTable first
+    in
+    List.all (\node -> normalize lookupTable node == normalizedFirst) rest
+
+
+normalize : ModuleNameLookupTable -> Node Expression -> Node Expression
+normalize lookupTable node =
+    case Node.value node of
+        Expression.ParenthesizedExpression expr ->
+            normalize lookupTable expr
+
+        Expression.Application nodes ->
+            toNode (Expression.Application (List.map (normalize lookupTable) nodes))
+
+        Expression.OperatorApplication string infixDirection left right ->
+            toNode (Expression.OperatorApplication string infixDirection (normalize lookupTable left) (normalize lookupTable right))
+
+        Expression.FunctionOrValue rawModuleName string ->
+            let
+                moduleName : ModuleName
+                moduleName =
+                    ModuleNameLookupTable.moduleNameFor lookupTable node
+                        |> Maybe.withDefault rawModuleName
+            in
+            toNode (Expression.FunctionOrValue moduleName string)
+
+        Expression.IfBlock cond then_ else_ ->
+            toNode (Expression.IfBlock (normalize lookupTable cond) (normalize lookupTable then_) (normalize lookupTable else_))
+
+        Expression.Negation expr ->
+            toNode (Expression.Negation (normalize lookupTable expr))
+
+        Expression.TupledExpression nodes ->
+            toNode (Expression.TupledExpression (List.map (normalize lookupTable) nodes))
+
+        Expression.LetExpression letBlock ->
+            toNode
+                (Expression.LetExpression
+                    { declarations =
+                        List.map
+                            (\decl ->
+                                case Node.value decl of
+                                    Expression.LetFunction function ->
+                                        let
+                                            declaration : Expression.FunctionImplementation
+                                            declaration =
+                                                Node.value function.declaration
+                                        in
+                                        toNode
+                                            (Expression.LetFunction
+                                                { documentation = Nothing
+                                                , signature = Nothing
+                                                , declaration =
+                                                    toNode
+                                                        { name = toNode (Node.value declaration.name)
+                                                        , arguments = List.map normalizePattern declaration.arguments
+                                                        , expression = normalize lookupTable declaration.expression
+                                                        }
+                                                }
+                                            )
+
+                                    Expression.LetDestructuring pattern expr ->
+                                        toNode (Expression.LetDestructuring (normalizePattern pattern) (normalize lookupTable expr))
+                            )
+                            letBlock.declarations
+                    , expression = normalize lookupTable letBlock.expression
+                    }
+                )
+
+        Expression.CaseExpression caseBlock ->
+            toNode
+                (Expression.CaseExpression
+                    { cases = List.map (\( pattern, expr ) -> ( normalizePattern pattern, normalize lookupTable expr )) caseBlock.cases
+                    , expression = toNode <| Node.value caseBlock.expression
+                    }
+                )
+
+        Expression.LambdaExpression lambda ->
+            toNode
+                (Expression.LambdaExpression
+                    { args = List.map normalizePattern lambda.args
+                    , expression = normalize lookupTable lambda.expression
+                    }
+                )
+
+        Expression.ListExpr nodes ->
+            toNode (Expression.ListExpr (List.map (normalize lookupTable) nodes))
+
+        Expression.RecordAccess expr (Node _ field) ->
+            toNode (Expression.RecordAccess (normalize lookupTable expr) (toNode field))
+
+        Expression.RecordExpr nodes ->
+            nodes
+                |> List.sortBy (\(Node _ ( Node _ fieldName, _ )) -> fieldName)
+                |> List.map (\(Node _ ( Node _ fieldName, expr )) -> toNode ( toNode fieldName, normalize lookupTable expr ))
+                |> Expression.RecordExpr
+                |> toNode
+
+        Expression.RecordUpdateExpression (Node _ value) nodes ->
+            nodes
+                |> List.sortBy (\(Node _ ( Node _ fieldName, _ )) -> fieldName)
+                |> List.map (\(Node _ ( Node _ fieldName, expr )) -> toNode ( toNode fieldName, normalize lookupTable expr ))
+                |> Expression.RecordUpdateExpression (toNode value)
+                |> toNode
+
+        expr ->
+            toNode expr
+
+
+normalizePattern : Node Pattern -> Node Pattern
+normalizePattern node =
+    case Node.value node of
+        Pattern.TuplePattern patterns ->
+            toNode (Pattern.TuplePattern (List.map normalizePattern patterns))
+
+        Pattern.RecordPattern fields ->
+            toNode (Pattern.RecordPattern (List.map (\(Node _ field) -> toNode field) fields))
+
+        Pattern.UnConsPattern element list ->
+            toNode (Pattern.UnConsPattern (normalizePattern element) (normalizePattern list))
+
+        Pattern.ListPattern patterns ->
+            toNode (Pattern.ListPattern (List.map normalizePattern patterns))
+
+        Pattern.NamedPattern qualifiedNameRef patterns ->
+            toNode (Pattern.NamedPattern qualifiedNameRef (List.map normalizePattern patterns))
+
+        Pattern.AsPattern pattern (Node _ asName) ->
+            toNode (Pattern.AsPattern (normalizePattern pattern) (toNode asName))
+
+        Pattern.ParenthesizedPattern pattern ->
+            normalizePattern pattern
+
+        pattern ->
+            toNode pattern
+
+
+toNode : a -> Node a
+toNode =
+    Node Range.emptyRange
+
+
+
+-- COMPARE
+
+
+type Comparison
+    = ConfirmedEquality
+    | ConfirmedInequality
+    | Unconfirmed
+
+
+compare : ModuleNameLookupTable -> Node Expression -> Node Expression -> Comparison
+compare lookupTable leftNode right =
+    compareHelp lookupTable leftNode right True
+
+
+compareHelp : ModuleNameLookupTable -> Node Expression -> Node Expression -> Bool -> Comparison
+compareHelp lookupTable leftNode right canFlip =
+    let
+        fallback : Node Expression -> Comparison
+        fallback rightNode =
+            if canFlip then
+                compareHelp lookupTable rightNode leftNode False
+
+            else if areTheSame lookupTable leftNode right then
+                ConfirmedEquality
+
+            else
+                Unconfirmed
+    in
+    case Node.value leftNode of
+        Expression.ParenthesizedExpression expr ->
+            compareHelp lookupTable expr right canFlip
+
+        Expression.Integer left ->
+            compareNumbers (Basics.toFloat left) right
+
+        Expression.Floatable left ->
+            compareNumbers left right
+
+        Expression.Hex left ->
+            compareNumbers (Basics.toFloat left) right
+
+        Expression.Negation left ->
+            case getNumberValue left of
+                Just leftValue ->
+                    compareNumbers -leftValue right
+
+                Nothing ->
+                    fallback right
+
+        Expression.OperatorApplication leftOp _ leftLeft leftRight ->
+            if List.member leftOp [ "+", "-", "*", "/" ] then
+                case getNumberValue leftNode of
+                    Just leftValue ->
+                        case getNumberValue right of
+                            Just rightValue ->
+                                fromEquality (leftValue == rightValue)
+
+                            Nothing ->
+                                fallback right
+
+                    Nothing ->
+                        fallback right
+
+            else
+                case Node.value (removeParens right) of
+                    Expression.OperatorApplication rightOp _ rightLeft rightRight ->
+                        if leftOp == rightOp then
+                            compareEqualityOfAll
+                                lookupTable
+                                [ leftLeft, leftRight ]
+                                [ rightLeft, rightRight ]
+
+                        else
+                            fallback right
+
+                    _ ->
+                        fallback right
+
+        Expression.Literal left ->
+            case Node.value (removeParens right) of
+                Expression.Literal rightValue ->
+                    fromEquality (left == rightValue)
+
+                _ ->
+                    fallback right
+
+        Expression.CharLiteral left ->
+            case Node.value (removeParens right) of
+                Expression.CharLiteral rightValue ->
+                    fromEquality (left == rightValue)
+
+                _ ->
+                    fallback right
+
+        Expression.FunctionOrValue _ leftName ->
+            let
+                right_ : Node Expression
+                right_ =
+                    removeParens right
+            in
+            case Node.value right_ of
+                Expression.FunctionOrValue _ rightName ->
+                    if
+                        isSameReference
+                            lookupTable
+                            ( Node.range leftNode, leftName )
+                            ( Node.range right_, rightName )
+                    then
+                        ConfirmedEquality
+
+                    else
+                        fallback right_
+
+                _ ->
+                    fallback right
+
+        Expression.ListExpr leftList ->
+            case Node.value (removeParens right) of
+                Expression.ListExpr rightList ->
+                    if List.length leftList /= List.length rightList then
+                        ConfirmedInequality
+
+                    else
+                        compareLists lookupTable leftList rightList ConfirmedEquality
+
+                _ ->
+                    fallback right
+
+        Expression.TupledExpression leftList ->
+            case Node.value (removeParens right) of
+                Expression.TupledExpression rightList ->
+                    compareLists lookupTable leftList rightList ConfirmedEquality
+
+                _ ->
+                    fallback right
+
+        Expression.RecordExpr leftList ->
+            case Node.value (removeParens right) of
+                Expression.RecordExpr rightList ->
+                    compareRecords lookupTable leftList rightList ConfirmedEquality
+
+                _ ->
+                    fallback right
+
+        Expression.RecordUpdateExpression leftBaseValue leftList ->
+            case Node.value (removeParens right) of
+                Expression.RecordUpdateExpression rightBaseValue rightList ->
+                    if Node.value leftBaseValue == Node.value rightBaseValue then
+                        compareRecords lookupTable leftList rightList ConfirmedEquality
+
+                    else
+                        compareRecords lookupTable leftList rightList Unconfirmed
+
+                _ ->
+                    fallback right
+
+        Expression.Application leftArgs ->
+            case Node.value (removeParens right) of
+                Expression.Application rightArgs ->
+                    compareEqualityOfAll lookupTable leftArgs rightArgs
+
+                _ ->
+                    fallback right
+
+        Expression.RecordAccess leftExpr leftName ->
+            case Node.value (removeParens right) of
+                Expression.RecordAccess rightExpr rightName ->
+                    if Node.value leftName == Node.value rightName then
+                        compareHelp lookupTable leftExpr rightExpr canFlip
+
+                    else
+                        Unconfirmed
+
+                _ ->
+                    fallback right
+
+        Expression.UnitExpr ->
+            ConfirmedEquality
+
+        Expression.IfBlock leftCond leftThen leftElse ->
+            case Node.value (removeParens right) of
+                Expression.IfBlock rightCond rightThen rightElse ->
+                    compareEqualityOfAll
+                        lookupTable
+                        [ leftCond, leftThen, leftElse ]
+                        [ rightCond, rightThen, rightElse ]
+
+                _ ->
+                    fallback right
+
+        _ ->
+            fallback right
+
+
+isSameReference : ModuleNameLookupTable -> ( Range, String ) -> ( Range, String ) -> Bool
+isSameReference lookupTable ( leftFnRange, leftFnName ) ( rightFnRange, rightFnName ) =
+    if leftFnName == rightFnName then
+        Maybe.map2 (==)
+            (ModuleNameLookupTable.moduleNameAt lookupTable leftFnRange)
+            (ModuleNameLookupTable.moduleNameAt lookupTable rightFnRange)
+            |> Maybe.withDefault False
+
+    else
+        False
+
+
+compareNumbers : Float -> Node Expression -> Comparison
+compareNumbers leftValue right =
+    case getNumberValue right of
+        Just rightValue ->
+            fromEquality (leftValue == rightValue)
+
+        Nothing ->
+            Unconfirmed
+
+
+getNumberValue : Node Expression -> Maybe Float
+getNumberValue node =
+    case Node.value node of
+        Expression.Integer value ->
+            Just (Basics.toFloat value)
+
+        Expression.Hex int ->
+            Just (Basics.toFloat int)
+
+        Expression.Floatable float ->
+            Just float
+
+        Expression.ParenthesizedExpression expr ->
+            getNumberValue expr
+
+        Expression.LetExpression { expression } ->
+            getNumberValue expression
+
+        Expression.OperatorApplication "+" _ left right ->
+            Maybe.map2 (+)
+                (getNumberValue left)
+                (getNumberValue right)
+
+        Expression.OperatorApplication "-" _ left right ->
+            Maybe.map2 (-)
+                (getNumberValue left)
+                (getNumberValue right)
+
+        Expression.OperatorApplication "*" _ left right ->
+            Maybe.map2 (*)
+                (getNumberValue left)
+                (getNumberValue right)
+
+        Expression.OperatorApplication "/" _ left right ->
+            Maybe.map2 (/)
+                (getNumberValue left)
+                (getNumberValue right)
+
+        Expression.Negation expr ->
+            getNumberValue expr
+                |> Maybe.map negate
+
+        _ ->
+            Nothing
+
+
+compareLists : ModuleNameLookupTable -> List (Node Expression) -> List (Node Expression) -> Comparison -> Comparison
+compareLists lookupTable leftList rightList acc =
+    case ( leftList, rightList ) of
+        ( left :: restOfLeft, right :: restOfRight ) ->
+            case compareHelp lookupTable left right True of
+                ConfirmedEquality ->
+                    compareLists lookupTable restOfLeft restOfRight acc
+
+                ConfirmedInequality ->
+                    ConfirmedInequality
+
+                Unconfirmed ->
+                    compareLists lookupTable restOfLeft restOfRight Unconfirmed
+
+        _ ->
+            acc
+
+
+compareEqualityOfAll : ModuleNameLookupTable -> List (Node Expression) -> List (Node Expression) -> Comparison
+compareEqualityOfAll lookupTable leftList rightList =
+    case ( leftList, rightList ) of
+        ( left :: restOfLeft, right :: restOfRight ) ->
+            case compareHelp lookupTable left right True of
+                ConfirmedEquality ->
+                    compareEqualityOfAll lookupTable restOfLeft restOfRight
+
+                ConfirmedInequality ->
+                    Unconfirmed
+
+                Unconfirmed ->
+                    Unconfirmed
+
+        _ ->
+            ConfirmedEquality
+
+
+type RecordFieldComparison
+    = MissingOtherValue
+    | HasBothValues (Node Expression) (Node Expression)
+
+
+compareRecords : ModuleNameLookupTable -> List (Node Expression.RecordSetter) -> List (Node Expression.RecordSetter) -> Comparison -> Comparison
+compareRecords lookupTable leftList rightList acc =
+    let
+        leftFields : List ( String, Node Expression )
+        leftFields =
+            List.map (Node.value >> Tuple.mapFirst Node.value) leftList
+
+        rightFields : List ( String, Node Expression )
+        rightFields =
+            List.map (Node.value >> Tuple.mapFirst Node.value) rightList
+
+        recordFieldComparisons : List RecordFieldComparison
+        recordFieldComparisons =
+            Dict.merge
+                (\key _ -> Dict.insert key MissingOtherValue)
+                (\key a b -> Dict.insert key (HasBothValues a b))
+                (\key _ -> Dict.insert key MissingOtherValue)
+                (Dict.fromList leftFields)
+                (Dict.fromList rightFields)
+                Dict.empty
+                |> Dict.values
+    in
+    compareRecordFields lookupTable recordFieldComparisons acc
+
+
+compareRecordFields : ModuleNameLookupTable -> List RecordFieldComparison -> Comparison -> Comparison
+compareRecordFields lookupTable recordFieldComparisons acc =
+    case recordFieldComparisons of
+        [] ->
+            acc
+
+        MissingOtherValue :: rest ->
+            compareRecordFields lookupTable rest Unconfirmed
+
+        (HasBothValues a b) :: rest ->
+            case compare lookupTable a b of
+                ConfirmedInequality ->
+                    ConfirmedInequality
+
+                ConfirmedEquality ->
+                    compareRecordFields lookupTable rest acc
+
+                Unconfirmed ->
+                    compareRecordFields lookupTable rest Unconfirmed
+
+
+fromEquality : Bool -> Comparison
+fromEquality bool =
+    if bool then
+        ConfirmedEquality
+
+    else
+        ConfirmedInequality
+
+
+removeParens : Node Expression -> Node Expression
+removeParens node =
+    case Node.value node of
+        Expression.ParenthesizedExpression expr ->
+            removeParens expr
+
+        _ ->
+            node

tests/TestProject.elm

@@ -0,0 +1,134 @@
+module TestProject exposing (application, lamderaApplication, package)
+
+import Elm.Package
+import Elm.Project
+import Elm.Version
+import Json.Decode as Decode
+import Review.Project as Project exposing (Project)
+
+
+application : Project
+application =
+    Project.new
+        |> Project.addElmJson applicationElmJson
+
+
+lamderaApplication : Project
+lamderaApplication =
+    Project.new
+        |> Project.addElmJson lamderaApplicationElmJson
+
+
+applicationElmJson : { path : String, raw : String, project : Elm.Project.Project }
+applicationElmJson =
+    { path = "elm.json"
+    , raw = """{
+    "type": "application",
+    "source-directories": [
+        "src"
+    ],
+    "elm-version": "0.19.1",
+    "dependencies": {
+        "direct": {
+            "elm/core": "1.0.0"
+        },
+        "indirect": {}
+    },
+    "test-dependencies": {
+        "direct": {},
+        "indirect": {}
+    }
+}"""
+    , project =
+        Elm.Project.Application
+            { elm = Elm.Version.one
+            , dirs = []
+            , depsDirect = [ ( unsafePackageName "elm/core", Elm.Version.one ) ]
+            , depsIndirect = []
+            , testDepsDirect = []
+            , testDepsIndirect = []
+            }
+    }
+
+
+lamderaApplicationElmJson : { path : String, raw : String, project : Elm.Project.Project }
+lamderaApplicationElmJson =
+    { path = "elm.json"
+    , raw = """{
+    "type": "application",
+    "source-directories": [
+        "src"
+    ],
+    "elm-version": "0.19.1",
+    "dependencies": {
+        "direct": {
+            "elm/core": "1.0.0",
+            "lamdera/core": "1.0.0"
+        },
+        "indirect": {}
+    },
+    "test-dependencies": {
+        "direct": {},
+        "indirect": {}
+    }
+}"""
+    , project =
+        Elm.Project.Application
+            { elm = Elm.Version.one
+            , dirs = []
+            , depsDirect =
+                [ ( unsafePackageName "elm/core", Elm.Version.one )
+                , ( unsafePackageName "lamdera/core", Elm.Version.one )
+                ]
+            , depsIndirect = []
+            , testDepsDirect = []
+            , testDepsIndirect = []
+            }
+    }
+
+
+unsafePackageName : String -> Elm.Package.Name
+unsafePackageName packageName =
+    case Elm.Package.fromString packageName of
+        Just name ->
+            name
+
+        Nothing ->
+            -- unsafe, but if the generation went well, it should all be good.
+            unsafePackageName packageName
+
+
+package : Project
+package =
+    Project.new
+        |> Project.addElmJson (createPackageElmJson ())
+
+
+createPackageElmJson : () -> { path : String, raw : String, project : Elm.Project.Project }
+createPackageElmJson () =
+    case Decode.decodeString Elm.Project.decoder rawPackageElmJson of
+        Ok elmJson ->
+            { path = "elm.json"
+            , raw = rawPackageElmJson
+            , project = elmJson
+            }
+
+        Err err ->
+            Debug.todo ("Invalid elm.json supplied to test: " ++ Debug.toString err)
+
+
+rawPackageElmJson : String
+rawPackageElmJson =
+    """{
+    "type": "package",
+    "name": "author/package",
+    "summary": "Summary",
+    "license": "BSD-3-Clause",
+    "version": "1.0.0",
+    "exposed-modules": [
+        "Exposed"
+    ],
+    "elm-version": "0.19.0 <= v < 0.20.0",
+    "dependencies": {},
+    "test-dependencies": {}
+}"""