From f28b7698d7a983c7b95291ca6d6ac017b4000ddf Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Tue, 11 Feb 2020 18:47:28 +0100 Subject: [PATCH] Give a nicer error message when under is passed empty --- src/Review/Test.elm | 54 +++++++++++++++++++------------- src/Review/Test/ErrorMessage.elm | 21 +++++++++++-- tests/ErrorMessageTest.elm | 26 +++++++++++++++ 3 files changed, 77 insertions(+), 24 deletions(-) diff --git a/src/Review/Test.elm b/src/Review/Test.elm index f9850348..c14b2b25 100644 --- a/src/Review/Test.elm +++ b/src/Review/Test.elm @@ -841,16 +841,16 @@ checkErrorsMatch runResult expectedErrors errors = checkErrorMatch : CodeInspector -> ExpectedError -> Error -> (() -> Expectation) checkErrorMatch codeInspector ((ExpectedError expectedError_) as expectedError) error_ = Expect.all - [ (expectedError_.message == Rule.errorMessage error_) - |> Expect.true - (ErrorMessage.messageMismatch - (extractExpectedErrorData expectedError) - error_ - ) - |> always + [ \() -> + (expectedError_.message == Rule.errorMessage error_) + |> Expect.true + (ErrorMessage.messageMismatch + (extractExpectedErrorData expectedError) + error_ + ) , checkMessageAppearsUnder codeInspector error_ expectedError , checkDetailsAreCorrect error_ expectedError - , always <| checkFixesAreCorrect codeInspector error_ expectedError + , \() -> checkFixesAreCorrect codeInspector error_ expectedError ] @@ -861,27 +861,37 @@ checkMessageAppearsUnder codeInspector error_ (ExpectedError expectedError) = case expectedError.under of Under under -> Expect.all - [ (codeAtLocation == under) - |> Expect.true (ErrorMessage.underMismatch error_ { under = under, codeAtLocation = codeAtLocation }) - |> always - , codeInspector.checkIfLocationIsAmbiguous error_ under - |> always + [ \() -> + case under of + "" -> + ErrorMessage.underMayNotBeEmpty + { message = expectedError.message + , codeAtLocation = codeAtLocation + } + |> Expect.fail + + _ -> + Expect.pass + , \() -> + (codeAtLocation == under) + |> Expect.true (ErrorMessage.underMismatch error_ { under = under, codeAtLocation = codeAtLocation }) + , \() -> codeInspector.checkIfLocationIsAmbiguous error_ under ] UnderExactly under range -> Expect.all - [ (codeAtLocation == under) - |> Expect.true (ErrorMessage.underMismatch error_ { under = under, codeAtLocation = codeAtLocation }) - |> always - , (Rule.errorRange error_ == range) - |> Expect.true (ErrorMessage.wrongLocation error_ range under) - |> always + [ \() -> + (codeAtLocation == under) + |> Expect.true (ErrorMessage.underMismatch error_ { under = under, codeAtLocation = codeAtLocation }) + , \() -> + (Rule.errorRange error_ == range) + |> Expect.true (ErrorMessage.wrongLocation error_ range under) ] Nothing -> - ErrorMessage.locationNotFound error_ - |> Expect.fail - |> always + \() -> + ErrorMessage.locationNotFound error_ + |> Expect.fail checkDetailsAreCorrect : Error -> ExpectedError -> (() -> Expectation) diff --git a/src/Review/Test/ErrorMessage.elm b/src/Review/Test/ErrorMessage.elm index 25d700fa..4bf61032 100644 --- a/src/Review/Test/ErrorMessage.elm +++ b/src/Review/Test/ErrorMessage.elm @@ -1,7 +1,7 @@ module Review.Test.ErrorMessage exposing ( ExpectedErrorData , parsingFailure, messageMismatch, emptyDetails, unexpectedDetails, wrongLocation, didNotExpectErrors - , underMismatch, expectedMoreErrors, tooManyErrors, locationNotFound, locationIsAmbiguousInSourceCode + , underMismatch, expectedMoreErrors, tooManyErrors, locationNotFound, underMayNotBeEmpty, locationIsAmbiguousInSourceCode , needToUsedExpectErrorsForModules, duplicateModuleName, unknownModulesInExpectedErrors , missingFixes, unexpectedFixes, fixedCodeMismatch, unchangedSourceAfterFix, invalidSourceAfterFix, hasCollisionsInFixRanges ) @@ -13,7 +13,7 @@ module Review.Test.ErrorMessage exposing @docs ExpectedErrorData @docs parsingFailure, messageMismatch, emptyDetails, unexpectedDetails, wrongLocation, didNotExpectErrors -@docs underMismatch, expectedMoreErrors, tooManyErrors, locationNotFound, locationIsAmbiguousInSourceCode +@docs underMismatch, expectedMoreErrors, tooManyErrors, locationNotFound, underMayNotBeEmpty, locationIsAmbiguousInSourceCode @docs needToUsedExpectErrorsForModules, duplicateModuleName, unknownModulesInExpectedErrors @docs missingFixes, unexpectedFixes, fixedCodeMismatch, unchangedSourceAfterFix, invalidSourceAfterFix, hasCollisionsInFixRanges @@ -232,6 +232,23 @@ Please try to have the error under the smallest region that makes sense. This will be the most helpful for the person who reads the error message.""" +underMayNotBeEmpty : { message : String, codeAtLocation : String } -> String +underMayNotBeEmpty { message, codeAtLocation } = + """COULD NOT FIND LOCATION FOR ERROR + +I was looking for the error with the following message: + + """ ++ wrapInQuotes message ++ """ + +and I found it, but the expected error has an empty string for `under`. I +need to point somewhere, so as to best help the people who encounter this +error. + +If this helps, this is where I found the error: + + """ ++ formatSourceCode codeAtLocation + + locationIsAmbiguousInSourceCode : SourceCode -> Error -> String -> List Int -> String locationIsAmbiguousInSourceCode sourceCode error under occurrencesInSourceCode = """AMBIGUOUS ERROR LOCATION diff --git a/tests/ErrorMessageTest.elm b/tests/ErrorMessageTest.elm index 4849f85b..912cb90c 100644 --- a/tests/ErrorMessageTest.elm +++ b/tests/ErrorMessageTest.elm @@ -18,6 +18,7 @@ all = , unexpectedDetailsTest , emptyDetailsTest , wrongLocationTest + , underMayNotBeEmptyTest , locationNotFoundTest , expectedMoreErrorsTest , tooManyErrorsTest @@ -458,6 +459,31 @@ This will be the most helpful for the person who reads the error message. """ +underMayNotBeEmptyTest : Test +underMayNotBeEmptyTest = + test "underMayNotBeEmpty" <| + \() -> + ErrorMessage.underMayNotBeEmpty + { message = "Some error" + , codeAtLocation = "abcd = 1" + } + |> expectMessageEqual """ +COULD NOT FIND LOCATION FOR ERROR + +I was looking for the error with the following message: + + `Some error` + +and I found it, but the expected error has an empty string for `under`. I +need to point somewhere, so as to best help the people who encounter this +error. + +If this helps, this is where I found the error: + + `abcd = 1` +""" + + expectedMoreErrorsTest : Test expectedMoreErrorsTest = test "expectedMoreErrors" <|