Replace expectErrorsForFiles by expectErrorsForModules

src/ListExtra.elm

@@ -1,4 +1,4 @@
-module ListExtra exposing (last, uniquePairs)
+module ListExtra exposing (find, last, uniquePairs)
 
 {-| Functions taken from elm-community/list-extra.
 
@@ -8,7 +8,7 @@ elm-community/list-extra would release a new major version.
 
 # List functions
 
-@docs last, uniquePairs
+@docs find, last, uniquePairs
 
 
 # Original Copyright notice
@@ -38,6 +38,24 @@ SOFTWARE.
 -}
 
 
+{-| Find the first element that satisfies a predicate and return
+Just that element. If none match, return Nothing.
+find (\\num -> num > 5) [2, 4, 6, 8] == Just 6
+-}
+find : (a -> Bool) -> List a -> Maybe a
+find predicate list =
+    case list of
+        [] ->
+            Nothing
+
+        first :: rest ->
+            if predicate first then
+                Just first
+
+            else
+                find predicate rest
+
+
 last : List a -> Maybe a
 last items =
     case items of

src/Review/Test.elm

@@ -1,6 +1,6 @@
 module Review.Test exposing
     ( ReviewResult, run, runWithProjectData, runMulti, runMultiWithProjectData
-    , ExpectedError, expectErrors, expectErrorsForFiles, expectNoErrors, error, atExactly, whenFixed
+    , ExpectedError, expectErrors, expectErrorsForModules, expectNoErrors, error, atExactly, whenFixed
     )
 
 {-| Module that helps you test your rules, using [`elm-test`](https://package.elm-lang.org/packages/elm-explorations/test/latest/).
@@ -100,7 +100,7 @@ for this module.
 
 # Making assertions
 
-@docs ExpectedError, expectErrors, expectErrorsForFiles, expectNoErrors, error, atExactly, whenFixed
+@docs ExpectedError, expectErrors, expectErrorsForModules, expectNoErrors, error, atExactly, whenFixed
 
 -}
 
@@ -109,6 +109,7 @@ import Elm.Syntax.Module as Module
 import Elm.Syntax.Node as Node
 import Elm.Syntax.Range exposing (Range)
 import Expect exposing (Expectation)
+import ListExtra
 import Review
 import Review.File as File
 import Review.Fix as Fix
@@ -126,7 +127,13 @@ import Set exposing (Set)
 -}
 type ReviewResult
     = FailedRun String
-    | SuccessfulRun (List { inspector : CodeInspector, errors : List Error })
+    | SuccessfulRun
+        (List
+            { moduleName : String
+            , inspector : CodeInspector
+            , errors : List Error
+            }
+        )
 
 
 type alias CodeInspector =
