Backport work from review packages

This commit is contained in:
Jeroen Engels 2020-06-03 18:23:19 +02:00
parent d311c44f3d
commit 932c788b9f
23 changed files with 4003 additions and 82 deletions

View File

@ -8,14 +8,21 @@ import Test exposing (Test, describe, test)
all : Test
all =
describe "NoExposingEverything"
[ test "should not when a module exposes a limited set of things" <|
[ test "should not report anything when a module exposes a limited set of things" <|
\_ ->
"module A exposing (B(..), C, d)"
"""
module A exposing (B(..), C, d)
type B = B
d = 1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report when a module exposes everything" <|
\_ ->
"module A exposing (..)"
"""
module A exposing (..)
import B exposing (..)
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
@ -25,5 +32,6 @@ all =
]
, under = "(..)"
}
|> Review.Test.atExactly { start = { row = 2, column = 19 }, end = { row = 2, column = 23 } }
]
]

View File

@ -0,0 +1,205 @@
module NoMissingSubscriptionsCall exposing (rule)
{-|
@docs rule
-}
import Dict exposing (Dict)
import Elm.Syntax.Declaration as Declaration exposing (Declaration)
import Elm.Syntax.Expression as Expression exposing (Expression)
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 Scope
import Set exposing (Set)
{-| Reports likely missing calls to a `subscriptions` function.
config =
[ NoMissingSubscriptionsCall.rule
]
## Fail
import SomeModule
update msg model =
case msg of
UsedMsg subMsg ->
SomeModule.update subMsg model.used
subscriptions model =
-- We used `SomeModule.update` but not `SomeModule.subscriptions`
Sub.none
This won't fail if `SomeModule` does not define a `subscriptions` function.
## Success
import SomeModule
update msg model =
case msg of
UsedMsg subMsg ->
SomeModule.update subMsg model.used
subscriptions model =
SomeModule.subscriptions
-}
rule : Rule
rule =
Rule.newProjectRuleSchema "NoMissingSubscriptionsCall" initialProjectContext
|> Scope.addProjectVisitors
|> Rule.withModuleVisitor moduleVisitor
|> Rule.withModuleContext
{ fromProjectToModule = fromProjectToModule
, fromModuleToProject = fromModuleToProject
, foldProjectContexts = foldProjectContexts
}
|> Rule.withContextFromImportedModules
|> Rule.fromProjectRuleSchema
moduleVisitor : Rule.ModuleRuleSchema schemaState ModuleContext -> Rule.ModuleRuleSchema { schemaState | hasAtLeastOneVisitor : () } ModuleContext
moduleVisitor schema =
schema
|> Rule.withDeclarationVisitor declarationVisitor
|> Rule.withExpressionVisitor expressionVisitor
|> Rule.withFinalModuleEvaluation finalEvaluation
type alias ProjectContext =
{ scope : Scope.ProjectContext
, modulesThatExposeSubscriptionsAndUpdate : Set ModuleName
}
type alias ModuleContext =
{ scope : Scope.ModuleContext
, modulesThatExposeSubscriptionsAndUpdate : Set ModuleName
--, usesUpdate : Bool
--, usesSubscription : Bool
, definesUpdate : Bool
, definesSubscriptions : Bool
, usesUpdateOfModule : Dict ModuleName Range
, usesSubscriptionsOfModule : Set ModuleName
}
initialProjectContext : ProjectContext
initialProjectContext =
{ scope = Scope.initialProjectContext
, modulesThatExposeSubscriptionsAndUpdate = Set.empty
}
fromProjectToModule : Rule.ModuleKey -> Node ModuleName -> ProjectContext -> ModuleContext
fromProjectToModule _ _ projectContext =
{ scope = Scope.fromProjectToModule projectContext.scope
, modulesThatExposeSubscriptionsAndUpdate = projectContext.modulesThatExposeSubscriptionsAndUpdate
, definesUpdate = False
, definesSubscriptions = False
, usesUpdateOfModule = Dict.empty
, usesSubscriptionsOfModule = Set.empty
}
fromModuleToProject : Rule.ModuleKey -> Node ModuleName -> ModuleContext -> ProjectContext
fromModuleToProject _ moduleName moduleContext =
{ scope = Scope.fromModuleToProject moduleName moduleContext.scope
, modulesThatExposeSubscriptionsAndUpdate =
if moduleContext.definesSubscriptions && moduleContext.definesUpdate then
Set.singleton (Node.value moduleName)
else
Set.empty
}
foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext
foldProjectContexts newContext previousContext =
{ scope = Scope.foldProjectContexts newContext.scope previousContext.scope
, modulesThatExposeSubscriptionsAndUpdate =
Set.union
newContext.modulesThatExposeSubscriptionsAndUpdate
previousContext.modulesThatExposeSubscriptionsAndUpdate
}
expressionVisitor : Node Expression -> Rule.Direction -> ModuleContext -> ( List (Error {}), ModuleContext )
expressionVisitor node direction moduleContext =
case ( direction, Node.value node ) of
( Rule.OnEnter, Expression.FunctionOrValue moduleName "update" ) ->
let
realModuleName : List String
realModuleName =
Scope.moduleNameForValue moduleContext.scope "update" moduleName
in
if Set.member realModuleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
( [], { moduleContext | usesUpdateOfModule = Dict.insert realModuleName (Node.range node) moduleContext.usesUpdateOfModule } )
else
( [], moduleContext )
( Rule.OnEnter, Expression.FunctionOrValue moduleName "subscriptions" ) ->
let
realModuleName : List String
realModuleName =
Scope.moduleNameForValue moduleContext.scope "subscriptions" moduleName
in
if Set.member realModuleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
( [], { moduleContext | usesSubscriptionsOfModule = Set.insert realModuleName moduleContext.usesSubscriptionsOfModule } )
else
( [], moduleContext )
_ ->
( [], moduleContext )
declarationVisitor : Node Declaration -> Rule.Direction -> ModuleContext -> ( List (Error nothing), ModuleContext )
declarationVisitor declaration direction moduleContext =
case ( direction, Node.value declaration ) of
( Rule.OnEnter, Declaration.FunctionDeclaration function ) ->
case
function.declaration
|> Node.value
|> .name
|> Node.value
of
"update" ->
( [], { moduleContext | definesUpdate = True } )
"subscriptions" ->
( [], { moduleContext | definesSubscriptions = True } )
_ ->
( [], moduleContext )
_ ->
( [], moduleContext )
finalEvaluation : ModuleContext -> List (Error {})
finalEvaluation moduleContext =
moduleContext.usesUpdateOfModule
|> Dict.filter (\moduleName _ -> not <| Set.member moduleName moduleContext.usesSubscriptionsOfModule)
|> Dict.toList
|> List.map
(\( moduleName, range ) ->
Rule.error
{ message = "Missing subscriptions call to " ++ String.join "." moduleName ++ ".subscriptions"
, details =
[ "The " ++ String.join "." moduleName ++ " module defines a `subscriptions` function, which you are not using even though you are using its `update` function. This makes me think that you are not subscribing to all the things you should."
]
}
range
)

View File

@ -0,0 +1,102 @@
module NoMissingSubscriptionsCallTest exposing (all)
import NoMissingSubscriptionsCall exposing (rule)
import Review.Test
import Test exposing (Test, describe, test)
all : Test
all =
describe "NoMissingSubscriptionsCall"
[ test "should report when a module defines when a module exposes a limited set of things" <|
\_ ->
[ """
module Main exposing (main)
import Imported
main = Browser.element
{ init = init
, update = update
, subscriptions = subscriptions
, view = view
}
update msg model =
case msg of
UsedMsg subMsg ->
Imported.update subMsg model.used
subscriptions model =
Sub.none
""", """
module Imported exposing (update, subscriptions)
update = Debug.todo ""
subscriptions = Debug.todo ""
""" ]
|> Review.Test.runOnModules rule
|> Review.Test.expectErrorsForModules
[ ( "Main"
, [ Review.Test.error
{ message = "Missing subscriptions call to Imported.subscriptions"
, details =
[ "The Imported module defines a `subscriptions` function, which you are not using even though you are using its `update` function. This makes me think that you are not subscribing to all the things you should."
]
, under = "Imported.update"
}
]
)
]
, test "should not report anything when the imported module's subscriptions function is used" <|
\_ ->
[ """
module Main exposing (main)
import Imported
main = Browser.element
{ init = init
, update = update
, subscriptions = subscriptions
, view = view
}
update msg model =
case msg of
UsedMsg subMsg ->
Imported.update subMsg model.used
subscriptions model =
Imported.subscriptions
""", """
module Imported exposing (update, subscriptions)
update = Debug.todo ""
subscriptions = Debug.todo ""
""" ]
|> Review.Test.runOnModules rule
|> Review.Test.expectNoErrors
, test "should not report anything when the imported module does not define a subscriptions function" <|
\_ ->
[ """
module Main exposing (main)
import Imported
main = Browser.element
{ init = init
, update = update
, subscriptions = subscriptions
, view = view
}
update msg model =
case msg of
UsedMsg subMsg ->
Imported.update subMsg model.used
subscriptions model =
Sub.none
""", """
module Imported exposing (update)
update = Debug.todo ""
""" ]
|> Review.Test.runOnModules rule
|> Review.Test.expectNoErrors
]

View File

@ -28,6 +28,7 @@ For that, enable [`NoMissingTypeAnnotation`](./NoMissingTypeAnnotation).
a : number
a =
let
-- Missing annotation
b =
2
in
@ -36,6 +37,8 @@ For that, enable [`NoMissingTypeAnnotation`](./NoMissingTypeAnnotation).
## Success
-- Top-level annotation is not necessary, but good to have!
a : number
a =
let
b : number

105
tests/NoRecursiveUpdate.elm Normal file
View File

