mirror of
https://github.com/jfmengels/elm-review.git
synced 2024-12-23 17:53:35 +03:00
Add rule ElmTest.NoDuplicateTestBodies
This commit is contained in:
parent
966a0de116
commit
11b37494a9
@ -15,6 +15,7 @@ import Lint.Rules.NoUselessPatternMatching
|
||||
import Lint.Rules.NoWarningComments
|
||||
import Lint.Rules.SimplifyPiping
|
||||
import Lint.Rules.SimplifyPropertyAccess
|
||||
import Lint.Rules.ElmTest.NoDuplicateTestBodies
|
||||
|
||||
|
||||
config : List ( Severity, LintRule )
|
||||
@ -33,4 +34,5 @@ config =
|
||||
, ( Warning, Lint.Rules.NoWarningComments.rule )
|
||||
, ( Critical, Lint.Rules.SimplifyPiping.rule )
|
||||
, ( Critical, Lint.Rules.SimplifyPropertyAccess.rule )
|
||||
, ( Critical, Lint.Rules.ElmTest.NoDuplicateTestBodies.rule )
|
||||
]
|
||||
|
@ -33,6 +33,7 @@ You can read the slides for my [presentation](http://slides.com/jeroenengels/elm
|
||||
- [NoWarningComments](rules/NoWarningComments.md) - Detect comments containing words like `TODO`, `FIXME` and `XXX`.
|
||||
- [SimplifyPiping](rules/SimplifyPiping.md) - Simplify piped functions like `List.map f >> List.map g` to `List.map (f >> g)`
|
||||
- [SimplifyPropertyAccess](rules/SimplifyPropertyAccess.md) - Replace functions that only return the property of its parameter by an access function, like `(\x -> x.foo)` to `.foo`
|
||||
- [ElmTest.NoDuplicateTestBodies](rules/ElmTest/NoDuplicateTestBodies.md) - Forbid having multiple tests with the same bodies. Often a consequence of copy-pasting tests.
|
||||
|
||||
More rule ideas in this [slide](http://slides.com/jeroenengels/elm-lint#/5/3) and the ones below it.
|
||||
Note that some rules were implemented but may not be good ideas. Think for yourself and ask the community whether you should enable them.
|
||||
|
@ -22,7 +22,8 @@
|
||||
"Lint.Rules.NoUselessPatternMatching",
|
||||
"Lint.Rules.NoWarningComments",
|
||||
"Lint.Rules.SimplifyPiping",
|
||||
"Lint.Rules.SimplifyPropertyAccess"
|
||||
"Lint.Rules.SimplifyPropertyAccess",
|
||||
"Lint.Rules.ElmTest.NoDuplicateTestBodies"
|
||||
],
|
||||
"dependencies": {
|
||||
"Bogdanp/elm-ast": "8.0.3 <= v < 9.0.0",
|
||||
|
@ -30,6 +30,7 @@ import Lint.Rules.NoUselessPatternMatching
|
||||
import Lint.Rules.NoWarningComments
|
||||
import Lint.Rules.SimplifyPiping
|
||||
import Lint.Rules.SimplifyPropertyAccess
|
||||
import Lint.Rules.ElmTest.NoDuplicateTestBodies
|
||||
|
||||
|
||||
type Msg
|
||||
@ -52,6 +53,7 @@ config =
|
||||
, ( Warning, Lint.Rules.NoWarningComments.rule )
|
||||
, ( Critical, Lint.Rules.SimplifyPiping.rule )
|
||||
, ( Critical, Lint.Rules.SimplifyPropertyAccess.rule )
|
||||
, ( Critical, Lint.Rules.ElmTest.NoDuplicateTestBodies.rule )
|
||||
]
|
||||
|
||||
|
||||
|
183
src/Lint/Rules/ElmTest/NoDuplicateTestBodies.elm
Normal file
183
src/Lint/Rules/ElmTest/NoDuplicateTestBodies.elm
Normal file
@ -0,0 +1,183 @@
|
||||
module Lint.Rules.ElmTest.NoDuplicateTestBodies exposing (rule)
|
||||
|
||||
{-|
|
||||
@docs rule
|
||||
|
||||
# Fail
|
||||
|
||||
module Addition exposing (..)
|
||||
|
||||
import Test exposing (test)
|
||||
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
|
||||
# Success
|
||||
|
||||
module Addition exposing (..)
|
||||
|
||||
import Test exposing (test)
|
||||
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\() -> 1 + 2
|
||||
|> Expect.equal 3
|
||||
]
|
||||
-}
|
||||
|
||||
import Ast.Expression exposing (..)
|
||||
import Ast.Statement exposing (..)
|
||||
import Lint exposing (lint, doNothing)
|
||||
import Lint.Types exposing (LintRule, LintRuleImplementation, LintError, Direction(..))
|
||||
import Dict exposing (Dict)
|
||||
|
||||
|
||||
type alias Context =
|
||||
{ availableTestAliases : List String
|
||||
}
|
||||
|
||||
|
||||
{-| Forbid dupicate test bodies.
|
||||
When copy-pasting tests, it can happen that the title is changed but the developer forgets to update the test body.
|
||||
This may result in specifications that are thought to be implemented but are not enforced by tests.
|
||||
|
||||
rules =
|
||||
[ ElmTest.NoDuplicateTestBodies.rule
|
||||
]
|
||||
-}
|
||||
rule : LintRule
|
||||
rule input =
|
||||
lint input implementation
|
||||
|
||||
|
||||
implementation : LintRuleImplementation Context
|
||||
implementation =
|
||||
{ statementFn = statementFn
|
||||
, typeFn = doNothing
|
||||
, expressionFn = expressionFn
|
||||
, moduleEndFn = (\ctx -> ( [], ctx ))
|
||||
, initialContext = Context []
|
||||
}
|
||||
|
||||
|
||||
error : ( String, String ) -> LintError
|
||||
error ( title1, title2 ) =
|
||||
LintError
|
||||
"ElmTest.NoDuplicateTestBodies"
|
||||
("Test `" ++ title1 ++ "` has the same body as test `" ++ title2 ++ "`")
|
||||
|
||||
|
||||
isTestFunctionCall : List String -> Expression -> Bool
|
||||
isTestFunctionCall availableTestAliases expr =
|
||||
case expr of
|
||||
Variable fnName ->
|
||||
List.member (String.join "." fnName) availableTestAliases
|
||||
|
||||
Access (Variable object) fields ->
|
||||
List.member (String.join "." <| object ++ fields) availableTestAliases
|
||||
|
||||
_ ->
|
||||
False
|
||||
|
||||
|
||||
filterTests : List String -> List Expression -> List ( String, Expression )
|
||||
filterTests availableTestAliases listItems =
|
||||
List.concatMap
|
||||
(\item ->
|
||||
case item of
|
||||
BinOp (Variable [ "<|" ]) (Application fn (String title)) testBody ->
|
||||
if isTestFunctionCall availableTestAliases fn then
|
||||
[ ( title, testBody ) ]
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
||||
)
|
||||
listItems
|
||||
|
||||
|
||||
expressionFn : Context -> Direction Expression -> ( List LintError, Context )
|
||||
expressionFn ctx node =
|
||||
case node of
|
||||
Enter (List listItems) ->
|
||||
let
|
||||
tests =
|
||||
filterTests ctx.availableTestAliases listItems
|
||||
|
||||
redundantTests =
|
||||
List.foldl
|
||||
(\( title, testBody ) { dict, redundant } ->
|
||||
let
|
||||
testBodyAsString =
|
||||
toString testBody
|
||||
|
||||
existingTest =
|
||||
Dict.get testBodyAsString dict
|
||||
in
|
||||
case existingTest of
|
||||
Nothing ->
|
||||
{ dict = Dict.insert testBodyAsString title dict, redundant = redundant }
|
||||
|
||||
Just existingTestTitle ->
|
||||
{ dict = dict, redundant = redundant ++ [ ( title, existingTestTitle ) ] }
|
||||
)
|
||||
{ dict = Dict.empty, redundant = [] }
|
||||
tests
|
||||
in
|
||||
( List.map error redundantTests.redundant, ctx )
|
||||
|
||||
_ ->
|
||||
( [], ctx )
|
||||
|
||||
|
||||
extractImported : ExportSet -> List String
|
||||
extractImported exportSet =
|
||||
case exportSet of
|
||||
AllExport ->
|
||||
[ "test" ]
|
||||
|
||||
SubsetExport list ->
|
||||
List.concatMap extractImported list
|
||||
|
||||
FunctionExport name ->
|
||||
if name == "test" then
|
||||
[ name ]
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
||||
|
||||
|
||||
computeAlias : Maybe String -> String
|
||||
computeAlias =
|
||||
Maybe.withDefault "Test"
|
||||
|
||||
|
||||
statementFn : Context -> Direction Statement -> ( List LintError, Context )
|
||||
statementFn ctx node =
|
||||
case node of
|
||||
Enter (ImportStatement [ "Test" ] testAlias exportSet) ->
|
||||
let
|
||||
moduleFnAccess =
|
||||
computeAlias testAlias ++ ".test"
|
||||
in
|
||||
case exportSet of
|
||||
Nothing ->
|
||||
( [], { availableTestAliases = [ moduleFnAccess ] } )
|
||||
|
||||
Just subExportSet ->
|
||||
( [], { availableTestAliases = [ moduleFnAccess ] ++ extractImported subExportSet } )
|
||||
|
||||
_ ->
|
||||
( [], ctx )
|
213
tests/ElmTest/NoDuplicateTestBodiesTest.elm
Normal file
213
tests/ElmTest/NoDuplicateTestBodiesTest.elm
Normal file
@ -0,0 +1,213 @@
|
||||
module ElmTest.NoDuplicateTestBodiesTest exposing (all)
|
||||
|
||||
import Test exposing (describe, test, Test)
|
||||
import Lint.Rules.ElmTest.NoDuplicateTestBodies exposing (rule)
|
||||
import Lint.Types exposing (LintRule, LintError, LintResult)
|
||||
import TestUtil exposing (ruleTester, expectErrors)
|
||||
|
||||
|
||||
testRule : String -> LintResult
|
||||
testRule =
|
||||
ruleTester rule
|
||||
|
||||
|
||||
error : String -> LintError
|
||||
error =
|
||||
LintError "ElmTest.NoDuplicateTestBodies"
|
||||
|
||||
|
||||
tests : List Test
|
||||
tests =
|
||||
[ test "should not report non-tests" <|
|
||||
\() ->
|
||||
testRule """
|
||||
foo = [ 1, 1 ]
|
||||
bar = [ foo 2 <| 1, foo 2 <| 1 ]
|
||||
tests =
|
||||
[ fn "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, fn "bar" <|
|
||||
\\() -> 1 + 2
|
||||
|> Expect.equal 3
|
||||
]
|
||||
"""
|
||||
|> expectErrors []
|
||||
, test "should not report tests that have distinct bodies" <|
|
||||
\() ->
|
||||
testRule """
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\\() -> 1 + 2
|
||||
|> Expect.equal 3
|
||||
]
|
||||
"""
|
||||
|> expectErrors []
|
||||
, test "should not report tests that have the same body when Test is not imported and used in list" <|
|
||||
\() ->
|
||||
testRule """
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors []
|
||||
, test "should report tests that have the same body when using test and exposing (test)" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test exposing (test, foo)
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "baz" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors
|
||||
[ error "Test `bar` has the same body as test `foo`"
|
||||
, error "Test `baz` has the same body as test `foo`"
|
||||
]
|
||||
, test "should report tests that have the same body when using Test.test and exposing (test)" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test exposing (test, foo)
|
||||
tests =
|
||||
[ Test.test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, Test.test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors [ error "Test `bar` has the same body as test `foo`" ]
|
||||
, test "should report tests that have the same body when using test and exposing (..)" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test exposing (..)
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors [ error "Test `bar` has the same body as test `foo`" ]
|
||||
, test "should report tests that have the same body when using Test.test and exposing (..)" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test exposing (..)
|
||||
tests =
|
||||
[ Test.test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, Test.test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors [ error "Test `bar` has the same body as test `foo`" ]
|
||||
, test "should report tests that have the same body when using Foo.test, aliasing as Foo and exposing (..)" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test as Foo exposing (..)
|
||||
tests =
|
||||
[ Foo.test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, Foo.test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors [ error "Test `bar` has the same body as test `foo`" ]
|
||||
, test "should report tests that have the same body when using Test.test without exposing anything" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test
|
||||
tests =
|
||||
[ Test.test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, Test.test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors [ error "Test `bar` has the same body as test `foo`" ]
|
||||
, test "should report tests that have the same body when using Foo.test aliasing as Foo and without exposing anything" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test as Foo
|
||||
tests =
|
||||
[ Foo.test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, Foo.test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors [ error "Test `bar` has the same body as test `foo`" ]
|
||||
, test "should not report tests that have the same body when using test and not exposing (test)" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test exposing (foo)
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors []
|
||||
, test "should not report tests that have the same body when using test without exposing anything" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test
|
||||
tests =
|
||||
[ test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors []
|
||||
, test "should not report tests that have the same body when using test and aliasing as Foo without exposing anything" <|
|
||||
\() ->
|
||||
testRule """
|
||||
import Test as Foo
|
||||
tests =
|
||||
[ Test.test "foo" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
, Test.test "bar" <|
|
||||
\\() -> 1 + 1
|
||||
|> Expect.equal 2
|
||||
]
|
||||
"""
|
||||
|> expectErrors []
|
||||
]
|
||||
|
||||
|
||||
all : Test
|
||||
all =
|
||||
describe "ElmTest.NoDuplicateTestBodies" tests
|
Loading…
Reference in New Issue
Block a user