@@ -261,7 +268,12 @@ runMultiWithProjectData project rule sources =
                     in
                     List.map
                         (\parsedFile ->
-                            { inspector = codeInspectorForSource parsedFile
+                            { moduleName =
+                                parsedFile.ast.moduleDefinition
+                                    |> Node.value
+                                    |> Module.moduleName
+                                    |> String.join "."
+                            , inspector = codeInspectorForSource parsedFile
                             , errors =
                                 errors
                                     |> List.filter (\error_ -> Rule.errorFilePath error_ == parsedFile.path)
@@ -438,29 +450,40 @@ an error at the end of the source code.
 -}
 expectErrors : List ExpectedError -> ReviewResult -> Expectation
 expectErrors expectedErrors reviewResult =
-    expectErrorsForFiles [ expectedErrors ] reviewResult
+    case reviewResult of
+        FailedRun errorMessage ->
+            Expect.fail errorMessage
+
+        SuccessfulRun ({ inspector, errors, moduleName } :: []) ->
+            checkAllErrorsMatch inspector expectedErrors errors
+
+        SuccessfulRun _ ->
+            Expect.fail ErrorMessage.needToUsedExpectErrorsForModules
 
 
 {-| TODO Documentation
 -}
-expectErrorsForFiles : List (List ExpectedError) -> ReviewResult -> Expectation
-expectErrorsForFiles expectedErrorsList reviewResult =
+expectErrorsForModules : List ( String, List ExpectedError ) -> ReviewResult -> Expectation
+expectErrorsForModules expectedErrorsList reviewResult =
     case reviewResult of
         FailedRun errorMessage ->
             Expect.fail errorMessage
 
         SuccessfulRun runResults ->
-            if List.length runResults /= List.length expectedErrorsList then
-                Expect.fail <| ErrorMessage.errorListLengthMismatch (List.length runResults) (List.length expectedErrorsList)
-
-            else
-                List.map2
-                    (\{ inspector, errors } expectedErrors () ->
-                        checkAllErrorsMatch inspector expectedErrors errors
+            runResults
+                |> List.map
+                    (\{ inspector, errors, moduleName } ->
+                        let
+                            expectedErrors : List ExpectedError
+                            expectedErrors =
+                                expectedErrorsList
+                                    |> ListExtra.find (\( moduleName_, _ ) -> moduleName_ == moduleName)
+                                    |> Maybe.map Tuple.second
+                                    |> Maybe.withDefault []
+                        in
+                        \() -> checkAllErrorsMatch inspector expectedErrors errors
                     )
-                    runResults
-                    expectedErrorsList
-                    |> (\expectations -> Expect.all expectations ())
+                |> (\expectations -> Expect.all expectations ())
 
 
 {-| Create an expectation for an error.

src/Review/Test/ErrorMessage.elm

@@ -2,7 +2,7 @@ module Review.Test.ErrorMessage exposing
     ( ExpectedErrorData
     , parsingFailure, messageMismatch, emptyDetails, unexpectedDetails, wrongLocation, didNotExpectErrors
     , underMismatch, expectedMoreErrors, tooManyErrors, locationIsAmbiguousInSourceCode
-    , errorListLengthMismatch, duplicateModuleName
+    , needToUsedExpectErrorsForModules, duplicateModuleName
     , missingFixes, unexpectedFixes, fixedCodeMismatch, unchangedSourceAfterFix, invalidSourceAfterFix, hasCollisionsInFixRanges
     , impossibleState
     )
@@ -15,7 +15,7 @@ module Review.Test.ErrorMessage exposing
 @docs ExpectedErrorData
 @docs parsingFailure, messageMismatch, emptyDetails, unexpectedDetails, wrongLocation, didNotExpectErrors
 @docs underMismatch, expectedMoreErrors, tooManyErrors, locationIsAmbiguousInSourceCode
-@docs errorListLengthMismatch, duplicateModuleName
+@docs needToUsedExpectErrorsForModules, duplicateModuleName
 @docs missingFixes, unexpectedFixes, fixedCodeMismatch, unchangedSourceAfterFix, invalidSourceAfterFix, hasCollisionsInFixRanges
 @docs impossibleState
 
@@ -239,26 +239,27 @@ Tip: I found them at:
 """ ++ listOccurrencesAsLocations sourceCode under occurrencesInSourceCode
 
 
-errorListLengthMismatch : Int -> Int -> String
-errorListLengthMismatch expectedListLength actualListLength =
-    """MISMATCH BETWEEN NUMBER OF MODULES AND NUMBER OF LISTS OF ERRORS
+needToUsedExpectErrorsForModules : String
+needToUsedExpectErrorsForModules =
+    """AMBIGUOUS MODULE FOR ERROR
 
-You passed a list of """ ++ String.fromInt expectedListLength ++ """ modules to this test, but a list of """ ++ String.fromInt actualListLength ++ """ lists
-of errors.
+You gave me several modules, and you expect some errors. I need to know for
+which module you expect these errors to be reported.
 
-I expect each item in the list of expected errors to correspond to the
-module at the same position in the module list. Since the two lists have
-different sizes, I'm not sure how to associate the last modules or errors.
-
-If you expect no errors to be reported for a module, use an empty list:
+You should use `expectErrorsForModules` to do this:
 
   test "..." <|
     \\() ->
-      [ sourceCode1, sourceCode2 ]
+      [ \"\"\"
+module A exposing (..)
+-- someCode
+\"\"\", \"\"\"
+module B exposing (..)
+-- someCode
+\"\"\" ]
       |> Review.Test.runMulti rule
-      |> Review.Test.expectErrorsForFiles
-          [ [] -- Expect no errors reported in `sourceCode1`
-          , [ Review.Test.error theErrorForSourceCode2 ]
+      |> Review.Test.expectErrorsForModules
+          [ ( "B", [ Review.Test.error someError ] )
           ]"""
 
 

tests/ErrorMessageTest.elm

@@ -21,7 +21,7 @@ all =
         , expectedMoreErrorsTest
         , tooManyErrorsTest
         , locationIsAmbiguousInSourceCodeTest
-        , errorListLengthMismatchTest
+        , needToUsedExpectErrorsForModulesTest
         , duplicateModuleNameTest
         , missingFixesTest
         , unexpectedFixesTest
@@ -607,32 +607,32 @@ Tip: I found them at:
         ]
 
 
-errorListLengthMismatchTest : Test
-errorListLengthMismatchTest =
-    test "errorListLengthMismatch" <|
+needToUsedExpectErrorsForModulesTest : Test
+needToUsedExpectErrorsForModulesTest =
+    test "needToUsedExpectErrorsForModules" <|
         \() ->
-            ErrorMessage.errorListLengthMismatch 1417 1418
+            ErrorMessage.needToUsedExpectErrorsForModules
                 |> expectMessageEqual """
-MISMATCH BETWEEN NUMBER OF MODULES AND NUMBER OF LISTS OF ERRORS
+AMBIGUOUS MODULE FOR ERROR
 
-You passed a list of 1417 modules to this test, but a list of 1418 lists
-of errors.
+You gave me several modules, and you expect some errors. I need to know for
+which module you expect these errors to be reported.
 
-I expect each item in the list of expected errors to correspond to the
-module at the same position in the module list. Since the two lists have
-different sizes, I'm not sure how to associate the last modules or errors.
-
-If you expect no errors to be reported for a module, use an empty list:
+You should use `expectErrorsForModules` to do this:
 
   test "..." <|
     \\() ->
-      [ sourceCode1, sourceCode2 ]
+      [ \"\"\"
+module A exposing (..)
+-- someCode
+\"\"\", \"\"\"
+module B exposing (..)
+-- someCode
+\"\"\" ]
       |> Review.Test.runMulti rule
-      |> Review.Test.expectErrorsForFiles
-          [ [] -- Expect no errors reported in `sourceCode1`
-          , [ Review.Test.error theErrorForSourceCode2 ]
-          ]
-"""
+      |> Review.Test.expectErrorsForModules
+          [ ( "B", [ Review.Test.error someError ] )
+          ]"""
 
 
 duplicateModuleNameTest : Test

tests/NoUnusedModulesTest.elm

@@ -89,17 +89,30 @@ main = text ""
             , """
 module Reported exposing (..)
 a = 1
+"""
+            , """
+module Other.Reported exposing (..)
+a = 1
 """
             ]
                 |> Review.Test.runMultiWithProjectData application rule
-                |> Review.Test.expectErrorsForFiles
-                    [ []
-                    , [ Review.Test.error
+                |> Review.Test.expectErrorsForModules
+                    [ ( "Reported"
+                      , [ Review.Test.error
                             { message = "Module `Reported` is never used."
                             , details = details
                             , under = "Reported"
                             }
-                      ]
+                        ]
+                      )
+                    , ( "Other.Reported"
+                      , [ Review.Test.error
+                            { message = "Module `Other.Reported` is never used."
+                            , details = details
+                            , under = "Other.Reported"
+                            }
+                        ]
+                      )
                     ]
     , test "should report a module even if it is the only module in the project" <|
         \() ->