@ -0,0 +1,105 @@
module NoRecursiveUpdate exposing (rule)
{-|
@docs rule
-}
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 Review.Rule as Rule exposing (Error, Rule)
{-| Reports when the `update` function calls itself.
This is often done in order to have one message (A) trigger (all or some of) the same
model updates and commands as another message (B).
update msg model =
case msg of
Foo ->
{ model | foo = True }
Bar ->
update Foo { model | bar = True }
This is advised against, because if the way that message B is handled changes,
that will implicitly change how message A is handled in ways that may not have
been foreseen.
A better solution is to move the common handling into a different function and
have it called in the handling of both messages.
update msg model =
case msg of
Foo ->
commonOperationOnModel model
Bar ->
commonOperationOnModel { model | bar = True }
commonOperationOnModel model =
{ model | foo = True }
Calls to other modules' `update` function are allowed.
To add the rule to your configuration:
config =
[ NoRecursiveUpdate.rule
]
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoRecursiveUpdate" { isInUpdateFunction = False }
|> Rule.withDeclarationVisitor declarationVisitor
|> Rule.withExpressionVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
type alias Context =
{ isInUpdateFunction : Bool
}
declarationVisitor : Node Declaration -> Rule.Direction -> Context -> ( List nothing, Context )
declarationVisitor declaration direction _ =
case ( direction, Node.value declaration ) of
( Rule.OnEnter, Declaration.FunctionDeclaration function ) ->
( []
, { isInUpdateFunction =
(function.declaration
|> Node.value
|> .name
|> Node.value
)
== "update"
}
)
_ ->
( [], { isInUpdateFunction = False } )
expressionVisitor : Node Expression -> Rule.Direction -> Context -> ( List (Error {}), Context )
expressionVisitor node direction context =
if context.isInUpdateFunction then
case ( direction, Node.value node ) of
( Rule.OnEnter, Expression.FunctionOrValue [] "update" ) ->
( [ Rule.error
{ message = "`update` shouldn't call itself"
, details = [ "If you wish to have the same behavior for different messages, move that behavior into a new function and call have it called in the handling of both messages." ]
}
(Node.range node)
]
, context
)
_ ->
( [], context )
else
( [], context )

View File

@ -0,0 +1,67 @@
module NoRecursiveUpdateTest exposing (all)
import NoRecursiveUpdate exposing (rule)
import Review.Test
import Test exposing (Test, describe, test)
all : Test
all =
describe "NoRecursiveUpdate"
[ test "should report when an update function calls itself" <|
\_ ->
"""
module A exposing (..)
update msg model =
case msg of
Foo ->
{ model | foo = True }
Bar ->
update Foo { model | bar = True }
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "`update` shouldn't call itself"
, details = [ "If you wish to have the same behavior for different messages, move that behavior into a new function and call have it called in the handling of both messages." ]
, under = "update"
}
|> Review.Test.atExactly { start = { row = 8, column = 7 }, end = { row = 8, column = 13 } }
]
, test "should not report other recursive functions" <|
\_ ->
"""
module A exposing (..)
recursive list =
case list of
[] -> Nothing
_ :: xs -> recursive xs
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report calls to update functions from other modules" <|
\_ ->
"""
module A exposing (..)
update msg model =
case msg of
Bar subMsg ->
Bar.update subMsg model.bar
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report references to update functions outside the update functions" <|
\_ ->
"""
module A exposing (..)
main = Browser.sandbox { init = init, update = update, view = view }
update msg model =
case msg of
Bar ->
1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]

View File

@ -319,7 +319,7 @@ moduleDefinitionVisitor moduleNode context =
List.filterMap
(\node ->
case Node.value node of
Exposing.TypeExpose { name, open } ->
Exposing.TypeExpose { name } ->
Just name
_ ->

View File

@ -142,15 +142,15 @@ type Foo = Bar | Baz"""
|> Review.Test.runWithProjectData project (rule [])
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Type constructor `Baz` is not used."
, details = details
, under = "Baz"
}
, Review.Test.error
{ message = "Type constructor `Bar` is not used."
, details = details
, under = "Bar"
}
, Review.Test.error
{ message = "Type constructor `Baz` is not used."
, details = details
, under = "Baz"
}
]
]

View File

@ -98,7 +98,7 @@ fromProjectToModule _ _ projectContext =
fromModuleToProject : Rule.ModuleKey -> Node ModuleName -> ModuleContext -> ProjectContext
fromModuleToProject moduleKey moduleName importedModuleNames =
fromModuleToProject _ _ importedModuleNames =
{ moduleNameToDependency = Dict.empty
, directProjectDependencies = Set.empty
, importedModuleNames = importedModuleNames
@ -194,7 +194,10 @@ error elmJsonKey packageName =
Rule.errorForElmJson elmJsonKey
(\elmJson ->
{ message = "Unused dependency `" ++ packageName ++ "`"
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall " ++ packageName ++ "`" ]
, details =
[ "To remove it, I recommend running the following command:"
, " elm-json uninstall " ++ packageName
]
, range = findPackageNameInElmJson packageName elmJson
}
)

View File

@ -176,12 +176,18 @@ a = 1
|> Review.Test.expectErrorsForElmJson
[ Review.Test.error
{ message = "Unused dependency `author/package-with-bar`"
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-bar`" ]
, details =
[ "To remove it, I recommend running the following command:"
, " elm-json uninstall author/package-with-bar"
]
, under = "author/package-with-bar"
}
, Review.Test.error
{ message = "Unused dependency `author/package-with-foo`"
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-foo`" ]
, details =
[ "To remove it, I recommend running the following command:"
, " elm-json uninstall author/package-with-foo"
]
, under = "author/package-with-foo"
}
]
@ -205,12 +211,18 @@ a = 1
|> Review.Test.expectErrorsForElmJson
[ Review.Test.error
{ message = "Unused dependency `author/package-with-bar`"
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-bar`" ]
, details =
[ "To remove it, I recommend running the following command:"
, " elm-json uninstall author/package-with-bar"
]
, under = "author/package-with-bar"
}
, Review.Test.error
{ message = "Unused dependency `author/package-with-foo`"
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-foo`" ]
, details =
[ "To remove it, I recommend running the following command:"
, " elm-json uninstall author/package-with-foo"
]
, under = "author/package-with-foo"
}
]

View File

@ -18,11 +18,13 @@ import Elm.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 Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
import Review.Fix as Fix exposing (Fix)
import Review.Rule as Rule exposing (Error, Rule)
import Scope
import Set exposing (Set)
@ -59,6 +61,7 @@ moduleVisitor : Rule.ModuleRuleSchema {} ModuleContext -> Rule.ModuleRuleSchema
moduleVisitor schema =
schema
|> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
|> Rule.withImportVisitor importVisitor
|> Rule.withDeclarationListVisitor declarationListVisitor
|> Rule.withExpressionVisitor expressionVisitor
@ -73,18 +76,25 @@ type alias ProjectContext =
, modules :
Dict ModuleName
{ moduleKey : Rule.ModuleKey
, exposed : Dict String { range : Range, exposedElement : ExposedElement }
, exposed : Dict String ExposedElement
}
, used : Set ( ModuleName, String )
}
type alias ExposedElement =
{ range : Range
, rangeToRemove : Maybe Range
, elementType : ExposedElementType
}
type ProjectType
= IsApplication
| IsPackage (Set (List String))
type ExposedElement
type ExposedElementType
= Function
| TypeOrTypeAlias
| ExposedType
@ -93,7 +103,7 @@ type ExposedElement
type alias ModuleContext =
{ scope : Scope.ModuleContext
, exposesEverything : Bool
, exposed : Dict String { range : Range, exposedElement : ExposedElement }
, exposed : Dict String ExposedElement
, used : Set ( ModuleName, String )
, elementsNotToReport : Set String
}
@ -109,7 +119,7 @@ initialProjectContext =
fromProjectToModule : Rule.ModuleKey -> Node ModuleName -> ProjectContext -> ModuleContext
fromProjectToModule moduleKey moduleName projectContext =
fromProjectToModule _ _ projectContext =
{ scope = Scope.fromProjectToModule projectContext.scope
, exposesEverything = False
, exposed = Dict.empty
@ -198,16 +208,16 @@ finalEvaluationForProject projectContext =
|> List.concatMap
(\( moduleName, { moduleKey, exposed } ) ->
exposed
|> removeApplicationExceptions projectContext moduleName
|> removeApplicationExceptions projectContext
|> removeReviewConfig moduleName
|> Dict.filter (\name _ -> not <| Set.member ( moduleName, name ) projectContext.used)
|> Dict.toList
|> List.concatMap
(\( name, { range, exposedElement } ) ->
(\( name, element ) ->
let
what : String
what =
case exposedElement of
case element.elementType of
Function ->
"Exposed function or value"
@ -216,12 +226,22 @@ finalEvaluationForProject projectContext =
ExposedType ->
"Exposed type"
fixes : List Fix
fixes =
case element.rangeToRemove of
Just rangeToRemove ->
[ Fix.removeRange rangeToRemove ]
Nothing ->
[]
in
[ Rule.errorForModule moduleKey
[ Rule.errorForModuleWithFix moduleKey
{ message = what ++ " `" ++ name ++ "` is never used outside this module."
, details = [ "This exposed element is never used. You may want to remove it to keep your project clean, and maybe detect some unused code in your project." ]
}
range
element.range
fixes
]
)
)
@ -237,8 +257,8 @@ removeExposedPackages projectContext dict =
Dict.filter (\name _ -> not <| Set.member name exposedModuleNames) dict
removeApplicationExceptions : ProjectContext -> ModuleName -> Dict String a -> Dict String a
removeApplicationExceptions projectContext moduleName dict =
removeApplicationExceptions : ProjectContext -> Dict String a -> Dict String a
removeApplicationExceptions projectContext dict =
case projectContext.projectType of
IsApplication ->
Dict.remove "main" dict
@ -267,27 +287,81 @@ moduleDefinitionVisitor moduleNode moduleContext =
( [], { moduleContext | exposesEverything = True } )
Exposing.Explicit list ->
( [], { moduleContext | exposed = exposedElements list } )
( [], { moduleContext | exposed = collectExposedElements list } )
exposedElements : List (Node Exposing.TopLevelExpose) -> Dict String { range : Range, exposedElement : ExposedElement }
exposedElements nodes =
collectExposedElements : List (Node Exposing.TopLevelExpose) -> Dict String ExposedElement
collectExposedElements nodes =
let
listWithPreviousRange : List (Maybe Range)
listWithPreviousRange =
Nothing
:: (nodes
|> List.map (Node.range >> Just)
|> List.take (List.length nodes - 1)
)
listWithNextRange : List Range
listWithNextRange =
(nodes
|> List.map Node.range
|> List.drop 1
)
++ [ { start = { row = 0, column = 0 }, end = { row = 0, column = 0 } } ]
in
nodes
|> List.filterMap
(\node ->
case Node.value node of
|> List.map3 (\prev next current -> ( prev, current, next )) listWithPreviousRange listWithNextRange
|> List.indexedMap
(\index ( maybePreviousRange, Node range value, nextRange ) ->
let
rangeToRemove : Maybe Range
rangeToRemove =
if List.length nodes == 1 then
Nothing
else if index == 0 then
Just { range | end = nextRange.start }
else
case maybePreviousRange of
Nothing ->
Just range
Just previousRange ->
Just { range | start = previousRange.end }
in
case value of
Exposing.FunctionExpose name ->
Just ( name, { range = untilEndOfVariable name (Node.range node), exposedElement = Function } )
Just
( name
, { range = untilEndOfVariable name range
, rangeToRemove = rangeToRemove
, elementType = Function
}
)
Exposing.TypeOrAliasExpose name ->
Just ( name, { range = untilEndOfVariable name (Node.range node), exposedElement = TypeOrTypeAlias } )
Just
( name
, { range = untilEndOfVariable name range
, rangeToRemove = rangeToRemove
, elementType = TypeOrTypeAlias
}
)
Exposing.TypeExpose { name } ->
Just ( name, { range = untilEndOfVariable name (Node.range node), exposedElement = ExposedType } )
Just
( name
, { range = untilEndOfVariable name range
, rangeToRemove = Nothing
, elementType = ExposedType
}
)
Exposing.InfixExpose _ ->
Nothing
)
|> List.filterMap identity
|> Dict.fromList
@ -301,6 +375,49 @@ untilEndOfVariable name range =
-- IMPORT VISITOR
importVisitor : Node Import -> ModuleContext -> ( List nothing, ModuleContext )
importVisitor node moduleContext =
case (Node.value node).exposingList |> Maybe.map Node.value of
Just (Exposing.Explicit list) ->
let
moduleName : ModuleName
moduleName =
Node.value (Node.value node).moduleName
usedElements : List ( ModuleName, String )
usedElements =
list
|> List.filterMap
(Node.value
>> (\element ->
case element of
Exposing.FunctionExpose name ->
Just ( moduleName, name )
Exposing.TypeOrAliasExpose name ->
Just ( moduleName, name )
Exposing.TypeExpose { name } ->
Just ( moduleName, name )
Exposing.InfixExpose _ ->
Nothing
)
)
in
( [], registerMultipleAsUsed usedElements moduleContext )
Just (Exposing.All _) ->
( [], moduleContext )
Nothing ->
( [], moduleContext )
-- DECLARATION LIST VISITOR
@ -436,7 +553,7 @@ typesUsedInDeclaration moduleContext declaration =
|> List.concatMap (Node.value >> .arguments)
|> List.concatMap (collectTypesFromTypeAnnotation moduleContext.scope)
, not <|
case Dict.get (Node.value type_.name) moduleContext.exposed |> Maybe.map .exposedElement of
case Dict.get (Node.value type_.name) moduleContext.exposed |> Maybe.map .elementType of
Just ExposedType ->
True

View File

@ -96,6 +96,7 @@ all =
, typesTests
, typeAliasesTests
, duplicateModuleNameTests
, importsTests
-- TODO Add tests that report exposing the type's variants if they are never used.
]
@ -183,6 +184,38 @@ main = exposed
}
|> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 27 } }
]
, test "should propose a fix for unused exports if there are others exposed elements" <|
\() ->
"""
module A exposing (exposed1, exposed2)
exposed1 = 1
exposed2 = 2
"""
|> Review.Test.runWithProjectData package_ rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Exposed function or value `exposed1` is never used outside this module."
, details = details
, under = "exposed1"
}
|> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 28 } }
|> Review.Test.whenFixed """
module A exposing (exposed2)
exposed1 = 1
exposed2 = 2
"""
, Review.Test.error
{ message = "Exposed function or value `exposed2` is never used outside this module."
, details = details
, under = "exposed2"
}
|> Review.Test.atExactly { start = { row = 2, column = 30 }, end = { row = 2, column = 38 } }
|> Review.Test.whenFixed """
module A exposing (exposed1)
exposed1 = 1
exposed2 = 2
"""
]
, test "should not report anything for modules that expose everything`" <|
\() ->
"""
@ -391,6 +424,11 @@ type alias B = A.OtherType
, under = "MyType"
}
|> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
|> Review.Test.whenFixed """
module A exposing (OtherType)
type MyType = VariantA | VariantB
type OtherType = Thing MyType
"""
]
)
]
@ -427,6 +465,11 @@ type alias B = A.OtherType
, under = "MyType"
}
|> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
|> Review.Test.whenFixed """
module A exposing (OtherType)
type MyType = VariantA | VariantB
type OtherType = OtherThing | SomeThing ((), List MyType)
"""
]
)
]
@ -566,6 +609,11 @@ type alias B = A.OtherType
, under = "MyType"
}
|> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
|> Review.Test.whenFixed """
module A exposing (OtherType)
type alias MyType = {}
type OtherType = OtherType MyType
"""
]
)
]
@ -602,6 +650,11 @@ type alias B = A.OtherType
, under = "MyType"
}
|> Review.Test.atExactly { start = { row = 2, column = 20 }, end = { row = 2, column = 26 } }
|> Review.Test.whenFixed """
module A exposing (OtherType)
type alias MyType = {}
type OtherType = OtherThing | SomeThing ((), List MyType)
"""
]
)
]
@ -630,6 +683,15 @@ a = A.Card A.init A.toElement
, under = "Link"
}
|> Review.Test.atExactly { start = { row = 3, column = 7 }, end = { row = 3, column = 11 } }
|> Review.Test.whenFixed """module A
exposing ( Card
, init
, toElement
)
type Card = Card
type Link = Link
init = 1
"""
]
)
]
@ -653,3 +715,22 @@ b = 2
|> Review.Test.runOnModulesWithProjectData application rule
|> Review.Test.expectNoErrors
]
importsTests : Test
importsTests =
describe "Imports"
[ test "should not report an export if it is imported by name" <|
\() ->
[ """module Main exposing (main)
import B exposing (Type1, Type2(..), TypeAlias, b)
main = 1
""", """module B exposing (Type1, Type2(..), TypeAlias, b)
type Type1 = Type1
type Type2 = Type2
type alias TypeAlias = {}
b = 2
""" ]
|> Review.Test.runOnModulesWithProjectData application rule
|> Review.Test.expectNoErrors
]

