report imports

src/Lint/Rule/NoUnusedVariables.elm

@@ -37,23 +37,6 @@ import List.Nonempty as Nonempty exposing (Nonempty)
 import Set exposing (Set)
 
 
-type alias Scope =
-    { declared : Dict String Range
-    , used : Set String
-    }
-
-
-type alias Context =
-    { scopes : Nonempty Scope
-    , exposesEverything : Bool
-    }
-
-
-emptyScope : Scope
-emptyScope =
-    Scope Dict.empty Set.empty
-
-
 {-| Reports variables that are declared but never used.
 
     rules =
@@ -66,10 +49,41 @@ rule input =
     lint input implementation
 
 
+type alias Scope =
+    { declared : Dict String Range
+    , used : Set String
+    }
+
+
+type alias Context =
+    { scopes : Nonempty Scope
+    , exposesEverything : Bool
+    , imports : Dict String Range
+    }
+
+
+emptyScope : Scope
+emptyScope =
+    Scope Dict.empty Set.empty
+
+
+error : Range -> String -> Error
+error range_ name =
+    Error "NoUnusedVariables" ("Variable `" ++ name ++ "` is not used") range_
+
+
+initialContext : Context
+initialContext =
+    { scopes = Nonempty.fromElement emptyScope
+    , exposesEverything = False
+    , imports = Dict.empty
+    }
+
+
 implementation : Implementation Context
 implementation =
     createRule
-        (Context (Nonempty.fromElement emptyScope) False)
+        initialContext
         (\v ->
             { v
                 | visitModuleDefinition = visitModuleDefinition
@@ -114,46 +128,90 @@ visitModuleDefinition ctx moduleNode =
 visitImport : Context -> Node Import -> ( List Error, Context )
 visitImport ctx node =
     let
-        newContext =
+        exposed =
-            value node
+            node
+                |> value
                 |> .exposingList
+
+        declaredImports =
+            exposed
                 |> Maybe.map (value >> collectFromExposing)
                 |> Maybe.withDefault []
-                |> List.foldl (\( range, name ) context -> register range name context) ctx
+
+        moduleName =
+            Maybe.withDefault (value node |> .moduleName) (value node |> .moduleAlias)
     in
-    ( [], newContext )
+    case Maybe.map value exposed of
+        Just (All _) ->
+            -- Do not attempt to report an import that exposes all
+            ( [], ctx )
 
-
+        _ ->
-collectFromExposing : Exposing -> List ( Range, String )
+            if List.isEmpty declaredImports then
-collectFromExposing exposing_ =
+                -- Only register the module name
-    case exposing_ of
+                ( []
-        All _ ->
+                , register
-            []
+                    (range moduleName)
-
+                    (value moduleName |> getModuleName)
-        Explicit list ->
+                    ctx
-            List.filterMap
+                )
-                (\node ->
+
-                    case value node of
+            else
-                        FunctionExpose name ->
+                -- Only register the exposed variables
-                            Just ( range node, name )
+                ( []
-
+                , List.foldl
-                        InfixExpose name ->
+                    (\( range, name ) context -> register range name context)
-                            Just ( range node, name )
+                    ctx
-
+                    declaredImports
-                        _ ->
-                            Nothing
                 )
-                list
 
 
-markAllAsUsed : List String -> Context -> Context
+visitExpression : Context -> Direction -> Node Expression -> ( List Error, Context )
-markAllAsUsed names ctx =
+visitExpression ctx direction node =
-    List.foldl markAsUsed ctx names
+    case ( direction, value node ) of
+        ( Enter, FunctionOrValue [] name ) ->
+            ( [], markAsUsed name ctx )
 
+        ( Enter, FunctionOrValue moduleName name ) ->
+            ( [], markAsUsed (getModuleName moduleName) ctx )
 
-error : Range -> String -> Error
+        ( Enter, OperatorApplication name _ _ _ ) ->
-error range_ name =
+            ( [], markAsUsed name ctx )
-    Error "NoUnusedVariables" ("Variable `" ++ name ++ "` is not used") range_
+
+        ( Enter, PrefixOperator name ) ->
+            ( [], markAsUsed name ctx )
+
+        ( Enter, LetExpression { declarations } ) ->
+            let
+                newContext =
+                    List.foldl
+                        (\declaration context ->
+                            case value declaration of
+                                LetFunction function ->
+                                    registerFunction function context
+
+                                LetDestructuring pattern _ ->
+                                    context
+                        )
+                        { ctx | scopes = Nonempty.cons emptyScope ctx.scopes }
+                        declarations
+            in
+            ( [], newContext )
+
+        ( Exit, LetExpression _ ) ->
+            let
+                ( errors, remainingUsed ) =
+                    makeReport (Nonempty.head ctx.scopes)
+
+                ctxWithPoppedScope =
+                    { ctx | scopes = Nonempty.pop ctx.scopes }
+            in
+            ( errors
+            , markAllAsUsed remainingUsed ctxWithPoppedScope
+            )
+
+        _ ->
+            ( [], ctx )
 
 
 visitDeclaration : Context -> Direction -> Node Declaration -> ( List Error, Context )
@@ -186,6 +244,52 @@ visitDeclaration ctx direction node =
             ( [], ctx )
 
 
+visitEnd : Context -> ( List Error, Context )
+visitEnd ctx =
+    let
+        errors =
+            if ctx.exposesEverything then
+                []
+
+            else
+                ctx.scopes
+                    |> Nonempty.head
+                    |> makeReportRoot ctx.imports
+    in
+    ( errors, ctx )
+
+
+registerFunction : Function -> Context -> Context
+registerFunction function ctx =
+    let
+        declaration =
+            value function.declaration
+    in
+    register (range declaration.name) (value declaration.name) ctx
+
+
+collectFromExposing : Exposing -> List ( Range, String )
+collectFromExposing exposing_ =
+    case exposing_ of
+        All _ ->
+            []
+
+        Explicit list ->
+            List.filterMap
+                (\node ->
+                    case value node of
+                        FunctionExpose name ->
+                            Just ( range node, name )
+
+                        InfixExpose name ->
+                            Just ( range node, name )
+
+                        _ ->
+                            Nothing
+                )
+                list
+
+
 collectNamesFromTypeAnnotation : Node TypeAnnotation -> List String
 collectNamesFromTypeAnnotation node =
     case value node of
@@ -229,6 +333,11 @@ register range name ctx =
     { ctx | scopes = scopes }
 
 
+markAllAsUsed : List String -> Context -> Context
+markAllAsUsed names ctx =
+    List.foldl markAsUsed ctx names
+
+
 markAsUsed : String -> Context -> Context
 markAsUsed name ctx =
     let
@@ -242,74 +351,9 @@ markAsUsed name ctx =
     { ctx | scopes = scopes }
 
 
-visitExpression : Context -> Direction -> Node Expression -> ( List Error, Context )
+getModuleName : List String -> String
-visitExpression ctx direction node =
+getModuleName name =
-    case ( direction, value node ) of
+    String.join "." name
-        ( Enter, FunctionOrValue [] name ) ->
-            ( [], markAsUsed name ctx )
-
-        ( Enter, OperatorApplication name _ _ _ ) ->
-            ( [], markAsUsed name ctx )
-
-        ( Enter, PrefixOperator name ) ->
-            ( [], markAsUsed name ctx )
-
-        ( Enter, LetExpression { declarations } ) ->
-            let
-                newContext =
-                    List.foldl
-                        (\declaration context ->
-                            case value declaration of
-                                LetFunction function ->
-                                    registerFunction function context
-
-                                LetDestructuring pattern _ ->
-                                    context
-                        )
-                        { ctx | scopes = Nonempty.cons emptyScope ctx.scopes }
-                        declarations
-            in
-            ( [], newContext )
-
-        ( Exit, LetExpression _ ) ->
-            let
-                ( errors, remainingUsed ) =
-                    makeReport (Nonempty.head ctx.scopes)
-
-                ctxWithPoppedScope =
-                    { ctx | scopes = Nonempty.pop ctx.scopes }
-            in
-            ( errors
-            , markAllAsUsed remainingUsed ctxWithPoppedScope
-            )
-
-        _ ->
-            ( [], ctx )
-
-
-registerFunction : Function -> Context -> Context
-registerFunction function ctx =
-    let
-        declaration =
-            value function.declaration
-    in
-    register (range declaration.name) (value declaration.name) ctx
-
-
-visitEnd : Context -> ( List Error, Context )
-visitEnd ctx =
-    let
-        errors =
-            if ctx.exposesEverything then
-                []
-
-            else
-                ctx.scopes
-                    |> Nonempty.head
-                    |> makeReport
-                    |> Tuple.first
-    in
-    ( errors, ctx )
 
 
 makeReport : Scope -> ( List Error, List String )
@@ -322,185 +366,25 @@ makeReport { declared, used } =
         errors =
             Dict.filter (\key _ -> not <| Set.member key used) declared
                 |> Dict.toList
-                |> List.map (\( key, node ) -> error node key)
+                |> List.map (\( key, range ) -> error range key)
     in
     ( errors, nonUsedVars )
 
 
+makeReportRoot : Dict String Range -> Scope -> List Error
+makeReportRoot imports { declared, used } =
+    let
+        nonUsedVariablesErrors =
+            Dict.filter (\key _ -> not <| Set.member key used) declared
+                |> Dict.toList
+                |> List.map (\( key, range ) -> error range key)
 
--- ( Enter, Variable names ) ->
+        nonUsedImportErrors =
---     case names of
+            Dict.filter (\key _ -> not <| Set.member key used) imports
---         [ name ] ->
+                |> Dict.toList
---             ( [], { ctx | scopes = addUsedToStack ctx.scopes [ name ] } )
+                |> List.map (\( key, range ) -> error range key)
---
+    in
---         _ ->
+    nonUsedImportErrors ++ nonUsedVariablesErrors
---             ( [], ctx )
---
--- ( Enter, LetExpression declarations ) ->
---     let
---         variables =
---             List.map Tuple.first declarations
---                 |> List.filterMap variableName
---                 |> List.concat
---                 |> Set.fromList
---
---         newScope =
---             Scope variables Set.empty
---     in
---     ( [], { ctx | scopes = newScope :: ctx.scopes } )
---
--- ( Exit, LetExpression _ ) ->
---     let
---         ( errors, variablesUsedButNotFromThisScope ) =
---             ctx.scopes
---                 |> List.head
---                 |> makeReport
---
---         newScopes =
---             List.drop 1 ctx.scopes
---     in
---     ( errors, { ctx | scopes = addUsedToStack newScopes (Set.toList variablesUsedButNotFromThisScope) } )
--- addUsedToStack : List Scope -> List String -> List Scope
--- addUsedToStack scopes variables =
---     let
---         lastScope =
---             case List.head scopes of
---                 Nothing ->
---                     Debug.log "Unexpected Empty scope stack" emptyScope
---
---                 Just scope ->
---                     { scope | used = Set.union scope.used (Set.fromList variables) }
---     in
---     lastScope :: List.drop 1 scopes
---
---
--- addFoundToStack : List Scope -> List String -> List Scope
--- addFoundToStack scopes variables =
---     let
---         lastScope =
---             case List.head scopes of
---                 Nothing ->
---                     Debug.log "Unexpected Empty scope stack" emptyScope
---
---                 Just scope ->
---                     { scope | declared = Set.union scope.declared (Set.fromList variables) }
---     in
---     lastScope :: List.drop 1 scopes
--- makeReport : Maybe Scope -> ( List Error, Set String )
--- makeReport maybeScope =
---     case maybeScope of
---         Nothing ->
---             Debug.log "Unexpected Empty scope stack" ( [], Set.empty )
---
---         Just scope ->
---             let
---                 notUsed =
---                     Set.diff scope.declared scope.used
---
---                 variablesUsedButNotFromThisScope =
---                     Set.diff scope.used scope.declared
---
---                 errors =
---                     Set.diff scope.declared scope.used
---                         |> Set.toList
---                         |> List.sort
---                         |> List.map error
---             in
---             ( errors, variablesUsedButNotFromThisScope )
---
--- variableName : Expression -> Maybe (List String)
--- variableName expr =
---     case expr of
---         Variable names ->
---             Just names
---
---         Application var _ ->
---             variableName var
---
---         _ ->
---             Nothing
---
---
--- getExported : ExportSet -> Set String
--- getExported exportType =
---     case exportType of
---         -- Ignore as this case is handled by `exposesEverything`
---         AllExport ->
---             Set.empty
---
---         SubsetExport exports ->
---             List.map getExported exports
---                 |> List.foldl Set.union Set.empty
---
---         FunctionExport name ->
---             Set.singleton name
---
---         TypeExport name _ ->
---             Set.singleton name
---
---
--- addExposedVariables : Context -> Ast.Statement.ExportSet -> Context
--- addExposedVariables ctx exportType =
---     { ctx
---         | scopes =
---             getExported exportType
---                 |> Set.toList
---                 |> addUsedToStack ctx.scopes
---     }
---
---
--- statementFn : Context -> Direction Statement -> ( List Error, Context )
--- statementFn ctx node =
---     case node of
---         Enter (FunctionDeclaration name args body) ->
---             ( [], { ctx | scopes = addFoundToStack ctx.scopes [ name ] } )
---
---         Enter (ModuleDeclaration names AllExport) ->
---             ( [], { ctx | exposesEverything = True } )
---
---         Enter (PortModuleDeclaration names AllExport) ->
---             ( [], { ctx | exposesEverything = True } )
---
---         Enter (ImportStatement module_ alias_ (Just (SubsetExport imported))) ->
---             let
---                 variables =
---                     List.foldl
---                         (\var res ->
---                             case var of
---                                 FunctionExport name ->
---                                     name :: res
---
---                                 _ ->
---                                     res
---                         )
---                         []
---                         imported
---             in
---             ( [], { ctx | scopes = addFoundToStack ctx.scopes variables } )
---
---         Enter (ModuleDeclaration names exportType) ->
---             ( [], addExposedVariables ctx exportType )
---
---         Enter (PortModuleDeclaration names exportType) ->
---             ( [], addExposedVariables ctx exportType )
---
---         _ ->
---             ( [], ctx )
---
---
--- moduleEndFn : Context -> ( List Error, Context )
--- moduleEndFn ctx =
---     let
---         ( errors, _ ) =
---             if ctx.exposesEverything then
---                 ( [], Set.empty )
---
---             else
---                 ctx.scopes
---                     |> List.head
---                     |> makeReport
---     in
---     ( errors, ctx )
 
 
 mapNonemptyHead : (a -> a) -> Nonempty a -> Nonempty a

tests/NoUnusedVariablesTest.elm

@@ -278,6 +278,44 @@ type A = A Int
 a : A
 a = 1"""
                 |> expectErrors []
+    , test "should report unused import" <|
+        \() ->
+            testRule """module A exposing (a)
+import Html"""
+                |> expectErrors [ error "Variable `Html` is not used" (location 2 8 12) ]
+    , test "should report unused import (multiples segments)" <|
+        \() ->
+            testRule """module A exposing (a)
+import Html.Styled.Attributes"""
+                |> expectErrors [ error "Variable `Html.Styled.Attributes` is not used" (location 2 8 30) ]
+    , test "should not report import if it exposes all (should be improved by detecting if any exposed value is used)" <|
+        \() ->
+            testRule """module A exposing (a)
+import Html.Styled.Attributes exposing (..)"""
+                |> expectErrors []
+    , test "should report unused variable even if a homonym from a module is used" <|
+        \() ->
+            testRule """module A exposing (a)
+href = 1
+a = Html.Styled.Attributes.href"""
+                |> expectErrors [ error "Variable `href` is not used" (location 2 1 5) ]
+    , test "should not report used import (function access)" <|
+        \() ->
+            testRule """module A exposing (a)
+import Html.Styled.Attributes
+a = Html.Styled.Attributes.href"""
+                |> expectErrors []
+    , test "should not report unused import if it is aliased" <|
+        \() ->
+            testRule """module A exposing (a)
+import Html.Styled.Attributes as Html
+a = Html.href"""
+                |> expectErrors []
+    , test "should report unused import alias" <|
+        \() ->
+            testRule """module A exposing (a)
+import Html.Styled.Attributes as Html"""
+                |> expectErrors [ error "Variable `Html` is not used" (location 2 34 38) ]
     ]