From a821e1f83b3ecbb0e6dcb217487b4c3818eac56b Mon Sep 17 00:00:00 2001 From: Mateus Date: Mon, 15 Jul 2024 23:45:08 -0300 Subject: [PATCH 1/3] fix: add tests based on issue --- tests/Install/InitializerTest.elm | 224 +++++++++++++++++++++++++++++- 1 file changed, 218 insertions(+), 6 deletions(-) diff --git a/tests/Install/InitializerTest.elm b/tests/Install/InitializerTest.elm index eae9f80..48fbfb0 100644 --- a/tests/Install/InitializerTest.elm +++ b/tests/Install/InitializerTest.elm @@ -8,7 +8,11 @@ import Test exposing (Test, describe) all : Test all = describe "Install.Initializer" - [ Run.testFix test1, Run.testFix test2 ] + [ Run.testFix test1 + , Run.testFix test2 + , Run.testFix test3 + , Run.testFix test4 + ] test1 = @@ -30,8 +34,7 @@ init = , fixed = """module Client exposing (..) init : (Model, Cmd Msg) init = - ( { age = 30 - , name = "Nancy" + ( { age = 30, name = "Nancy" } , Cmd.none ) @@ -59,12 +62,221 @@ init = , fixed = """module Client exposing (..) init : (Model, Cmd Msg) init = - ( { age = 30 - , name = "Nancy" - , count = 0 + ( { age = 30, name = "Nancy", count = 0 } , Cmd.none ) """ , message = "Add fields to the model" } + + +test3 = + { description = "should insert a field in a function with multiple arguments" + , src = src3 + , rule = rule3 + , under = under3 + , fixed = fixed3 + , message = "Add fields to the model" + } + + +src3 = + """module Frontend exposing (app) + +import Browser +import Browser.Dom +import Browser.Events +import Browser.Navigation +import Json.Decode +import Lamdera exposing (sendToBackend) +import Route +import Task +import Time +import Types exposing (..) +import Url +import View.Main + + +{-| Lamdera applications define 'app' instead of 'main'. + +Lamdera.frontend is the same as Browser.application with the +additional update function; updateFromBackend. + +-} +app = + Lamdera.frontend + { init = init + , onUrlRequest = UrlClicked + , onUrlChange = UrlChanged + , update = update + , updateFromBackend = updateFromBackend + , subscriptions = subscriptions + , view = View.Main.view + } + + +subscriptions : FrontendModel -> Sub FrontendMsg +subscriptions _ = + Sub.batch + [ Browser.Events.onResize GotWindowSize + , Browser.Events.onMouseUp (Json.Decode.succeed MouseDown) + , Time.every 1000 Tick + ] + + +init : Url.Url -> Browser.Navigation.Key -> ( FrontendModel, Cmd FrontendMsg ) +init url key = + let + route = + Route.decode url + in + ( Loading + { key = key + , initUrl = url + , now = Time.millisToPosix 0 + , window = Nothing + , route = route + } + , Cmd.batch + [ Browser.Dom.getViewport + |> Task.perform (\\{ viewport } -> GotWindowSize (round viewport.width) (round viewport.height)) + ] + )""" + + +rule3 = + Install.Initializer.makeRule "Frontend" "init" [ { field = "authFlow", value = "Auth.Common.Idle" } ] + + +fixed3 = + """module Frontend exposing (app) + +import Browser +import Browser.Dom +import Browser.Events +import Browser.Navigation +import Json.Decode +import Lamdera exposing (sendToBackend) +import Route +import Task +import Time +import Types exposing (..) +import Url +import View.Main + + +{-| Lamdera applications define 'app' instead of 'main'. + +Lamdera.frontend is the same as Browser.application with the +additional update function; updateFromBackend. + +-} +app = + Lamdera.frontend + { init = init + , onUrlRequest = UrlClicked + , onUrlChange = UrlChanged + , update = update + , updateFromBackend = updateFromBackend + , subscriptions = subscriptions + , view = View.Main.view + } + + +subscriptions : FrontendModel -> Sub FrontendMsg +subscriptions _ = + Sub.batch + [ Browser.Events.onResize GotWindowSize + , Browser.Events.onMouseUp (Json.Decode.succeed MouseDown) + , Time.every 1000 Tick + ] + + +init : Url.Url -> Browser.Navigation.Key -> ( FrontendModel, Cmd FrontendMsg ) +init url key = + let + route = + Route.decode url + in + ( Loading + { key = key + , initUrl = url + , now = Time.millisToPosix 0 + , window = Nothing + , route = route, authFlow = Auth.Common.Idle + } + , Cmd.batch + [ Browser.Dom.getViewport + |> Task.perform (\\{ viewport } -> GotWindowSize (round viewport.width) (round viewport.height)) + ] + )""" + + +under3 = + """init url key = + let + route = + Route.decode url + in + ( Loading + { key = key + , initUrl = url + , now = Time.millisToPosix 0 + , window = Nothing + , route = route + } + , Cmd.batch + [ Browser.Dom.getViewport + |> Task.perform (\\{ viewport } -> GotWindowSize (round viewport.width) (round viewport.height)) + ] + )""" + + +test4 = + { description = "should insert multiple fields in a function with multiple arguments" + , src = src4 + , rule = rule4 + , under = under4 + , fixed = fixed4 + , message = "Add fields to the model" + } + + +rule4 = + Install.Initializer.makeRule "Backend" + "init" + [ { field = "pendingAuths", value = "Dict.empty" } + , { field = "sessions", value = "Dict.empty" } + , { field = "users", value = "Dict.empty" } + ] + + +src4 = + """module Backend exposing (app, init) + +init : ( BackendModel, Cmd BackendMsg ) +init = + ( { counter = 0 + } + , Cmd.none + )""" + + +under4 = + """init = + ( { counter = 0 + } + , Cmd.none + )""" + + +fixed4 = + """module Backend exposing (app, init) + +init : ( BackendModel, Cmd BackendMsg ) +init = + ( { counter = 0, pendingAuths = Dict.empty, sessions = Dict.empty, users = Dict.empty + } + , Cmd.none + )""" From 0b76dcb9a495e9c00ef8b469ee349422b6f13c85 Mon Sep 17 00:00:00 2001 From: Mateus Date: Mon, 15 Jul 2024 23:45:37 -0300 Subject: [PATCH 2/3] fix: cover more cases when searching for lastRange and fieldNames --- src/Install/Library.elm | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/Install/Library.elm b/src/Install/Library.elm index 285f823..9ffbd86 100644 --- a/src/Install/Library.elm +++ b/src/Install/Library.elm @@ -7,6 +7,7 @@ import Elm.Syntax.ModuleName exposing (ModuleName) import Elm.Syntax.Node as Node exposing (Node(..)) import Elm.Syntax.Pattern exposing (Pattern(..)) import Elm.Syntax.Range as Range exposing (Range) +import List.Extra import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable) import Review.Rule as Rule import Set exposing (Set) @@ -28,6 +29,9 @@ fieldNames expr = RecordExpr fields -> List.map (\(Node _ ( name, _ )) -> Node.value name) fields + Application children -> + List.concatMap (Node.value >> fieldNames) children + _ -> [] @@ -36,9 +40,33 @@ lastRange : Expression -> Range lastRange expr = case expr of RecordExpr fields -> - List.map (\(Node rg _) -> rg) fields + fields |> List.reverse |> List.head + |> Maybe.map + (\setter -> + Node.value setter + |> Tuple.second + |> Node.range + ) + |> Maybe.withDefault Range.empty + + Application children -> + List.Extra.findMap + (\child -> + let + expression = + Node.value child + in + case expression of + RecordExpr _ -> + Just expression + + _ -> + Nothing + ) + children + |> Maybe.map lastRange |> Maybe.withDefault Range.empty _ -> From e4ad0f6d8399b96bd34dcc232df53c067ef7eb2b Mon Sep 17 00:00:00 2001 From: Mateus Date: Mon, 15 Jul 2024 23:45:56 -0300 Subject: [PATCH 3/3] fix: cover more cases of init syntax --- src/Install/Initializer.elm | 78 +++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 29 deletions(-) diff --git a/src/Install/Initializer.elm b/src/Install/Initializer.elm index c3c8f65..6262408 100644 --- a/src/Install/Initializer.elm +++ b/src/Install/Initializer.elm @@ -31,6 +31,7 @@ import Elm.Syntax.ModuleName exposing (ModuleName) import Elm.Syntax.Node as Node exposing (Node(..)) import Elm.Syntax.Range exposing (Range) import Install.Library +import List.Extra import Review.Fix as Fix exposing (Fix) import Review.Rule as Rule exposing (Error, Rule) import Set exposing (Set) @@ -93,10 +94,6 @@ declarationVisitor moduleName functionName data (Node _ declaration) context = isInCorrectModule = Install.Library.isInCorrectModule moduleName context - - namespace : String - namespace = - String.join "." context.moduleName ++ "." ++ name in if name == functionName && isInCorrectModule then visitFunction data Set.empty function context @@ -116,30 +113,7 @@ visitFunction data ignored function context = Node.value function.declaration ( fieldNames, lastRange ) = - case declaration.expression |> Node.value of - TupledExpression expressions -> - let - lastRange_ = - case expressions |> List.map Node.value |> List.head of - Just recordExpr -> - Install.Library.lastRange recordExpr - - Nothing -> - Elm.Syntax.Range.empty - - fieldNames_ : List String - fieldNames_ = - case expressions |> List.map Node.value |> List.head of - Just recordExpr -> - Install.Library.fieldNames recordExpr - - Nothing -> - [] - in - ( fieldNames_, lastRange_ ) - - _ -> - ( [], Elm.Syntax.Range.empty ) + getFieldNamesAndLastRange (Node.value declaration.expression) existingFields = Set.fromList fieldNames @@ -180,10 +154,56 @@ addMissingCases : { row : Int, column : Int } -> List { field : String, value : addMissingCases insertionPoint data = let insertion = - "\n , " ++ (List.map (\{ field, value } -> field ++ " = " ++ value) data |> String.join "\n , ") + ", " ++ (List.map (\{ field, value } -> field ++ " = " ++ value) data |> String.join ", ") in Fix.insertAt { row = insertionPoint.row , column = insertionPoint.column + 4 } insertion + + +getFieldNamesAndLastRange : Expression -> ( List String, Range ) +getFieldNamesAndLastRange expr = + case expr of + TupledExpression expressions -> + let + lastRange_ = + case expressions |> List.head |> Maybe.map Node.value of + Just expression -> + Install.Library.lastRange expression + + Nothing -> + Elm.Syntax.Range.empty + + fieldNames_ : List String + fieldNames_ = + case expressions |> List.map Node.value |> List.head of + Just recordExpr -> + Install.Library.fieldNames recordExpr + + Nothing -> + [] + in + ( fieldNames_, lastRange_ ) + + LetExpression { expression } -> + getFieldNamesAndLastRange (Node.value expression) + + Application children -> + children + |> List.Extra.find + (\child -> + case Node.value child of + TupledExpression _ -> + True + + _ -> + False + ) + |> Maybe.map Node.value + |> Maybe.withDefault (TupledExpression []) + |> getFieldNamesAndLastRange + + _ -> + ( [], Elm.Syntax.Range.empty )