View File

@ -92,7 +92,7 @@ fromElement x =
{-| Return the head of the list.
-}
head : Nonempty a -> a
head (Nonempty x xs) =
head (Nonempty x _) =
x

View File

@ -0,0 +1,520 @@
module NoUnused.Parameters exposing (rule)
{-| Report parameters that are not used.
# Rule
@docs rule
-}
import Elm.Syntax.Declaration as Declaration exposing (Declaration)
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.ModuleName exposing (ModuleName)
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.Writer as Writer
import NoUnused.Patterns.NameVisitor as NameVisitor
import Review.Fix as Fix
import Review.Rule as Rule exposing (Rule)
import Set exposing (Set)
{-| Report parameters that are not used.
config =
[ NoUnused.Parameters.rule
]
This rule looks within function arguments, let functions and lambdas to find any values that are unused. It will report any parameters that are not used.
## Fixes for lambdas
We're only offering fixes for lambdas here because we believe unused parameters in functions are a code smell that should be refactored.
## Fail
Value `something` is not used:
add1 number =
1
## Success
add1 number =
number + 1
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoUnused.Parameters" initialContext
|> Rule.withDeclarationVisitor declarationVisitor
|> Rule.withExpressionVisitor expressionVisitor
|> NameVisitor.withValueVisitor valueVisitor
|> Rule.fromModuleRuleSchema
declarationVisitor : Node Declaration -> Rule.Direction -> Context -> ( List (Rule.Error {}), Context )
declarationVisitor node direction context =
case ( direction, Node.value node ) of
( Rule.OnEnter, Declaration.FunctionDeclaration { declaration } ) ->
( [], rememberFunctionImplementation declaration context )
( Rule.OnExit, Declaration.FunctionDeclaration { declaration } ) ->
errorsForFunctionImplementation declaration context
_ ->
( [], context )
expressionVisitor : Node Expression -> Rule.Direction -> Context -> ( List (Rule.Error {}), Context )
expressionVisitor (Node _ expression) direction context =
case ( direction, expression ) of
( Rule.OnEnter, Expression.LambdaExpression { args } ) ->
( [], rememberPatternList args context )
( Rule.OnExit, Expression.LambdaExpression { args } ) ->
errorsForPatternList Lambda args context
( Rule.OnEnter, Expression.LetExpression { declarations } ) ->
( [], rememberLetDeclarationList declarations context )
( Rule.OnExit, Expression.LetExpression { declarations } ) ->
errorsForLetDeclarationList declarations context
_ ->
( [], context )
valueVisitor : Node ( ModuleName, String ) -> Context -> ( List (Rule.Error {}), Context )
valueVisitor (Node _ ( moduleName, value )) context =
case moduleName of
[] ->
( [], useValue value context )
_ ->
( [], context )
--- ON ENTER
rememberFunctionImplementation : Node Expression.FunctionImplementation -> Context -> Context
rememberFunctionImplementation (Node _ { arguments }) context =
rememberPatternList arguments context
rememberLetDeclarationList : List (Node Expression.LetDeclaration) -> Context -> Context
rememberLetDeclarationList list context =
List.foldl rememberLetDeclaration context list
rememberLetDeclaration : Node Expression.LetDeclaration -> Context -> Context
rememberLetDeclaration (Node _ letDeclaration) context =
case letDeclaration of
Expression.LetFunction { declaration } ->
rememberLetFunctionImplementation declaration context
Expression.LetDestructuring _ _ ->
context
rememberLetFunctionImplementation : Node Expression.FunctionImplementation -> Context -> Context
rememberLetFunctionImplementation (Node _ { arguments }) context =
rememberPatternList arguments context
rememberPatternList : List (Node Pattern) -> Context -> Context
rememberPatternList list context =
List.foldl rememberPattern context list
rememberPattern : Node Pattern -> Context -> Context
rememberPattern (Node _ pattern) context =
case pattern of
Pattern.AllPattern ->
context
Pattern.VarPattern value ->
rememberValue value context
Pattern.TuplePattern patterns ->
rememberPatternList patterns context
Pattern.RecordPattern values ->
rememberValueList values context
Pattern.UnConsPattern first second ->
context
|> rememberPattern first
|> rememberPattern second
Pattern.ListPattern patterns ->
rememberPatternList patterns context
Pattern.NamedPattern _ patterns ->
rememberPatternList patterns context
Pattern.AsPattern inner name ->
context
|> rememberPattern inner
|> rememberValue (Node.value name)
Pattern.ParenthesizedPattern inner ->
rememberPattern inner context
_ ->
context
rememberValueList : List (Node String) -> Context -> Context
rememberValueList list context =
List.foldl (Node.value >> rememberValue) context list
--- ON EXIT
singularDetails : List String
singularDetails =
[ "You should either use this parameter somewhere, or remove it at the location I pointed at." ]
pluralDetails : List String
pluralDetails =
[ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
removeDetails : List String
removeDetails =
[ "You should remove it at the location I pointed at." ]
andThen :
(Context -> ( List (Rule.Error {}), Context ))
-> ( List (Rule.Error {}), Context )
-> ( List (Rule.Error {}), Context )
andThen function ( errors, context ) =
let
( newErrors, newContext ) =
function context
in
( newErrors ++ errors, newContext )
errorsForFunctionImplementation : Node Expression.FunctionImplementation -> Context -> ( List (Rule.Error {}), Context )
errorsForFunctionImplementation (Node _ { arguments }) context =
errorsForPatternList Function arguments context
errorsForLetDeclarationList : List (Node Expression.LetDeclaration) -> Context -> ( List (Rule.Error {}), Context )
errorsForLetDeclarationList list context =
case list of
[] ->
( [], context )
first :: rest ->
context
|> errorsForLetDeclaration first
|> andThen (errorsForLetDeclarationList rest)
errorsForLetDeclaration : Node Expression.LetDeclaration -> Context -> ( List (Rule.Error {}), Context )
errorsForLetDeclaration (Node _ letDeclaration) context =
case letDeclaration of
Expression.LetFunction { declaration } ->
errorsForLetFunctionImplementation declaration context
Expression.LetDestructuring _ _ ->
( [], context )
errorsForLetFunctionImplementation : Node Expression.FunctionImplementation -> Context -> ( List (Rule.Error {}), Context )
errorsForLetFunctionImplementation (Node _ { arguments }) context =
errorsForPatternList Function arguments context
type PatternUse
= Lambda
| Function
errorsForPatternList : PatternUse -> List (Node Pattern) -> Context -> ( List (Rule.Error {}), Context )
errorsForPatternList use list context =
case list of
[] ->
( [], context )
first :: rest ->
context
|> errorsForPattern use first
|> andThen (errorsForPatternList use rest)
errorsForPattern : PatternUse -> Node Pattern -> Context -> ( List (Rule.Error {}), Context )
errorsForPattern use (Node range pattern) context =
case pattern of
Pattern.AllPattern ->
( [], context )
Pattern.VarPattern value ->
errorsForValue use value range context
Pattern.RecordPattern values ->
errorsForRecordValueList use range values context
Pattern.TuplePattern [ Node _ Pattern.AllPattern, Node _ Pattern.AllPattern ] ->
errorsForUselessTuple use range context
Pattern.TuplePattern [ Node _ Pattern.AllPattern, Node _ Pattern.AllPattern, Node _ Pattern.AllPattern ] ->
errorsForUselessTuple use range context
Pattern.TuplePattern patterns ->
errorsForPatternList use patterns context
Pattern.UnConsPattern first second ->
errorsForPatternList use [ first, second ] context
Pattern.ListPattern patterns ->
errorsForPatternList use patterns context
Pattern.NamedPattern _ patterns ->
if List.all isAllPattern patterns then
errorsForUselessNamePattern use range context
else
errorsForPatternList use patterns context
Pattern.AsPattern inner name ->
context
|> errorsForAsPattern use range inner name
|> andThen (errorsForPattern use inner)
Pattern.ParenthesizedPattern inner ->
errorsForPattern use inner context
_ ->
( [], context )
errorsForUselessNamePattern : PatternUse -> Range -> Context -> ( List (Rule.Error {}), Context )
errorsForUselessNamePattern use range context =
let
fix =
case use of
Lambda ->
[ Fix.replaceRangeBy range "_" ]
Function ->
[]
in
( [ Rule.errorWithFix
{ message = "Named pattern is not needed."
, details = removeDetails
}
range
fix
]
, context
)
errorsForUselessTuple : PatternUse -> Range -> Context -> ( List (Rule.Error {}), Context )
errorsForUselessTuple use range context =
let
fix =
case use of
Lambda ->
[ Fix.replaceRangeBy range "_" ]
Function ->
[]
in
( [ Rule.errorWithFix
{ message = "Tuple pattern is not needed."
, details = removeDetails
}
range
fix
]
, context
)
errorsForRecordValueList : PatternUse -> Range -> List (Node String) -> Context -> ( List (Rule.Error {}), Context )
errorsForRecordValueList use recordRange list context =
let
( unused, used ) =
List.partition (\(Node _ value) -> Set.member value context) list
in
case unused of
[] ->
( [], context )
firstNode :: restNodes ->
let
first =
firstNode |> Node.value
rest =
List.map Node.value restNodes
errorRange =
Range.combine (List.map Node.range unused)
fix =
case ( use, used ) of
( Lambda, [] ) ->
[ Fix.replaceRangeBy recordRange "_" ]
( Lambda, _ ) ->
[ Node Range.emptyRange (Pattern.RecordPattern used)
|> Writer.writePattern
|> Writer.write
|> Fix.replaceRangeBy recordRange
]
( Function, _ ) ->
[]
in
( [ Rule.errorWithFix
{ message = listToMessage first rest
, details = listToDetails first rest
}
errorRange
fix
]
, List.foldl forgetNode context unused
)
listToMessage : String -> List String -> String
listToMessage first rest =
case List.reverse rest of
[] ->
"Parameter `" ++ first ++ "` is not used."
last :: middle ->
"Parameters `" ++ String.join "`, `" (first :: middle) ++ "` and `" ++ last ++ "` are not used."
listToDetails : String -> List String -> List String
listToDetails _ rest =
case rest of
[] ->
singularDetails
_ ->
pluralDetails
errorsForAsPattern : PatternUse -> Range -> Node Pattern -> Node String -> Context -> ( List (Rule.Error {}), Context )
errorsForAsPattern use patternRange inner (Node range name) context =
if Set.member name context then
let
fix =
case use of
Lambda ->
[ inner
|> Writer.writePattern
|> Writer.write
|> Fix.replaceRangeBy patternRange
]
Function ->
[]
in
( [ Rule.errorWithFix
{ message = "Pattern alias `" ++ name ++ "` is not used."
, details = singularDetails
}
range
fix
]
, Set.remove name context
)
else if isAllPattern inner then
( [ Rule.errorWithFix
{ message = "Pattern `_` is not needed."
, details = removeDetails
}
(Node.range inner)
[ Fix.replaceRangeBy patternRange name ]
]
, Set.remove name context
)
else
( [], context )
isAllPattern : Node Pattern -> Bool
isAllPattern (Node _ pattern) =
case pattern of
Pattern.AllPattern ->
True
_ ->
False
forgetNode : Node String -> Context -> Context
forgetNode (Node _ value) context =
Set.remove value context
--- CONTEXT
type alias Context =
Set String
initialContext : Context
initialContext =
Set.empty
errorsForValue : PatternUse -> String -> Range -> Context -> ( List (Rule.Error {}), Context )
errorsForValue use value range context =
if Set.member value context then
let
fix =
case use of
Lambda ->
[ Fix.replaceRangeBy range "_" ]
Function ->
[]
in
( [ Rule.errorWithFix
{ message = "Parameter `" ++ value ++ "` is not used."
, details = singularDetails
}
range
fix
]
, Set.remove value context
)
else
( [], context )
rememberValue : String -> Context -> Context
rememberValue value context =
Set.insert value context
useValue : String -> Context -> Context
useValue value context =
Set.remove value context

View File

@ -0,0 +1,889 @@
module NoUnused.ParametersTests exposing (all)
import NoUnused.Parameters exposing (rule)
import Review.Test
import Test exposing (Test, describe, test)
details : List String
details =
[ "You should either use this parameter somewhere, or remove it at the location I pointed at." ]
all : Test
all =
describe "NoUnused.Patterns"
[ describe "in Function arguments" functionArgumentTests
, describe "in Lambda arguments" lambdaArgumentTests
, describe "in Let Functions" letFunctionTests
--- un-tests
, describe "in Case branches" caseTests
, describe "in Let destructuring" letDestructuringTests
--- in lambda
, describe "with as pattern in lambdas" lambdaAsPatternTests
, describe "with named pattern in lambdas" lambdaNamedPatternTests
, describe "with record pattern in lambdas" lambdaRecordPatternTests
, describe "with tuple pattern in lambdas" lambdaTuplePatternTests
--- in function
, describe "with as pattern in functions" functionAsPatternTests
, describe "with named pattern in functions" functionNamedPatternTests
, describe "with record pattern in functions" functionRecordPatternTests
, describe "with tuple pattern in functions" functionTuplePatternTests
]
caseTests : List Test
caseTests =
[ test "should not report unused values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
bish ->
Nothing
bash ->
Nothing
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report list of unused values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
[] -> 0
[one] -> 1
[first, two] -> 2
more -> 3
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report uncons of unused values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
[] -> 0
one :: [] -> 1
first :: two :: [] -> 2
_ :: _ :: more -> 3
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
functionArgumentTests : List Test
functionArgumentTests =
[ test "should report unused arguments" <|
\() ->
"""
module A exposing (..)
foo : Int -> String -> String -> String
foo one two three =
three
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `one` is not used."
, details = details
, under = "one"
}
, Review.Test.error
{ message = "Parameter `two` is not used."
, details = details
, under = "two"
}
]
, test "should not consider values from other modules" <|
\() ->
"""
module A exposing (..)
foo one =
Bar.one
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `one` is not used."
, details = details
, under = "one"
}
|> Review.Test.atExactly { start = { row = 3, column = 5 }, end = { row = 3, column = 8 } }
]
]
lambdaArgumentTests : List Test
lambdaArgumentTests =
[ test "should report unused arguments" <|
\() ->
"""
module A exposing (..)
foo =
List.map (\\value -> Nothing) list
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `value` is not used."
, details = details
, under = "value"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
List.map (\\_ -> Nothing) list
"""
]
]
letDestructuringTests : List Test
letDestructuringTests =
[ test "should not report unused values" <|
\() ->
"""
module A exposing (..)
foo =
let
( left, right ) =
tupleValue
in
left
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report unused patterns that are aliased" <|
\() ->
"""
module A exposing (..)
foo =
let
(_ as bar) = 1
in
bar
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
letFunctionTests : List Test
letFunctionTests =
[ test "should report unused arguments" <|
\() ->
"""
module A exposing (..)
foo =
let
one oneValue =
1
two twoValue =
2
in
one two 3
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `oneValue` is not used."
, details = details
, under = "oneValue"
}
, Review.Test.error
{ message = "Parameter `twoValue` is not used."
, details = details
, under = "twoValue"
}
]
, test "should not report unused let functions" <|
\() ->
"""
module A exposing (..)
foo =
let
value =
something 5
in
bar
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
--- LAMBDA PATTERN TESTS ------------------------
lambdaAsPatternTests : List Test
lambdaAsPatternTests =
[ test "should report unused pattern aliases" <|
\() ->
"""
module A exposing (..)
foo =
\\({ bish, bash } as bosh) ->
( bish, bash )
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern alias `bosh` is not used."
, details = details
, under = "bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\({bish, bash}) ->
( bish, bash )
"""
]
, test "should report unused patterns in an as pattern" <|
\() ->
"""
module A exposing (..)
foo =
\\({ bish, bash } as bosh) ->
( bish, bosh )
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bash` is not used."
, details = details
, under = "bash"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\({bish} as bosh) ->
( bish, bosh )
"""
]
, test "should report unused patterns and unused aliases" <|
\() ->
"""
module A exposing (..)
foo =
\\({ bish, bash } as bosh) ->
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bash` is not used."
, details = details
, under = "bash"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\({bish} as bosh) ->
bish
"""
, Review.Test.error
{ message = "Pattern alias `bosh` is not used."
, details = details
, under = "bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\({bish, bash}) ->
bish
"""
]
, test "should report unused patterns that are aliased" <|
\() ->
"""
module A exposing (..)
foo =
\\(_ as bar) -> bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern `_` is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "_"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\(bar) -> bar
"""
]
, test "should report nested unused pattern aliases" <|
\() ->
"""
module A exposing (..)
foo =
\\(Named ( _, ( Named bash ) as bish )) ->
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern alias `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\(Named ( _, ( Named bash ) )) ->
bash
"""
]
]
lambdaNamedPatternTests : List Test
lambdaNamedPatternTests =
[ test "should report unused named patterns" <|
\() ->
"""
module A exposing (..)
foo =
\\Named bish ->
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\Named _ ->
bash
"""
]
, test "should report unused nested named patterns" <|
\() ->
"""
module A exposing (..)
foo =
\\Named (Bish bish) ->
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\Named (Bish _) ->
bash
"""
]
, test "should report unused named patterns with multiple segments" <|
\() ->
"""
module A exposing (..)
foo =
\\(Pair _ _) -> bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Pair _ _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\(_) -> bash
"""
]
, test "should report unused named patterns in tuples" <|
\() ->
"""
module A exposing (..)
foo =
\\(Singular _, Pair _ _) -> bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Singular _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\(_, Pair _ _) -> bish
"""
, Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Pair _ _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\(Singular _, _) -> bish
"""
]
]
lambdaRecordPatternTests : List Test
lambdaRecordPatternTests =
[ test "should replace unused record with `_`" <|
\() ->
"""
module A exposing (..)
foo =
\\{ bish, bash, bosh } ->
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameters `bish`, `bash` and `bosh` are not used."
, details = [ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
, under = "bish, bash, bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\_ ->
bar
"""
]
, test "should report unused record values" <|
\() ->
"""
module A exposing (..)
foo =
\\{ bish, bash, bosh } ->
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameters `bish` and `bosh` are not used."
, details = [ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
, under = "bish, bash, bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\{bash} ->
bash
"""
]
, test "should report highlight the least amount of values possible" <|
\() ->
"""
module A exposing (..)
foo =
\\{ bish, bash, bosh } ->
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameters `bash` and `bosh` are not used."
, details = [ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
, under = "bash, bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\{bish} ->
bish
"""
]
]
lambdaTuplePatternTests : List Test
lambdaTuplePatternTests =
[ test "should report unused tuple values" <|
\() ->
"""
module A exposing (..)
foo =
\\( bish, bash, bosh ) ->
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\( _, bash, bosh ) ->
bash
"""
, Review.Test.error
{ message = "Parameter `bosh` is not used."
, details = details
, under = "bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\( bish, bash, _ ) ->
bash
"""
]
, test "should replace unused tuple with `_`" <|
\() ->
"""
module A exposing (..)
foo =
\\( _, _ ) ->
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Tuple pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "( _, _ )"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\_ ->
bar
"""
]
, test "should replace unused threeple with `_`" <|
\() ->
"""
module A exposing (..)
foo =
\\( _, _, _ ) ->
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Tuple pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "( _, _, _ )"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
\\_ ->
bar
"""
]
]
--- FUNCTION PATTERN TESTS
functionAsPatternTests : List Test
functionAsPatternTests =
[ test "should report unused pattern aliases" <|
\() ->
"""
module A exposing (..)
foo ({ bish, bash } as bosh) =
( bish, bash )
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern alias `bosh` is not used."
, details = details
, under = "bosh"
}
]
, test "should report unused patterns in an as pattern" <|
\() ->
"""
module A exposing (..)
foo ({ bish, bash } as bosh) =
( bish, bosh )
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bash` is not used."
, details = details
, under = "bash"
}
]
, test "should report unused patterns and unused aliases" <|
\() ->
"""
module A exposing (..)
foo ({ bish, bash } as bosh) =
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bash` is not used."
, details = details
, under = "bash"
}
, Review.Test.error
{ message = "Pattern alias `bosh` is not used."
, details = details
, under = "bosh"
}
]
, test "should report unused patterns that are aliased" <|
\() ->
"""
module A exposing (..)
foo (_ as bar) =
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern `_` is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "_"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo (bar) =
bar
"""
]
, test "should report nested unused pattern aliases" <|
\() ->
"""
module A exposing (..)
foo (Named ( _, ( Just bash ) as bish )) =
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern alias `bish` is not used."
, details = details
, under = "bish"
}
]
]
functionNamedPatternTests : List Test
functionNamedPatternTests =
[ test "should report unused named patterns" <|
\() ->
"""
module A exposing (..)
foo (Named bish) =
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bish` is not used."
, details = details
, under = "bish"
}
]
, test "should report unused nested named patterns" <|
\() ->
"""
module A exposing (..)
foo (Named (Bish bish)) =
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bish` is not used."
, details = details
, under = "bish"
}
]
, test "should report unused named patterns with multiple segments" <|
\() ->
"""
module A exposing (..)
foo (Pair _ _) =
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Pair _ _"
}
]
, test "should report unused named patterns in tuples" <|
\() ->
"""
module A exposing (..)
foo (Singular _, Pair _ _) =
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Singular _"
}
, Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Pair _ _"
}
]
]
functionRecordPatternTests : List Test
functionRecordPatternTests =
[ test "should replace unused record with `_`" <|
\() ->
"""
module A exposing (..)
foo { bish, bash, bosh } =
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameters `bish`, `bash` and `bosh` are not used."
, details = [ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
, under = "bish, bash, bosh"
}
]
, test "should report unused record values" <|
\() ->
"""
module A exposing (..)
foo { bish, bash, bosh } =
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameters `bish` and `bosh` are not used."
, details = [ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
, under = "bish, bash, bosh"
}
]
, test "should report highlight the least amount of values possible" <|
\() ->
"""
module A exposing (..)
foo { bish, bash, bosh } =
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameters `bash` and `bosh` are not used."
, details = [ "You should either use these parameters somewhere, or remove them at the location I pointed at." ]
, under = "bash, bosh"
}
]
]
functionTuplePatternTests : List Test
functionTuplePatternTests =
[ test "should report unused tuple values" <|
\() ->
"""
module A exposing (..)
foo ( bish, bash, bosh ) =
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Parameter `bish` is not used."
, details = details
, under = "bish"
}
, Review.Test.error
{ message = "Parameter `bosh` is not used."
, details = details
, under = "bosh"
}
]
, test "should report unused tuple" <|
\() ->
"""
module A exposing (..)
foo ( _, _ ) =
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Tuple pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "( _, _ )"
}
]
, test "should report unused threeple" <|
\() ->
"""
module A exposing (..)
foo ( _, _, _ ) =
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Tuple pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "( _, _, _ )"
}
]
]

478
tests/NoUnused/Patterns.elm Normal file
View File

@ -0,0 +1,478 @@
module NoUnused.Patterns exposing (rule)
{-| Report useless patterns and pattern values that are not used.
# Rule
@docs rule
-}
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.ModuleName exposing (ModuleName)
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.Writer as Writer
import NoUnused.Patterns.NameVisitor as NameVisitor
import Review.Fix as Fix
import Review.Rule as Rule exposing (Rule)
import Set exposing (Set)
{-| Report useless patterns and pattern values that are not used.
config =
[ NoUnused.Patterns.rule
]
This rule looks within let..in blocks and case branches to find any patterns that are unused. It will report any useless patterns as well as any pattern values that are not used.
## Fail
Value `something` is not used:
case maybe of
Just something ->
True
Nothing ->
False
## Success
case maybe of
Just _ ->
True
Nothing ->
False
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoUnused.Patterns" initialContext
|> Rule.withExpressionVisitor expressionVisitor
|> NameVisitor.withValueVisitor valueVisitor
|> Rule.fromModuleRuleSchema
expressionVisitor : Node Expression -> Rule.Direction -> Context -> ( List (Rule.Error {}), Context )
expressionVisitor (Node _ expression) direction context =
case ( direction, expression ) of
( Rule.OnEnter, Expression.LetExpression { declarations } ) ->
( [], rememberLetDeclarationList declarations context )
( Rule.OnExit, Expression.LetExpression { declarations } ) ->
errorsForLetDeclarationList declarations context
( Rule.OnEnter, Expression.CaseExpression { cases } ) ->
( [], rememberCaseList cases context )
( Rule.OnExit, Expression.CaseExpression { cases } ) ->
errorsForCaseList cases context
_ ->
( [], context )
valueVisitor : Node ( ModuleName, String ) -> Context -> ( List (Rule.Error {}), Context )
valueVisitor (Node _ ( moduleName, value )) context =
case moduleName of
[] ->
( [], useValue value context )
_ ->
( [], context )
--- ON ENTER
rememberCaseList : List Expression.Case -> Context -> Context
rememberCaseList list context =
List.foldl rememberCase context list
rememberCase : Expression.Case -> Context -> Context
rememberCase ( pattern, _ ) context =
rememberPattern pattern context
rememberLetDeclarationList : List (Node Expression.LetDeclaration) -> Context -> Context
rememberLetDeclarationList list context =
List.foldl rememberLetDeclaration context list
rememberLetDeclaration : Node Expression.LetDeclaration -> Context -> Context
rememberLetDeclaration (Node _ letDeclaration) context =
case letDeclaration of
Expression.LetFunction _ ->
context
Expression.LetDestructuring pattern _ ->
rememberPattern pattern context
rememberPatternList : List (Node Pattern) -> Context -> Context
rememberPatternList list context =
List.foldl rememberPattern context list
rememberPattern : Node Pattern -> Context -> Context
rememberPattern (Node _ pattern) context =
case pattern of
Pattern.AllPattern ->
context
Pattern.VarPattern value ->
rememberValue value context
Pattern.TuplePattern patterns ->
rememberPatternList patterns context
Pattern.RecordPattern values ->
rememberValueList values context
Pattern.UnConsPattern first second ->
context
|> rememberPattern first
|> rememberPattern second
Pattern.ListPattern patterns ->
rememberPatternList patterns context
Pattern.NamedPattern _ patterns ->
rememberPatternList patterns context
Pattern.AsPattern inner name ->
context
|> rememberPattern inner
|> rememberValue (Node.value name)
Pattern.ParenthesizedPattern inner ->
rememberPattern inner context
_ ->
context
rememberValueList : List (Node String) -> Context -> Context
rememberValueList list context =
List.foldl (Node.value >> rememberValue) context list
--- ON EXIT
singularDetails : List String
singularDetails =
[ "You should either use this value somewhere, or remove it at the location I pointed at." ]
pluralDetails : List String
pluralDetails =
[ "You should either use these values somewhere, or remove them at the location I pointed at." ]
removeDetails : List String
removeDetails =
[ "You should remove it at the location I pointed at." ]
andThen :
(Context -> ( List (Rule.Error {}), Context ))
-> ( List (Rule.Error {}), Context )
-> ( List (Rule.Error {}), Context )
andThen function ( errors, context ) =
let
( newErrors, newContext ) =
function context
in
( newErrors ++ errors, newContext )
errorsForCaseList : List Expression.Case -> Context -> ( List (Rule.Error {}), Context )
errorsForCaseList list context =
case list of
[] ->
( [], context )
first :: rest ->
context
|> errorsForCase first
|> andThen (errorsForCaseList rest)
errorsForCase : Expression.Case -> Context -> ( List (Rule.Error {}), Context )
errorsForCase ( pattern, _ ) context =
errorsForPattern Matching pattern context
errorsForLetDeclarationList : List (Node Expression.LetDeclaration) -> Context -> ( List (Rule.Error {}), Context )
errorsForLetDeclarationList list context =
case list of
[] ->
( [], context )
first :: rest ->
context
|> errorsForLetDeclaration first
|> andThen (errorsForLetDeclarationList rest)
errorsForLetDeclaration : Node Expression.LetDeclaration -> Context -> ( List (Rule.Error {}), Context )
errorsForLetDeclaration (Node _ letDeclaration) context =
case letDeclaration of
Expression.LetFunction _ ->
( [], context )
Expression.LetDestructuring pattern _ ->
errorsForPattern Destructuring pattern context
type PatternUse
= Destructuring
| Matching
errorsForPatternList : PatternUse -> List (Node Pattern) -> Context -> ( List (Rule.Error {}), Context )
errorsForPatternList use list context =
case list of
[] ->
( [], context )
first :: rest ->
context
|> errorsForPattern use first
|> andThen (errorsForPatternList use rest)
errorsForPattern : PatternUse -> Node Pattern -> Context -> ( List (Rule.Error {}), Context )
errorsForPattern use (Node range pattern) context =
case pattern of
Pattern.AllPattern ->
( [], context )
Pattern.VarPattern value ->
errorsForValue value range context
Pattern.RecordPattern values ->
errorsForRecordValueList range values context
Pattern.TuplePattern [ Node _ Pattern.AllPattern, Node _ Pattern.AllPattern ] ->
errorsForUselessTuple range context
Pattern.TuplePattern [ Node _ Pattern.AllPattern, Node _ Pattern.AllPattern, Node _ Pattern.AllPattern ] ->
errorsForUselessTuple range context
Pattern.TuplePattern patterns ->
errorsForPatternList use patterns context
Pattern.UnConsPattern first second ->
errorsForPatternList use [ first, second ] context
Pattern.ListPattern patterns ->
errorsForPatternList use patterns context
Pattern.NamedPattern _ patterns ->
if use == Destructuring && List.all isAllPattern patterns then
errorsForUselessNamePattern range context
else
errorsForPatternList use patterns context
Pattern.AsPattern inner name ->
context
|> errorsForAsPattern range inner name
|> andThen (errorsForPattern use inner)
Pattern.ParenthesizedPattern inner ->
errorsForPattern use inner context
_ ->
( [], context )
errorsForUselessNamePattern : Range -> Context -> ( List (Rule.Error {}), Context )
errorsForUselessNamePattern range context =
( [ Rule.errorWithFix
{ message = "Named pattern is not needed."
, details = removeDetails
}
range
[ Fix.replaceRangeBy range "_" ]
]
, context
)
errorsForUselessTuple : Range -> Context -> ( List (Rule.Error {}), Context )
errorsForUselessTuple range context =
( [ Rule.errorWithFix
{ message = "Tuple pattern is not needed."
, details = removeDetails
}
range
[ Fix.replaceRangeBy range "_" ]
]
, context
)
errorsForRecordValueList : Range -> List (Node String) -> Context -> ( List (Rule.Error {}), Context )
errorsForRecordValueList recordRange list context =
let
( unused, used ) =
List.partition (\(Node _ value) -> Set.member value context) list
in
case unused of
[] ->
( [], context )
firstNode :: restNodes ->
let
first =
firstNode |> Node.value
rest =
List.map Node.value restNodes
( errorRange, fix ) =
case used of
[] ->
( recordRange, Fix.replaceRangeBy recordRange "_" )
_ ->
( Range.combine (List.map Node.range unused)
, Node Range.emptyRange (Pattern.RecordPattern used)
|> Writer.writePattern
|> Writer.write
|> Fix.replaceRangeBy recordRange
)
in
( [ Rule.errorWithFix
{ message = listToMessage first rest
, details = listToDetails first rest
}
errorRange
[ fix ]
]
, List.foldl forgetNode context unused
)
listToMessage : String -> List String -> String
listToMessage first rest =
case List.reverse rest of
[] ->
"Value `" ++ first ++ "` is not used."
last :: middle ->
"Values `" ++ String.join "`, `" (first :: middle) ++ "` and `" ++ last ++ "` are not used."
listToDetails : String -> List String -> List String
listToDetails _ rest =
case rest of
[] ->
singularDetails
_ ->
pluralDetails
errorsForAsPattern : Range -> Node Pattern -> Node String -> Context -> ( List (Rule.Error {}), Context )
errorsForAsPattern patternRange inner (Node range name) context =
if Set.member name context then
let
fix =
[ inner
|> Writer.writePattern
|> Writer.write
|> Fix.replaceRangeBy patternRange
]
in
( [ Rule.errorWithFix
{ message = "Pattern alias `" ++ name ++ "` is not used."
, details = singularDetails
}
range
fix
]
, Set.remove name context
)
else if isAllPattern inner then
( [ Rule.errorWithFix
{ message = "Pattern `_` is not needed."
, details = removeDetails
}
(Node.range inner)
[ Fix.replaceRangeBy patternRange name ]
]
, Set.remove name context
)
else
( [], context )
isAllPattern : Node Pattern -> Bool
isAllPattern (Node _ pattern) =
case pattern of
Pattern.AllPattern ->
True
_ ->
False
forgetNode : Node String -> Context -> Context
forgetNode (Node _ value) context =
Set.remove value context
--- CONTEXT
type alias Context =
Set String
initialContext : Context
initialContext =
Set.empty
errorsForValue : String -> Range -> Context -> ( List (Rule.Error {}), Context )
errorsForValue value range context =
if Set.member value context then
( [ Rule.errorWithFix
{ message = "Value `" ++ value ++ "` is not used."
, details = singularDetails
}
range
[ Fix.replaceRangeBy range "_" ]
]
, Set.remove value context
)
else
( [], context )
rememberValue : String -> Context -> Context
rememberValue value context =
Set.insert value context
useValue : String -> Context -> Context
useValue value context =
Set.remove value context

View File

@ -0,0 +1,215 @@
module NoUnused.Patterns.NameVisitor exposing (withValueVisitor)
{-| Visit each name in the module.
A "name" is a `Node ( ModuleName, String )` and represents a value or type reference. Here are some value examples:
- `Html.Attributes.class` -> `( [ "Html", "Attributes" ], "class" )`
- `view` -> `( [], "view" )`
These can appear in many places throughout declarations and expressions, and picking them out each time is a lot of work. Instead of writing 1000 lines of code and tests each time, you can write one `nameVisitor` and plug it straight into your module schema, or separate `valueVisitor` and `typeVisitor`s.
@docs withValueVisitor
## Scope
This makes no attempt to resolve module names from imports, it just returns what's written in the code. It would be trivial to connect [elm-review-scope] with the name visitor if you want to do this.
[elm-review-scope]: http://github.com/jfmengels/elm-review-scope/
-}
import Elm.Syntax.Declaration as Declaration exposing (Declaration)
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.ModuleName exposing (ModuleName)
import Elm.Syntax.Node as Node exposing (Node(..))
import Elm.Syntax.Pattern as Pattern exposing (Pattern)
import Review.Rule as Rule exposing (Error)
type alias VisitorFunction context =
Node ( ModuleName, String ) -> context -> ( List (Error {}), context )
type Name
= Value (Node ( ModuleName, String ))
withValueVisitor :
(Node ( ModuleName, String ) -> context -> ( List (Error {}), context ))
-> Rule.ModuleRuleSchema { schemaState | canCollectProjectData : () } context
-> Rule.ModuleRuleSchema { schemaState | canCollectProjectData : (), hasAtLeastOneVisitor : () } context
withValueVisitor valueVisitor rule =
rule
|> Rule.withDeclarationListVisitor (declarationListVisitor valueVisitor)
|> Rule.withExpressionVisitor (expressionVisitor valueVisitor)
--- VISITORS
declarationListVisitor :
VisitorFunction context
-> (List (Node Declaration) -> context -> ( List (Error {}), context ))
declarationListVisitor visitor list context =
visitDeclarationList list
|> folder visitor context
expressionVisitor :
VisitorFunction context
-> (Node Expression -> Rule.Direction -> context -> ( List (Error {}), context ))
expressionVisitor visitor node direction context =
case direction of
Rule.OnEnter ->
visitExpression node
|> folder visitor context
Rule.OnExit ->
( [], context )
--- FOLDER
folder :
VisitorFunction context
-> (context -> List Name -> ( List (Error {}), context ))
folder visitor context list =
case list of
[] ->
( [], context )
head :: rest ->
let
( headErrors, headContext ) =
applyVisitor visitor head context
( restErrors, restContext ) =
folder visitor headContext rest
in
( headErrors ++ restErrors, restContext )
applyVisitor : VisitorFunction context -> Name -> context -> ( List (Error {}), context )
applyVisitor visitor (Value node) context =
visitor node context
--- PRIVATE
visitDeclarationList : List (Node Declaration) -> List Name
visitDeclarationList nodes =
List.concatMap visitDeclaration nodes
visitDeclaration : Node Declaration -> List Name
visitDeclaration node =
case Node.value node of
Declaration.FunctionDeclaration { declaration } ->
visitFunctionImplementation declaration
_ ->
[]
visitFunctionImplementation : Node Expression.FunctionImplementation -> List Name
visitFunctionImplementation node =
visitPatternList (node |> Node.value |> .arguments)
visitExpression : Node Expression -> List Name
visitExpression (Node range expression) =
case expression of
Expression.FunctionOrValue moduleName function ->
visitValue (Node range ( moduleName, function ))
Expression.LetExpression { declarations } ->
visitLetDeclarationList declarations
Expression.CaseExpression { cases } ->
visitCaseList cases
Expression.LambdaExpression { args } ->
visitPatternList args
Expression.RecordUpdateExpression name _ ->
visitValue (Node.map (\function -> ( [], function )) name)
_ ->
[]
visitLetDeclarationList : List (Node Expression.LetDeclaration) -> List Name
visitLetDeclarationList list =
List.concatMap visitLetDeclaration list
visitLetDeclaration : Node Expression.LetDeclaration -> List Name
visitLetDeclaration node =
case Node.value node of
Expression.LetFunction { declaration } ->
visitFunctionImplementation declaration
Expression.LetDestructuring pattern _ ->
visitPattern pattern
visitCaseList : List Expression.Case -> List Name
visitCaseList list =
List.concatMap visitCase list
visitCase : Expression.Case -> List Name
visitCase ( pattern, _ ) =
visitPattern pattern
visitPatternList : List (Node Pattern) -> List Name
visitPatternList list =
List.concatMap visitPattern list
visitPattern : Node Pattern -> List Name
visitPattern node =
case Node.value node of
Pattern.TuplePattern patterns ->
visitPatternList patterns
Pattern.UnConsPattern head rest ->
visitPattern head ++ visitPattern rest
Pattern.ListPattern list ->
visitPatternList list
Pattern.NamedPattern { moduleName, name } _ ->
let
{ start } =
Node.range node
newEnd =
{ start | column = start.column + (name :: moduleName |> String.join "." |> String.length) }
range =
{ start = start, end = newEnd }
in
visitValue (Node range ( moduleName, name ))
Pattern.AsPattern pattern _ ->
visitPattern pattern
Pattern.ParenthesizedPattern pattern ->
visitPattern pattern
_ ->
[]
visitValue : Node ( ModuleName, String ) -> List Name
visitValue node =
[ Value node ]

View File

@ -0,0 +1,960 @@
module NoUnused.PatternsTests exposing (all)
import NoUnused.Patterns exposing (rule)
import Review.Test
import Test exposing (Test, describe, test)
details : List String
details =
[ "You should either use this value somewhere, or remove it at the location I pointed at."
]
all : Test
all =
describe "NoUnused.Patterns"
[ describe "in Case branches" caseTests
, describe "in Let destructuring" letDestructuringTests
--- un-tests
, describe "in Function arguments" functionArgumentTests
, describe "in Lambda arguments" lambdaArgumentTests
, describe "in Let Functions" letFunctionTests
--- patterns
, describe "with as pattern" asPatternTests
, describe "with list pattern" listPatternTests
, describe "with named pattern" namedPatternTests
, describe "with record pattern" recordPatternTests
, describe "with tuple pattern" tuplePatternTests
, describe "with uncons pattern" unconsPatternTests
]
caseTests : List Test
caseTests =
[ test "reports unused values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
bish ->
Nothing
bash ->
Nothing
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
_ ->
Nothing
bash ->
Nothing
"""
, Review.Test.error
{ message = "Value `bash` is not used."
, details = details
, under = "bash"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
bish ->
Nothing
_ ->
Nothing
"""
]
, test "should not remove all list of unused values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
[] -> 0
[one] -> 1
[first, two] -> 2
more -> 3
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `one` is not used."
, details = details
, under = "one"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
[_] -> 1
[first, two] -> 2
more -> 3
"""
, Review.Test.error
{ message = "Value `first` is not used."
, details = details
, under = "first"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
[one] -> 1
[_, two] -> 2
more -> 3
"""
, Review.Test.error
{ message = "Value `two` is not used."
, details = details
, under = "two"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
[one] -> 1
[first, _] -> 2
more -> 3
"""
, Review.Test.error
{ message = "Value `more` is not used."
, details = details
, under = "more"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
[one] -> 1
[first, two] -> 2
_ -> 3
"""
]
, test "should not remove all uncons of unused values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
[] -> 0
one :: [] -> 1
first :: two :: [] -> 2
_ :: _ :: more -> 3
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `one` is not used."
, details = details
, under = "one"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
_ :: [] -> 1
first :: two :: [] -> 2
_ :: _ :: more -> 3
"""
, Review.Test.error
{ message = "Value `first` is not used."
, details = details
, under = "first"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
one :: [] -> 1
_ :: two :: [] -> 2
_ :: _ :: more -> 3
"""
, Review.Test.error
{ message = "Value `two` is not used."
, details = details
, under = "two"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
one :: [] -> 1
first :: _ :: [] -> 2
_ :: _ :: more -> 3
"""
, Review.Test.error
{ message = "Value `more` is not used."
, details = details
, under = "more"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[] -> 0
one :: [] -> 1
first :: two :: [] -> 2
_ :: _ :: _ -> 3
"""
]
]
functionArgumentTests : List Test
functionArgumentTests =
[ test "should not report unused arguments" <|
\() ->
"""
module A exposing (..)
foo : Int -> String -> String -> String
foo one two three =
three
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
lambdaArgumentTests : List Test
lambdaArgumentTests =
[ test "should not report unused arguments" <|
\() ->
"""
module A exposing (..)
foo =
List.map (\\value -> Nothing) list
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
letDestructuringTests : List Test
letDestructuringTests =
[ test "should report unused values" <|
\() ->
"""
module A exposing (..)
foo =
let
( left, right ) =
tupleValue
in
left
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `right` is not used."
, details = details
, under = "right"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
( left, _ ) =
tupleValue
in
left
"""
]
, test "should report unused patterns that are aliased" <|
\() ->
"""
module A exposing (..)
foo =
let
(_ as bar) = 1
in
bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern `_` is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "_"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
(bar) = 1
in
bar
"""
]
]
letFunctionTests : List Test
letFunctionTests =
[ test "should not report unused arguments" <|
\() ->
"""
module A exposing (..)
foo =
let
one oneValue =
1
two twoValue =
2
in
one two 3
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report unused let functions" <|
\() ->
"""
module A exposing (..)
foo =
let
value foo =
something foo
in
bar
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
--- PATTERN TESTS ------------------------
asPatternTests : List Test
asPatternTests =
[ test "should report unused pattern aliases" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
({ bish, bash } as bosh) ->
( bish, bash )
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern alias `bosh` is not used."
, details = details
, under = "bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
({bish, bash}) ->
( bish, bash )
"""
]
, test "should report unused patterns in an as pattern" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
({ bish, bash } as bosh) ->
( bish, bosh )
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `bash` is not used."
, details = details
, under = "bash"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
({bish} as bosh) ->
( bish, bosh )
"""
]
, test "should report unused patterns and unused aliases" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
({ bish, bash } as bosh) ->
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `bash` is not used."
, details = details
, under = "bash"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
({bish} as bosh) ->
bish
"""
, Review.Test.error
{ message = "Pattern alias `bosh` is not used."
, details = details
, under = "bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
({bish, bash}) ->
bish
"""
]
, test "should report unused patterns that are aliased" <|
\() ->
"""
module A exposing (..)
foo =
case 1 of
(_ as bar) -> bar
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern `_` is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "_"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case 1 of
(bar) -> bar
"""
]
, test "should report nested unused pattern aliases" <|
\() ->
"""
module A exposing (..)
foo =
case maybeTupleMaybe of
Just ( _, ( Just _ ) as bish ) ->
bash
_ ->
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Pattern alias `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case maybeTupleMaybe of
Just ( _, ( Just _ ) ) ->
bash
_ ->
bosh
"""
]
]
listPatternTests : List Test
listPatternTests =
[ test "should report unused list values" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
[ first, second ] ->
another
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `first` is not used."
, details = details
, under = "first"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[ _, second ] ->
another
"""
, Review.Test.error
{ message = "Value `second` is not used."
, details = details
, under = "second"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
[ first, _ ] ->
another
"""
]
]
namedPatternTests : List Test
namedPatternTests =
[ test "should report unused named patterns" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
Just bish ->
bash
Nothing ->
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
Just _ ->
bash
Nothing ->
bosh
"""
]
, test "should report unused parenthesized patterns" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
Just (Bish bish) ->
bash
Nothing ->
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case bar of
Just (Bish _) ->
bash
Nothing ->
bosh
"""
]
, test "should not report unused named patterns in case" <|
\() ->
"""
module A exposing (..)
foo =
case maybeTupleMaybe of
Just ( Just _, (Just _) as bish ) ->
bish
Just _ ->
bash
_ ->
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report unused named patterns in destructuring" <|
\() ->
"""
module A exposing (..)
foo =
let
(Singular _) = bish
(Pair _ _) = bash
in
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Singular _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
(_) = bish
(Pair _ _) = bash
in
bosh
"""
, Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Pair _ _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
(Singular _) = bish
(_) = bash
in
bosh
"""
]
, test "should report unused named patterns in destructuring tuples" <|
\() ->
"""
module A exposing (..)
foo =
let
(Singular _, Pair _ _) = bish
in
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Singular _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
(_, Pair _ _) = bish
in
bosh
"""
, Review.Test.error
{ message = "Named pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "Pair _ _"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
(Singular _, _) = bish
in
bosh
"""
]
]
recordPatternTests : List Test
recordPatternTests =
[ test "should replace unused record with `_`" <|
\() ->
"""
module A exposing (..)
foo =
let
{ bish, bash } =
bar
in
bosh
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Values `bish` and `bash` are not used."
, details = [ "You should either use these values somewhere, or remove them at the location I pointed at." ]
, under = "{ bish, bash }"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
_ =
bar
in
bosh
"""
]
, test "should report unused record values" <|
\() ->
"""
module A exposing (..)
foo =
let
{ bish, bash, bosh } =
bar
in
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Values `bish` and `bosh` are not used."
, details = [ "You should either use these values somewhere, or remove them at the location I pointed at." ]
, under = "bish, bash, bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
{bash} =
bar
in
bash
"""
]
, test "should report highlight the least amount of values possible" <|
\() ->
"""
module A exposing (..)
foo =
let
{ bish, bash, bosh } =
bar
in
bish
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Values `bash` and `bosh` are not used."
, details = [ "You should either use these values somewhere, or remove them at the location I pointed at." ]
, under = "bash, bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
{bish} =
bar
in
bish
"""
]
]
tuplePatternTests : List Test
tuplePatternTests =
[ test "should report unused tuple values" <|
\() ->
"""
module A exposing (..)
foo =
let
( bish, bash, bosh ) =
bar
in
bash
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `bish` is not used."
, details = details
, under = "bish"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
( _, bash, bosh ) =
bar
in
bash
"""
, Review.Test.error
{ message = "Value `bosh` is not used."
, details = details
, under = "bosh"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
( bish, bash, _ ) =
bar
in
bash
"""
]
, test "should replace unused tuple with `_`" <|
\() ->
"""
module A exposing (..)
foo =
let
( _, _ ) =
bar
in
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Tuple pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "( _, _ )"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
_ =
bar
in
1
"""
]
, test "should replace unused threeple with `_`" <|
\() ->
"""
module A exposing (..)
foo =
let
( _, _, _ ) =
bar
in
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Tuple pattern is not needed."
, details = [ "You should remove it at the location I pointed at." ]
, under = "( _, _, _ )"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
let
_ =
bar
in
1
"""
]
, test "should not report a tuple containing an empty list" <|
\() ->
"""
module A exposing (..)
foo =
case bar of
( [], _ ) ->
1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]
unconsPatternTests : List Test
unconsPatternTests =
[ test "should report unused uncons values" <|
\() ->
"""
module A exposing (..)
foo =
case list of
first :: rest ->
list
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Value `first` is not used."
, details = details
, under = "first"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case list of
_ :: rest ->
list
"""
, Review.Test.error
{ message = "Value `rest` is not used."
, details = details
, under = "rest"
}
|> Review.Test.whenFixed
"""
module A exposing (..)
foo =
case list of
first :: _ ->
list
"""
]
]

