NoUnusedExports: Mark types used in type alias and custom type arguments as used

src/NoUnusedExports.elm

@@ -20,7 +20,7 @@ import Elm.Syntax.Exposing as Exposing
 import Elm.Syntax.Expression as Expression exposing (Expression)
 import Elm.Syntax.Module as Module exposing (Module)
 import Elm.Syntax.ModuleName exposing (ModuleName)
-import Elm.Syntax.Node as Node exposing (Node)
+import Elm.Syntax.Node as Node exposing (Node(..))
 import Elm.Syntax.Range exposing (Range)
 import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
 import Review.Rule as Rule exposing (Error, Rule)
@@ -160,17 +160,10 @@ foldGlobalContexts newContext previousContext =
     }
 
 
-registerAsUsed : ModuleContext -> ModuleName -> String -> ModuleContext
-registerAsUsed moduleContext moduleName name =
-    let
-        ( realModuleName, realName ) =
-            Scope.realFunctionOrType moduleName name moduleContext.scope
-    in
-    if realModuleName /= [] then
-        { moduleContext
-            | used =
-                Set.insert ( realModuleName, realName ) moduleContext.used
-        }
+registerAsUsed : ( ModuleName, String ) -> ModuleContext -> ModuleContext
+registerAsUsed ( moduleName, name ) moduleContext =
+    if moduleName /= [] then
+        { moduleContext | used = Set.insert ( moduleName, name ) moduleContext.used }
 
     else
         moduleContext
@@ -292,8 +285,7 @@ exposedElements nodes =
                         Just <| ( name, { range = Node.range node, exposedElement = TypeOrTypeAlias } )
 
                     Exposing.TypeExpose { name } ->
-                        -- TODO
-                        Nothing
+                        Just <| ( name, { range = Node.range node, exposedElement = ExposedType } )
 
                     Exposing.InfixExpose name ->
                         Nothing
@@ -307,10 +299,6 @@ exposedElements nodes =
 
 declarationListVisitor : List (Node Declaration) -> ModuleContext -> ( List Error, ModuleContext )
 declarationListVisitor declarations moduleContext =
-    if moduleContext.exposesEverything then
-        ( [], moduleContext )
-
-    else
     let
         declaredNames : Set String
         declaredNames =
@@ -318,33 +306,39 @@ declarationListVisitor declarations moduleContext =
                 |> List.filterMap (Node.value >> declarationName)
                 |> Set.fromList
 
-            typesUsedInSignature_ : List ( ModuleName, String )
-            typesUsedInSignature_ =
+        typesUsedInDeclaration_ : List ( List ( ModuleName, String ), Bool )
+        typesUsedInDeclaration_ =
             declarations
-                    |> List.concatMap typesUsedInSignature
+                |> List.map (typesUsedInDeclaration moduleContext)
+
+        allUsedTypes : List ( ModuleName, String )
+        allUsedTypes =
+            typesUsedInDeclaration_
+                |> List.concatMap Tuple.first
 
         contextWithUsedTypes : ModuleContext
         contextWithUsedTypes =
