mirror of
https://github.com/jfmengels/elm-review.git
synced 2024-12-23 17:53:35 +03:00
Backport rules from elm-review-unused
This commit is contained in:
Jeroen Engels
parent
871a9faae1
commit
9992afbff8
@ -17,6 +17,7 @@ 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 exposing (Range)
|
||||
import Elm.Syntax.TypeAnnotation as TypeAnnotation
|
||||
import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
|
||||
import Review.Rule as Rule exposing (Error, Rule)
|
||||
import Set exposing (Set)
|
||||
@ -33,8 +34,10 @@ This rule will warn arguments that are always pattern matched using a wildcard (
|
||||
|
||||
For package projects, custom types whose constructors are exposed as part of the package API are not reported.
|
||||
|
||||
Note that this rule may report false positives if you compare custom types with the `==`/`/=` operators (and never destructure the custom type), like when you do `value == Just 0`.
|
||||
If you have a solution in mind to better handle these cases in a manageable way, please open an issue so we can talk about it.
|
||||
Note that this rule **may report false positives** if you compare custom types with the `==` or `/=` operators
|
||||
(and never destructure the custom type), like when you do `value == Just 0`, or store them in lists for instance with
|
||||
[`assoc-list`](https://package.elm-lang.org/packages/pzp1997/assoc-list/latest).
|
||||
This rule attempts to detect when the custom type is used in comparisons, but it may still result in false positives.
|
||||
|
||||
|
||||
## Fail
|
||||
@ -95,6 +98,7 @@ type alias ProjectContext =
|
||||
, args : Dict String (List Range)
|
||||
}
|
||||
, usedArguments : Dict ( ModuleName, String ) (Set Int)
|
||||
, customTypesNotToReport : Set ( ModuleName, String )
|
||||
}
|
||||
|
||||
|
||||
@ -104,6 +108,7 @@ type alias ModuleContext =
|
||||
, exposed : Exposing
|
||||
, customTypeArgs : Dict String (Dict String (List Range))
|
||||
, usedArguments : Dict ( ModuleName, String ) (Set Int)
|
||||
, customTypesNotToReport : Set ( ModuleName, String )
|
||||
}
|
||||
|
||||
|
||||
@ -147,6 +152,7 @@ initialProjectContext =
|
||||
{ exposedModules = Set.empty
|
||||
, customTypeArgs = Dict.empty
|
||||
, usedArguments = Dict.empty
|
||||
, customTypesNotToReport = Set.empty
|
||||
}
|
||||
|
||||
|
||||
@ -159,6 +165,7 @@ fromProjectToModule =
|
||||
, exposed = Exposing.Explicit []
|
||||
, customTypeArgs = Dict.empty
|
||||
, usedArguments = Dict.empty
|
||||
, customTypesNotToReport = Set.empty
|
||||
}
|
||||
)
|
||||
|> Rule.withModuleNameLookupTable
|
||||
@ -176,24 +183,43 @@ fromModuleToProject =
|
||||
{ moduleKey = moduleKey
|
||||
, args = getNonExposedCustomTypes moduleContext
|
||||
}
|
||||
, usedArguments =
|
||||
Dict.foldl
|
||||
(\( moduleNameForType, name ) value dict ->
|
||||
case moduleNameForType of
|
||||
[] ->
|
||||
Dict.insert ( Rule.moduleNameFromMetadata metadata, name ) value dict
|
||||
|
||||
_ ->
|
||||
Dict.insert ( moduleNameForType, name ) value dict
|
||||
)
|
||||
Dict.empty
|
||||
moduleContext.usedArguments
|
||||
, usedArguments = replaceLocalModuleNameForDict (Rule.moduleNameFromMetadata metadata) moduleContext.usedArguments
|
||||
, customTypesNotToReport = replaceLocalModuleNameForSet (Rule.moduleNameFromMetadata metadata) moduleContext.customTypesNotToReport
|
||||
}
|
||||
)
|
||||
|> Rule.withModuleKey
|
||||
|> Rule.withMetadata
|
||||
|
||||
|
||||
replaceLocalModuleNameForSet : ModuleName -> Set ( ModuleName, comparable ) -> Set ( ModuleName, comparable )
|
||||
replaceLocalModuleNameForSet moduleName set =
|
||||
Set.map
|
||||
(\( moduleNameForType, name ) ->
|
||||
case moduleNameForType of
|
||||
[] ->
|
||||
( moduleName, name )
|
||||
|
||||
_ ->
|
||||
( moduleNameForType, name )
|
||||
)
|
||||
set
|
||||
|
||||
|
||||
replaceLocalModuleNameForDict : ModuleName -> Dict ( ModuleName, comparable ) b -> Dict ( ModuleName, comparable ) b
|
||||
replaceLocalModuleNameForDict moduleName dict =
|
||||
Dict.foldl
|
||||
(\( moduleNameForType, name ) value acc ->
|
||||
case moduleNameForType of
|
||||
[] ->
|
||||
Dict.insert ( moduleName, name ) value acc
|
||||
|
||||
_ ->
|
||||
Dict.insert ( moduleNameForType, name ) value acc
|
||||
)
|
||||
Dict.empty
|
||||
dict
|
||||
|
||||
|
||||
getNonExposedCustomTypes : ModuleContext -> Dict String (List Range)
|
||||
getNonExposedCustomTypes moduleContext =
|
||||
if moduleContext.isModuleExposed then
|
||||
@ -248,6 +274,7 @@ foldProjectContexts newContext previousContext =
|
||||
newContext.usedArguments
|
||||
previousContext.usedArguments
|
||||
Dict.empty
|
||||
, customTypesNotToReport = Set.union newContext.customTypesNotToReport previousContext.customTypesNotToReport
|
||||
}
|
||||
|
||||
|
||||
@ -269,20 +296,42 @@ declarationListVisitor nodes context =
|
||||
let
|
||||
customTypeArgs : List ( String, Dict String (List Range) )
|
||||
customTypeArgs =
|
||||
List.filterMap collectCustomType nodes
|
||||
List.filterMap (collectCustomType context.lookupTable) nodes
|
||||
in
|
||||
( [], { context | customTypeArgs = Dict.fromList customTypeArgs } )
|
||||
|
||||
|
||||
collectCustomType : Node Declaration -> Maybe ( String, Dict String (List Range) )
|
||||
collectCustomType node =
|
||||
collectCustomType : ModuleNameLookupTable -> Node Declaration -> Maybe ( String, Dict String (List Range) )
|
||||
collectCustomType lookupTable node =
|
||||
case Node.value node of
|
||||
Declaration.CustomTypeDeclaration typeDeclaration ->
|
||||
let
|
||||
customTypeConstructors : List ( String, List Range )
|
||||
customTypeConstructors =
|
||||
typeDeclaration.constructors
|
||||
|> List.map (Node.value >> (\{ name, arguments } -> ( Node.value name, List.map Node.range arguments )))
|
||||
List.map
|
||||
(Node.value
|
||||
>> (\{ name, arguments } ->
|
||||
( Node.value name
|
||||
, arguments
|
||||
|> List.filter
|
||||
(\arg ->
|
||||
case Node.value arg of
|
||||
TypeAnnotation.Typed (Node _ ( _, "Never" )) [] ->
|
||||
case ModuleNameLookupTable.moduleNameFor lookupTable arg of
|
||||
Just [ "Basics" ] ->
|
||||
False
|
||||
|
||||
_ ->
|
||||
True
|
||||
|
||||
_ ->
|
||||
True
|
||||
)
|
||||
|> List.map Node.range
|
||||
)
|
||||
)
|
||||
)
|
||||
typeDeclaration.constructors
|
||||
in
|
||||
Just ( Node.value typeDeclaration.name, Dict.fromList customTypeConstructors )
|
||||
|
||||
@ -361,10 +410,77 @@ expressionVisitor node context =
|
||||
}
|
||||
)
|
||||
|
||||
Expression.OperatorApplication operator _ left right ->
|
||||
if operator == "==" || operator == "/=" then
|
||||
let
|
||||
customTypesNotToReport : Set ( ModuleName, String )
|
||||
customTypesNotToReport =
|
||||
Set.union
|
||||
(findCustomTypes context.lookupTable left)
|
||||
(findCustomTypes context.lookupTable right)
|
||||
in
|
||||
( [], { context | customTypesNotToReport = Set.union customTypesNotToReport context.customTypesNotToReport } )
|
||||
|
||||
else
|
||||
( [], context )
|
||||
|
||||
_ ->
|
||||
( [], context )
|
||||
|
||||
|
||||
findCustomTypes : ModuleNameLookupTable -> Node Expression -> Set ( ModuleName, String )
|
||||
findCustomTypes lookupTable node =
|
||||
case Node.value node of
|
||||
Expression.FunctionOrValue rawModuleName functionName ->
|
||||
if isCustomTypeConstructor functionName then
|
||||
case ModuleNameLookupTable.moduleNameFor lookupTable node of
|
||||
Just moduleName ->
|
||||
Set.singleton ( moduleName, functionName )
|
||||
|
||||
Nothing ->
|
||||
Set.singleton ( rawModuleName, functionName )
|
||||
|
||||
else
|
||||
Set.empty
|
||||
|
||||
Expression.TupledExpression expressions ->
|
||||
List.foldl (findCustomTypes lookupTable >> Set.union) Set.empty expressions
|
||||
|
||||
Expression.ParenthesizedExpression expression ->
|
||||
findCustomTypes lookupTable expression
|
||||
|
||||
Expression.Application [] ->
|
||||
Set.empty
|
||||
|
||||
Expression.Application (((Node _ (Expression.FunctionOrValue _ functionName)) as first) :: expressions) ->
|
||||
if isCustomTypeConstructor functionName then
|
||||
List.foldl (findCustomTypes lookupTable >> Set.union) Set.empty (first :: expressions)
|
||||
|
||||
else
|
||||
Set.empty
|
||||
|
||||
Expression.OperatorApplication _ _ left right ->
|
||||
Set.union
|
||||
(findCustomTypes lookupTable left)
|
||||
(findCustomTypes lookupTable right)
|
||||
|
||||
Expression.Negation expression ->
|
||||
findCustomTypes lookupTable expression
|
||||
|
||||
Expression.ListExpr expressions ->
|
||||
List.foldl (findCustomTypes lookupTable >> Set.union) Set.empty expressions
|
||||
|
||||
_ ->
|
||||
Set.empty
|
||||
|
||||
|
||||
isCustomTypeConstructor : String -> Bool
|
||||
isCustomTypeConstructor functionName =
|
||||
String.toList functionName
|
||||
|> List.take 1
|
||||
|> List.all Char.isUpper
|
||||
|
||||
|
||||
registerUsedPatterns : List ( ( ModuleName, String ), Set Int ) -> Dict ( ModuleName, String ) (Set Int) -> Dict ( ModuleName, String ) (Set Int)
|
||||
registerUsedPatterns newUsedArguments previouslyUsedArguments =
|
||||
List.foldl
|
||||
@ -452,15 +568,25 @@ finalEvaluation context =
|
||||
|> Dict.toList
|
||||
|> List.concatMap
|
||||
(\( name, ranges ) ->
|
||||
case Dict.get ( moduleName, name ) context.usedArguments of
|
||||
Just usedArgumentPositions ->
|
||||
ranges
|
||||
|> List.indexedMap Tuple.pair
|
||||
|> List.filter (\( index, _ ) -> not <| Set.member index usedArgumentPositions)
|
||||
|> List.map (Tuple.second >> error moduleKey)
|
||||
if Set.member ( moduleName, name ) context.customTypesNotToReport then
|
||||
[]
|
||||
|
||||
Nothing ->
|
||||
List.map (error moduleKey) ranges
|
||||
else
|
||||
case Dict.get ( moduleName, name ) context.usedArguments of
|
||||
Just usedArgumentPositions ->
|
||||
ranges
|
||||
|> List.indexedMap Tuple.pair
|
||||
|> List.filterMap
|
||||
(\( index, range ) ->
|
||||
if Set.member index usedArgumentPositions then
|
||||
Nothing
|
||||
|
||||
else
|
||||
Just (error moduleKey range)
|
||||
)
|
||||
|
||||
Nothing ->
|
||||
List.map (error moduleKey) ranges
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@ -5,6 +5,7 @@ import Json.Decode as Decode
|
||||
import NoUnused.CustomTypeConstructorArgs exposing (rule)
|
||||
import Review.Project as Project exposing (Project)
|
||||
import Review.Test
|
||||
import Review.Test.Dependencies
|
||||
import Test exposing (Test, describe, test)
|
||||
|
||||
|
||||
@ -384,12 +385,146 @@ type Msg
|
||||
]
|
||||
|> Review.Test.runOnModules rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report Never arguments" <|
|
||||
\() ->
|
||||
"""
|
||||
module Main exposing (a)
|
||||
a = 1
|
||||
type CustomType
|
||||
= B Never
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report Never arguments even when aliased" <|
|
||||
\() ->
|
||||
"""
|
||||
module Main exposing (a)
|
||||
import Basics as B
|
||||
a = 1
|
||||
type CustomType
|
||||
= B B.Never
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report arguments next to Never" <|
|
||||
-- Honestly I'm unsure about doing this, but I currently don't see
|
||||
-- the point of having other args next to a Never arg.
|
||||
\() ->
|
||||
"""
|
||||
module Main exposing (a)
|
||||
a = 1
|
||||
type CustomType
|
||||
= B SomeData Never
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = message
|
||||
, details = details
|
||||
, under = "SomeData"
|
||||
}
|
||||
]
|
||||
, test "should not report args for type constructors used in an equality expression (==)" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = Unused 0 == b
|
||||
b = B
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report args for type constructors used in an inequality expression (/=)" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = Unused 0 /= b
|
||||
b = B
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report args for type constructors used in non-equality operator expressions" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = Unused <| b
|
||||
b = B
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = message
|
||||
, details = details
|
||||
, under = "Int"
|
||||
}
|
||||
]
|
||||
, test "should not report args for type constructors used in an equality expression with parenthesized expressions" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = ( Unused 0 ) == b
|
||||
b = ( B )
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report args for type constructors used in an equality expression with tuples" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = ( Unused 0, Unused 1 ) == b
|
||||
b = ( B, B )
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report args for type constructors used in an equality expression with lists" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = [ Unused 0 ] == b
|
||||
b = [ B ]
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report args for type constructors used in an equality expression (==) in a different module" <|
|
||||
\() ->
|
||||
[ """
|
||||
module MyModule exposing (a, b)
|
||||
import Foo as F
|
||||
a = F.Unused 0 == b
|
||||
b = F.B
|
||||
""", """
|
||||
module Foo exposing (Foo(..))
|
||||
type Foo = Unused Int | B
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData packageProject rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report args for type constructors used in an equality expression when value is passed to a function" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (a, b)
|
||||
type Foo = Unused Int | B
|
||||
a = foo (Unused 0) == b
|
||||
b = B
|
||||
"""
|
||||
|> Review.Test.runWithProjectData packageProject rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = message
|
||||
, details = details
|
||||
, under = "Int"
|
||||
}
|
||||
]
|
||||
]
|
||||
|
||||
|
||||
packageProject : Project
|
||||
packageProject =
|
||||
Project.new
|
||||
Review.Test.Dependencies.projectWithElmCore
|
||||
|> Project.addElmJson (createElmJson packageElmJson)
|
||||
|
||||
|
||||
|
||||
@ -504,8 +504,8 @@ declarationName declaration =
|
||||
Declaration.CustomTypeDeclaration type_ ->
|
||||
Just <| Node.value type_.name
|
||||
|
||||
Declaration.AliasDeclaration alias ->
|
||||
Just <| Node.value alias.name
|
||||
Declaration.AliasDeclaration alias_ ->
|
||||
Just <| Node.value alias_.name
|
||||
|
||||
Declaration.PortDeclaration port_ ->
|
||||
Just <| Node.value port_.name
|
||||
@ -569,8 +569,8 @@ typesUsedInDeclaration moduleContext declaration =
|
||||
False
|
||||
)
|
||||
|
||||
Declaration.AliasDeclaration alias ->
|
||||
( collectTypesFromTypeAnnotation moduleContext alias.typeAnnotation, False )
|
||||
Declaration.AliasDeclaration alias_ ->
|
||||
( collectTypesFromTypeAnnotation moduleContext alias_.typeAnnotation, False )
|
||||
|
||||
Declaration.PortDeclaration _ ->
|
||||
( [], False )
|
||||
@ -648,5 +648,24 @@ expressionVisitor node moduleContext =
|
||||
Nothing ->
|
||||
( [], moduleContext )
|
||||
|
||||
Expression.LetExpression { declarations } ->
|
||||
let
|
||||
used : List ( ModuleName, String )
|
||||
used =
|
||||
List.concatMap
|
||||
(\declaration ->
|
||||
case Node.value declaration of
|
||||
Expression.LetFunction function ->
|
||||
function.signature
|
||||
|> Maybe.map (Node.value >> .typeAnnotation >> collectTypesFromTypeAnnotation moduleContext)
|
||||
|> Maybe.withDefault []
|
||||
|
||||
Expression.LetDestructuring _ _ ->
|
||||
[]
|
||||
)
|
||||
declarations
|
||||
in
|
||||
( [], registerMultipleAsUsed used moduleContext )
|
||||
|
||||
_ ->
|
||||
( [], moduleContext )
|
||||
|
||||
@ -47,8 +47,8 @@ applicationElmJson =
|
||||
}
|
||||
|
||||
|
||||
package : Project
|
||||
package =
|
||||
package_ : Project
|
||||
package_ =
|
||||
Project.new
|
||||
|> Project.addElmJson (createPackageElmJson ())
|
||||
|
||||
@ -203,7 +203,7 @@ module A exposing (exposed)
|
||||
exposed = 1
|
||||
main = exposed
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Exposed function or value `exposed` is never used outside this module."
|
||||
@ -219,7 +219,7 @@ module A exposing (exposed1, exposed2)
|
||||
exposed1 = 1
|
||||
exposed2 = 2
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Exposed function or value `exposed1` is never used outside this module."
|
||||
@ -250,7 +250,7 @@ exposed2 = 2
|
||||
module A exposing (..)
|
||||
a = 1
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report the `main` function for an application even if it is unused" <|
|
||||
\() ->
|
||||
@ -266,7 +266,7 @@ main = text ""
|
||||
module Main exposing (main)
|
||||
main = text ""
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Exposed function or value `main` is never used outside this module."
|
||||
@ -281,7 +281,7 @@ main = text ""
|
||||
module A exposing (b)
|
||||
a = 1
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report exposed tests" <|
|
||||
\() ->
|
||||
@ -291,7 +291,7 @@ import Test exposing (Test)
|
||||
a : Test
|
||||
a = Test.describe "thing" []
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not ReviewConfig.config" <|
|
||||
\() ->
|
||||
@ -299,7 +299,7 @@ a = Test.describe "thing" []
|
||||
module ReviewConfig exposing (config)
|
||||
config = []
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a function in a 'shadowed' module" <|
|
||||
\() ->
|
||||
@ -360,7 +360,7 @@ import A
|
||||
type alias B = A.ExposedB
|
||||
type alias C = A.ExposedC
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's part of the package's exposed API" <|
|
||||
\() ->
|
||||
@ -368,7 +368,7 @@ type alias C = A.ExposedC
|
||||
module Exposed exposing (MyType)
|
||||
type MyType = VariantA | VariantB
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's present in the signature of an exposed function" <|
|
||||
\() ->
|
||||
@ -391,7 +391,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's present in an exposed type alias" <|
|
||||
\() ->
|
||||
@ -404,7 +404,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's present in an exposed type alias (nested)" <|
|
||||
\() ->
|
||||
@ -417,7 +417,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments" <|
|
||||
\() ->
|
||||
@ -430,7 +430,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments but the constructors are not exposed" <|
|
||||
\() ->
|
||||
@ -443,7 +443,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
@ -471,7 +471,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed custom type if it's present in an exposed custom type constructor's arguments (nested) but the constructors are not exposed" <|
|
||||
\() ->
|
||||
@ -484,7 +484,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
@ -501,6 +501,22 @@ type OtherType = OtherThing | SomeThing ((), List MyType)
|
||||
]
|
||||
)
|
||||
]
|
||||
, test "should not report an exposed type if it is used in a let block type annotation" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main =
|
||||
let
|
||||
type1 : B.Type1
|
||||
type1 =
|
||||
1
|
||||
in
|
||||
type1
|
||||
""", """module B exposing (Type1)
|
||||
type Type1 = Type1
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
]
|
||||
|
||||
|
||||
@ -532,6 +548,34 @@ module B exposing (main)
|
||||
import A
|
||||
main : A.Exposed
|
||||
main = {}
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an exposed type alias if it is used in a let block type annotation" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main =
|
||||
let
|
||||
type1 : B.Type1
|
||||
type1 =
|
||||
1
|
||||
in
|
||||
type1
|
||||
""", """module B exposing (Type1)
|
||||
type alias Type1 = Int
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an exposed type if it is used as value in a let block" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main =
|
||||
let type1 = B.Type1
|
||||
in type1
|
||||
""", """module B exposing (Type1)
|
||||
type alias Type1 = {}
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
@ -545,7 +589,7 @@ module Exposed exposing (B)
|
||||
import A
|
||||
type alias B = A.ExposedB
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed type alias if it's part of the package's exposed API" <|
|
||||
\() ->
|
||||
@ -553,7 +597,7 @@ type alias B = A.ExposedB
|
||||
module Exposed exposing (MyType)
|
||||
type alias MyType = {}
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed type alias if it's present in the signature of an exposed function" <|
|
||||
\() ->
|
||||
@ -576,7 +620,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed type alias if it's present in an exposed type alias" <|
|
||||
\() ->
|
||||
@ -589,7 +633,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed type alias if it's present in an exposed type alias (nested)" <|
|
||||
\() ->
|
||||
@ -602,7 +646,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed type alias if it's present in an exposed custom type constructor's arguments" <|
|
||||
\() ->
|
||||
@ -615,7 +659,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an unused exposed type alias if it's present in an exposed custom type constructor's arguments but the constructors are not exposed" <|
|
||||
\() ->
|
||||
@ -628,7 +672,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
@ -656,7 +700,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report an unused exposed type alias if it's present in an exposed custom type constructor's arguments (nested) but the constructors are not exposed" <|
|
||||
\() ->
|
||||
@ -669,7 +713,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
type alias B = A.OtherType
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
@ -702,7 +746,7 @@ module Exposed exposing (..)
|
||||
import A
|
||||
a = A.Card A.init A.toElement
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData package rule
|
||||
|> Review.Test.runOnModulesWithProjectData package_ rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
|
||||
@ -47,8 +47,8 @@ applicationElmJson =
|
||||
}
|
||||
|
||||
|
||||
package : Project
|
||||
package =
|
||||
package_ : Project
|
||||
package_ =
|
||||
Project.new
|
||||
|> Project.addElmJson (createPackageElmJson ())
|
||||
|
||||
@ -203,7 +203,7 @@ config = []
|
||||
module Exposed exposing (..)
|
||||
a = 1
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report non-exposed and non-used modules from a package" <|
|
||||
\() ->
|
||||
@ -211,7 +211,7 @@ a = 1
|
||||
module NotExposed exposing (..)
|
||||
a = 1
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Module `NotExposed` is never used."
|
||||
@ -225,7 +225,7 @@ a = 1
|
||||
module Reported exposing (main)
|
||||
main = text ""
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Module `Reported` is never used."
|
||||
@ -240,7 +240,7 @@ module Reported exposing (a)
|
||||
main = text ""
|
||||
a = 1
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.runWithProjectData package_ rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Module `Reported` is never used."
|
||||
|
||||
@ -154,8 +154,7 @@ expressionEnterVisitorHelp node context =
|
||||
Expression.CaseExpression { cases } ->
|
||||
( []
|
||||
, { context
|
||||
| scopes = { declared = [], used = Set.empty } :: context.scopes
|
||||
, scopesToCreate =
|
||||
| scopesToCreate =
|
||||
List.foldl
|
||||
(\( pattern, expr ) scopesToCreate -> RangeDict.insert (Node.range expr) (findPatterns Matching pattern) scopesToCreate)
|
||||
context.scopesToCreate
|
||||
|
||||
@ -300,6 +300,37 @@ bar =
|
||||
case bar of
|
||||
_ ->
|
||||
Nothing
|
||||
"""
|
||||
]
|
||||
, test "should report patterns in nested case declarations" <|
|
||||
\() ->
|
||||
"""
|
||||
module A exposing (..)
|
||||
foo =
|
||||
case thing of
|
||||
Just data ->
|
||||
case () of
|
||||
_ -> 1
|
||||
_ ->
|
||||
bosh
|
||||
"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Value `data` is not used."
|
||||
, details = useOrReplaceDetails
|
||||
, under = "data"
|
||||
}
|
||||
|> Review.Test.whenFixed
|
||||
"""
|
||||
module A exposing (..)
|
||||
foo =
|
||||
case thing of
|
||||
Just _ ->
|
||||
case () of
|
||||
_ -> 1
|
||||
_ ->
|
||||
bosh
|
||||
"""
|
||||
]
|
||||
]
|
||||
|
||||
@ -315,26 +315,27 @@ moduleDefinitionVisitor (Node _ moduleNode) context =
|
||||
let
|
||||
names : List String
|
||||
names =
|
||||
List.map
|
||||
(\(Node _ node) ->
|
||||
case node of
|
||||
Exposing.FunctionExpose name ->
|
||||
name
|
||||
|
||||
Exposing.TypeOrAliasExpose name ->
|
||||
name
|
||||
|
||||
Exposing.TypeExpose { name } ->
|
||||
name
|
||||
|
||||
Exposing.InfixExpose name ->
|
||||
name
|
||||
)
|
||||
list
|
||||
List.map getExposingName list
|
||||
in
|
||||
( [], markAllAsUsed names context )
|
||||
|
||||
|
||||
getExposingName : Node Exposing.TopLevelExpose -> String
|
||||
getExposingName node =
|
||||
case Node.value node of
|
||||
Exposing.FunctionExpose name ->
|
||||
name
|
||||
|
||||
Exposing.TypeOrAliasExpose name ->
|
||||
name
|
||||
|
||||
Exposing.TypeExpose { name } ->
|
||||
name
|
||||
|
||||
Exposing.InfixExpose name ->
|
||||
name
|
||||
|
||||
|
||||
importVisitor : Node Import -> ModuleContext -> ( List (Error {}), ModuleContext )
|
||||
importVisitor ((Node importRange import_) as node) context =
|
||||
let
|
||||
@ -363,12 +364,6 @@ importVisitor ((Node importRange import_) as node) context =
|
||||
|
||||
Just declaredImports ->
|
||||
let
|
||||
customTypesFromModule : Dict String (List String)
|
||||
customTypesFromModule =
|
||||
context.customTypes
|
||||
|> Dict.get (Node.value import_.moduleName)
|
||||
|> Maybe.withDefault Dict.empty
|
||||
|
||||
contextWithAlias : ModuleContext
|
||||
contextWithAlias =
|
||||
case import_.moduleAlias of
|
||||
@ -381,20 +376,31 @@ importVisitor ((Node importRange import_) as node) context =
|
||||
( errors
|
||||
, case Node.value declaredImports of
|
||||
Exposing.All _ ->
|
||||
{ contextWithAlias
|
||||
| exposingAllModules =
|
||||
{ name = Node.value import_.moduleName
|
||||
, alias = Maybe.map (Node.value >> String.join ".") import_.moduleAlias
|
||||
, moduleNameRange = Node.range import_.moduleName
|
||||
, exposingRange = Node.range declaredImports
|
||||
, importRange = importRange
|
||||
, wasUsedImplicitly = False
|
||||
, wasUsedWithModuleName = False
|
||||
}
|
||||
:: context.exposingAllModules
|
||||
}
|
||||
if Dict.member (Node.value import_.moduleName) context.customTypes then
|
||||
{ contextWithAlias
|
||||
| exposingAllModules =
|
||||
{ name = Node.value import_.moduleName
|
||||
, alias = Maybe.map (Node.value >> String.join ".") import_.moduleAlias
|
||||
, moduleNameRange = Node.range import_.moduleName
|
||||
, exposingRange = Node.range declaredImports
|
||||
, importRange = importRange
|
||||
, wasUsedImplicitly = False
|
||||
, wasUsedWithModuleName = False
|
||||
}
|
||||
:: context.exposingAllModules
|
||||
}
|
||||
|
||||
else
|
||||
contextWithAlias
|
||||
|
||||
Exposing.Explicit list ->
|
||||
let
|
||||
customTypesFromModule : Dict String (List String)
|
||||
customTypesFromModule =
|
||||
context.customTypes
|
||||
|> Dict.get (Node.value import_.moduleName)
|
||||
|> Maybe.withDefault Dict.empty
|
||||
in
|
||||
List.foldl
|
||||
(registerExposedElements customTypesFromModule)
|
||||
contextWithAlias
|
||||
@ -403,27 +409,27 @@ importVisitor ((Node importRange import_) as node) context =
|
||||
|
||||
|
||||
registerExposedElements : Dict String (List String) -> ExposedElement -> ModuleContext -> ModuleContext
|
||||
registerExposedElements customTypesFromModule importedElement context_ =
|
||||
registerExposedElements customTypesFromModule importedElement context =
|
||||
case importedElement of
|
||||
CustomType name variableInfo ->
|
||||
case Dict.get name customTypesFromModule of
|
||||
Just constructorNames ->
|
||||
{ context_
|
||||
| unusedImportedCustomTypes = Dict.insert name variableInfo context_.unusedImportedCustomTypes
|
||||
{ context
|
||||
| unusedImportedCustomTypes = Dict.insert name variableInfo context.unusedImportedCustomTypes
|
||||
, importedCustomTypeLookup =
|
||||
Dict.union
|
||||
(constructorNames
|
||||
|> List.map (\constructorName -> ( constructorName, name ))
|
||||
|> Dict.fromList
|
||||
)
|
||||
context_.importedCustomTypeLookup
|
||||
context.importedCustomTypeLookup
|
||||
}
|
||||
|
||||
Nothing ->
|
||||
context_
|
||||
context
|
||||
|
||||
TypeOrValue name variableInfo ->
|
||||
registerVariable variableInfo name context_
|
||||
registerVariable variableInfo name context
|
||||
|
||||
|
||||
collectExplicitlyExposedElements : Range -> List (Node Exposing.TopLevelExpose) -> List ExposedElement
|
||||
@ -584,7 +590,7 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
Expression.FunctionOrValue [] name ->
|
||||
case Dict.get name context.constructorNameToTypeName of
|
||||
Just typeName ->
|
||||
( [], markAsUsed typeName context )
|
||||
( [], markValueAsUsed typeName context )
|
||||
|
||||
Nothing ->
|
||||
case Dict.get name context.importedCustomTypeLookup of
|
||||
@ -596,12 +602,12 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
Just realModuleName ->
|
||||
( []
|
||||
, context
|
||||
|> markAsUsed name
|
||||
|> markValueAsUsed name
|
||||
|> markModuleAsUsed ( realModuleName, [] )
|
||||
)
|
||||
|
||||
Nothing ->
|
||||
( [], markAsUsed name context )
|
||||
( [], markValueAsUsed name context )
|
||||
|
||||
Expression.FunctionOrValue moduleName _ ->
|
||||
case ModuleNameLookupTable.moduleNameAt context.lookupTable range of
|
||||
@ -612,10 +618,10 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
( [], context )
|
||||
|
||||
Expression.OperatorApplication name _ _ _ ->
|
||||
( [], markAsUsed name context )
|
||||
( [], markValueAsUsed name context )
|
||||
|
||||
Expression.PrefixOperator name ->
|
||||
( [], markAsUsed name context )
|
||||
( [], markValueAsUsed name context )
|
||||
|
||||
Expression.LetExpression { declarations, expression } ->
|
||||
let
|
||||
@ -628,7 +634,7 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
HasMultipleDeclarations
|
||||
in
|
||||
List.foldl
|
||||
(\declaration (( errors, foldContext ) as unchangedResult) ->
|
||||
(\declaration ( errors, foldContext ) ->
|
||||
case Node.value declaration of
|
||||
Expression.LetFunction function ->
|
||||
let
|
||||
@ -651,15 +657,15 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
}
|
||||
in
|
||||
( errors
|
||||
, foldContext
|
||||
, List.foldl markValueAsUsed foldContext namesUsedInArgumentPatterns.types
|
||||
|> markAllModulesAsUsed namesUsedInArgumentPatterns.modules
|
||||
|> registerFunction letBlockContext function
|
||||
|> markUsedTypesAndModules namesUsedInArgumentPatterns
|
||||
|> markAsInTheDeclarationOf (function.declaration |> Node.value |> .name |> Node.value)
|
||||
)
|
||||
|
||||
Expression.LetDestructuring pattern _ ->
|
||||
case isAllPattern pattern of
|
||||
Just wildCardRange ->
|
||||
case removeParens pattern of
|
||||
Node wildCardRange Pattern.AllPattern ->
|
||||
( Rule.errorWithFix
|
||||
{ message = "Value assigned to `_` is unused"
|
||||
, details =
|
||||
@ -667,21 +673,34 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
]
|
||||
}
|
||||
wildCardRange
|
||||
[ case letBlockContext of
|
||||
HasMultipleDeclarations ->
|
||||
Fix.removeRange (Node.range declaration)
|
||||
|
||||
HasNoOtherDeclarations letDeclarationsRange ->
|
||||
-- If there are no other declarations in the let in block,
|
||||
-- we also need to remove the `let in` keywords.
|
||||
Fix.removeRange letDeclarationsRange
|
||||
]
|
||||
[ Fix.removeRange (letDeclarationToRemoveRange letBlockContext (Node.range declaration)) ]
|
||||
:: errors
|
||||
, foldContext
|
||||
)
|
||||
|
||||
Nothing ->
|
||||
unchangedResult
|
||||
Node unitPattern Pattern.UnitPattern ->
|
||||
( Rule.errorWithFix
|
||||
{ message = "Unit value is unused"
|
||||
, details =
|
||||
[ "This value has no data, which makes the value unusable. You should remove it at the location I pointed at."
|
||||
]
|
||||
}
|
||||
unitPattern
|
||||
[ Fix.removeRange (letDeclarationToRemoveRange letBlockContext (Node.range declaration)) ]
|
||||
:: errors
|
||||
, foldContext
|
||||
)
|
||||
|
||||
_ ->
|
||||
let
|
||||
namesUsedInPattern : { types : List String, modules : List ( ModuleName, ModuleName ) }
|
||||
namesUsedInPattern =
|
||||
getUsedVariablesFromPattern context pattern
|
||||
in
|
||||
( []
|
||||
, List.foldl markValueAsUsed foldContext namesUsedInPattern.types
|
||||
|> markAllModulesAsUsed namesUsedInPattern.modules
|
||||
)
|
||||
)
|
||||
( [], { context | scopes = NonemptyList.cons emptyScope context.scopes } )
|
||||
declarations
|
||||
@ -694,23 +713,35 @@ expressionEnterVisitorHelp (Node range value) context =
|
||||
|> List.map (getUsedVariablesFromPattern context)
|
||||
|> foldUsedTypesAndModules
|
||||
in
|
||||
( [], markUsedTypesAndModules namesUsedInArgumentPatterns context )
|
||||
( []
|
||||
, List.foldl markValueAsUsed context namesUsedInArgumentPatterns.types
|
||||
|> markAllModulesAsUsed namesUsedInArgumentPatterns.modules
|
||||
)
|
||||
|
||||
_ ->
|
||||
( [], context )
|
||||
|
||||
|
||||
isAllPattern : Node Pattern -> Maybe Range
|
||||
isAllPattern node =
|
||||
letDeclarationToRemoveRange : LetBlockContext -> Range -> Range
|
||||
letDeclarationToRemoveRange letBlockContext range =
|
||||
case letBlockContext of
|
||||
HasMultipleDeclarations ->
|
||||
range
|
||||
|
||||
HasNoOtherDeclarations letDeclarationsRange ->
|
||||
-- If there are no other declarations in the let in block,
|
||||
-- we also need to remove the `let in` keywords.
|
||||
letDeclarationsRange
|
||||
|
||||
|
||||
removeParens : Node Pattern -> Node Pattern
|
||||
removeParens node =
|
||||
case Node.value node of
|
||||
Pattern.ParenthesizedPattern pattern ->
|
||||
isAllPattern pattern
|
||||
|
||||
Pattern.AllPattern ->
|
||||
Just (Node.range node)
|
||||
removeParens pattern
|
||||
|
||||
_ ->
|
||||
Nothing
|
||||
node
|
||||
|
||||
|
||||
expressionExitVisitor : Node Expression -> ModuleContext -> ( List (Error {}), ModuleContext )
|
||||
@ -731,7 +762,7 @@ expressionExitVisitorHelp : Node Expression -> ModuleContext -> ( List (Error {}
|
||||
expressionExitVisitorHelp node context =
|
||||
case Node.value node of
|
||||
Expression.RecordUpdateExpression expr _ ->
|
||||
( [], markAsUsed (Node.value expr) context )
|
||||
( [], markValueAsUsed (Node.value expr) context )
|
||||
|
||||
Expression.CaseExpression { cases } ->
|
||||
let
|
||||
@ -745,7 +776,8 @@ expressionExitVisitorHelp node context =
|
||||
|> foldUsedTypesAndModules
|
||||
in
|
||||
( []
|
||||
, markUsedTypesAndModules usedVariables context
|
||||
, List.foldl markValueAsUsed context usedVariables.types
|
||||
|> markAllModulesAsUsed usedVariables.modules
|
||||
)
|
||||
|
||||
Expression.LetExpression _ ->
|
||||
@ -922,15 +954,24 @@ declarationListVisitor nodes context =
|
||||
)
|
||||
|
||||
Declaration.AliasDeclaration { name, documentation } ->
|
||||
let
|
||||
contextWithRemovedShadowedImports : ModuleContext
|
||||
contextWithRemovedShadowedImports =
|
||||
{ context | importedCustomTypeLookup = Dict.remove (Node.value name) context.importedCustomTypeLookup }
|
||||
in
|
||||
( []
|
||||
, registerVariable
|
||||
{ typeName = "Type"
|
||||
, under = Node.range name
|
||||
, rangeToRemove = Just (rangeToRemoveForNodeWithDocumentation node documentation)
|
||||
, warning = ""
|
||||
}
|
||||
(Node.value name)
|
||||
{ context | importedCustomTypeLookup = Dict.remove (Node.value name) context.importedCustomTypeLookup }
|
||||
, if context.exposesEverything then
|
||||
contextWithRemovedShadowedImports
|
||||
|
||||
else
|
||||
registerVariable
|
||||
{ typeName = "Type"
|
||||
, under = Node.range name
|
||||
, rangeToRemove = Just (rangeToRemoveForNodeWithDocumentation node documentation)
|
||||
, warning = ""
|
||||
}
|
||||
(Node.value name)
|
||||
contextWithRemovedShadowedImports
|
||||
)
|
||||
|
||||
_ ->
|
||||
@ -944,7 +985,7 @@ declarationListVisitor nodes context =
|
||||
-- DECLARATION VISITOR
|
||||
|
||||
|
||||
declarationVisitor : Node Declaration -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
declarationVisitor : Node Declaration -> ModuleContext -> ( List (Error {}), ModuleContext )
|
||||
declarationVisitor node context =
|
||||
case Node.value node of
|
||||
Declaration.FunctionDeclaration function ->
|
||||
@ -953,6 +994,10 @@ declarationVisitor node context =
|
||||
functionImplementation =
|
||||
Node.value function.declaration
|
||||
|
||||
functionName : String
|
||||
functionName =
|
||||
Node.value functionImplementation.name
|
||||
|
||||
namesUsedInSignature : { types : List String, modules : List ( ModuleName, ModuleName ) }
|
||||
namesUsedInSignature =
|
||||
function.signature
|
||||
@ -972,7 +1017,7 @@ declarationVisitor node context =
|
||||
if
|
||||
context.exposesEverything
|
||||
-- The main function is "exposed" by default for applications
|
||||
|| (context.isApplication && Node.value functionImplementation.name == "main")
|
||||
|| (context.isApplication && functionName == "main")
|
||||
then
|
||||
context
|
||||
|
||||
@ -983,16 +1028,31 @@ declarationVisitor node context =
|
||||
, rangeToRemove = Just (rangeToRemoveForNodeWithDocumentation node function.documentation)
|
||||
, warning = ""
|
||||
}
|
||||
(Node.value functionImplementation.name)
|
||||
functionName
|
||||
context
|
||||
|
||||
newContext : ModuleContext
|
||||
newContext =
|
||||
{ newContextWhereFunctionIsRegistered | inTheDeclarationOf = [ Node.value functionImplementation.name ], declarations = Dict.empty }
|
||||
|> markUsedTypesAndModules namesUsedInSignature
|
||||
|> markUsedTypesAndModules namesUsedInArgumentPatterns
|
||||
{ newContextWhereFunctionIsRegistered | inTheDeclarationOf = [ functionName ], declarations = Dict.empty }
|
||||
|> (\ctx -> List.foldl markAsUsed ctx namesUsedInSignature.types)
|
||||
|> (\ctx -> List.foldl markValueAsUsed ctx namesUsedInArgumentPatterns.types)
|
||||
|> markAllModulesAsUsed namesUsedInSignature.modules
|
||||
|> markAllModulesAsUsed namesUsedInArgumentPatterns.modules
|
||||
|
||||
shadowingImportError : List (Error {})
|
||||
shadowingImportError =
|
||||
case Dict.get functionName (NonemptyList.head context.scopes).declared of
|
||||
Just existingVariable ->
|
||||
if existingVariable.typeName == "Imported variable" then
|
||||
[ error existingVariable functionName ]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
||||
in
|
||||
( [], newContext )
|
||||
( shadowingImportError, newContext )
|
||||
|
||||
Declaration.CustomTypeDeclaration { name, constructors } ->
|
||||
let
|
||||
@ -1003,11 +1063,10 @@ declarationVisitor node context =
|
||||
|> foldUsedTypesAndModules
|
||||
in
|
||||
( []
|
||||
, markUsedTypesAndModules
|
||||
{ types = List.filter ((/=) (Node.value name)) types
|
||||
, modules = modules
|
||||
}
|
||||
context
|
||||
, types
|
||||
|> List.filter ((/=) (Node.value name))
|
||||
|> List.foldl markAsUsed context
|
||||
|> markAllModulesAsUsed modules
|
||||
)
|
||||
|
||||
Declaration.AliasDeclaration { name, typeAnnotation } ->
|
||||
@ -1017,7 +1076,8 @@ declarationVisitor node context =
|
||||
collectNamesFromTypeAnnotation context.lookupTable typeAnnotation
|
||||
in
|
||||
( []
|
||||
, markUsedTypesAndModules namesUsedInTypeAnnotation context
|
||||
, List.foldl markAsUsed context namesUsedInTypeAnnotation.types
|
||||
|> markAllModulesAsUsed namesUsedInTypeAnnotation.modules
|
||||
)
|
||||
|
||||
Declaration.PortDeclaration { name, typeAnnotation } ->
|
||||
@ -1025,23 +1085,31 @@ declarationVisitor node context =
|
||||
namesUsedInTypeAnnotation : { types : List String, modules : List ( ModuleName, ModuleName ) }
|
||||
namesUsedInTypeAnnotation =
|
||||
collectNamesFromTypeAnnotation context.lookupTable typeAnnotation
|
||||
|
||||
contextWithUsedElements : ModuleContext
|
||||
contextWithUsedElements =
|
||||
List.foldl markAsUsed context namesUsedInTypeAnnotation.types
|
||||
|> markAllModulesAsUsed namesUsedInTypeAnnotation.modules
|
||||
in
|
||||
( []
|
||||
, context
|
||||
|> markUsedTypesAndModules namesUsedInTypeAnnotation
|
||||
|> registerVariable
|
||||
, if context.exposesEverything then
|
||||
contextWithUsedElements
|
||||
|
||||
else
|
||||
registerVariable
|
||||
{ typeName = "Port"
|
||||
, under = Node.range name
|
||||
, rangeToRemove = Nothing
|
||||
, warning = " (Warning: Removing this port may break your application if it is used in the JS code)"
|
||||
}
|
||||
(Node.value name)
|
||||
contextWithUsedElements
|
||||
)
|
||||
|
||||
Declaration.InfixDeclaration { operator, function } ->
|
||||
( []
|
||||
, context
|
||||
|> markAsUsed (Node.value function)
|
||||
|> markValueAsUsed (Node.value function)
|
||||
|> registerVariable
|
||||
{ typeName = "Declared operator"
|
||||
, under = Node.range operator
|
||||
@ -1060,13 +1128,6 @@ foldUsedTypesAndModules =
|
||||
List.foldl (\a b -> { types = a.types ++ b.types, modules = a.modules ++ b.modules }) { types = [], modules = [] }
|
||||
|
||||
|
||||
markUsedTypesAndModules : { types : List String, modules : List ( ModuleName, ModuleName ) } -> ModuleContext -> ModuleContext
|
||||
markUsedTypesAndModules { types, modules } context =
|
||||
context
|
||||
|> markAllAsUsed types
|
||||
|> markAllModulesAsUsed modules
|
||||
|
||||
|
||||
rangeToRemoveForNodeWithDocumentation : Node Declaration -> Maybe (Node a) -> Range
|
||||
rangeToRemoveForNodeWithDocumentation (Node nodeRange _) documentation =
|
||||
case documentation of
|
||||
@ -1231,18 +1292,22 @@ finalEvaluation context =
|
||||
|
||||
customTypeErrors : List (Error {})
|
||||
customTypeErrors =
|
||||
context.localCustomTypes
|
||||
|> Dict.toList
|
||||
|> List.filter (\( name, _ ) -> not <| Set.member name usedLocally)
|
||||
|> List.map
|
||||
(\( name, customType ) ->
|
||||
Rule.errorWithFix
|
||||
{ message = "Type `" ++ name ++ "` is not used"
|
||||
, details = details
|
||||
}
|
||||
customType.under
|
||||
[ Fix.removeRange customType.rangeToRemove ]
|
||||
)
|
||||
if context.exposesEverything then
|
||||
[]
|
||||
|
||||
else
|
||||
context.localCustomTypes
|
||||
|> Dict.toList
|
||||
|> List.filter (\( name, _ ) -> not <| Set.member name usedLocally)
|
||||
|> List.map
|
||||
(\( name, customType ) ->
|
||||
Rule.errorWithFix
|
||||
{ message = "Type `" ++ name ++ "` is not used"
|
||||
, details = details
|
||||
}
|
||||
customType.under
|
||||
[ Fix.removeRange customType.rangeToRemove ]
|
||||
)
|
||||
in
|
||||
List.concat
|
||||
[ newRootScope
|
||||
@ -1282,23 +1347,15 @@ registerFunction letBlockContext function context =
|
||||
Nothing ->
|
||||
Node.range function.declaration
|
||||
in
|
||||
context
|
||||
List.foldl markAsUsed context namesUsedInSignature.types
|
||||
|> markAllModulesAsUsed namesUsedInSignature.modules
|
||||
|> registerVariable
|
||||
{ typeName = "`let in` variable"
|
||||
, under = Node.range declaration.name
|
||||
, rangeToRemove =
|
||||
case letBlockContext of
|
||||
HasMultipleDeclarations ->
|
||||
Just functionRange
|
||||
|
||||
HasNoOtherDeclarations letDeclarationsRange ->
|
||||
-- If there are no other declarations in the let in block,
|
||||
-- we also need to remove the `let in` keywords.
|
||||
Just letDeclarationsRange
|
||||
, rangeToRemove = Just (letDeclarationToRemoveRange letBlockContext functionRange)
|
||||
, warning = ""
|
||||
}
|
||||
(Node.value declaration.name)
|
||||
|> markUsedTypesAndModules namesUsedInSignature
|
||||
|
||||
|
||||
type ExposedElement
|
||||
@ -1421,37 +1478,46 @@ markAllAsUsed names context =
|
||||
List.foldl markAsUsed context names
|
||||
|
||||
|
||||
markValueAsUsed : String -> ModuleContext -> ModuleContext
|
||||
markValueAsUsed name context =
|
||||
if Dict.member name context.constructorNameToTypeName then
|
||||
markAsUsed name context
|
||||
|
||||
else
|
||||
case Dict.get name context.importedCustomTypeLookup of
|
||||
Just customTypeName ->
|
||||
{ context | unusedImportedCustomTypes = Dict.remove customTypeName context.unusedImportedCustomTypes }
|
||||
|
||||
_ ->
|
||||
markAsUsed name context
|
||||
|
||||
|
||||
markAsUsed : String -> ModuleContext -> ModuleContext
|
||||
markAsUsed name context =
|
||||
case ( Dict.get name <| context.importedCustomTypeLookup, Dict.get name context.constructorNameToTypeName ) of
|
||||
( Just customTypeName, Nothing ) ->
|
||||
{ context | unusedImportedCustomTypes = Dict.remove customTypeName context.unusedImportedCustomTypes }
|
||||
if List.member name context.inTheDeclarationOf then
|
||||
context
|
||||
|
||||
_ ->
|
||||
if List.member name context.inTheDeclarationOf then
|
||||
context
|
||||
|
||||
else
|
||||
let
|
||||
scopes : Nonempty Scope
|
||||
scopes =
|
||||
NonemptyList.mapHead
|
||||
(\scope ->
|
||||
{ scope
|
||||
| used =
|
||||
Dict.update []
|
||||
(\set ->
|
||||
set
|
||||
|> Maybe.withDefault Set.empty
|
||||
|> Set.insert name
|
||||
|> Just
|
||||
)
|
||||
scope.used
|
||||
}
|
||||
)
|
||||
context.scopes
|
||||
in
|
||||
{ context | scopes = scopes }
|
||||
else
|
||||
let
|
||||
scopes : Nonempty Scope
|
||||
scopes =
|
||||
NonemptyList.mapHead
|
||||
(\scope ->
|
||||
{ scope
|
||||
| used =
|
||||
Dict.update []
|
||||
(\set ->
|
||||
set
|
||||
|> Maybe.withDefault Set.empty
|
||||
|> Set.insert name
|
||||
|> Just
|
||||
)
|
||||
scope.used
|
||||
}
|
||||
)
|
||||
context.scopes
|
||||
in
|
||||
{ context | scopes = scopes }
|
||||
|
||||
|
||||
markAllModulesAsUsed : List ( ModuleName, ModuleName ) -> ModuleContext -> ModuleContext
|
||||
@ -1602,14 +1668,9 @@ positionAsInt { row, column } =
|
||||
|
||||
comparePosition : { row : Int, column : Int } -> { row : Int, column : Int } -> Order
|
||||
comparePosition a b =
|
||||
let
|
||||
order : Order
|
||||
order =
|
||||
compare a.row b.row
|
||||
in
|
||||
case order of
|
||||
case compare a.row b.row of
|
||||
EQ ->
|
||||
compare a.column b.column
|
||||
|
||||
_ ->
|
||||
order ->
|
||||
order
|
||||
|
||||
@ -188,13 +188,25 @@ b = 2"""
|
||||
|> Review.Test.whenFixed """module SomeModule exposing (b)
|
||||
b = 2"""
|
||||
]
|
||||
, test "should not report unused top-level variables if everything is exposed" <|
|
||||
, test "should not report unused top-level variables if everything is exposed (functions)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (..)
|
||||
a n = 1
|
||||
b = a 1"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report unused top-level variables if everything is exposed (custom types)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (..)
|
||||
type A = A"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report unused top-level variables if everything is exposed (type aliases)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (..)
|
||||
type alias A = ()"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report unused top-level variables that are exposed by name" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a, b)
|
||||
@ -437,6 +449,23 @@ a = let _ = 1
|
||||
, under = "_"
|
||||
}
|
||||
|> Review.Test.whenFixed """module SomeModule exposing (a)
|
||||
a = 2"""
|
||||
]
|
||||
, test "should report () destructuring" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
a = let () = b
|
||||
in 2"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Unit value is unused"
|
||||
, details =
|
||||
[ "This value has no data, which makes the value unusable. You should remove it at the location I pointed at."
|
||||
]
|
||||
, under = "()"
|
||||
}
|
||||
|> Review.Test.whenFixed """module SomeModule exposing (a)
|
||||
a = 2"""
|
||||
]
|
||||
, test "should report parenthesized wildcard assignments" <|
|
||||
@ -743,6 +772,18 @@ import Html.Styled.Attributes
|
||||
a = Html.Styled.Attributes.href"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report used import (let destructuring)" <|
|
||||
\() ->
|
||||
[ """module SomeModule exposing (a)
|
||||
import B
|
||||
a = let (B.B ()) = x
|
||||
in 1
|
||||
"""
|
||||
, """module B exposing (B)
|
||||
type B = B ()"""
|
||||
]
|
||||
|> Review.Test.runOnModules rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report unused import if it is aliased" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
@ -1225,6 +1266,20 @@ a = 1"""
|
||||
]
|
||||
)
|
||||
]
|
||||
, test "should not report import if it exposes all and its contents are unknown" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
import Unknown exposing (..)
|
||||
a = 1"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report imported type if it exposes the constructors and the module is unknown" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
import Unknown exposing (A(..))
|
||||
a = 1"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report unused import from dependency that exposes everything" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (..)
|
||||
@ -1374,6 +1429,77 @@ b = 1
|
||||
]
|
||||
|> Review.Test.runOnModules rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report unused imported value if it is redefined" <|
|
||||
\() ->
|
||||
[ """module A exposing (a)
|
||||
import Used exposing (shadowed)
|
||||
shadowed = 1
|
||||
a = shadowed"""
|
||||
, """module Used exposing (shadowed)
|
||||
shadowed = 1
|
||||
"""
|
||||
]
|
||||
|> Review.Test.runOnModules rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
{ message = "Imported variable `shadowed` is not used"
|
||||
, details = details
|
||||
, under = "shadowed"
|
||||
}
|
||||
|> Review.Test.atExactly { start = { row = 2, column = 23 }, end = { row = 2, column = 31 } }
|
||||
|> Review.Test.whenFixed ("""module A exposing (a)
|
||||
import Used$
|
||||
shadowed = 1
|
||||
a = shadowed""" |> String.replace "$" " ")
|
||||
]
|
||||
)
|
||||
]
|
||||
, test "should report unused imported value if it is redefined, and should not report the top-level one even if used before declaration" <|
|
||||
\() ->
|
||||
[ """module A exposing (a)
|
||||
import Used exposing (shadowed)
|
||||
a = shadowed
|
||||
shadowed = 1"""
|
||||
, """module Used exposing (shadowed)
|
||||
shadowed = 1
|
||||
"""
|
||||
]
|
||||
|> Review.Test.runOnModules rule
|
||||
|> Review.Test.expectErrorsForModules
|
||||
[ ( "A"
|
||||
, [ Review.Test.error
|
||||
{ message = "Imported variable `shadowed` is not used"
|
||||
, details = details
|
||||
, under = "shadowed"
|
||||
}
|
||||
|> Review.Test.atExactly { start = { row = 2, column = 23 }, end = { row = 2, column = 31 } }
|
||||
|> Review.Test.whenFixed ("""module A exposing (a)
|
||||
import Used$
|
||||
a = shadowed
|
||||
shadowed = 1""" |> String.replace "$" " ")
|
||||
]
|
||||
)
|
||||
]
|
||||
, test "should not report imported type as unused when it's used in a type annotation, and the name conflicts with an imported custom type constructor" <|
|
||||
\() ->
|
||||
[ """module Main exposing (thing, main)
|
||||
import ModuleA exposing (A)
|
||||
import ModuleB exposing (Variants(..))
|
||||
|
||||
thing : Variants
|
||||
thing = A
|
||||
|
||||
main : A
|
||||
main = ()
|
||||
"""
|
||||
, """module ModuleA exposing (A)
|
||||
type alias A = ()"""
|
||||
, """module ModuleB exposing (Variants(..))
|
||||
type Variants = A"""
|
||||
]
|
||||
|> Review.Test.runOnModules rule
|
||||
|> Review.Test.expectNoErrors
|
||||
]
|
||||
|
||||
|
||||
@ -1934,6 +2060,22 @@ a = output ()
|
||||
subscriptions = input GotInput"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report exposed ports" <|
|
||||
\() ->
|
||||
"""port module SomeModule exposing (output, input)
|
||||
import Json.Decode
|
||||
port output : () -> Cmd msg
|
||||
port input : (Json.Decode.Value -> msg) -> Sub msg"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report exposed ports using (..)" <|
|
||||
\() ->
|
||||
"""port module SomeModule exposing (..)
|
||||
import Json.Decode
|
||||
port output : () -> Cmd msg
|
||||
port input : (Json.Decode.Value -> msg) -> Sub msg"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report unused ports (ingoing)" <|
|
||||
\() ->
|
||||
"""port module SomeModule exposing (a)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user