View File

@ -56,8 +56,8 @@ rule =
Rule.newModuleRuleSchema "NoUnused.Variables" initialContext
|> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
|> Rule.withImportVisitor importVisitor
|> Rule.withExpressionVisitor expressionVisitor
|> Rule.withDeclarationVisitor declarationVisitor
|> Rule.withExpressionVisitor expressionVisitor
|> Rule.withFinalModuleEvaluation finalEvaluation
|> Rule.fromModuleRuleSchema
@ -216,7 +216,7 @@ fix declaredModules { variableType, rangeToRemove } =
True
Port ->
True
False
in
if shouldOfferFix then
[ Fix.removeRange rangeToRemove ]
@ -264,7 +264,7 @@ importVisitor ((Node _ import_) as node) context =
Just moduleAlias ->
if Node.value moduleAlias == Node.value import_.moduleName then
[ Rule.errorWithFix
{ message = "Module `Html` is aliased as `Html`"
{ message = "Module `" ++ String.join "." (Node.value moduleAlias) ++ "` is aliased as itself"
, details = [ "The alias is the same as the module name, and brings no useful value" ]
}
(Node.range moduleAlias)
@ -291,7 +291,7 @@ importVisitor ((Node _ import_) as node) context =
registerModuleNameOrAlias : Node Import -> Context -> Context
registerModuleNameOrAlias ((Node range { exposingList, moduleAlias, moduleName }) as node) context =
registerModuleNameOrAlias ((Node range { moduleAlias, moduleName }) as node) context =
case moduleAlias of
Just _ ->
registerModuleAlias node context
@ -381,7 +381,7 @@ expressionVisitor (Node range value) direction context =
|> registerFunction letBlockContext function
|> markUsedTypesAndModules namesUsedInArgumentPatterns
Expression.LetDestructuring pattern _ ->
Expression.LetDestructuring _ _ ->
context_
)
{ context | scopes = NonemptyList.cons emptyScope context.scopes }
@ -408,7 +408,7 @@ expressionVisitor (Node range value) direction context =
usedVariables =
cases
|> List.map
(\( patternNode, expressionNode ) ->
(\( patternNode, _ ) ->
getUsedVariablesFromPattern patternNode
)
|> foldUsedTypesAndModules
@ -480,19 +480,14 @@ getUsedTypesFromPattern patternNode =
[]
Pattern.NamedPattern qualifiedNameRef patterns ->
let
usedVariable : List String
usedVariable =
case qualifiedNameRef.moduleName of
[] ->
[ qualifiedNameRef.name ]
case qualifiedNameRef.moduleName of
[] ->
qualifiedNameRef.name :: List.concatMap getUsedTypesFromPattern patterns
moduleName ->
[]
in
usedVariable ++ List.concatMap getUsedTypesFromPattern patterns
_ ->
List.concatMap getUsedTypesFromPattern patterns
Pattern.AsPattern pattern alias_ ->
Pattern.AsPattern pattern _ ->
getUsedTypesFromPattern pattern
Pattern.ParenthesizedPattern pattern ->
@ -539,19 +534,14 @@ getUsedModulesFromPattern patternNode =
[]
Pattern.NamedPattern qualifiedNameRef patterns ->
let
usedVariable : List String
usedVariable =
case qualifiedNameRef.moduleName of
[] ->
[]
case qualifiedNameRef.moduleName of
[] ->
List.concatMap getUsedModulesFromPattern patterns
moduleName ->
[ getModuleName moduleName ]
in
usedVariable ++ List.concatMap getUsedModulesFromPattern patterns
moduleName ->
getModuleName moduleName :: List.concatMap getUsedModulesFromPattern patterns
Pattern.AsPattern pattern alias_ ->
Pattern.AsPattern pattern _ ->
getUsedModulesFromPattern pattern
Pattern.ParenthesizedPattern pattern ->
@ -842,6 +832,7 @@ collectFromExposing exposingNode =
)
Exposing.TypeOrAliasExpose name ->
-- TODO Detect whether it is a custom type or type alias
Just
( name
, { variableType = ImportedItem ImportedType
@ -852,7 +843,7 @@ collectFromExposing exposingNode =
Exposing.TypeExpose { name, open } ->
case open of
Just openRange ->
Just _ ->
-- TODO Change this behavior once we know the contents of the open range, using dependencies or the interfaces of the other modules
Nothing
@ -898,7 +889,7 @@ collectTypesFromTypeAnnotation node =
( [], str ) ->
[ str ]
( moduleName, _ ) ->
_ ->
[]
in
name ++ List.concatMap collectTypesFromTypeAnnotation params
@ -935,7 +926,7 @@ collectModuleNamesFromTypeAnnotation node =
name : List String
name =
case Node.value nameNode of
( [], str ) ->
( [], _ ) ->
[]
( moduleName, _ ) ->

View File

@ -781,7 +781,7 @@ a = Html.div"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Module `Html` is aliased as `Html`"
{ message = "Module `Html` is aliased as itself"
, details = [ "The alias is the same as the module name, and brings no useful value" ]
, under = "Html"
}
@ -1396,9 +1396,6 @@ port input : (() -> msg) -> Sub msg"""
, details = details
, under = "input"
}
|> Review.Test.whenFixed """port module SomeModule exposing (a)
a = 1
"""
]
, test "should report unused ports (outgoing)" <|
\() ->
@ -1412,8 +1409,5 @@ port output : String -> Cmd msg"""
, details = details
, under = "output"
}
|> Review.Test.whenFixed """port module SomeModule exposing (a)
a = 1
"""
]
]

