Skip to content

Commit

Permalink
Variables: Avoid false positive around type name confusion
Browse files Browse the repository at this point in the history 
Fixes #15
Avoids a false positive between imported types and imported
custom type constructors.
  • Loading branch information
@jfmengels
jfmengels committed Mar 25, 2021
1 parent 88fbda2 commit 89b5b48
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 38 deletions.
80 changes: 42 additions & 38 deletions src/NoUnused/Variables.elm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,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
Expand All @@ -602,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
Expand All @@ -618,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
Expand Down Expand Up @@ -657,9 +657,9 @@ 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)
)

Expand Down Expand Up @@ -705,7 +705,10 @@ expressionEnterVisitorHelp (Node range value) context =
|> List.map (getUsedVariablesFromPattern context)
|> foldUsedTypesAndModules
in
( [], markUsedTypesAndModules namesUsedInArgumentPatterns context )
( []
, List.foldl markValueAsUsed context namesUsedInArgumentPatterns.types
|> markAllModulesAsUsed namesUsedInArgumentPatterns.modules
)

_ ->
( [], context )
Expand Down Expand Up @@ -751,7 +754,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
Expand All @@ -765,7 +768,8 @@ expressionExitVisitorHelp node context =
|> foldUsedTypesAndModules
in
( []
, markUsedTypesAndModules usedVariables context
, List.foldl markValueAsUsed context usedVariables.types
|> markAllModulesAsUsed usedVariables.modules
)

Expression.LetExpression _ ->
Expand Down Expand Up @@ -1022,8 +1026,10 @@ declarationVisitor node context =
newContext : ModuleContext
newContext =
{ newContextWhereFunctionIsRegistered | inTheDeclarationOf = [ functionName ], declarations = Dict.empty }
|> markUsedTypesAndModules namesUsedInSignature
|> markUsedTypesAndModules namesUsedInArgumentPatterns
|> (\ctx -> List.foldl markAsUsed ctx namesUsedInSignature.types)
|> (\ctx -> List.foldl markValueAsUsed ctx namesUsedInArgumentPatterns.types)
|> markAllModulesAsUsed namesUsedInSignature.modules
|> markAllModulesAsUsed namesUsedInArgumentPatterns.modules

shadowingImportError : List (Error {})
shadowingImportError =
Expand All @@ -1049,11 +1055,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 } ->
Expand All @@ -1063,7 +1068,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 } ->
Expand All @@ -1074,7 +1080,8 @@ declarationVisitor node context =

contextWithUsedElements : ModuleContext
contextWithUsedElements =
markUsedTypesAndModules namesUsedInTypeAnnotation context
List.foldl markAsUsed context namesUsedInTypeAnnotation.types
|> markAllModulesAsUsed namesUsedInTypeAnnotation.modules
in
( []
, if context.exposesEverything then
Expand All @@ -1094,7 +1101,7 @@ declarationVisitor node context =
Declaration.InfixDeclaration { operator, function } ->
( []
, context
|> markAsUsed (Node.value function)
|> markValueAsUsed (Node.value function)
|> registerVariable
{ typeName = "Declared operator"
, under = Node.range operator
Expand All @@ -1113,13 +1120,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
Expand Down Expand Up @@ -1339,15 +1339,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 = Just (letDeclarationToRemoveRange letBlockContext functionRange)
, warning = ""
}
(Node.value declaration.name)
|> markUsedTypesAndModules namesUsedInSignature


type ExposedElement
Expand Down Expand Up @@ -1470,18 +1470,22 @@ markAllAsUsed names context =
List.foldl markAsUsed context names


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 }
markValueAsUsed : String -> ModuleContext -> ModuleContext
markValueAsUsed name context =
if Dict.member name context.constructorNameToTypeName then
markAsUsed name context

_ ->
markNameAsUserHelper name context
else
case Dict.get name context.importedCustomTypeLookup of
Just customTypeName ->
{ context | unusedImportedCustomTypes = Dict.remove customTypeName context.unusedImportedCustomTypes }

_ ->
markAsUsed name context

markNameAsUserHelper : String -> ModuleContext -> ModuleContext
markNameAsUserHelper name context =

markAsUsed : String -> ModuleContext -> ModuleContext
markAsUsed name context =
if List.member name context.inTheDeclarationOf then
context

Expand Down
19 changes: 19 additions & 0 deletions tests/NoUnused/VariablesTest.elm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,25 @@ 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
]


Expand Down

0 comments on commit 89b5b48

Please sign in to comment.