jfmengels/elm-review
1
1
Fork 0
mirror of https://github.com/jfmengels/elm-review.git synced 2024-09-11 08:05:46 +03:00
Code Issues Packages Projects Releases Wiki Activity

Rewrite the Lint.Test module to have a nicer API and much better error messages

Browse Source
This commit is contained in:
Jeroen Engels 2019-06-27 22:25:45 +02:00
parent 08cf4213b3
commit 75e80d0926
3 changed files with 660 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/Lint/Test.elm
View File

@ -10,8 +10,9 @@ TODO Rework API, we can do something much nicer than this
-}
import Array exposing (Array)
import Elm.Syntax.Range exposing (Range)
import Expect
import Expect exposing (Expectation)
import Lint exposing (Severity(..), lintSource)
import Lint.Rule as Rule exposing (Error, Rule)
@ -53,7 +54,7 @@ run rule str =
|> Result.map (List.map (\( severity, { message, range } ) -> Rule.error message range))
expectErrors : List Error -> LintResult -> Expect.Expectation
expectErrors : List Error -> LintResult -> Expectation
expectErrors expectedErrors result =
case result of
Err errors ->
@ -63,7 +64,7 @@ expectErrors expectedErrors result =
Expect.equal expectedErrors errors
expectErrorsWithoutRange : List Error -> LintResult -> Expect.Expectation
expectErrorsWithoutRange : List Error -> LintResult -> Expectation
expectErrorsWithoutRange expectedErrors result =
case result of
Err errors ->

568
src/Lint/Test2.elm Normal file
View File

