From ab9fd57a573bee68a75066bb313ac23ec38679b1 Mon Sep 17 00:00:00 2001 From: perkee Date: Wed, 5 Feb 2025 21:13:48 -0700 Subject: [PATCH] apply fixes including imports --- src/UseHtmlExtraNothing.elm | 51 +++- tests/UseHtmlExtraNothingTest.elm | 475 ++++++++++++++++++++++++------ 2 files changed, 438 insertions(+), 88 deletions(-) diff --git a/src/UseHtmlExtraNothing.elm b/src/UseHtmlExtraNothing.elm index c936435..9149f15 100644 --- a/src/UseHtmlExtraNothing.elm +++ b/src/UseHtmlExtraNothing.elm @@ -58,6 +58,7 @@ rule = Rule.newModuleRuleSchemaUsingContextCreator "UseHtmlExtraNothing" initialContext |> Rule.withImportVisitor importVisitor |> Rule.withExpressionEnterVisitor expressionVisitor + |> Rule.providesFixesForModuleRule |> Rule.fromModuleRuleSchema @@ -96,7 +97,7 @@ toImportContext import_ = |> Maybe.map (\exposingList -> case exposingList of - Elm.Syntax.Exposing.All nodes -> + Elm.Syntax.Exposing.All _ -> AllExposed Elm.Syntax.Exposing.Explicit nodes -> @@ -111,7 +112,6 @@ toImportContext import_ = ) nodes |> SomeExposed - -- (Node.value nodes) ) |> Maybe.withDefault (SomeExposed []) } @@ -141,7 +141,16 @@ importVisitor node context = , { context | importContext = context.importContext |> Dict.insert key value - , firstImport = context.firstImport |> Maybe.withDefault (Node.range node) |> Just + , firstImport = + case ( context.firstImport, key ) of + ( _, [ "Html" ] ) -> + Just (Node.range node) + + ( Nothing, _ ) -> + Just (Node.range node) + + ( Just _, _ ) -> + context.firstImport } ) @@ -160,11 +169,45 @@ expressionVisitor node context = case ( Node.value firstNode, Node.value secondNode ) of ( FunctionOrValue _ "text", Literal "" ) -> if ModuleNameLookupTable.moduleNameFor context.lookupTable firstNode == Just [ "Html" ] then - ( [ Rule.error + ( [ Rule.errorWithFix { message = "Replace `Html.text \"\" with Html.Extra.nothing" , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] } (Node.range node) + (case + Dict.get + [ "Html", "Extra" ] + context.importContext + |> Debug.log "htmlExtraImport" + of + Just { exposedFunctions, moduleAlias, moduleName } -> + [ Fix.replaceRangeBy (Node.range node) + (case exposedFunctions of + AllExposed -> + "nothing" + + SomeExposed exposedFnNames -> + if List.member "nothing" exposedFnNames then + "nothing" + + else + Maybe.withDefault moduleName + moduleAlias + ++ [ "nothing" ] + |> String.join "." + ) + ] + + Nothing -> + case context.firstImport of + Just { start } -> + [ Fix.replaceRangeBy (Node.range node) "Html.Extra.nothing" + , Fix.insertAt start "import Html.Extra\n" + ] + + Nothing -> + [] + ) ] , context ) diff --git a/tests/UseHtmlExtraNothingTest.elm b/tests/UseHtmlExtraNothingTest.elm index 50cae9f..5e1287c 100644 --- a/tests/UseHtmlExtraNothingTest.elm +++ b/tests/UseHtmlExtraNothingTest.elm @@ -21,130 +21,437 @@ run = all : Test all = describe "UseHtmlExtraNothing" - [ test "should not report an error when calling Html.Extra.nothing" <| - \() -> - """module A exposing (..) + [ describe "should not throw report an error when calling" + [ test "calling Html.Extra.nothing" <| + \() -> + """module A exposing (..) import Html.Extra blank = Html.Extra.nothing """ - |> run - |> Review.Test.expectNoErrors - , test "should not report an error when calling Html.Styled.text \"\"" <| - \() -> - """module A exposing (..) + |> run + |> Review.Test.expectNoErrors + , test " Html.Styled.text \"\"" <| + \() -> + """module A exposing (..) import Html.Styled as Html blank = Html.text "" """ - |> run - |> Review.Test.expectNoErrors - , test "should not report an error when calling aliased Html.Extra.nothing" <| - \() -> - """module A exposing (..) + |> run + |> Review.Test.expectNoErrors + , test " aliased Html.Extra.nothing" <| + \() -> + """module A exposing (..) import Html.Extra as Html blank = Html.nothing """ - |> run - |> Review.Test.expectNoErrors - , test "should not report an error when calling exposed Html.Extra.nothing" <| - \() -> - """module A exposing (..) + |> run + |> Review.Test.expectNoErrors + , test "exposed Html.Extra.nothing" <| + \() -> + """module A exposing (..) import Html.Extra exposing (nothing) blank = nothing """ - |> run - |> Review.Test.expectNoErrors - , test "should not report an error when calling everything-exposed Html.Extra.nothing" <| - \() -> - """module A exposing (..) + |> run + |> Review.Test.expectNoErrors + , test "everything-exposed Html.Extra.nothing" <| + \() -> + """module A exposing (..) import Html.Extra exposing (..) blank = nothing """ - |> run - |> Review.Test.expectNoErrors - , test "should report an error when calling `Html.text \"\"`" <| - \() -> - """module A exposing (..) + |> run + |> Review.Test.expectNoErrors + ] + , describe "Should fix `Html.text \"\"`" + [ describe "when importing Html by plain `import Html`" + [ test "and Html.Extra is not imported" <| + \() -> + """module A exposing (..) import Html a = Html.text "" """ - |> run - |> Review.Test.expectErrors - [ Review.Test.error - { message = "Replace `Html.text \"\" with Html.Extra.nothing" - , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] - , under = "Html.text \"\"" - } - ] - , test "should report an error when calling aliased `Html.text \"\"`" <| - \() -> - """module A exposing (..) + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) +import Html.Extra +import Html + +a = Html.Extra.nothing +""" + ] + , describe "when `Html.Extra` is imported aliased and exposes" + [ test "`nothing`" <| + \() -> + """module A exposing (..) + + +import Html +import Html.Extra as Html exposing (nothing) + +a = Html.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html +import Html.Extra as Html exposing (nothing) + +a = nothing +""" + ] + , test "exposes other things but not `nothing`" <| + \() -> + """module A exposing (..) + + +import Html +import Html.Extra as Html exposing (viewIf) + +a = Html.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html +import Html.Extra as Html exposing (viewIf) + +a = Html.nothing +""" + ] + , test "∅" <| + \() -> + """module A exposing (..) + + +import Html +import Html.Extra as Html + +a = Html.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html +import Html.Extra as Html + +a = Html.nothing +""" + ] + ] + ] + , describe "when `Html.Extra` is imported un-aliased and exposes" + [ test "`nothing`" <| + \() -> + """module A exposing (..) + + +import Html +import Html.Extra exposing (nothing) + +a = Html.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html +import Html.Extra exposing (nothing) + +a = nothing +""" + ] + , test "exposes other things but not `nothing`" <| + \() -> + """module A exposing (..) + + +import Html +import Html.Extra exposing (viewIf) + +a = Html.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html +import Html.Extra exposing (viewIf) + +a = Html.Extra.nothing +""" + ] + , test "∅" <| + \() -> + """module A exposing (..) + + +import Html +import Html.Extra + +a = Html.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "Html.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html +import Html.Extra + +a = Html.Extra.nothing +""" + ] + ] + ] + , describe "when importing Html aliased" + [ test "and Html.Extra is not imported" <| + \() -> + """module A exposing (..) + + +import Html as H + +a = H.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html.Extra import Html as H +a = Html.Extra.nothing +""" + ] + , describe "when `Html.Extra` is imported aliased and exposes" + [ test "`nothing`" <| + \() -> + """module A exposing (..) + + +import Html as H +import Html.Extra as H exposing (nothing) a = H.text "" - """ - |> run - |> Review.Test.expectErrors - [ Review.Test.error - { message = "Replace `Html.text \"\" with Html.Extra.nothing" - , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] - , under = "H.text \"\"" - } - ] - , test "should report an error when calling exposed `Html.text \"\"`" <| - \() -> - """module A exposing (..) - - -import Html as H exposing (text) - - -a = text "" - """ - |> run - |> Review.Test.expectErrors - [ Review.Test.error - { message = "Replace `Html.text \"\" with Html.Extra.nothing" - , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] - , under = "text \"\"" - } - ] - , test "should report an error when calling entirely exposed `Html.text \"\"`" <| - \() -> - """module A exposing (..) - - -import Html as H exposing (..) - - -a = text "" - """ - |> run - |> Review.Test.expectErrors - [ Review.Test.error - { message = "Replace `Html.text \"\" with Html.Extra.nothing" - , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] - , under = "text \"\"" - } - ] +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html as H +import Html.Extra as H exposing (nothing) + +a = nothing +""" + ] + , test "exposes other things but not `nothing`" <| + \() -> + """module A exposing (..) + + +import Html as H +import Html.Extra as H exposing (viewIf) + +a = H.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html as H +import Html.Extra as H exposing (viewIf) + +a = H.nothing +""" + ] + , test "∅" <| + \() -> + """module A exposing (..) + + +import Html as H +import Html.Extra as H + +a = H.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html as H +import Html.Extra as H + +a = H.nothing +""" + ] + ] + ] + , describe "when `Html.Extra` is imported un-aliased and exposes" + [ test "`nothing`" <| + \() -> + """module A exposing (..) + + +import Html as H +import Html.Extra exposing (nothing) + +a = H.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html as H +import Html.Extra exposing (nothing) + +a = nothing +""" + ] + , test "exposes other things but not `nothing`" <| + \() -> + """module A exposing (..) + + +import Html as H +import Html.Extra exposing (viewIf) + +a = H.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html as H +import Html.Extra exposing (viewIf) + +a = Html.Extra.nothing +""" + ] + , test "∅" <| + \() -> + """module A exposing (..) + + +import Html as H +import Html.Extra + +a = H.text "" +""" + |> run + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Replace `Html.text \"\" with Html.Extra.nothing" + , details = [ "We prefer Html.Extra.nothing when we must create an empty node." ] + , under = "H.text \"\"" + } + |> Review.Test.whenFixed """module A exposing (..) + + +import Html as H +import Html.Extra + +a = Html.Extra.nothing +""" + ] + ] ]