Backport rules from elm-review-code-style

tests/NoSimpleLetBody.elm

@@ -8,6 +8,8 @@ module NoSimpleLetBody exposing (rule)
 
 import Elm.Syntax.Expression as Expression exposing (Expression)
 import Elm.Syntax.Node as Node exposing (Node)
+import Elm.Syntax.Range exposing (Location, Range)
+import Review.Fix as Fix
 import Review.Rule as Rule exposing (Rule)
 
 
@@ -89,25 +91,53 @@ expressionVisitor node =
             case Node.value expression of
                 Expression.FunctionOrValue [] name ->
                     let
-                        declared : List String
+                        declarationData :
-                        declared =
+                            { previousEnd : Maybe Location
-                            List.filterMap
+                            , lastEnd : Maybe Location
-                                (\declaration ->
+                            , last : Maybe { name : String, declarationRange : Range, expressionRange : Range }
+                            , foundDeclaredWithName : Bool
+                            }
+                        declarationData =
+                            List.foldl
+                                (\declaration { lastEnd, foundDeclaredWithName } ->
                                     case Node.value declaration of
                                         Expression.LetFunction function ->
-                                            function.declaration
+                                            let
-                                                |> Node.value
+                                                functionDeclaration : Expression.FunctionImplementation
-                                                |> .name
+                                                functionDeclaration =
-                                                |> Node.value
+                                                    Node.value function.declaration
-                                                |> Just
+                                            in
+                                            { previousEnd = lastEnd
+                                            , lastEnd = Just (Node.range functionDeclaration.expression).end
+                                            , last =
+                                                if List.isEmpty functionDeclaration.arguments then
+                                                    Just
+                                                        { name = Node.value functionDeclaration.name
+                                                        , declarationRange = Node.range declaration
+                                                        , expressionRange = Node.range functionDeclaration.expression
+                                                        }
+
+                                                else
+                                                    Nothing
+                                            , foundDeclaredWithName = foundDeclaredWithName || Node.value functionDeclaration.name == name
+                                            }
 
                                         Expression.LetDestructuring _ _ ->
-                                            Nothing
+                                            { previousEnd = lastEnd
+                                            , lastEnd = Just (Node.range declaration).end
+                                            , last = Nothing
+                                            , foundDeclaredWithName = foundDeclaredWithName
+                                            }
                                 )
+                                { previousEnd = Nothing
+                                , lastEnd = Nothing
+                                , last = Nothing
+                                , foundDeclaredWithName = False
+                                }
                                 declarations
                     in
-                    if List.member name declared then
+                    if declarationData.foundDeclaredWithName then
-                        [ Rule.error
+                        [ Rule.errorWithFix
                             { message = "The referenced value should be inlined."
                             , details =
                                 [ "The name of the value is redundant with the surrounding expression."
@@ -115,6 +145,33 @@ expressionVisitor node =
                                 ]
                             }
                             (Node.range expression)
+                            (case declarationData.last of
+                                Just last ->
+                                    if last.name == name then
+                                        case declarationData.previousEnd of
+                                            Nothing ->
+                                                -- It's the only element in the destructuring, we should remove move of the let expression
+                                                [ Fix.removeRange { start = (Node.range node).start, end = last.expressionRange.start }
+                                                , Fix.removeRange { start = last.expressionRange.end, end = (Node.range node).end }
+                                                ]
+
+                                            Just previousEnd ->
+                                                -- There are other elements in the let body that we need to keep
+                                                let
+                                                    indentation : String
+                                                    indentation =
+                                                        String.repeat ((Node.range node).start.column - 1) " "
+                                                in
+                                                [ Fix.replaceRangeBy { start = previousEnd, end = last.expressionRange.start } ("\n" ++ indentation ++ "in\n" ++ indentation)
+                                                , Fix.removeRange { start = last.expressionRange.end, end = (Node.range node).end }
+                                                ]
+
+                                    else
+                                        []
+
+                                Nothing ->
+                                    []
+                            )
                         ]
 
                     else

tests/NoSimpleLetBodyTest.elm

@@ -25,6 +25,95 @@ a = let b = 1
                             , under = "b"
                             }
                             |> Review.Test.atExactly { start = { row = 3, column = 8 }, end = { row = 3, column = 9 } }
+                            |> Review.Test.whenFixed """module A exposing (..)
+a = 1
+"""
+                        ]
+        , test "should report an error when let body is a simple function or value and is the last declaration" <|
+            \() ->
+                """module A exposing (..)
+a = let
+        c = 2
+        b = c
+    in b
+"""
+                    |> Review.Test.run rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "The referenced value should be inlined."
+                            , details =
+                                [ "The name of the value is redundant with the surrounding expression."
+                                , "If you believe that the expression needs a name because it is too complex, consider splitting the expression up more or extracting it to a new function."
+                                ]
+                            , under = "b"
+                            }
+                            |> Review.Test.atExactly { start = { row = 5, column = 8 }, end = { row = 5, column = 9 } }
+                            |> Review.Test.whenFixed """module A exposing (..)
+a = let
+        c = 2
+    in
+    c
+"""
+                        ]
+        , test "should report an error but not suggest a fix when value is not the last declaration" <|
+            \() ->
+                """module A exposing (..)
+a = let
+        b = c
+        c = 2
+    in b
+"""
+                    |> Review.Test.run rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "The referenced value should be inlined."
+                            , details =
+                                [ "The name of the value is redundant with the surrounding expression."
+                                , "If you believe that the expression needs a name because it is too complex, consider splitting the expression up more or extracting it to a new function."
+                                ]
+                            , under = "b"
+                            }
+                            |> Review.Test.atExactly { start = { row = 5, column = 8 }, end = { row = 5, column = 9 } }
+                        ]
+        , test "should report an error but not suggest a fix when value is not the last declaration (last is destructuring)" <|
+            \() ->
+                """module A exposing (..)
+a = let
+        b = c
+        {c} = 2
+    in b
+"""
+                    |> Review.Test.run rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "The referenced value should be inlined."
+                            , details =
+                                [ "The name of the value is redundant with the surrounding expression."
+                                , "If you believe that the expression needs a name because it is too complex, consider splitting the expression up more or extracting it to a new function."
+                                ]
+                            , under = "b"
+                            }
+                            |> Review.Test.atExactly { start = { row = 5, column = 8 }, end = { row = 5, column = 9 } }
+                        ]
+        , test "should report an error but not suggest a fix when value is a function that takes arguments" <|
+            \() ->
+                """module A exposing (..)
+a = let
+        c = 2
+        b n = c
+    in b
+"""
+                    |> Review.Test.run rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "The referenced value should be inlined."
+                            , details =
+                                [ "The name of the value is redundant with the surrounding expression."
+                                , "If you believe that the expression needs a name because it is too complex, consider splitting the expression up more or extracting it to a new function."
+                                ]
+                            , under = "b"
+                            }
+                            |> Review.Test.atExactly { start = { row = 5, column = 8 }, end = { row = 5, column = 9 } }
                         ]
         , test "should not report an error when let body is a function call" <|
             \() ->

tests/NoUnnecessaryTrailingUnderscore.elm

@@ -298,6 +298,9 @@ reservedElmKeywords =
         , "exposing_"
         , "as_"
         , "port_"
+
+        -- Until `elm-format` and `elm-syntax` allow `infix` as an identifier
+        , "infix_"
         ]