Add NoUnused rules like they are in jfmengels/review-unused

This commit is contained in:
Jeroen Engels 2020-04-03 15:50:37 +02:00
parent 02d7665e91
commit 60106d7958
16 changed files with 1188 additions and 507 deletions

View File

@ -14,11 +14,11 @@ when inside the directory containing this file.
import NoDebug.Log
import NoDebug.TodoOrToString
import NoTodoComment
import NoUnused.CustomTypeConstructors2
import NoUnused.CustomTypeConstructors
import NoUnused.Dependencies
import NoUnused.Exports
import NoUnused.Modules
import NoUnused.Variables
import NoUnusedDependencies
import NoUnusedExports
import NoUnusedModules
import Review.Rule as Rule exposing (Rule)
@ -27,11 +27,12 @@ config =
[ NoDebug.Log.rule
, NoDebug.TodoOrToString.rule
|> Rule.ignoreErrorsForDirectories [ "tests/" ]
, NoUnused.CustomTypeConstructors2.rule
, NoUnused.CustomTypeConstructors.rule
, NoUnused.Variables.rule
, NoUnusedDependencies.rule
, NoUnusedExports.rule
, NoUnusedModules.rule
, NoUnused.Dependencies.rule
, NoUnused.Exports.rule
|> Rule.ignoreErrorsForFiles [ "tests/Scope.elm" ]
, NoUnused.Modules.rule
--, NoTodoComment.rule
-- |> Rule.ignoreErrorsForFiles [ "NoTodoComment" ]

View File

@ -2,8 +2,7 @@
"type": "application",
"source-directories": [
".",
"/home/jeroen/dev/review-debug/src/",
"/home/jeroen/dev/review-unused/src/"
"../tests"
],
"elm-version": "0.19.1",
"dependencies": {

View File

@ -1,6 +1,6 @@
module NoUnused.CustomTypeConstructors2 exposing (rule)
module NoUnused.CustomTypeConstructors exposing (rule)
{-| Forbid having unused custom type constructors in a file.
{-| Forbid having unused custom type constructors inside the project.
# Rule
@ -21,22 +21,30 @@ import Elm.Syntax.Node as Node exposing (Node)
import Elm.Syntax.Signature exposing (Signature)
import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
import Review.Rule as Rule exposing (Direction, Error, Rule)
import Review.Scope as Scope
import Scope
import Set exposing (Set)
{-| Forbid having unused custom type constructors in a file.
{-| Forbid having unused custom type constructors.
config =
[ NoUnused.CustomTypeConstructors.rule
]
Note that this does not report a constructor if it is exposed in the module, even
if it is not used anywhere in the project. For a more accurate detection of
unused constructors (and functions) across your project, you might want to check
out [elm-xref](https://github.com/zwilias/elm-xref). You may still want to use
this rule in your config so that you get notified of unused constructors earlier
in your editor, rather than when running your tests or [elm-xref](https://github.com/zwilias/elm-xref).
Note that this rule reports any custom type constructor that isn't used
anywhere _in the project_.
If the project is a package and the module that declared the type is exposed and
the type's constructors are exposed, then the constructors will not be reported.
Phantom types are supported by this rule. A constructor won't be reported if
- It is the only constructor of a type that has no type variable
- It has no parameters
- It is used as an argument of a custom type, in the stead of a type variable that is not used in the definition in any of the type's constructors.
**Note**: At the time of writing, there may be cases where this is not well handled.
If you find one, please open an issue!
## Fail
@ -478,9 +486,9 @@ finalProjectEvaluation projectContext =
-- ERROR
errorInformation : Node String -> { message : String, details : List String }
errorInformation node =
{ message = "Type constructor `" ++ Node.value node ++ "` is not used."
errorInformation : String -> { message : String, details : List String }
errorInformation name =
{ message = "Type constructor `" ++ name ++ "` is not used."
, details = [ "This type constructor is never used. It might be handled everywhere it might appear, but there is no location where this value actually gets created." ]
}
@ -489,7 +497,7 @@ errorForModule : Rule.ModuleKey -> Node String -> Error scope
errorForModule moduleKey node =
Rule.errorForModule
moduleKey
(errorInformation node)
(errorInformation (Node.value node))
(Node.range node)

View File

@ -1,8 +1,8 @@
module NoUnusedCustomTypeConstructorsTest exposing (all)
module NoUnused.CustomTypeConstructorsTest exposing (all)
import Elm.Project
import Json.Decode as Decode
import NoUnused.CustomTypeConstructors2 exposing (rule)
import NoUnused.CustomTypeConstructors exposing (rule)
import Review.Project as Project exposing (Project)
import Review.Test
import Test exposing (Test, describe, test)

View File

@ -1,6 +1,6 @@
module NoUnusedDependencies exposing (rule)
module NoUnused.Dependencies exposing (rule)
{-| Forbid the use of modules that are never used in your project.
{-| Forbid the use of dependencies that are never used in your project.
# Rule
@ -21,40 +21,26 @@ import Review.Rule as Rule exposing (Error, Rule)
import Set exposing (Set)
{-| Forbid the use of modules that are never used in your project.
{-| Forbid the use of dependencies that are never used in your project.
A module is considered unused if it does not contain a `main` function
(be it exposed or not), does not import `Test` module, and is never imported in
other modules. For packages, modules listed in the `elm.json`'s
`exposed-modules` are considered used. The `ReviewConfig` is also always
considered as used.
A module will be considered as used if it gets imported, even if none of its
functions or types are used. Other rules from this package will help detect and
remove code so that the import statement is removed.
A dependency is considered unused if none of its modules are imported in the project.
config =
[ NoUnused.Dependencies.rule
]
# When (not) to use this rule
You may not want to enable this rule if you are not concerned about having
unused modules in your application or package.
-}
rule : Rule
rule =
Rule.newProjectRuleSchema "NoUnused.Dependencies" initialProjectContext
|> Rule.withElmJsonProjectVisitor elmJsonVisitor
|> Rule.withDependenciesProjectVisitor dependenciesVisitor
|> Rule.withModuleVisitor moduleVisitor
|> Rule.withModuleContext
{ fromProjectToModule = fromProjectToModule
, fromModuleToProject = fromModuleToProject
, foldProjectContexts = foldProjectContexts
}
|> Rule.withElmJsonProjectVisitor elmJsonVisitor
|> Rule.withDependenciesProjectVisitor dependenciesVisitor
|> Rule.withFinalProjectEvaluation finalEvaluationForProject
|> Rule.fromProjectRuleSchema
@ -208,7 +194,7 @@ error elmJsonKey packageName =
Rule.errorForElmJson elmJsonKey
(\elmJson ->
{ message = "Unused dependency `" ++ packageName ++ "`"
, details = [ "You can simplify your project a tiny bit by removing this dependency." ]
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall " ++ packageName ++ "`" ]
, range = findPackageNameInElmJson packageName elmJson
}
)

View File

@ -1,9 +1,9 @@
module NoUnusedDependenciesTest exposing (all)
module NoUnused.DependenciesTest exposing (all)
import Elm.Docs
import Elm.Project
import Json.Decode as Decode
import NoUnusedDependencies exposing (rule)
import NoUnused.Dependencies exposing (rule)
import Review.Project as Project exposing (Project)
import Review.Project.Dependency as Dependency exposing (Dependency)
import Review.Test
@ -155,12 +155,6 @@ packageWithBar =
modules
details : List String
details =
[ "You can simplify your project a tiny bit by removing this dependency."
]
all : Test
all =
describe "NoUnused.Dependencies"
@ -182,12 +176,12 @@ a = 1
|> Review.Test.expectErrorsForElmJson
[ Review.Test.error
{ message = "Unused dependency `author/package-with-bar`"
, details = details
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-bar`" ]
, under = "author/package-with-bar"
}
, Review.Test.error
{ message = "Unused dependency `author/package-with-foo`"
, details = details
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-foo`" ]
, under = "author/package-with-foo"
}
]
@ -211,12 +205,12 @@ a = 1
|> Review.Test.expectErrorsForElmJson
[ Review.Test.error
{ message = "Unused dependency `author/package-with-bar`"
, details = details
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-bar`" ]
, under = "author/package-with-bar"
}
, Review.Test.error
{ message = "Unused dependency `author/package-with-foo`"
, details = details
, details = [ "To remove it, I recommend installing `elm-json` and running `elm-json uninstall author/package-with-foo`" ]
, under = "author/package-with-foo"
}
]

View File

@ -1,6 +1,6 @@
module NoUnusedExports exposing (rule)
module NoUnused.Exports exposing (rule)
{-| Forbid the use of modules that are never used in your project.
{-| Forbid the use of exposed elements that are never used in your project.
# Rule
@ -24,32 +24,20 @@ import Elm.Syntax.Node as Node exposing (Node(..))
import Elm.Syntax.Range exposing (Range)
import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
import Review.Rule as Rule exposing (Error, Rule)
import Review.Scope as Scope
import Scope
import Set exposing (Set)
{-| Forbid the use of modules that are never used in your project.
{-| Report functions and types that are exposed from a module but that are never
used in other modules.
A module is considered unused if it does not contain a `main` function
(be it exposed or not), does not import `Test` module, and is never imported in
other modules. For packages, modules listed in the `elm.json`'s
`exposed-modules` are considered used. The `ReviewConfig` is also always
considered as used.
A module will be considered as used if it gets imported, even if none of its
functions or types are used. Other rules from this package will help detect and
remove code so that the import statement is removed.
If the project is a package and the module that declared the element is exposed,
then nothing will be reported.
config =
[ NoUnused.Modules.rule
[ NoUnused.Exports.rule
]
# When (not) to use this rule
You may not want to enable this rule if you are not concerned about having
unused modules in your application or package.
-}
rule : Rule
rule =
@ -495,7 +483,7 @@ collectTypesFromTypeAnnotation scope node =
-- EXPRESSION VISITOR
expressionVisitor : Node Expression -> Rule.Direction -> ModuleContext -> ( List (Error scope), ModuleContext )
expressionVisitor : Node Expression -> Rule.Direction -> ModuleContext -> ( List nothing, ModuleContext )
expressionVisitor node direction moduleContext =
case ( direction, Node.value node ) of
( Rule.OnEnter, Expression.FunctionOrValue moduleName name ) ->

View File

@ -1,9 +1,9 @@
module NoUnusedExportsTest exposing (all)
module NoUnused.ExportsTest exposing (all)
import Elm.Project
import Elm.Version
import Json.Decode as Decode
import NoUnusedExports exposing (rule)
import NoUnused.Exports exposing (rule)
import Review.Project as Project exposing (Project)
import Review.Test
import Test exposing (Test, describe, test)

View File

@ -1,4 +1,4 @@
module NoUnusedModules exposing (rule)
module NoUnused.Modules exposing (rule)
{-| Forbid the use of modules that are never used in your project.
@ -23,25 +23,19 @@ import Set exposing (Set)
{-| Forbid the use of modules that are never used in your project.
A module is considered unused if it does not contain a `main` function
(be it exposed or not), does not import `Test` module, and is never imported in
other modules. For packages, modules listed in the `elm.json`'s
`exposed-modules` are considered used. The `ReviewConfig` is also always
considered as used.
A module is considered used if
A module will be considered as used if it gets imported, even if none of its
functions or types are used. Other rules from this package will help detect and
remove code so that the import statement is removed.
- it contains a `main` function (be it exposed or not)
- it imports the `Test` module
- it is imported in any other modules, even if it is not used.
- the project is a package and the module is part of the `elm.json`'s `exposed-modules`
- it is named `ReviewConfig`
config =
[ NoUnused.Modules.rule
]
# When (not) to use this rule
You may not want to enable this rule if you are not concerned about having
unused modules in your application or package.
```elm
config =
[ NoUnused.Modules.rule
]
```
-}
rule : Rule

View File

@ -1,9 +1,9 @@
module NoUnusedModulesTest exposing (all)
module NoUnused.ModulesTest exposing (all)
import Elm.Project
import Elm.Version
import Json.Decode as Decode
import NoUnusedModules exposing (rule)
import NoUnused.Modules exposing (rule)
import Review.Project as Project exposing (Project)
import Review.Test
import Test exposing (Test, describe, test)

View File

@ -1,4 +1,4 @@
module NonemptyList exposing
module NoUnused.NonemptyList exposing
( Nonempty(..)
, fromElement
, head
@ -26,12 +26,7 @@ available.
# Access
@docs head
# Inspect
@docs any
@docs head, sample
# Convert

View File

@ -1,6 +1,6 @@
module Review.Rule.NoUnusedVariables exposing (rule)
module NoUnused.Variables exposing (rule)
{-| Report variables or types that are declared or imported but never used.
{-| Report variables or types that are declared or imported but never used inside of a module.
# Rule
@ -19,7 +19,7 @@ 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 exposing (TypeAnnotation(..))
import NonemptyList as NonemptyList exposing (Nonempty)
import NoUnused.NonemptyList as NonemptyList exposing (Nonempty)
import Review.Fix as Fix
import Review.Rule as Rule exposing (Direction, Error, Rule)
import Set exposing (Set)

View File

@ -1,6 +1,6 @@
module NoUnusedVariablesTest exposing (all)
module NoUnused.VariablesTest exposing (all)
import Review.Rule.NoUnusedVariables exposing (rule)
import NoUnused.Variables exposing (rule)
import Review.Test exposing (ReviewResult)
import Test exposing (Test, describe, test)

View File

@ -1,289 +0,0 @@
module Review.NoDebugTest exposing (all)
import Review.Rule.NoDebug exposing (rule)
import Review.Test exposing (ReviewResult)
import Test exposing (Test, describe, test)
testRule : String -> ReviewResult
testRule string =
"module A exposing (..)\n\n"
++ string
|> Review.Test.run rule
message : String
message =
"Remove the use of `Debug` before shipping to production"
details : List String
details =
[ "The `Debug` module is useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing its use before committing and attempting to push to production."
]
tests : List Test
tests =
[ test "should not report normal function calls" <|
\() ->
testRule """
a = foo n
b = bar.foo n
c = debug
e = debug.log n
d = debug.log n
"""
|> Review.Test.expectNoErrors
, test "should report Debug.log use" <|
\() ->
testRule "a = Debug.log"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug.log calls" <|
\() ->
testRule "a = Debug.log z"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report multiple Debug.log calls" <|
\() ->
testRule """
a = Debug.log z
b = Debug.log z
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 14 } }
, Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 14 } }
]
, test "should report Debug.todo calls" <|
\() ->
testRule "a = Debug.todo 1"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.todo"
}
]
, test "should report any Debug method" <|
\() ->
testRule "a = Debug.foo 1"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.foo"
}
]
, test "should report Debug in a binary expression" <|
\() ->
testRule "a = ( Debug.log z ) + 2"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in a << binary expression" <|
\() ->
testRule "a = fn << Debug.log"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in a pipe expression" <|
\() ->
testRule "a = fn |> Debug.log z"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an list expression" <|
\() ->
testRule "a = [ Debug.log z y ]"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an record expression" <|
\() ->
testRule "a = { foo = Debug.log z y }"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an record update expression" <|
\() ->
testRule "a = { model | foo = Debug.log z y }"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an lambda expression" <|
\() ->
testRule "a = (\\foo -> Debug.log z foo)"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an if expression condition" <|
\() ->
testRule "a = if Debug.log a b then True else False"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an if expression then branch" <|
\() ->
testRule "a = if True then Debug.log a b else False"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in an if expression else branch" <|
\() ->
testRule "a = if True then True else Debug.log a b"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in a case value" <|
\() ->
testRule """
a = case Debug.log a b of
_ -> []
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in a case body" <|
\() ->
testRule """
a = case a of
_ -> Debug.log a b
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in let declaration section" <|
\() ->
testRule """
a = let b = Debug.log a b
in b
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should report Debug in let body" <|
\() ->
testRule """
a = let b = c
in Debug.log a b
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
]
, test "should not report calls from a module containing Debug but that is not Debug" <|
\() ->
testRule """
a = Foo.Debug.log 1
b = Debug.Foo.log 1
"""
|> Review.Test.expectNoErrors
, test "should report the import of the Debug module" <|
\() ->
testRule "import Debug"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug"
}
]
, test "should report the import of the Debug module (with exposing of some things)" <|
\() ->
testRule "import Debug exposing (log)"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug"
}
]
, test "should not report imports of modules containing Debug but that is not Debug" <|
\() ->
testRule """
import Foo.Debug
import Debug.Foo
"""
|> Review.Test.expectNoErrors
]
all : Test
all =
describe "NoDebug" tests

View File

@ -1,104 +0,0 @@
module Review.Rule.NoDebug exposing (rule)
{-| Forbid the use of the [`Debug`](https://package.elm-lang.org/packages/elm/core/latest/Debug) module before it goes into production or fails in the CI.
# Rule
@docs rule
-}
import Elm.Syntax.Expression exposing (Expression(..))
import Elm.Syntax.Import exposing (Import)
import Elm.Syntax.Node as Node exposing (Node)
import Review.Rule as Rule exposing (Error, Rule)
{-| Forbid the use of the [`Debug`](https://package.elm-lang.org/packages/elm/core/latest/Debug) module before it goes into production or fails in the CI.
config =
[ NoDebug.rule
]
## Fail
if Debug.log "condition" condition then
a
else
b
if condition then
Debug.todo "Nooo!"
else
value
## Success
if condition then
a
else
b
# When (not) to use this rule
You should use this rule if you're developing a package meant to be published,
or an application that is put into production, and wish to know about the use of
the [`Debug`](https://package.elm-lang.org/packages/elm/core/latest/Debug)
module before committing your changes.
You should not use this rule if you are developing an application that is not
put into production, and you do not care about having stray debug logs or
runtime exceptions caused by [`Debug.todo`](https://package.elm-lang.org/packages/elm/core/latest/Debug#todo),
and you do not ship to production.
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoDebug" ()
|> Rule.withSimpleImportVisitor importVisitor
|> Rule.withSimpleExpressionVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
error : Node a -> Error {}
error node =
Rule.error
{ message = "Remove the use of `Debug` before shipping to production"
, details = [ "The `Debug` module is useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing its use before committing and attempting to push to production." ]
}
(Node.range node)
importVisitor : Node Import -> List (Error {})
importVisitor node =
let
moduleNameNode : Node (List String)
moduleNameNode =
node |> Node.value |> .moduleName
in
if Node.value moduleNameNode == [ "Debug" ] then
[ error moduleNameNode ]
else
[]
expressionVisitor : Node Expression -> List (Error {})
expressionVisitor node =
case Node.value node of
FunctionOrValue moduleName fnName ->
if moduleName == [ "Debug" ] then
[ error node ]
else
[]
_ ->
[]

1109
tests/Scope.elm Normal file

File diff suppressed because it is too large Load Diff