From 251bca2c12f5ebeb00efaab06deef02356f81380 Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Tue, 14 Jan 2020 09:48:26 +0100 Subject: [PATCH] Use Scope in NoUnusedExports to know about the exports from other modules --- src/NoUnusedExports.elm | 127 ++++++++++++++++++++++++++++++---- src/NoUnusedModules.elm | 8 +-- src/Review/Rule.elm | 1 + src/Scope2.elm | 4 +- tests/NoUnusedExportsTest.elm | 25 ++++--- 5 files changed, 132 insertions(+), 33 deletions(-) diff --git a/src/NoUnusedExports.elm b/src/NoUnusedExports.elm index 3dee5694..ebd4764f 100644 --- a/src/NoUnusedExports.elm +++ b/src/NoUnusedExports.elm @@ -17,12 +17,14 @@ import Elm.Module import Elm.Project exposing (Project) import Elm.Syntax.Declaration as Declaration exposing (Declaration) import Elm.Syntax.Exposing as Exposing +import Elm.Syntax.Expression as Expression exposing (Expression) import Elm.Syntax.Import exposing (Import) 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.Range exposing (Range) import Review.Rule as Rule exposing (Error, Rule) +import Scope2 as Scope import Set exposing (Set) @@ -55,13 +57,23 @@ rule = { moduleVisitorSchema = \schema -> schema + |> Scope.addModuleVisitors + { set = \scope context -> { context | scope = scope } + , get = .scope + } |> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor + |> Rule.withExpressionVisitor expressionVisitor , initGlobalContext = initGlobalContext , fromGlobalToModule = fromGlobalToModule , fromModuleToGlobal = fromModuleToGlobal , foldGlobalContexts = foldGlobalContexts } + |> Scope.addGlobalVisitors + { set = \scope context -> { context | scope = scope } + , get = .scope + } |> Rule.traversingImportedModulesFirst + |> Rule.withMultiElmJsonVisitor elmJsonVisitor |> Rule.withMultiFinalEvaluation finalEvaluationForProject |> Rule.fromMultiSchema @@ -71,14 +83,22 @@ rule = type alias GlobalContext = - { modules : + { scope : Scope.GlobalContext + , projectType : ProjectType + , modules : Dict ModuleName { fileKey : Rule.FileKey , exposed : Dict String { range : Range, exposedElement : ExposedElement } } + , used : Set ( ModuleName, String ) } +type ProjectType + = IsApplication + | IsPackage (Set (List String)) + + type ExposedElement = Function | TypeOrTypeAlias @@ -86,38 +106,51 @@ type ExposedElement type alias ModuleContext = - { exposesEverything : Bool + { scope : Scope.ModuleContext + , exposesEverything : Bool , exposed : Dict String { range : Range, exposedElement : ExposedElement } + , used : Set ( ModuleName, String ) } initGlobalContext : GlobalContext initGlobalContext = - { modules = Dict.empty + { scope = Scope.initGlobalContext + , projectType = IsApplication + , modules = Dict.empty + , used = Set.empty } fromGlobalToModule : Rule.FileKey -> Node ModuleName -> GlobalContext -> ModuleContext -fromGlobalToModule fileKey moduleNameNode globalContext = - { exposesEverything = False +fromGlobalToModule fileKey moduleName globalContext = + { scope = Scope.fromGlobalToModule globalContext.scope + , exposesEverything = False , exposed = Dict.empty + , used = Set.empty } fromModuleToGlobal : Rule.FileKey -> Node ModuleName -> ModuleContext -> GlobalContext fromModuleToGlobal fileKey moduleName moduleContext = - { modules = + { scope = Scope.fromModuleToGlobal moduleName moduleContext.scope + , projectType = IsApplication + , modules = Dict.singleton (Node.value moduleName) { fileKey = fileKey , exposed = moduleContext.exposed } + , used = moduleContext.used } foldGlobalContexts : GlobalContext -> GlobalContext -> GlobalContext -foldGlobalContexts contextA contextB = - { modules = Dict.union contextA.modules contextB.modules +foldGlobalContexts newContext previousContext = + { scope = Scope.foldGlobalContexts previousContext.scope newContext.scope + , projectType = previousContext.projectType + , modules = Dict.union previousContext.modules newContext.modules + , used = Set.union newContext.used previousContext.used } @@ -131,17 +164,49 @@ error ( moduleName, { fileKey, moduleNameLocation } ) = +-- ELM JSON VISITOR + + +elmJsonVisitor : Maybe Project -> GlobalContext -> GlobalContext +elmJsonVisitor maybeProject globalContext = + case maybeProject of + Just (Elm.Project.Package { exposed }) -> + let + exposedModuleNames : List Elm.Module.Name + exposedModuleNames = + case exposed of + Elm.Project.ExposedList names -> + names + + Elm.Project.ExposedDict fakeDict -> + List.concatMap Tuple.second fakeDict + in + { globalContext + | projectType = + exposedModuleNames + |> List.map (Elm.Module.toString >> String.split ".") + |> Set.fromList + |> IsPackage + } + + _ -> + { globalContext | projectType = IsApplication } + + + -- GLOBAL EVALUATION finalEvaluationForProject : GlobalContext -> List Error finalEvaluationForProject globalContext = globalContext.modules - |> Dict.values + |> removeExposedPackages globalContext + |> Dict.toList |> List.concatMap - (\{ fileKey, exposed } -> + (\( moduleName, { fileKey, exposed } ) -> exposed - |> removeExceptions + |> removeApplicationExceptions globalContext moduleName + |> Dict.filter (\name _ -> not <| Set.member ( moduleName, name ) globalContext.used) |> Dict.toList |> List.map (\( name, { range, exposedElement } ) -> @@ -154,9 +219,24 @@ finalEvaluationForProject globalContext = ) -removeExceptions : Dict String a -> Dict String a -removeExceptions dict = - Dict.filter (\name _ -> name /= "main") dict +removeExposedPackages : GlobalContext -> Dict ModuleName a -> Dict ModuleName a +removeExposedPackages globalContext dict = + case globalContext.projectType of + IsApplication -> + dict + + IsPackage exposedModuleNames -> + Dict.filter (\name _ -> not <| Set.member name exposedModuleNames) dict + + +removeApplicationExceptions : GlobalContext -> ModuleName -> Dict String a -> Dict String a +removeApplicationExceptions globalContext moduleName dict = + case globalContext.projectType of + IsApplication -> + Dict.remove "main" dict + + IsPackage _ -> + dict @@ -194,3 +274,22 @@ exposedElements nodes = Nothing ) |> Dict.fromList + + + +-- EXPRESSION VISITOR + + +expressionVisitor : Node Expression -> Rule.Direction -> ModuleContext -> ( List Error, ModuleContext ) +expressionVisitor node direction moduleContext = + case ( direction, Node.value node ) of + ( Rule.OnEnter, Expression.FunctionOrValue moduleName name ) -> + ( [] + , { moduleContext + | used = + Set.insert (Scope.realFunctionOrType moduleName name moduleContext.scope) moduleContext.used + } + ) + + _ -> + ( [], moduleContext ) diff --git a/src/NoUnusedModules.elm b/src/NoUnusedModules.elm index f0f09e0a..29ed4320 100644 --- a/src/NoUnusedModules.elm +++ b/src/NoUnusedModules.elm @@ -114,10 +114,10 @@ fromModuleToGlobal fileKey moduleName moduleContext = foldGlobalContexts : GlobalContext -> GlobalContext -> GlobalContext -foldGlobalContexts contextA contextB = - { modules = Dict.union contextA.modules contextB.modules - , usedModules = Set.union contextA.usedModules contextB.usedModules - , isPackage = contextA.isPackage +foldGlobalContexts newContext previousContext = + { modules = Dict.union previousContext.modules newContext.modules + , usedModules = Set.union previousContext.usedModules newContext.usedModules + , isPackage = previousContext.isPackage } diff --git a/src/Review/Rule.elm b/src/Review/Rule.elm index d7f21cc4..8e98608a 100644 --- a/src/Review/Rule.elm +++ b/src/Review/Rule.elm @@ -846,6 +846,7 @@ importedModulesFirst (MultiSchema schema) startCache project = contextsAndErrorsPerFile : List ( List Error, globalContext ) contextsAndErrorsPerFile = + -- TODO select leaf nodes and fold their contexts only newCache |> Dict.values |> List.map (\cacheEntry -> ( cacheEntry.errors, cacheEntry.context )) diff --git a/src/Scope2.elm b/src/Scope2.elm index ec331997..af8a861e 100644 --- a/src/Scope2.elm +++ b/src/Scope2.elm @@ -1,6 +1,6 @@ module Scope2 exposing ( GlobalContext, ModuleContext - , addGlobalVisitors, addModuleVisitors, initGlobalContext, fromGlobalToModule, fromModuleToGlobal, foldGlobalContexts + , GlobalSetterGetter, addGlobalVisitors, ModuleSetterGetter, addModuleVisitors, initGlobalContext, fromGlobalToModule, fromModuleToGlobal, foldGlobalContexts , realFunctionOrType ) @@ -14,7 +14,7 @@ module Scope2 exposing # Usage -@docs addGlobalVisitors, addModuleVisitors, initGlobalContext, fromGlobalToModule, fromModuleToGlobal, foldGlobalContexts +@docs GlobalSetterGetter, addGlobalVisitors, ModuleSetterGetter, addModuleVisitors, initGlobalContext, fromGlobalToModule, fromModuleToGlobal, foldGlobalContexts # Access diff --git a/tests/NoUnusedExportsTest.elm b/tests/NoUnusedExportsTest.elm index 631f19d2..c0f63a57 100644 --- a/tests/NoUnusedExportsTest.elm +++ b/tests/NoUnusedExportsTest.elm @@ -161,22 +161,21 @@ main = text "" """ |> Review.Test.runWithProjectData application rule |> Review.Test.expectNoErrors - , Test.skip <| - test "should report the `main` function for a package when it is never used outside the module" <| - \() -> - """ + , test "should report the `main` function for a package when it is never used outside the module" <| + \() -> + """ module Main exposing (main) main = text "" """ - |> Review.Test.runWithProjectData package_ rule - |> Review.Test.expectErrors - [ Review.Test.error - { message = "Exposed function or type `main` is never used outside this module." - , details = details - , under = "main" - } - |> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 24 } } - ] + |> Review.Test.runWithProjectData package_ rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Exposed function or type `main` is never used outside this module." + , details = details + , under = "main" + } + |> Review.Test.atExactly { start = { row = 2, column = 23 }, end = { row = 2, column = 27 } } + ] , Test.skip <| test "should not report a function that does not refer to anything" <| \() ->