Fail tests when there are duplicate module names

src/Review/Test.elm

@@ -105,6 +105,8 @@ for this module.
 -}
 
 import Array exposing (Array)
+import Elm.Syntax.Module as Module
+import Elm.Syntax.Node as Node
 import Elm.Syntax.Range exposing (Range)
 import Expect exposing (Expectation)
 import Review
@@ -113,6 +115,7 @@ import Review.Fix as Fix
 import Review.Project as Project exposing (Project)
 import Review.Rule as Rule exposing (Error, Rule)
 import Review.Test.ErrorMessage as ErrorMessage
+import Set exposing (Set)
 
 
 
@@ -122,7 +125,7 @@ import Review.Test.ErrorMessage as ErrorMessage
 {-| The result of running a rule on a `String` containing source code.
 -}
 type ReviewResult
-    = ParseFailure
+    = FailedRun String
     | SuccessfulRun (List { inspector : CodeInspector, errors : List Error })
 
 
@@ -245,28 +248,53 @@ runMultiWithProjectData : Project -> Rule -> List String -> ReviewResult
 runMultiWithProjectData project rule sources =
     case parseSources sources of
         Ok parsedFiles ->
-            let
-                errors : List Error
-                errors =
-                    Review.reviewFiles [ rule ] project parsedFiles
-                        |> Tuple.first
-            in
-            -- TODO Fail if modules have the same module name
-            List.map
-                (\parsedFile ->
-                    { inspector = codeInspectorForSource parsedFile
-                    , errors =
-                        errors
-                            |> List.filter (\error_ -> Rule.errorFilePath error_ == parsedFile.path)
-                            |> List.sortWith compareErrorPositions
-                    }
-                )
-                parsedFiles
-                |> SuccessfulRun
+            case findDuplicateModuleNames Set.empty parsedFiles of
+                Just moduleName ->
+                    FailedRun <| ErrorMessage.duplicateModuleName moduleName
 
-        Err parsingError ->
+                Nothing ->
+                    let
+                        errors : List Error
+                        errors =
+                            Review.reviewFiles [ rule ] project parsedFiles
+                                |> Tuple.first
+                    in
+                    List.map
+                        (\parsedFile ->
+                            { inspector = codeInspectorForSource parsedFile
+                            , errors =
+                                errors
+                                    |> List.filter (\error_ -> Rule.errorFilePath error_ == parsedFile.path)
+                                    |> List.sortWith compareErrorPositions
+                            }
+                        )
+                        parsedFiles
+                        |> SuccessfulRun
+
+        Err _ ->
             -- TODO Explain in which file there was a parsing error
-            ParseFailure
+            FailedRun ErrorMessage.parsingFailure
+
+
+findDuplicateModuleNames : Set (List String) -> List File.ParsedFile -> Maybe (List String)
+findDuplicateModuleNames previousModuleNames parsedFiles =
+    case parsedFiles of
+        [] ->
+            Nothing
+
+        file :: restOfFiles ->
+            let
+                moduleName : List String
+                moduleName =
+                    file.ast.moduleDefinition
+                        |> Node.value
+                        |> Module.moduleName
+            in
+            if Set.member moduleName previousModuleNames then
+                Just moduleName
+
+            else
+                findDuplicateModuleNames (Set.insert moduleName previousModuleNames) restOfFiles
 
 
 parseSources : List String -> Result Error (List File.ParsedFile)
@@ -360,8 +388,8 @@ like `expectErrors []`.
 expectNoErrors : ReviewResult -> Expectation
 expectNoErrors reviewResult =
     case reviewResult of
-        ParseFailure ->
-            Expect.fail ErrorMessage.parsingFailure
+        FailedRun errorMessage ->
+            Expect.fail errorMessage
 
         SuccessfulRun runResults ->
             let
@@ -418,15 +446,14 @@ expectErrors expectedErrors reviewResult =
 expectErrorsForFiles : List (List ExpectedError) -> ReviewResult -> Expectation
 expectErrorsForFiles expectedErrorsList reviewResult =
     case reviewResult of
-        ParseFailure ->
-            Expect.fail ErrorMessage.parsingFailure
+        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
-                -- TODO Add expectation that we have as many elements in expectedErrorsList as runResults
                 List.map2
                     (\{ inspector, errors } expectedErrors () ->
                         checkAllErrorsMatch inspector expectedErrors errors

src/Review/Test/ErrorMessage.elm

@@ -1,7 +1,8 @@
 module Review.Test.ErrorMessage exposing
     ( ExpectedErrorData
     , parsingFailure, messageMismatch, emptyDetails, unexpectedDetails, wrongLocation, didNotExpectErrors
-    , underMismatch, expectedMoreErrors, tooManyErrors, locationIsAmbiguousInSourceCode, errorListLengthMismatch
+    , underMismatch, expectedMoreErrors, tooManyErrors, locationIsAmbiguousInSourceCode
+    , errorListLengthMismatch, duplicateModuleName
     , missingFixes, unexpectedFixes, fixedCodeMismatch, unchangedSourceAfterFix, invalidSourceAfterFix, hasCollisionsInFixRanges
     , impossibleState
     )
@@ -13,7 +14,8 @@ module Review.Test.ErrorMessage exposing
 
 @docs ExpectedErrorData
 @docs parsingFailure, messageMismatch, emptyDetails, unexpectedDetails, wrongLocation, didNotExpectErrors
-@docs underMismatch, expectedMoreErrors, tooManyErrors, locationIsAmbiguousInSourceCode, errorListLengthMismatch
+@docs underMismatch, expectedMoreErrors, tooManyErrors, locationIsAmbiguousInSourceCode
+@docs errorListLengthMismatch, duplicateModuleName
 @docs missingFixes, unexpectedFixes, fixedCodeMismatch, unchangedSourceAfterFix, invalidSourceAfterFix, hasCollisionsInFixRanges
 @docs impossibleState
 
@@ -238,6 +240,19 @@ If you expect no errors to be reported for a module, use an empty list:
           ]"""
 
 
+duplicateModuleName : List String -> String
+duplicateModuleName moduleName =
+    """DUPLICATE MODULE NAMES
+
+I found several modules named `""" ++ String.join "." moduleName ++ """` in the test source codes.
+
+I expect all modules to be able to exist together in the same project,
+but having several modules with the same name is not allowed by the Elm
+compiler.
+
+Please rename the modules so that they all have different names."""
+
+
 impossibleState : String
 impossibleState =
     """ELM-REVIEW IMPOSSIBLE STATE

tests/ErrorMessageTest.elm

@@ -22,6 +22,7 @@ all =
         , tooManyErrorsTest
         , locationIsAmbiguousInSourceCodeTest
         , errorListLengthMismatchTest
+        , duplicateModuleNameTest
         , missingFixesTest
         , unexpectedFixesTest
         , fixedCodeMismatchTest
@@ -616,6 +617,24 @@ If you expect no errors to be reported for a module, use an empty list:
 """
 
 
+duplicateModuleNameTest : Test
+duplicateModuleNameTest =
+    test "duplicateModuleName" <|
+        \() ->
+            ErrorMessage.duplicateModuleName [ "My", "Module" ]
+                |> expectMessageEqual """
+DUPLICATE MODULE NAMES
+
+I found several modules named `My.Module` in the test source codes.
+
+I expect all modules to be able to exist together in the same project,
+but having several modules with the same name is not allowed by the Elm
+compiler.
+
+Please rename the modules so that they all have different names.
+"""
+
+
 missingFixesTest : Test
 missingFixesTest =
     test "missingFixes" <|