@ -0,0 +1,568 @@
module Lint.Test2 exposing
( LintResult, run
, ExpectedError, expectErrors, expectNoErrors, error, atExactly
)
{-| Module that helps you test your linting rules, using [`elm-test`](https://package.elm-lang.org/packages/elm-explorations/test/latest).
import Lint.Test2 exposing (LintResult)
import Test exposing (Test, describe, test)
import The.Rule.You.Want.To.Test exposing (rule)
testRule : String -> LintResult
testRule string =
Lint.Test2.run rule string
-- In this example, the rule we're testing is `NoDebug`
tests : Test
tests =
describe "NoDebug"
[ test "should not report calls to normal functions" <|
\() ->
testRule """module A exposing (..)
a = foo n"""
|> Lint.Test2.expectNoErrors
, test "should report Debug.log use" <|
\() ->
testRule """module A exposing (..)
a = Debug.log "some" "message\""""
|> Lint.Test2.expectErrors
[ Lint.Test2.error
{ message = "Forbidden use of Debug"
, under = "Debug.log"
}
]
]
# Running tests
@docs LintResult, run
# Making assertions
@docs ExpectedError, expectErrors, expectNoErrors, error, atExactly
TODO Add instructions and examples
# Tips on testing
## What should you test?
TODO Add helpful tips
-}
import Array exposing (Array)
import Elm.Syntax.Range exposing (Range)
import Expect exposing (Expectation)
import Lint exposing (Severity(..), lintSource)
import Lint.Rule as Rule exposing (Error, Rule)
import List.Extra
{-| The result of running a rule on a `String` containing source code.
-}
type LintResult
= ParseFailure
| SuccessfulRun CodeInspector (List Error)
type alias CodeInspector =
{ getCodeAtLocation : Range -> Maybe String
, checkIfLocationIsAmbiguous : Error -> String -> Expectation
}
{-| An expectation of an error. Use [`error`](#error) to create one.
-}
type ExpectedError
= ExpectedError
{ message : String
, under : Under
}
type Under
= Under String
| UnderExactly String Range
type SourceCode
= SourceCode String
{-| Run a `Rule` on a `String` containing source code. You can then use
[`expectNoErrors`](#expectNoErrors) or [`expectErrors`](#expectErrors) to assert
the errors reported by the rule.
The source code needs to be syntactically valid Elm code. If the code
can't be parsed, the test will fail regardless of the expectations you set on it.
Note that t be syntactically valid, you need at least a module declaration at the
top of the file (like `module A exposing (..)`) and one declaration (like `a = 1`).
You can't just have an expression like `1 + 2`.
-}
run : Rule -> String -> LintResult
run rule sourceCode =
case lintSource [ ( Critical, rule ) ] sourceCode of
Ok errors ->
SuccessfulRun
{ getCodeAtLocation = getCodeAtLocationInSourceCode (SourceCode sourceCode)
, checkIfLocationIsAmbiguous = checkIfLocationIsAmbiguousInSourceCode (SourceCode sourceCode)
}
(List.map (\( _, error_ ) -> Rule.error error_.message error_.range) errors)
Err _ ->
ParseFailure
{-| Assert that the rule reprted no errors. Note, this is equivalent to using [`expectErrors`](#expectErrors)
like `expectErrors []`.
import Lint.Test2 exposing (LintResult)
import Test exposing (Test, describe, test)
import The.Rule.You.Want.To.Test exposing (rule)
testRule : String -> LintResult
testRule string =
Lint.Test2.run rule string
-- In this example, the rule we're testing is `NoDebug`
tests : Test
tests =
describe "NoDebug"
[ test "should not report calls to normal functions" <|
\() ->
testRule """module A exposing (..)
a = foo n"""
|> Lint.Test2.expectNoErrors
]
-}
expectNoErrors : LintResult -> Expectation
expectNoErrors lintResult =
case lintResult of
ParseFailure ->
Expect.fail parsingErrorMessage
SuccessfulRun _ errors ->
Expect.true
("I expected no errors but found:\n\n" ++ (List.map errorToString errors |> String.join "\n"))
(List.isEmpty errors)
{-| Assert that the rule reprted some errors, by specifying which one.
Assert which errors are reported using [`error`](#error). The test will fail if
a different number of errors than expected are reported, or if the message or the
location is incorrect.
import Lint.Test2 exposing (LintResult)
import Test exposing (Test, describe, test)
import The.Rule.You.Want.To.Test exposing (rule)
testRule : String -> LintResult
testRule string =
Lint.Test2.run rule string
-- In this example, the rule we're testing is `NoDebug`
tests : Test
tests =
describe "NoDebug"
[ test "should report Debug.log use" <|
\() ->
testRule """module A exposing (..)
a = Debug.log "some" "message\""""
|> Lint.Test2.expectErrors
[ Lint.Test2.error
{ message = "Forbidden use of Debug"
, under = "Debug.log"
}
]
]
-}
expectErrors : List ExpectedError -> LintResult -> Expectation
expectErrors expectedErrors lintResult =
case lintResult of
ParseFailure ->
Expect.fail parsingErrorMessage
SuccessfulRun codeInspector errors ->
checkAllErrorsMatch codeInspector expectedErrors errors
{-| Create an expectation for an error.
`message` should be the message you're expecting to be shown to the user.
`under` is the part of the code where you are expecting the error to be shown to
the user. If it helps, imagine `under` to be the text under which the squiggly
lines will appear if the error appeared in an editor.
tests : Test
tests =
describe "NoDebug"
[ test "should report Debug.log use" <|
\() ->
testRule """module A exposing (..)
a = Debug.log "some" "message\""""
|> Lint.Test2.expectErrors
[ Lint.Test2.error
{ message = "Forbidden use of Debug"
, under = "Debug.log"
}
]
]
If there are multiple locations where the value of `under` appears, the test will
fail unless you use [`atExactly`](#atExactly) to remove any ambiguity of where the
error should be used.
-}
error : { message : String, under : String } -> ExpectedError
error input =
ExpectedError
{ message = input.message
, under = Under input.under
}
getUnder : ExpectedError -> String
getUnder (ExpectedError expectedError) =
case expectedError.under of
Under str ->
str
UnderExactly str _ ->
str
{-| Precise the exact position where the error should be shown to the user. This
is only necessary when the `under` field is ambiguous.
`atExactly` takes a record with start and end positions.
tests : Test
tests =
describe "NoDebug"
[ test "should report multiple Debug.log calls" <|
\() ->
testRule """
a = Debug.log z
b = Debug.log z
"""
|> Lint.Test2.expectErrors
[ Lint.Test2.error
{ message = message
, under = "Debug.log"
}
|> Lint.Test2.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 14 } }
, Lint.Test2.error
{ message = message
, under = "Debug.log"
}
|> Lint.Test2.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 14 } }
]
]
Tip: By default, do not provide this field. If the test fails because there is some
ambiguity, the test error will give you a recommendation of what to use as a parameter
of `atExactly`, so you do not have to bother writing this hard to write argument.
-}
atExactly : { start : { row : Int, column : Int }, end : { row : Int, column : Int } } -> ExpectedError -> ExpectedError
atExactly range ((ExpectedError expectedError_) as expectedError) =
ExpectedError { expectedError_ | under = UnderExactly (getUnder expectedError) range }
checkAllErrorsMatch : CodeInspector -> List ExpectedError -> List Error -> Expectation
checkAllErrorsMatch codeInspector expectedErrors errors =
checkErrorsMatch codeInspector expectedErrors errors
|> List.reverse
|> (\expectations -> Expect.all expectations ())
checkErrorsMatch : CodeInspector -> List ExpectedError -> List Error -> List (() -> Expectation)
checkErrorsMatch codeInspector expectedErrors errors =
case ( expectedErrors, errors ) of
( [], [] ) ->
[ always Expect.pass ]
( expected :: restOfExpectedErrors, error_ :: restOfErrors ) ->
checkErrorMatch codeInspector expected error_ :: checkErrorsMatch codeInspector restOfExpectedErrors restOfErrors
( expected :: restOfExpectedErrors, [] ) ->
[ always <| Expect.fail <| notEnoughErrors expected restOfExpectedErrors ]
( [], error_ :: restOfErrors ) ->
[ always <| Expect.fail <| tooManyErrors error_ restOfErrors ]
checkErrorMatch : CodeInspector -> ExpectedError -> Error -> (() -> Expectation)
checkErrorMatch codeInspector ((ExpectedError expectedError_) as expectedError) error_ =
Expect.all
[ always <| Expect.true (messageMismatchError expectedError error_) (expectedError_.message == Rule.errorMessage error_)
, checkMessageAppearsUnder codeInspector error_ expectedError
]
checkMessageAppearsUnder : CodeInspector -> Error -> ExpectedError -> (() -> Expectation)
checkMessageAppearsUnder codeInspector error_ (ExpectedError expectedError) =
case codeInspector.getCodeAtLocation (Rule.errorRange error_) of
Just codeAtLocation ->
case expectedError.under of
Under under ->
Expect.all
[ always <| Expect.true (underMismatchError error_ under codeAtLocation) (codeAtLocation == under)
, always <| codeInspector.checkIfLocationIsAmbiguous error_ under
]
UnderExactly under range ->
Expect.all
[ always <| Expect.true (underMismatchError error_ under codeAtLocation) (codeAtLocation == under)
, always <| Expect.true (wrongLocationError error_ range under) (Rule.errorRange error_ == range)
]
Nothing ->
always <| Expect.fail impossibleStateError
getCodeAtLocationInSourceCode : SourceCode -> Range -> Maybe String
getCodeAtLocationInSourceCode (SourceCode sourceCode) =
let
lines : Array String
lines =
String.lines sourceCode
|> Array.fromList
in
\{ start, end } ->
if start.row == end.row then
Array.get (start.row - 1) lines
|> Maybe.map (String.slice (start.column - 1) (end.column - 1))
else
let
firstLine : Maybe String
firstLine =
Array.get (start.row - 1) lines
|> Maybe.map (String.dropLeft (start.column - 1))
lastLine : Maybe String
lastLine =
Array.get (end.row - 1) lines
|> Maybe.map (String.dropRight end.column)
in
[ [ firstLine ]
, Array.slice start.row (end.row - 1) lines
|> Array.toList
|> List.map Just
, [ lastLine ]
]
|> List.concat
|> List.filterMap identity
|> String.join "\n"
|> Just
formatSourceCode : String -> String
formatSourceCode string =
let
lines =
String.lines string
in
if List.length lines == 1 then
"`" ++ string ++ "`"
else
lines
|> List.map (\str -> " " ++ str)
|> String.join "\n"
|> (\str -> "\n\n```\n" ++ str ++ "\n```")
checkIfLocationIsAmbiguousInSourceCode : SourceCode -> Error -> String -> Expectation
checkIfLocationIsAmbiguousInSourceCode ((SourceCode sourceCodeContent) as sourceCode) error_ under =
let
occurrencesInSourceCode : List Int
occurrencesInSourceCode =
String.indexes under sourceCodeContent
in
Expect.true
(locationIsAmbiguousInSourceCodeError sourceCode error_ under occurrencesInSourceCode)
(List.length occurrencesInSourceCode == 1)
-- ERROR MESSAGES
parsingErrorMessage : String
parsingErrorMessage =
"""I could not parse the test source code, because it was not syntactically valid Elm code.
Maybe you forgot to add the module definition at the top, like:
module A exposing (..)"""
messageMismatchError : ExpectedError -> Error -> String
messageMismatchError (ExpectedError expectedError) error_ =
"""I was looking for the error with the following message:
`""" ++ expectedError.message ++ """`
but I found the following error message:
`""" ++ Rule.errorMessage error_ ++ "`"
wrongLocationError : Error -> Range -> String -> String
wrongLocationError error_ range under =
"""I was looking for the error with the following message:
`""" ++ Rule.errorMessage error_ ++ """`
under the following code:
""" ++ formatSourceCode under ++ """
and I found it. But there are multiple occurrences for this piece of code, and the exact location you specified is not the one I found. I was expecting the error at:
""" ++ rangeAsString range ++ """
but I found it at:
""" ++ rangeAsString (Rule.errorRange error_)
underMismatchError : Error -> String -> String -> String
underMismatchError error_ under codeAtLocation =
"""I found an error with the right message, but at the wrong location:
Message: `""" ++ Rule.errorMessage error_ ++ """`
I saw it under: """ ++ formatSourceCode codeAtLocation ++ """
But I expected to see it under: """ ++ formatSourceCode under
listOccurrencesAsLocations : SourceCode -> String -> List Int -> String
listOccurrencesAsLocations sourceCode under occurrences =
occurrences
|> List.map
(\occurrence ->
occurrence
|> positionAsRange sourceCode under
|> rangeAsString
|> (++) " - "
)
|> String.join "\n"
positionAsRange : SourceCode -> String -> Int -> Range
positionAsRange (SourceCode sourceCode) under position =
let
linesBeforeAndIncludingPosition : List String
linesBeforeAndIncludingPosition =
sourceCode
|> String.slice 0 position
|> String.lines
startRow : Int
startRow =
List.length linesBeforeAndIncludingPosition
in
-- TODO Get these values right
{ start =
{ row = startRow
, column = -1
}
, end =
{ row = startRow + (under |> String.indexes "\n" |> List.length)
, column = under |> String.indexes "\n" |> List.length
}
}
rangeAsString : Range -> String
rangeAsString { start, end } =
"{ start = { row = " ++ String.fromInt start.row ++ ", column = " ++ String.fromInt start.column ++ " }, end = { row = " ++ String.fromInt end.row ++ ", column = " ++ String.fromInt end.column ++ " } }"
errorToString : Error -> String
errorToString error_ =
"- \"" ++ Rule.errorMessage error_ ++ "\" at " ++ Debug.toString (Rule.errorRange error_)
notEnoughErrors : ExpectedError -> List ExpectedError -> String
notEnoughErrors expected restOfExpectedErrors =
let
numberOfErrors : Int
numberOfErrors =
List.length restOfExpectedErrors + 1
in
"I expected to see "
++ String.fromInt numberOfErrors
++ " more "
++ pluralizeErrors numberOfErrors
++ ":\n"
++ (List.map expectedErrorToString (expected :: restOfExpectedErrors) |> String.join "\n")
tooManyErrors : Error -> List Error -> String
tooManyErrors error_ restOfErrors =
let
numberOfErrors : Int
numberOfErrors =
List.length restOfErrors + 1
in
"I found "
++ String.fromInt numberOfErrors
++ " "
++ pluralizeErrors numberOfErrors
++ " too many:\n"
++ (List.map errorToString (error_ :: restOfErrors) |> String.join "\n")
locationIsAmbiguousInSourceCodeError : SourceCode -> Error -> String -> List Int -> String
locationIsAmbiguousInSourceCodeError sourceCode error_ under occurrencesInSourceCode =
"""Your test passes, but where the message appears is ambiguous.
You are looking for the following error message:
`""" ++ Rule.errorMessage error_ ++ """`
and expecting to see it under:
""" ++ formatSourceCode under ++ """
I found """ ++ String.fromInt (List.length occurrencesInSourceCode) ++ """ locations where that code appeared. Please use `Lint.Rule.atExactly` to make the part you were targetting unambiguous.
Tip: I found them at:
""" ++ listOccurrencesAsLocations sourceCode under occurrencesInSourceCode
impossibleStateError : String
impossibleStateError =
"Oh no! I'm in an impossible state. I found an error at a location that I could not find back. Please let me know and give me an SSCCE (http://sscce.org/) here: https://github.com/jfmengels/elm-lint/issues."
pluralizeErrors : Int -> String
pluralizeErrors n =
case n of
1 ->
"error"
_ ->
"errors"
expectedErrorToString : ExpectedError -> String
expectedErrorToString (ExpectedError expectedError) =
"- " ++ expectedError.message

89
tests/NoImportingEverythingTest.elm
View File

@ -4,6 +4,7 @@ import Elm.Syntax.Range exposing (Location, Range)
import Lint.Rule as Rule exposing (Error, Rule)
import Lint.Rule.NoImportingEverything exposing (Configuration, rule)
import Lint.Test exposing (LintResult)
import Lint.Test2
import Test exposing (Test, describe, test)
@ -95,6 +96,92 @@ import Html exposing (..)
]
testRule2 : Configuration -> String -> Lint.Test2.LintResult
testRule2 options =
Lint.Test2.run (rule options)
tests2 : List Test
tests2 =
[ test "A-should not report imports that do not expose anything" <|
\() ->
"""module A exposing (..)
import Html
import Http
"""
|> testRule2 { exceptions = [] }
|> Lint.Test2.expectNoErrors
, test "A-should not report imports that expose functions by name" <|
\() ->
"""module A exposing (..)
import Html exposing (a)
import Http exposing (a, b)"""
|> testRule2 { exceptions = [] }
|> Lint.Test2.expectNoErrors
, test "A-should report imports that expose everything" <|
\() ->
"""module A exposing (..)
import Html exposing
(..)"""
|> testRule2 { exceptions = [] }
|> Lint.Test2.expectErrors
[ Lint.Test2.error "Do not expose everything from Html"
|> Lint.Test2.under "(..)"
]
-- [ error "Do not expose everything from Html" ]
-- , test "A-should report imports from sub-modules" <|
-- \() ->
-- """module A exposing (..)
-- import Html.App exposing (..)
-- """
-- |> testRule { exceptions = [] }
-- |> Lint.Test.expectErrorsWithoutRange [ error "Do not expose everything from Html.App" ]
-- , test "A-should report imports from sub-modules (multiple dots)" <|
-- \() ->
-- """module A exposing (..)
-- import Html.Foo.Bar exposing (..)
-- """
-- |> testRule { exceptions = [] }
-- |> Lint.Test.expectErrorsWithoutRange [ error "Do not expose everything from Html.Foo.Bar" ]
-- , test "A-should not report imports that expose everything that are in the exception list" <|
-- \() ->
-- """module A exposing (..)
-- import Html exposing (..)
-- """
-- |> testRule { exceptions = [ "Html" ] }
-- |> Lint.Test.expectErrorsWithoutRange []
-- , test "A-should not report imports from sub-modules that are in the exception list" <|
-- \() ->
-- """module A exposing (..)
-- import Html.App exposing (..)
-- """
-- |> testRule { exceptions = [ "Html.App" ] }
-- |> Lint.Test.expectErrorsWithoutRange []
-- , test "A-should not report imports from sub-modules (multiple dots)" <|
-- \() ->
-- """module A exposing (..)
-- import Html.Foo.Bar exposing (..)
-- """
-- |> testRule { exceptions = [ "Html.Foo.Bar" ] }
-- |> Lint.Test.expectErrorsWithoutRange []
-- , test "A-should report imports whose parent is ignored" <|
-- \() ->
-- """module A exposing (..)
-- import Html.Foo.Bar exposing (..)
-- """
-- |> testRule { exceptions = [ "Html" ] }
-- |> Lint.Test.expectErrorsWithoutRange [ error "Do not expose everything from Html.Foo.Bar" ]
-- , test "A-should report imports whose sub-module is ignored" <|
-- \() ->
-- """module A exposing (..)
-- import Html exposing (..)
-- """
-- |> testRule { exceptions = [ "Html.App" ] }
-- |> Lint.Test.expectErrorsWithoutRange [ error "Do not expose everything from Html" ]
]
all : Test
all =
describe "NoImportingEverything" tests
describe "NoImportingEverything" (tests ++ tests2)