View File

@ -0,0 +1,97 @@
module NoUselessSubscriptions exposing (rule)
{-|
@docs rule
-}
import Elm.Syntax.Declaration as Declaration exposing (Declaration)
import Elm.Syntax.Expression as Expression
import Elm.Syntax.Node as Node exposing (Node(..))
import Review.Rule as Rule exposing (Error, Rule)
{-| Reports `subscriptions` functions that never return a subscription.
In my opinion, this is often a sign of premature architectural work, where you
set up an `init`, `view`, `update` and `subscriptions` functions. I think
it is better to define them as they are needed, to avoid adding upfront complexity
that turn out to be unnecessary later.
config =
[ NoUselessSubscriptions.rule
]
## Fail
subscriptions : Model -> Sub Msg
subscriptions model =
Sub.none
## Success
main =
Browser.element
{ init = init
, update = update
, subscriptions = \_ -> Sub.none
, view = view
}
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoUselessSubscriptions" ()
|> Rule.withSimpleDeclarationVisitor declarationVisitor
|> Rule.fromModuleRuleSchema
declarationVisitor : Node Declaration -> List (Error {})
declarationVisitor declaration =
case Node.value declaration of
Declaration.FunctionDeclaration function ->
if
(function.declaration
|> Node.value
|> .name
|> Node.value
)
== "subscriptions"
then
case Node.value function.declaration |> .expression |> Node.value of
Expression.FunctionOrValue [ "Sub" ] "none" ->
[ error function
]
Expression.Application [ Node _ (Expression.FunctionOrValue [] "always"), Node _ (Expression.FunctionOrValue [ "Sub" ] "none") ] ->
[ error function
]
Expression.Application [ Node _ (Expression.FunctionOrValue [ "Basics" ] "always"), Node _ (Expression.FunctionOrValue [ "Sub" ] "none") ] ->
[ error function
]
Expression.Application [ Node _ (Expression.FunctionOrValue [ "Sub" ] "batch"), Node _ (Expression.ListExpr []) ] ->
[ error function
]
_ ->
[]
else
[]
_ ->
[]
error : Expression.Function -> Error {}
error function =
Rule.error
{ message = "The `subscription` function never returns any subscriptions"
, details = [ "The `subscription` function never returns any subscriptions. You might as well remove it." ]
}
(Node.value function.declaration |> .expression |> Node.range)

View File

@ -0,0 +1,75 @@
module NoUselessSubscriptionsTest exposing (all)
import NoUselessSubscriptions exposing (rule)
import Review.Test
import Test exposing (Test, describe, test)
all : Test
all =
describe "NoUselessSubscriptions"
[ test "should report an error when subscriptions returns Sub.none" <|
\_ ->
"""
module Main exposing (..)
subscriptions _ = Sub.none
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "The `subscription` function never returns any subscriptions"
, details = [ "The `subscription` function never returns any subscriptions. You might as well remove it." ]
, under = "Sub.none"
}
]
, test "should not report an error if it returns anything else" <|
\_ ->
"""
module Main exposing (..)
subscriptions _ = Time.every 5000 SomeMsg
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report an error when subscriptions returns `Sub.batch []`" <|
\_ ->
"""
module Main exposing (..)
subscriptions _ = Sub.batch []
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "The `subscription` function never returns any subscriptions"
, details = [ "The `subscription` function never returns any subscriptions. You might as well remove it." ]
, under = "Sub.batch []"
}
]
, test "should report an error when subscriptions returns `always Sub.none`" <|
\_ ->
"""
module Main exposing (..)
subscriptions = always Sub.none
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "The `subscription` function never returns any subscriptions"
, details = [ "The `subscription` function never returns any subscriptions. You might as well remove it." ]
, under = "always Sub.none"
}
]
, test "should report an error when subscriptions returns `Basics.always Sub.none`" <|
\_ ->
"""
module Main exposing (..)
subscriptions = Basics.always Sub.none
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "The `subscription` function never returns any subscriptions"
, details = [ "The `subscription` function never returns any subscriptions. You might as well remove it." ]
, under = "Basics.always Sub.none"
}
]
]

View File

@ -27,7 +27,7 @@ module Scope exposing
{- Copied over from https://github.com/jfmengels/elm-review-scope
Version: 0.2.0
Version: 0.2.1
Copyright (c) 2020, Jeroen Engels
All rights reserved.
@ -225,9 +225,9 @@ fromProjectToModule (ProjectContext projectContext) =
fromModuleToProject : Node ModuleName -> ModuleContext -> ProjectContext
fromModuleToProject moduleName (ModuleContext moduleContext) =
ProjectContext
{ dependenciesModules = moduleContext.dependenciesModules
{ dependenciesModules = Dict.empty
, modules =
Dict.insert (Node.value moduleName)
Dict.singleton (Node.value moduleName)
{ name = String.join "." (Node.value moduleName)
, comment = ""
, unions = moduleContext.exposedUnions
@ -235,7 +235,6 @@ fromModuleToProject moduleName (ModuleContext moduleContext) =
, values = moduleContext.exposedValues
, binops = moduleContext.exposedBinops
}
moduleContext.modules
}
@ -250,10 +249,10 @@ fromModuleToProject moduleName (ModuleContext moduleContext) =
-}
foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext
foldProjectContexts (ProjectContext a) (ProjectContext b) =
foldProjectContexts (ProjectContext newContext) (ProjectContext previousContext) =
ProjectContext
{ dependenciesModules = Dict.union b.dependenciesModules a.dependenciesModules
, modules = Dict.union b.modules a.modules
{ dependenciesModules = previousContext.dependenciesModules
, modules = Dict.union previousContext.modules newContext.modules
}