Backport scope v0.2.0

2024-12-24 02:02:29 +03:00 · 2020-05-16 22:33:36 +02:00 · 2020-05-16 22:33:36 +02:00 · ee8f294c18
commit ee8f294c18
parent 52fd8d968e
8 changed files with 532 additions and 293 deletions
--- a/tests/MiscRules/NoHtmlButton.elm
+++ b/tests/MiscRules/NoHtmlButton.elm
@ -32,7 +32,7 @@ expressionVisitor : Node Expression -> Direction -> Context -> ( List (Error {})
 expressionVisitor node direction context =
    case ( direction, Node.value node ) of
        ( Rule.OnEnter, FunctionOrValue moduleName "button" ) ->
-            if Scope.realModuleName context.scope "button" moduleName == [ "Html" ] then
+            if Scope.moduleNameForValue context.scope "button" moduleName == [ "Html" ] then
                ( [ Rule.error
                        { message = "Do not use `Html.button` directly"
                        , details = [ "At fruits.com, we've built a nice `Button` module that suits our needs better. Using this module instead of `Html.button` ensures we have a consistent button experience across the website." ]
--- a/tests/ModuleNameForTypeTest.elm
+++ b/tests/ModuleNameForTypeTest.elm
@ -0,0 +1,202 @@
+module ModuleNameForTypeTest exposing (all)
+
+import Elm.Syntax.Declaration as Declaration exposing (Declaration)
+import Elm.Syntax.Node as Node exposing (Node(..))
+import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
+import Fixtures.Dependencies as Dependencies
+import Review.Project as Project exposing (Project)
+import Review.Rule as Rule exposing (Error, Rule)
+import Review.Test
+import Scope
+import Test exposing (Test, test)
+
+
+all : Test
+all =
+    Test.describe "Scope.moduleNameForType"
+        [ test "should return the module that defined the type" <|
+            \() ->
+                [ """module A exposing (..)
+import Bar as Baz exposing (baz)
+import ExposesSomeThings exposing (..)
+import ExposesEverything exposing (..)
+import Foo.Bar
+import Html exposing (..)
+import Http exposing (get)
+import Something.B as Something
+
+type A = B | C
+type Role = NormalUser Bool | Admin (Maybe A)
+type alias User =
+  { role : Role
+  , age : ( Msg, Unknown )
+  }
+type alias GenericRecord generic = { generic | foo : A }
+
+a : SomeCustomType -> SomeTypeAlias -> SomeOtherTypeAlias -> NonExposedCustomType
+a = 1
+""", """module ExposesSomeThings exposing (SomeOtherTypeAlias)
+type NonExposedCustomType = Variant
+type alias SomeOtherTypeAlias = {}
+""", """module ExposesEverything exposing (..)
+type SomeCustomType = VariantA | VariantB
+type alias SomeTypeAlias = {}
+type Msg = SomeMsgToBeShadowed | SomeOtherMsg
+""", """module Something.B exposing (..)
+b = 1
+type Foo = Bar
+type alias BAlias = {}
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData project projectRule
+                    |> Review.Test.expectErrorsForModules
+                        [ ( "A"
+                          , [ Review.Test.error
+                                { message = """
+<nothing>.Bool -> Basics.Bool
+<nothing>.Maybe -> Maybe.Maybe
+<nothing>.A -> <nothing>.A
+<nothing>.Role -> <nothing>.Role
+<nothing>.Msg -> ExposesEverything.Msg
+<nothing>.Unknown -> <nothing>.Unknown
+<nothing>.A -> <nothing>.A
+<nothing>.SomeCustomType -> ExposesEverything.SomeCustomType
+<nothing>.SomeTypeAlias -> ExposesEverything.SomeTypeAlias
+<nothing>.SomeOtherTypeAlias -> ExposesSomeThings.SomeOtherTypeAlias
+<nothing>.NonExposedCustomType -> <nothing>.NonExposedCustomType
+"""
+                                , details = [ "details" ]
+                                , under = "module"
+                                }
+                            ]
+                          )
+                        ]
+        ]
+
+
+type alias ModuleContext =
+    { scope : Scope.ModuleContext
+    , texts : List String
+    }
+
+
+project : Project
+project =
+    Project.new
+        |> Project.addDependency Dependencies.elmCore
+        |> Project.addDependency Dependencies.elmHtml
+
+
+projectRule : Rule
+projectRule =
+    Rule.newProjectRuleSchema "TestRule" { scope = Scope.initialProjectContext }
+        |> Scope.addProjectVisitors
+        |> Rule.withModuleVisitor moduleVisitor
+        |> Rule.withModuleContext
+            { fromProjectToModule =
+                \_ _ projectContext ->
+                    { scope = Scope.fromProjectToModule projectContext.scope
+                    , texts = []
+                    }
+            , fromModuleToProject =
+                \_ moduleNameNode moduleContext ->
+                    { scope = Scope.fromModuleToProject moduleNameNode moduleContext.scope
+                    }
+            , foldProjectContexts = \a b -> { scope = Scope.foldProjectContexts a.scope b.scope }
+            }
+        |> Rule.fromProjectRuleSchema
+
+
+moduleVisitor : Rule.ModuleRuleSchema schemaState ModuleContext -> Rule.ModuleRuleSchema { schemaState | hasAtLeastOneVisitor : () } ModuleContext
+moduleVisitor schema =
+    schema
+        |> Rule.withDeclarationVisitor declarationVisitor
+        |> Rule.withFinalModuleEvaluation finalEvaluation
+
+
+declarationVisitor : Node Declaration -> Rule.Direction -> ModuleContext -> ( List nothing, ModuleContext )
+declarationVisitor node direction context =
+    case ( direction, Node.value node ) of
+        ( Rule.OnEnter, Declaration.CustomTypeDeclaration { constructors } ) ->
+            let
+                types : List String
+                types =
+                    constructors
+                        |> List.concatMap (Node.value >> .arguments)
+                        |> List.concatMap (typeAnnotationNames context.scope)
+            in
+            ( [], { context | texts = context.texts ++ types } )
+
+        ( Rule.OnEnter, Declaration.AliasDeclaration { typeAnnotation } ) ->
+            ( [], { context | texts = context.texts ++ typeAnnotationNames context.scope typeAnnotation } )
+
+        ( Rule.OnEnter, Declaration.FunctionDeclaration function ) ->
+            case function.signature |> Maybe.map (Node.value >> .typeAnnotation) of
+                Nothing ->
+                    ( [], context )
+
+                Just typeAnnotation ->
+                    ( [], { context | texts = context.texts ++ typeAnnotationNames context.scope typeAnnotation } )
+
+        _ ->
+            ( [], context )
+
+
+typeAnnotationNames : Scope.ModuleContext -> Node TypeAnnotation -> List String
+typeAnnotationNames scope typeAnnotation =
+    case Node.value typeAnnotation of
+        TypeAnnotation.GenericType name ->
+            [ "<nothing>." ++ name ++ " -> <generic>" ]
+
+        TypeAnnotation.Typed (Node _ ( moduleName, typeName )) typeParameters ->
+            let
+                nameInCode : String
+                nameInCode =
+                    case moduleName of
+                        [] ->
+                            "<nothing>." ++ typeName
+
+                        _ ->
+                            String.join "." moduleName ++ "." ++ typeName
+
+                realName : String
+                realName =
+                    case Scope.moduleNameForType scope typeName moduleName of
+                        [] ->
+                            "<nothing>." ++ typeName
+
+                        moduleName_ ->
+                            String.join "." moduleName_ ++ "." ++ typeName
+            in
+            (nameInCode ++ " -> " ++ realName)
+                :: List.concatMap (typeAnnotationNames scope) typeParameters
+
+        TypeAnnotation.Unit ->
+            []
+
+        TypeAnnotation.Tupled typeAnnotations ->
+            List.concatMap (typeAnnotationNames scope) typeAnnotations
+
+        TypeAnnotation.Record typeAnnotations ->
+            List.concatMap (Node.value >> Tuple.second >> typeAnnotationNames scope) typeAnnotations
+
+        TypeAnnotation.GenericRecord _ typeAnnotations ->
+            List.concatMap (Node.value >> Tuple.second >> typeAnnotationNames scope) (Node.value typeAnnotations)
+
+        TypeAnnotation.FunctionTypeAnnotation arg returnType ->
+            typeAnnotationNames scope arg ++ typeAnnotationNames scope returnType
+
+
+finalEvaluation : ModuleContext -> List (Error {})
+finalEvaluation context =
+    if List.isEmpty context.texts then
+        []
+
+    else
+        [ Rule.error
+            { message = "\n" ++ String.join "\n" context.texts ++ "\n"
+            , details = [ "details" ]
+            }
+            { start = { row = 1, column = 1 }
+            , end = { row = 1, column = 7 }
+            }
+        ]
--- a/tests/ModuleNameForValueTest.elm
+++ b/tests/ModuleNameForValueTest.elm
@ -1,9 +1,7 @@
-module ScopeTest exposing (all)
+module ModuleNameForValueTest exposing (all)

-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.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
 import Fixtures.Dependencies as Dependencies
 import Review.Project as Project exposing (Project)
 import Review.Rule as Rule exposing (Error, Rule)
@ -14,16 +12,16 @@ import Test exposing (Test, test)

 all : Test
 all =
-    Test.describe "Scope"
-        [ realModuleNameTestsForModuleRule
-        , realModuleNameTestsForProjectRule
+    Test.describe "Scope.moduleNameForValue"
+        [ forModuleRule
+        , forProjectRule
        ]


-realModuleNameTestsForModuleRule : Test
-realModuleNameTestsForModuleRule =
-    Test.describe "Scope.realModuleName (module rule)"
-        [ test "should indicate that module from which a function or value comes from, with knowledge of what is in other modules" <|
+forModuleRule : Test
+forModuleRule =
+    Test.describe "module rule"
+        [ test "should return the module that defined the value" <|
            \() ->
                """module A exposing (..)
 import Bar as Baz exposing (baz)
@ -35,7 +33,6 @@ import Http exposing (get)

 localValue = 1

-a : SomeCustomType -> SomeTypeAlias -> SomeOtherTypeAlias -> NonExposedCustomType
 a = localValue
    unknownValue
    exposedElement
@ -57,10 +54,6 @@ a = localValue
                    |> Review.Test.expectErrors
                        [ Review.Test.error
                            { message = """
-<nothing>.SomeCustomType -> <nothing>.SomeCustomType
-<nothing>.SomeTypeAlias -> <nothing>.SomeTypeAlias
-<nothing>.SomeOtherTypeAlias -> <nothing>.SomeOtherTypeAlias
-<nothing>.NonExposedCustomType -> <nothing>.NonExposedCustomType
 <nothing>.localValue -> <nothing>.localValue
 <nothing>.unknownValue -> <nothing>.unknownValue
 <nothing>.exposedElement -> <nothing>.exposedElement
@ -84,10 +77,10 @@ Http.get -> Http.get
        ]


-realModuleNameTestsForProjectRule : Test
-realModuleNameTestsForProjectRule =
-    Test.describe "Scope.realModuleName (project rule)"
-        [ test "should indicate that module from which a function or value comes from, with knowledge of what is in other modules" <|
+forProjectRule : Test
+forProjectRule =
+    Test.describe "project rule"
+        [ test "should return the module that defined the value" <|
            \() ->
                [ """module A exposing (..)
 import Bar as Baz exposing (baz)
@ -103,7 +96,6 @@ localValue = 1
 localValueValueToBeShadowed = 1
 type Msg = SomeMsgToBeShadowed

-a : SomeCustomType -> SomeTypeAlias -> SomeOtherTypeAlias -> NonExposedCustomType
 a = localValue
    localValueValueToBeShadowed
    SomeMsgToBeShadowed
@ -111,7 +103,6 @@ a = localValue
    Something.b
    Something.c
    Something.BAlias
-    Something.Foo
    Something.Bar
    unknownValue
    exposedElement
@ -151,10 +142,6 @@ c = 1
                        [ ( "A"
                          , [ Review.Test.error
                                { message = """
-<nothing>.SomeCustomType -> ExposesEverything.SomeCustomType
-<nothing>.SomeTypeAlias -> ExposesEverything.SomeTypeAlias
-<nothing>.SomeOtherTypeAlias -> ExposesSomeThings.SomeOtherTypeAlias
-<nothing>.NonExposedCustomType -> <nothing>.NonExposedCustomType
 <nothing>.localValue -> <nothing>.localValue
 <nothing>.localValueValueToBeShadowed -> <nothing>.localValueValueToBeShadowed
 <nothing>.SomeMsgToBeShadowed -> <nothing>.SomeMsgToBeShadowed
@ -162,7 +149,6 @@ c = 1
 Something.b -> Something.B.b
 Something.c -> Something.C.c
 Something.BAlias -> Something.B.BAlias
-Something.Foo -> Something.B.Foo
 Something.Bar -> Something.B.Bar
 <nothing>.unknownValue -> <nothing>.unknownValue
 <nothing>.exposedElement -> ExposesSomeThings.exposedElement
@ -184,45 +170,13 @@ Http.get -> Http.get
                                }
                            ]
                          )
-                        , ( "ExposesSomeThings"
-                          , [ Review.Test.error
-                                { message = ""
-                                , details = [ "details" ]
-                                , under = "module"
-                                }
-                            ]
-                          )
-                        , ( "ExposesEverything"
-                          , [ Review.Test.error
-                                { message = ""
-                                , details = [ "details" ]
-                                , under = "module"
-                                }
-                            ]
-                          )
-                        , ( "Something.B"
-                          , [ Review.Test.error
-                                { message = ""
-                                , details = [ "details" ]
-                                , under = "module"
-                                }
-                            ]
-                          )
-                        , ( "Something.C"
-                          , [ Review.Test.error
-                                { message = ""
-                                , details = [ "details" ]
-                                , under = "module"
-                                }
-                            ]
-                          )
                        ]
        ]


 type alias ModuleContext =
    { scope : Scope.ModuleContext
-    , text : String
+    , texts : List String
    }


@ -242,7 +196,7 @@ projectRule =
            { fromProjectToModule =
                \_ _ projectContext ->
                    { scope = Scope.fromProjectToModule projectContext.scope
-                    , text = ""
+                    , texts = []
                    }
            , fromModuleToProject =
                \_ moduleNameNode moduleContext ->
@ -255,7 +209,7 @@ projectRule =

 moduleRule : Rule
 moduleRule =
-    Rule.newModuleRuleSchema "TestRule" { scope = Scope.initialModuleContext, text = "" }
+    Rule.newModuleRuleSchema "TestRule" { scope = Scope.initialModuleContext, texts = [] }
        |> Scope.addModuleVisitors
        |> moduleVisitor
        |> Rule.fromModuleRuleSchema
@ -264,71 +218,10 @@ moduleRule =
 moduleVisitor : Rule.ModuleRuleSchema schemaState ModuleContext -> Rule.ModuleRuleSchema { schemaState | hasAtLeastOneVisitor : () } ModuleContext
 moduleVisitor schema =
    schema
-        |> Rule.withDeclarationVisitor declarationVisitor
        |> Rule.withExpressionVisitor expressionVisitor
        |> Rule.withFinalModuleEvaluation finalEvaluation


-declarationVisitor : Node Declaration -> Rule.Direction -> ModuleContext -> ( List nothing, ModuleContext )
-declarationVisitor node direction context =
-    case ( direction, Node.value node ) of
-        ( Rule.OnEnter, Declaration.FunctionDeclaration function ) ->
-            case function.signature |> Maybe.map (Node.value >> .typeAnnotation) of
-                Nothing ->
-                    ( [], context )
-
-                Just typeAnnotation ->
-                    ( [], { context | text = context.text ++ "\n" ++ typeAnnotationNames context.scope typeAnnotation } )
-
-        _ ->
-            ( [], context )
-
-
-typeAnnotationNames : Scope.ModuleContext -> Node TypeAnnotation -> String
-typeAnnotationNames scope typeAnnotation =
-    case Node.value typeAnnotation of
-        TypeAnnotation.GenericType name ->
-            "<nothing>." ++ name ++ " -> <generic>"
-
-        TypeAnnotation.Typed (Node _ ( moduleName, typeName )) typeParameters ->
-            -- Elm.Type.Type (String.join "." moduleName ++ "." ++ typeName) (List.map syntaxTypeAnnotationToDocsType typeParameters)
-            let
-                nameInCode : String
-                nameInCode =
-                    case moduleName of
-                        [] ->
-                            "<nothing>." ++ typeName
-
-                        _ ->
-                            String.join "." moduleName ++ "." ++ typeName
-
-                realName : String
-                realName =
-                    case Scope.realModuleName scope typeName moduleName of
-                        [] ->
-                            "<nothing>." ++ typeName
-
-                        moduleName_ ->
-                            String.join "." moduleName_ ++ "." ++ typeName
-            in
-            nameInCode ++ " -> " ++ realName
-
-        TypeAnnotation.Unit ->
-            "unknown"
-
-        TypeAnnotation.Tupled typeAnnotationTypeAnnotationSyntaxElmNodeNodeSyntaxElmListList ->
-            "unknown"
-
-        TypeAnnotation.Record recordDefinitionTypeAnnotationSyntaxElm ->
-            "unknown"
-
-        TypeAnnotation.GenericRecord stringStringNodeNodeSyntaxElm recordDefinitionTypeAnnotationSyntaxElmNodeNodeSyntaxElm ->
-            "unknown"
-
-        TypeAnnotation.FunctionTypeAnnotation arg returnType ->
-            typeAnnotationNames scope arg ++ "\n" ++ typeAnnotationNames scope returnType
-
-
 expressionVisitor : Node Expression -> Rule.Direction -> ModuleContext -> ( List nothing, ModuleContext )
 expressionVisitor node direction context =
    case ( direction, Node.value node ) of
@ -345,14 +238,14 @@ expressionVisitor node direction context =

                realName : String
                realName =
-                    case Scope.realModuleName context.scope name moduleName of
+                    case Scope.moduleNameForValue context.scope name moduleName of
                        [] ->
                            "<nothing>." ++ name

                        moduleName_ ->
                            String.join "." moduleName_ ++ "." ++ name
            in
-            ( [], { context | text = context.text ++ "\n" ++ nameInCode ++ " -> " ++ realName } )
+            ( [], { context | texts = context.texts ++ [ nameInCode ++ " -> " ++ realName ] } )

        _ ->
            ( [], context )
@ -360,7 +253,14 @@ expressionVisitor node direction context =

 finalEvaluation : ModuleContext -> List (Error {})
 finalEvaluation context =
-    [ Rule.error { message = context.text, details = [ "details" ] }
+    if List.isEmpty context.texts then
+        []
+
+    else
+        [ Rule.error
+            { message = "\n" ++ String.join "\n" context.texts
+            , details = [ "details" ]
+            }
            { start = { row = 1, column = 1 }
            , end = { row = 1, column = 7 }
            }
--- a/tests/NoUnused/CustomTypeConstructors.elm
+++ b/tests/NoUnused/CustomTypeConstructors.elm
@ -465,7 +465,7 @@ registerUsedFunctionOrValue moduleName name moduleContext =
        let
            realModuleName : ModuleName
            realModuleName =
-                Scope.realModuleName moduleContext.scope name moduleName
+                Scope.moduleNameForValue moduleContext.scope name moduleName
        in
        { moduleContext
            | usedFunctionsOrValues =
@ -614,7 +614,7 @@ collectTypesUsedAsPhantomVariables scope phantomVariables node =
            let
                realModuleNameOfPhantomContainer : ModuleName
                realModuleNameOfPhantomContainer =
-                    Scope.realModuleName scope name moduleNameOfPhantomContainer
+                    Scope.moduleNameForType scope name moduleNameOfPhantomContainer

                typesUsedInThePhantomVariablePosition : List ( ModuleName, CustomTypeName )
                typesUsedInThePhantomVariablePosition =
@ -625,7 +625,7 @@ collectTypesUsedAsPhantomVariables scope phantomVariables node =
                            (\( _, index ) ->
                                case listAtIndex index params |> Maybe.map Node.value of
                                    Just (TypeAnnotation.Typed (Node.Node _ ( moduleNameOfPhantomVariable, typeName )) _) ->
-                                        Just ( Scope.realModuleName scope typeName moduleNameOfPhantomVariable, typeName )
+                                        Just ( Scope.moduleNameForType scope typeName moduleNameOfPhantomVariable, typeName )

                                    _ ->
                                        Nothing
--- a/tests/NoUnused/CustomTypeConstructorsTest.elm
+++ b/tests/NoUnused/CustomTypeConstructorsTest.elm
@ -559,4 +559,20 @@ type Msg = NoOp
                            ]
                          )
                        ]
+        , test "should not report type constructors in confusing situations" <|
+            \() ->
+                [ """module A exposing (A(..))
+type A = Foo | Bar
+"""
+                , """module B exposing (B(..))
+type B = A | B
+"""
+                , """module C exposing (foo)
+import A exposing (A(..))
+import B exposing (B(..))
+foo = [ (Foo, A), (Bar, B) ]
+"""
+                ]
+                    |> Review.Test.runOnModulesWithProjectData project (rule [])
+                    |> Review.Test.expectNoErrors
        ]
--- a/tests/NoUnused/Exports.elm
+++ b/tests/NoUnused/Exports.elm
@ -404,7 +404,7 @@ testFunctionName scope declaration =
                    |> Maybe.map (Node.value >> .typeAnnotation >> Node.value)
            of
                Just (TypeAnnotation.Typed (Node _ ( moduleName, name )) _) ->
-                    if name == "Test" && Scope.realModuleName scope name moduleName == [ "Test" ] then
+                    if name == "Test" && Scope.moduleNameForType scope name moduleName == [ "Test" ] then
                        function.declaration
                            |> Node.value
                            |> .name
@ -464,7 +464,7 @@ collectTypesFromTypeAnnotation scope node =
            collectTypesFromTypeAnnotation scope a ++ collectTypesFromTypeAnnotation scope b

        TypeAnnotation.Typed (Node _ ( moduleName, name )) params ->
-            ( Scope.realModuleName scope name moduleName, name )
+            ( Scope.moduleNameForType scope name moduleName, name )
                :: List.concatMap (collectTypesFromTypeAnnotation scope) params

        TypeAnnotation.Record list ->
@ -498,7 +498,7 @@ expressionVisitor node direction moduleContext =
        ( Rule.OnEnter, Expression.FunctionOrValue moduleName name ) ->
            ( []
            , registerAsUsed
-                ( Scope.realModuleName moduleContext.scope name moduleName, name )
+                ( Scope.moduleNameForValue moduleContext.scope name moduleName, name )
                moduleContext
            )

--- a/tests/NoUnused/ExportsTest.elm
+++ b/tests/NoUnused/ExportsTest.elm
@ -240,6 +240,19 @@ config = []
 """
                    |> Review.Test.runWithProjectData package_ rule
                    |> Review.Test.expectNoErrors
+        , test "should not report a function in a 'shadowed' module" <|
+            \() ->
+                [ """module Foo exposing (foo)
+foo = 1
+""", """module Bar exposing (bar)
+bar = 2
+""", """module Main exposing (main)
+import Bar as Foo
+import Foo
+main = [ Foo.foo, Foo.bar ]
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData application rule
+                    |> Review.Test.expectNoErrors
        ]


--- a/tests/Scope.elm
+++ b/tests/Scope.elm
@ -2,7 +2,7 @@ module Scope exposing
    ( ModuleContext, addModuleVisitors, initialModuleContext
    , ProjectContext, addProjectVisitors
    , initialProjectContext, fromProjectToModule, fromModuleToProject, foldProjectContexts
-    , realModuleName
+    , moduleNameForValue, moduleNameForType
    )

 {-| Collect and infer information automatically for you
@ -21,13 +21,13 @@ module Scope exposing

 # Access

-@docs realModuleName
+@docs moduleNameForValue, moduleNameForType

 -}

 {- Copied over from https://github.com/jfmengels/elm-review-scope

-   Version: 0.1.1
+   Version: 0.2.0

   Copyright (c) 2020, Jeroen Engels
   All rights reserved.
@ -70,10 +70,13 @@ 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 Elm.Syntax.Signature exposing (Signature)
+import Elm.Syntax.Type
+import Elm.Syntax.TypeAlias
 import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
 import Elm.Type
 import Review.Project.Dependency as Dependency exposing (Dependency)
 import Review.Rule as Rule exposing (Direction)
+import Set exposing (Set)



@ -88,8 +91,10 @@ type ModuleContext

 type alias InnerModuleContext =
    { scopes : Nonempty Scope
+    , localTypes : Set String
    , importAliases : Dict String (List ModuleName)
-    , importedFunctionOrTypes : Dict String (List String)
+    , importedFunctions : Dict String (List String)
+    , importedTypes : Dict String (List String)
    , dependenciesModules : Dict String Elm.Docs.Module
    , modules : Dict ModuleName Elm.Docs.Module
    , exposesEverything : Bool
@ -189,8 +194,10 @@ initialProjectContext =
 fromProjectToModule : ProjectContext -> ModuleContext
 fromProjectToModule (ProjectContext projectContext) =
    { scopes = nonemptyList_fromElement emptyScope
+    , localTypes = Set.empty
    , importAliases = Dict.empty
-    , importedFunctionOrTypes = Dict.empty
+    , importedFunctions = Dict.empty
+    , importedTypes = Dict.empty
    , dependenciesModules = projectContext.dependenciesModules
    , modules = projectContext.modules
    , exposesEverything = False
@ -583,115 +590,123 @@ createFakeImport { moduleName, moduleAlias, exposingList } =
 declarationListVisitor : List (Node Declaration) -> InnerModuleContext -> InnerModuleContext
 declarationListVisitor declarations innerContext =
    List.foldl registerDeclaration innerContext declarations
-        |> (\newInnerContext -> List.foldl registerExposed newInnerContext declarations)


 registerDeclaration : Node Declaration -> InnerModuleContext -> InnerModuleContext
 registerDeclaration declaration innerContext =
-    let
-        scope : Nonempty Scope
-        scope =
-            List.foldl
-                (\( variableType, nameNode ) accumulatorScope ->
-                    registerVariable
-                        { variableType = variableType
-                        , node = nameNode
-                        }
-                        (Node.value nameNode)
-                        accumulatorScope
-                )
-                innerContext.scopes
-                (declarationNameNode declaration)
-    in
-    updateScope innerContext scope
-
-
-declarationNameNode : Node Declaration -> List ( VariableType, Node String )
-declarationNameNode (Node _ declaration) =
-    case declaration of
-        Declaration.FunctionDeclaration function ->
-            [ ( TopLevelVariable
-              , function.declaration
-                    |> Node.value
-                    |> .name
-              )
-            ]
-
-        Declaration.CustomTypeDeclaration { name, constructors } ->
-            ( TopLevelVariable, name )
-                :: List.map
-                    (\constructor ->
-                        ( CustomTypeConstructor
-                        , constructor
-                            |> Node.value
-                            |> .name
-                        )
-                    )
-                    constructors
-
-        Declaration.AliasDeclaration alias_ ->
-            [ ( TopLevelVariable, alias_.name ) ]
-
-        Declaration.PortDeclaration port_ ->
-            [ ( Port, port_.name ) ]
-
-        Declaration.InfixDeclaration _ ->
-            []
-
-        Declaration.Destructuring _ _ ->
-            []
-
-
-registerExposed : Node Declaration -> InnerModuleContext -> InnerModuleContext
-registerExposed declaration innerContext =
    case Node.value declaration of
        Declaration.FunctionDeclaration function ->
            let
-                name : String
-                name =
+                nameNode : Node String
+                nameNode =
                    function.declaration
                        |> Node.value
                        |> .name
-                        |> Node.value
            in
-            if innerContext.exposesEverything || Dict.member name innerContext.exposedNames then
+            innerContext
+                |> addToScope
+                    { variableType = TopLevelVariable
+                    , node = nameNode
+                    }
+                |> registerIfExposed (registerExposedValue function) (Node.value nameNode)
+
+        Declaration.AliasDeclaration alias_ ->
+            { innerContext | localTypes = Set.insert (Node.value alias_.name) innerContext.localTypes }
+                |> addToScope
+                    { variableType = TopLevelVariable
+                    , node = alias_.name
+                    }
+                |> registerIfExposed (registerExposedTypeAlias alias_) (Node.value alias_.name)
+
+        Declaration.CustomTypeDeclaration { name, constructors } ->
+            List.foldl
+                (\constructor innerContext_ ->
+                    let
+                        constructorName : Node String
+                        constructorName =
+                            constructor |> Node.value |> .name
+                    in
+                    addToScope
+                        { variableType = CustomTypeConstructor
+                        , node = constructorName
+                        }
+                        innerContext_
+                )
+                { innerContext | localTypes = Set.insert (Node.value name) innerContext.localTypes }
+                constructors
+                |> registerIfExposed (registerExposedCustomType constructors) (Node.value name)
+
+        Declaration.PortDeclaration signature ->
+            addToScope
+                { variableType = Port
+                , node = signature.name
+                }
+                innerContext
+
+        Declaration.InfixDeclaration _ ->
+            -- TODO Support operators
+            -- I could use help adding this.
+            innerContext
+
+        Declaration.Destructuring _ _ ->
+            -- Not possible in 0.19 code
+            innerContext
+
+
+addToScope : { variableType : VariableType, node : Node String } -> InnerModuleContext -> InnerModuleContext
+addToScope variableData innerContext =
+    let
+        newScopes : Nonempty Scope
+        newScopes =
+            registerVariable
+                variableData
+                (Node.value variableData.node)
+                innerContext.scopes
+    in
+    { innerContext | scopes = newScopes }
+
+
+registerExposedValue : Expression.Function -> String -> InnerModuleContext -> InnerModuleContext
+registerExposedValue function name innerContext =
    { innerContext
        | exposedValues =
            { name = name
-                        , comment = ""
+            , comment =
+                case Maybe.map Node.value function.documentation of
+                    Just str ->
+                        str
+
+                    Nothing ->
+                        ""
            , tipe = convertTypeSignatureToDocsType function.signature
            }
                :: innerContext.exposedValues
    }

-            else
-                innerContext

-        Declaration.CustomTypeDeclaration type_ ->
-            if innerContext.exposesEverything || Dict.member (Node.value type_.name) innerContext.exposedNames then
+registerExposedCustomType : List (Node Elm.Syntax.Type.ValueConstructor) -> String -> InnerModuleContext -> InnerModuleContext
+registerExposedCustomType constructors name innerContext =
    { innerContext
        | exposedUnions =
-                        { name = Node.value type_.name
+            { name = name
            , comment = ""

            -- TODO
            , args = []
            , tags =
-                            type_.constructors
+                constructors
                    -- TODO Constructor args?
                    |> List.map (\constructor -> ( Node.value (Node.value constructor).name, [] ))
            }
                :: innerContext.exposedUnions
    }

-            else
-                innerContext

-        Declaration.AliasDeclaration alias_ ->
-            if innerContext.exposesEverything || Dict.member (Node.value alias_.name) innerContext.exposedNames then
+registerExposedTypeAlias : Elm.Syntax.TypeAlias.TypeAlias -> String -> InnerModuleContext -> InnerModuleContext
+registerExposedTypeAlias alias_ name innerContext =
    { innerContext
        | exposedAliases =
-                        { name = Node.value alias_.name
+            { name = name
            , comment = ""
            , args = []
            , tipe = Elm.Type.Tuple []
@ -699,18 +714,15 @@ registerExposed declaration innerContext =
                :: innerContext.exposedAliases
    }

+
+registerIfExposed : (String -> InnerModuleContext -> InnerModuleContext) -> String -> InnerModuleContext -> InnerModuleContext
+registerIfExposed registerFn name innerContext =
+    if innerContext.exposesEverything || Dict.member name innerContext.exposedNames then
+        registerFn name innerContext
+
    else
        innerContext

-        Declaration.PortDeclaration _ ->
-            innerContext
-
-        Declaration.InfixDeclaration _ ->
-            innerContext
-
-        Declaration.Destructuring _ _ ->
-            innerContext
-

 convertTypeSignatureToDocsType : Maybe (Node Signature) -> Elm.Type.Type
 convertTypeSignatureToDocsType maybeSignature =
@ -756,8 +768,8 @@ registerVariable variableInfo name scopes =


 updateScope : InnerModuleContext -> Nonempty Scope -> InnerModuleContext
-updateScope context scopes =
-    { context | scopes = scopes }
+updateScope innerContext scopes =
+    { innerContext | scopes = scopes }



@ -810,6 +822,22 @@ registerImportAlias : Import -> InnerModuleContext -> InnerModuleContext
 registerImportAlias import_ innerContext =
    case import_.moduleAlias of
        Nothing ->
+            let
+                moduleName : List String
+                moduleName =
+                    Node.value import_.moduleName
+            in
+            case moduleName of
+                singleSegmentModuleName :: [] ->
+                    { innerContext
+                        | importAliases =
+                            Dict.update
+                                singleSegmentModuleName
+                                (\previousValue -> Just <| moduleName :: Maybe.withDefault [] previousValue)
+                                innerContext.importAliases
+                    }
+
+                _ ->
                    innerContext

        Just alias_ ->
@ -864,8 +892,7 @@ registerImportExposed import_ innerContext =
                            List.concat
                                [ List.concatMap
                                    (\union ->
-                                        nameWithModuleName union
-                                            :: List.map (\( name, _ ) -> ( name, moduleName )) union.tags
+                                        List.map (\( name, _ ) -> ( name, moduleName )) union.tags
                                    )
                                    module_.unions
                                , List.map nameWithModuleName module_.values
@ -873,10 +900,18 @@ registerImportExposed import_ innerContext =
                                , List.map nameWithModuleName module_.binops
                                ]
                                |> Dict.fromList
+
+                        exposedTypes : Dict String (List String)
+                        exposedTypes =
+                            List.concat
+                                [ List.map nameWithModuleName module_.unions
+                                , List.map nameWithModuleName module_.aliases
+                                ]
+                                |> Dict.fromList
                    in
                    { innerContext
-                        | importedFunctionOrTypes =
-                            Dict.union innerContext.importedFunctionOrTypes exposedValues
+                        | importedFunctions = Dict.union innerContext.importedFunctions exposedValues
+                        , importedTypes = Dict.union innerContext.importedTypes exposedTypes
                    }

                Exposing.Explicit topLevelExposeList ->
@ -884,18 +919,25 @@ registerImportExposed import_ innerContext =
                        exposedValues : Dict String (List String)
                        exposedValues =
                            topLevelExposeList
-                                |> List.concatMap (namesFromExposingList module_)
+                                |> List.concatMap (valuesFromExposingList module_)
+                                |> List.map (\name -> ( name, moduleName ))
+                                |> Dict.fromList
+
+                        exposedTypes : Dict String (List String)
+                        exposedTypes =
+                            topLevelExposeList
+                                |> List.filterMap typesFromExposingList
                                |> List.map (\name -> ( name, moduleName ))
                                |> Dict.fromList
                    in
                    { innerContext
-                        | importedFunctionOrTypes =
-                            Dict.union innerContext.importedFunctionOrTypes exposedValues
+                        | importedFunctions = Dict.union innerContext.importedFunctions exposedValues
+                        , importedTypes = Dict.union innerContext.importedTypes exposedTypes
                    }


-namesFromExposingList : Elm.Docs.Module -> Node TopLevelExpose -> List String
-namesFromExposingList module_ topLevelExpose =
+valuesFromExposingList : Elm.Docs.Module -> Node TopLevelExpose -> List String
+valuesFromExposingList module_ topLevelExpose =
    case Node.value topLevelExpose of
        Exposing.InfixExpose operator ->
            [ operator ]
@ -903,21 +945,40 @@ namesFromExposingList module_ topLevelExpose =
        Exposing.FunctionExpose function ->
            [ function ]

-        Exposing.TypeOrAliasExpose type_ ->
-            [ type_ ]
+        Exposing.TypeOrAliasExpose name ->
+            if List.any (\alias_ -> alias_.name == name) module_.aliases then
+                [ name ]
+
+            else
+                -- Type is a custom type
+                []

        Exposing.TypeExpose { name, open } ->
            case open of
                Just _ ->
-                    name
-                        :: (module_.unions
+                    module_.unions
                        |> List.filter (\union -> union.name == name)
                        |> List.concatMap .tags
                        |> List.map Tuple.first
-                           )

                Nothing ->
-                    [ name ]
+                    []
+
+
+typesFromExposingList : Node TopLevelExpose -> Maybe String
+typesFromExposingList topLevelExpose =
+    case Node.value topLevelExpose of
+        Exposing.InfixExpose _ ->
+            Nothing
+
+        Exposing.FunctionExpose _ ->
+            Nothing
+
+        Exposing.TypeOrAliasExpose name ->
+            Just name
+
+        Exposing.TypeExpose { name } ->
+            Just name


 unboxProjectContext : ProjectContext -> InnerProjectContext
@ -1029,7 +1090,7 @@ collectNamesFromPattern pattern =


 popScope : Node Expression -> Direction -> InnerModuleContext -> InnerModuleContext
-popScope ((Node range value) as node) direction context =
+popScope node direction context =
    let
        currentScope : Scope
        currentScope =
@ -1136,10 +1197,11 @@ findInList predicate list =
 -- ACCESS


-{-| Get the name of the module where a function or type was defined.
+{-| Get the name of the module where a value was defined.
+A value can be either a function, a constant, a custom type constructor or a type alias (used as a function).

-  - The second argument (`String`) is the function name
-  - The third argument (`List String`) is the module name that was used next to the function name where you found it
+  - The second argument (`String`) is the name of the value
+  - The third argument (`List String`) is the module name that was used next to the value's name where you found it

 If the element was defined in the current module, then the result will be `[]`.

@ -1147,7 +1209,7 @@ If the element was defined in the current module, then the result will be `[]`.
    expressionVisitor node direction context =
        case ( direction, Node.value node ) of
            ( Rule.OnEnter, Expression.FunctionOrValue moduleName "button" ) ->
-                if Scope.realModuleName context.scope "button" moduleName == [ "Html" ] then
+                if Scope.moduleNameForValue context.scope "button" moduleName == [ "Html" ] then
                    ( [ createError node ], context )

                else
@ -1156,23 +1218,16 @@ If the element was defined in the current module, then the result will be `[]`.
            _ ->
                ( [], context )

-**Known problems**:
-
-  - Does not make the distinction between types and functions/values/custom type constructors.
-    This will likely warrant splitting this function into two later on.
-
-Help is welcome!
-
 -}
-realModuleName : ModuleContext -> String -> List String -> List String
-realModuleName (ModuleContext context) functionOrType moduleName =
+moduleNameForValue : ModuleContext -> String -> List String -> List String
+moduleNameForValue (ModuleContext context) valueName moduleName =
    case moduleName of
        [] ->
-            if isInScope functionOrType context.scopes then
+            if isInScope valueName context.scopes then
                []

            else
-                Dict.get functionOrType context.importedFunctionOrTypes
+                Dict.get valueName context.importedFunctions
                    |> Maybe.withDefault []

        _ :: [] ->
@ -1186,7 +1241,7 @@ realModuleName (ModuleContext context) functionOrType moduleName =
                            (\aliasedModuleName ->
                                case Dict.get aliasedModuleName context.modules of
                                    Just module_ ->
-                                        isDeclaredInModule functionOrType module_
+                                        isValueDeclaredInModule valueName module_

                                    Nothing ->
                                        False
@ -1207,18 +1262,71 @@ realModuleName (ModuleContext context) functionOrType moduleName =
            moduleName


-isDeclaredInModule : String -> Elm.Docs.Module -> Bool
-isDeclaredInModule functionOrType module_ =
-    List.any (.name >> (==) functionOrType) module_.values
-        || List.any (.name >> (==) functionOrType) module_.aliases
-        || List.any
-            (\union ->
-                (union.name == functionOrType)
-                    || List.any (Tuple.first >> (==) functionOrType) union.tags
+{-| Get the name of the module where a type was defined.
+A type can be either a custom type or a type alias.
+
+  - The second argument (`String`) is the name of the type
+  - The third argument (`List String`) is the module name that was used next to the type name where you found it
+
+-}
+moduleNameForType : ModuleContext -> String -> List String -> List String
+moduleNameForType (ModuleContext context) typeName moduleName =
+    case moduleName of
+        [] ->
+            if Set.member typeName context.localTypes then
+                []
+
+            else
+                Dict.get typeName context.importedTypes
+                    |> Maybe.withDefault []
+
+        _ :: [] ->
+            case Dict.get (getModuleName moduleName) context.importAliases of
+                Just [ aliasedModuleName ] ->
+                    aliasedModuleName
+
+                Just aliases ->
+                    case
+                        findInList
+                            (\aliasedModuleName ->
+                                case Dict.get aliasedModuleName context.modules of
+                                    Just module_ ->
+                                        isTypeDeclaredInModule typeName module_
+
+                                    Nothing ->
+                                        False
                            )
+                            aliases
+                    of
+                        Just aliasedModuleName ->
+                            aliasedModuleName
+
+                        Nothing ->
+                            List.head aliases
+                                |> Maybe.withDefault moduleName
+
+                Nothing ->
+                    moduleName
+
+        _ ->
+            moduleName
+
+
+isValueDeclaredInModule : String -> Elm.Docs.Module -> Bool
+isValueDeclaredInModule valueName module_ =
+    List.any (.name >> (==) valueName) module_.values
+        || List.any (.name >> (==) valueName) module_.aliases
+        || List.any
+            (\union -> List.any (Tuple.first >> (==) valueName) union.tags)
            module_.unions


+isTypeDeclaredInModule : String -> Elm.Docs.Module -> Bool
+isTypeDeclaredInModule typeName module_ =
+    List.any (.name >> (==) typeName) module_.aliases
+        || List.any (.name >> (==) typeName) module_.unions
+
+
 isInScope : String -> Nonempty Scope -> Bool
 isInScope name scopes =
    nonemptyList_any (.names >> Dict.member name) scopes