-                List.foldl
-                    (\( moduleName, name ) context -> registerAsUsed context moduleName name)
-                    moduleContext
-                    typesUsedInSignature_
+            List.foldl registerAsUsed moduleContext allUsedTypes
     in
     ( []
     , { contextWithUsedTypes
         | exposed =
-                Dict.filter
-                    (\name _ -> Set.member name declaredNames)
             contextWithUsedTypes.exposed
-            , typesNotToReport =
-                typesUsedInSignature_
-                    |> List.filter
-                        (\( moduleName, name ) ->
-                            (Scope.realFunctionOrType moduleName name contextWithUsedTypes.scope
-                                |> Tuple.first
-                                |> List.isEmpty
+                |> (if moduleContext.exposesEverything then
+                        identity
+
+                    else
+                        Dict.filter (\name _ -> Set.member name declaredNames)
                    )
-                                && isType name
+        , typesNotToReport =
+            typesUsedInDeclaration_
+                |> List.concatMap
+                    (\( list, comesFromCustomTypeWithHiddenConstructors ) ->
+                        if comesFromCustomTypeWithHiddenConstructors then
+                            []
+
+                        else
+                            List.filter (\( moduleName, name ) -> isType name && moduleName == []) list
                     )
                 |> List.map Tuple.second
                 |> Set.fromList
@@ -388,40 +382,65 @@ declarationName declaration =
             Nothing
 
 
-typesUsedInSignature : Node Declaration -> List ( ModuleName, String )
-typesUsedInSignature declaration =
+typesUsedInDeclaration : ModuleContext -> Node Declaration -> ( List ( ModuleName, String ), Bool )
+typesUsedInDeclaration moduleContext declaration =
     case Node.value declaration of
         Declaration.FunctionDeclaration function ->
-            function.signature
-                |> Maybe.map (Node.value >> .typeAnnotation >> collectTypesFromTypeAnnotation)
+            ( function.signature
+                |> Maybe.map (Node.value >> .typeAnnotation >> collectTypesFromTypeAnnotation moduleContext.scope)
                 |> Maybe.withDefault []
+            , False
+            )
+
+        Declaration.CustomTypeDeclaration type_ ->
+            ( type_.constructors
+                |> List.concatMap (Node.value >> .arguments)
+                |> List.concatMap (collectTypesFromTypeAnnotation moduleContext.scope)
+            , not <|
+                case Dict.get (Node.value type_.name) moduleContext.exposed |> Maybe.map .exposedElement of
+                    Just ExposedType ->
+                        True
 
                     _ ->
-            []
+                        False
+            )
+
+        Declaration.AliasDeclaration alias_ ->
+            ( collectTypesFromTypeAnnotation moduleContext.scope alias_.typeAnnotation, False )
+
+        Declaration.PortDeclaration _ ->
+            ( [], False )
+
+        Declaration.InfixDeclaration _ ->
+            ( [], False )
+
+        Declaration.Destructuring _ _ ->
+            ( [], False )
 
 
-collectTypesFromTypeAnnotation : Node TypeAnnotation -> List ( ModuleName, String )
-collectTypesFromTypeAnnotation node =
+collectTypesFromTypeAnnotation : Scope.ModuleContext -> Node TypeAnnotation -> List ( ModuleName, String )
+collectTypesFromTypeAnnotation scope node =
     case Node.value node of
         TypeAnnotation.FunctionTypeAnnotation a b ->
-            collectTypesFromTypeAnnotation a ++ collectTypesFromTypeAnnotation b
+            collectTypesFromTypeAnnotation scope a ++ collectTypesFromTypeAnnotation scope b
 
-        TypeAnnotation.Typed nameNode params ->
-            Node.value nameNode :: List.concatMap collectTypesFromTypeAnnotation params
+        TypeAnnotation.Typed (Node _ ( moduleName, name )) params ->
+            Scope.realFunctionOrType moduleName name scope
+                :: List.concatMap (collectTypesFromTypeAnnotation scope) params
 
         TypeAnnotation.Record list ->
             list
                 |> List.map (Node.value >> Tuple.second)
-                |> List.concatMap collectTypesFromTypeAnnotation
+                |> List.concatMap (collectTypesFromTypeAnnotation scope)
 
         TypeAnnotation.GenericRecord name list ->
             list
                 |> Node.value
                 |> List.map (Node.value >> Tuple.second)
-                |> List.concatMap collectTypesFromTypeAnnotation
+                |> List.concatMap (collectTypesFromTypeAnnotation scope)
 
         TypeAnnotation.Tupled list ->
-            List.concatMap collectTypesFromTypeAnnotation list
+            List.concatMap (collectTypesFromTypeAnnotation scope) list
 
         TypeAnnotation.GenericType _ ->
             []
@@ -438,7 +457,11 @@ expressionVisitor : Node Expression -> Rule.Direction -> ModuleContext -> ( List
 expressionVisitor node direction moduleContext =
     case ( direction, Node.value node ) of
         ( Rule.OnEnter, Expression.FunctionOrValue moduleName name ) ->
-            ( [], registerAsUsed moduleContext moduleName name )
+            ( []
+            , registerAsUsed
+                (Scope.realFunctionOrType moduleName name moduleContext.scope)
+                moduleContext
+            )
 
         _ ->
             ( [], moduleContext )

tests/NoUnusedExportsTest.elm

@@ -66,8 +66,11 @@ all =
     describe "NoUnusedExports"
         [ functionsAndValuesTests
         , typesTests
+        , typeAliasesTests
 
         -- TODO Add tests that report exposing the type's variants if they are never used.
+        -- TODO Add tests to add exceptions to tests
+        -- TODO Add tests to add exceptions for ReviewConfig.config (for applications only)
         ]
 
 
@@ -226,8 +229,7 @@ main = VariantA
 """ ]
                     |> Review.Test.runOnModulesWithProjectData application rule
                     |> Review.Test.expectNoErrors
-        , Test.skip <|
-            test "should not report a used exposed custom type (type alias)" <|
+        , test "should not report a used exposed custom type (used in type alias)" <|
             \() ->
                 [ """
 module A exposing (ExposedB, ExposedC)
@@ -259,8 +261,7 @@ main = 1
 """
                     |> Review.Test.runWithProjectData application rule
                     |> Review.Test.expectNoErrors
-        , Test.skip <|
-            test "should not report an unused exposed custom type if it's aliased by an exposed type alias" <|
+        , test "should not report an unused exposed custom type if it's aliased by an exposed type alias" <|
             \() ->
                 [ """
 module A exposing (MyType, OtherType)
@@ -273,8 +274,7 @@ type alias B = A.OtherType
 """ ]
                     |> Review.Test.runOnModulesWithProjectData package_ rule
                     |> Review.Test.expectNoErrors
-        , Test.skip <|
-            test "should not report an unused exposed custom type if it's present in an exposed type alias" <|
+        , test "should not report an unused exposed custom type if it's present in an exposed type alias" <|
             \() ->
                 [ """
 module A exposing (MyType, OtherType)
@@ -287,13 +287,12 @@ type alias B = A.OtherType
 """ ]
                     |> Review.Test.runOnModulesWithProjectData package_ rule
                     |> Review.Test.expectNoErrors
-        , Test.skip <|
-            test "should not report an unused exposed custom type if it's present in an exposed type alias (nested)" <|
+        , test "should not report an unused exposed custom type if it's present in an exposed type alias (nested)" <|
             \() ->
                 [ """
 module A exposing (MyType, OtherType)
 type MyType = VariantA | VariantB
-type alias OtherType = { other { thing : ((), MyType) } }
+type alias OtherType = { other : { thing : ((), MyType) } }
 """, """
 module Exposed exposing (..)
 import A
@@ -301,8 +300,20 @@ type alias B = A.OtherType
 """ ]
                     |> Review.Test.runOnModulesWithProjectData package_ rule
                     |> Review.Test.expectNoErrors
-        , Test.skip <|
-            test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments" <|
+        , test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType(..))
+type MyType = VariantA | VariantB
+type OtherType = Thing MyType
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments but the constructors are not exposed" <|
             \() ->
                 [ """
 module A exposing (MyType, OtherType)
@@ -312,11 +323,33 @@ type OtherType = Thing MyType
 module Exposed exposing (..)
 import A
 type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectErrorsForModules
+                        [ ( "A"
+                          , [ Review.Test.error
+                                { message = "Exposed type or type alias `MyType` is never used outside this module."
+                                , details = details
+                                , under = "MyType"
+                                }
+                                |> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
+                            ]
+                          )
+                        ]
+        , test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments (nested)" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType(..))
+type MyType = VariantA | VariantB
+type OtherType = OtherThing | SomeThing ((), List MyType)
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
 """ ]
                     |> Review.Test.runOnModulesWithProjectData package_ rule
                     |> Review.Test.expectNoErrors
-        , Test.skip <|
-            test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments (nested)" <|
+        , test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments (nested) but the constructors are not exposed" <|
             \() ->
                 [ """
 module A exposing (MyType, OtherType)
@@ -328,5 +361,190 @@ import A
 type alias B = A.OtherType
 """ ]
                     |> Review.Test.runOnModulesWithProjectData package_ rule
-                        |> Review.Test.expectNoErrors
+                    |> Review.Test.expectErrorsForModules
+                        [ ( "A"
+                          , [ Review.Test.error
+                                { message = "Exposed type or type alias `MyType` is never used outside this module."
+                                , details = details
+                                , under = "MyType"
+                                }
+                                |> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
+                            ]
+                          )
+                        ]
+        ]
+
+
+typeAliasesTests : Test
+typeAliasesTests =
+    describe "Type aliases"
+        [ test "should report an unused exposed type alias" <|
+            \() ->
+                """
+module A exposing (Exposed)
+type alias Exposed = {}
+"""
+                    |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.expectErrors
+                        [ Review.Test.error
+                            { message = "Exposed type or type alias `Exposed` is never used outside this module."
+                            , details = details
+                            , under = "Exposed"
+                            }
+                            |> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 27 } }
+                        ]
+        , test "should not report a used exposed type alias (type signature)" <|
+            \() ->
+                [ """
+module A exposing (Exposed)
+type alias Exposed = {}
+""", """
+module B exposing (main)
+import A
+main : A.Exposed
+main = {}
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData application rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report a used exposed type alias (used in type alias)" <|
+            \() ->
+                [ """
+module A exposing (ExposedB)
+type alias ExposedB = {}
+""", """
+module Exposed exposing (B)
+import A
+type alias B = A.ExposedB
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's part of the package's exposed API" <|
+            \() ->
+                """
+module Exposed exposing (MyType)
+type alias MyType = {}
+"""
+                    |> Review.Test.runWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's present in the signature of an exposed function" <|
+            \() ->
+                """
+module A exposing (main, MyType)
+type alias MyType = {}
+main : () -> MyType
+main = 1
+"""
+                    |> Review.Test.runWithProjectData application rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's aliased by an exposed type alias" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType)
+type alias MyType = {}
+type alias OtherType = MyType
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's present in an exposed type alias" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType)
+type alias MyType = {}
+type alias OtherType = { thing : MyType }
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's present in an exposed type alias (nested)" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType)
+type alias MyType = {}
+type alias OtherType = { other : { thing : ((), MyType) } }
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's present in an exposed custom type constructor's arguments" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType(..))
+type alias MyType = {}
+type OtherType = OtherType MyType
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should not report an unused exposed type alias if it's present in an exposed custom type constructor's arguments but the constructors are not exposed" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType)
+type alias MyType = {}
+type OtherType = OtherType MyType
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectErrorsForModules
+                        [ ( "A"
+                          , [ Review.Test.error
+                                { message = "Exposed type or type alias `MyType` is never used outside this module."
+                                , details = details
+                                , under = "MyType"
+                                }
+                                |> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
+                            ]
+                          )
+                        ]
+        , test "should not report an unused exposed type alias if it's present in an exposed custom type constructor's arguments (nested)" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType(..))
+type alias MyType = {}
+type OtherType = OtherThing | SomeThing ((), List MyType)
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectNoErrors
+        , test "should report an unused exposed type alias if it's present in an exposed custom type constructor's arguments (nested) but the constructors are not exposed" <|
+            \() ->
+                [ """
+module A exposing (MyType, OtherType)
+type alias MyType = {}
+type OtherType = OtherThing | SomeThing ((), List MyType)
+""", """
+module Exposed exposing (..)
+import A
+type alias B = A.OtherType
+""" ]
+                    |> Review.Test.runOnModulesWithProjectData package_ rule
+                    |> Review.Test.expectErrorsForModules
+                        [ ( "A"
+                          , [ Review.Test.error
+                                { message = "Exposed type or type alias `MyType` is never used outside this module."
+                                , details = details
+                                , under = "MyType"
+                                }
+                                |> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
+                            ]
+                          )
+                        ]
         ]