Add details to the message of every rule

This will make it easier to have helpful explanations and suggestions
like the Elm compiler does
This commit is contained in:
Jeroen Engels 2019-07-27 17:31:04 +02:00
parent ad8fccb116
commit d593bae052
9 changed files with 102 additions and 27 deletions

View File

@ -27,6 +27,7 @@ type alias LintError =
{ file : Maybe String
, ruleName : String
, message : String
, details : List String
, range : Range
}
@ -113,6 +114,7 @@ errorToRuleError file rule error =
{ file = file
, ruleName = Rule.name rule
, message = Rule.errorMessage error
, details = Rule.errorDetails error
, range = Rule.errorRange error
}

View File

@ -3,7 +3,7 @@ module Lint.Rule exposing
, newSchema, fromSchema
, withSimpleModuleDefinitionVisitor, withSimpleImportVisitor, withSimpleDeclarationVisitor, withSimpleExpressionVisitor
, withInitialContext, withModuleDefinitionVisitor, withImportVisitor, Direction(..), withDeclarationVisitor, withExpressionVisitor, withFinalEvaluation
, Error, error, errorMessage, errorRange
, Error, error, errorMessage, errorDetails, errorRange
, name, analyzer
)
@ -127,7 +127,7 @@ implementing the rule you wish to create.
## Errors
@docs Error, error, errorMessage, errorRange
@docs Error, error, errorMessage, errorDetails, errorRange
# ACCESS
@ -933,6 +933,7 @@ name of the rule that emitted it and the file name.
type Error
= Error
{ message : String
, details : List String
, range : Range
}
@ -944,13 +945,18 @@ In most cases, you can get it using [`Node.range`](https://package.elm-lang.org/
error : Node a -> Error
error node =
Rule.error "Remove the use of `Debug` before shipping to production" (Node.range node)
Rule.error
{ message = "Remove the use of `Debug` before shipping to production"
, details = "The `Debug` module is useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing it's use before committing and attempting to push to production."
}
(Node.range node)
-}
error : String -> Range -> Error
error message range =
error : { message : String, details : List String } -> Range -> Error
error { message, details } range =
Error
{ message = message
, details = details
, range = range
}
@ -962,6 +968,13 @@ errorMessage (Error err) =
err.message
{-| Get the error details of an [`Error`](#Error).
-}
errorDetails : Error -> List String
errorDetails (Error err) =
err.details
{-| Get the [`Range`](https://package.elm-lang.org/packages/stil4m/elm-syntax/7.1.0/Elm-Syntax-Range)
of an [`Error`](#Error).
-}

View File

@ -69,7 +69,11 @@ rule =
error : Node a -> Error
error node =
Rule.error "Remove the use of `Debug` before shipping to production" (Node.range node)
Rule.error
{ message = "Remove the use of `Debug` before shipping to production"
, details = [ "The `Debug` module is useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing it's use before committing and attempting to push to production." ]
}
(Node.range node)
importVisitor : Node Import -> List Error

View File

@ -67,10 +67,12 @@ rule =
|> Rule.fromSchema
error : Node a -> String -> Error
error node comparedValue =
error : Node a -> String -> String -> Error
error node operator comparedValue =
Rule.error
("Unnecessary comparison with `" ++ comparedValue ++ "`")
{ message = "Unnecessary comparison with `" ++ comparedValue ++ "`"
, details = [ "You can simplify this expression by removing the `" ++ operator ++ "` operator and the value `" ++ comparedValue ++ "`." ]
}
(Node.range node)
@ -80,7 +82,7 @@ expressionVisitor node =
Expression.OperatorApplication operator _ left right ->
if isEqualityOperator operator then
List.filterMap isTrueOrFalse [ left, right ]
|> List.map (error node)
|> List.map (error node operator)
else
[]

View File

@ -70,7 +70,14 @@ rule config =
error : Range -> String -> Error
error range name =
Rule.error ("Do not expose everything from " ++ name) range
Rule.error
{ message = "Do not expose everything from " ++ name
, details =
[ "Exposing `(..)` from a module means making all it's exposed functions and types available in the file's namespace. This makes it hard to tell which module a function or type comes from."
, "A preferred pattern is to import functions by name (`import Html exposing (div, span)`) or to use qualified imports (`import Html`, then `Html.div`). If the module name is too long, you can give an alias to the imported module (`import Html.Attributes as Attr`)."
]
}
range
importVisitor : Configuration -> Node Import -> List Error

View File

@ -101,7 +101,9 @@ initialContext =
error : Node String -> Error
error node =
Rule.error
("Type constructor `" ++ Node.value node ++ "` is not used.")
{ message = "Type constructor `" ++ Node.value node ++ "` is not used."
, details = [ "Since it is not being used, I recommend removing it. It should make the code clearer to read for other people." ]
}
(Node.range node)

View File

@ -106,7 +106,9 @@ emptyScope =
error : VariableType -> Range -> String -> Error
error variableType range_ name =
Rule.error
(variableTypeToString variableType ++ " `" ++ name ++ "` is not used" ++ variableTypeWarning variableType)
{ message = variableTypeToString variableType ++ " `" ++ name ++ "` is not used" ++ variableTypeWarning variableType
, details = [ "Since it is not being used, I recommend removing it. It should make the code clearer to read for other people." ]
}
range_

View File

@ -141,7 +141,16 @@ run rule sourceCode =
{ getCodeAtLocation = getCodeAtLocationInSourceCode sourceCode
, checkIfLocationIsAmbiguous = checkIfLocationIsAmbiguousInSourceCode sourceCode
}
(List.map (\error_ -> Rule.error error_.message error_.range) errors)
(List.map
(\error_ ->
Rule.error
{ message = error_.message
, details = error_.details
}
error_.range
)
errors
)
Err _ ->
ParseFailure

View File

@ -60,8 +60,16 @@ didNotExpectErrorsTest =
let
errors : List Error
errors =
[ Rule.error "Some error" dummyRange
, Rule.error "Some other error" dummyRange
[ Rule.error
{ message = "Some error"
, details = [ "Some details" ]
}
dummyRange
, Rule.error
{ message = "Some other error"
, details = [ "Some other details" ]
}
dummyRange
]
in
ErrorMessage.didNotExpectErrors errors
@ -90,7 +98,11 @@ messageMismatchTest =
error : Error
error =
Rule.error "Some error message" dummyRange
Rule.error
{ message = "Some error"
, details = [ "Some details" ]
}
dummyRange
in
ErrorMessage.messageMismatch expectedError error
|> expectMessageEqual """
@ -102,7 +114,7 @@ I was looking for the error with the following message:
but I found the following error message:
`Some error message`"""
`Some error`"""
underMismatchTest : Test
@ -113,7 +125,11 @@ underMismatchTest =
let
error : Error
error =
Rule.error "Some error" dummyRange
Rule.error
{ message = "Some error"
, details = [ "Some details" ]
}
dummyRange
in
ErrorMessage.underMismatch
error
@ -142,7 +158,11 @@ calling `Rule.error`"""
let
error : Error
error =
Rule.error "Some other error" dummyRange
Rule.error
{ message = "Some other error"
, details = [ "Some other details" ]
}
dummyRange
in
ErrorMessage.underMismatch
error
@ -185,7 +205,9 @@ wrongLocationTest =
error : Error
error =
Rule.error
"Some error"
{ message = "Some error"
, details = [ "Some details" ]
}
{ start = { row = 3, column = 1 }, end = { row = 3, column = 5 } }
in
ErrorMessage.wrongLocation
@ -219,7 +241,9 @@ but I found it at:
error : Error
error =
Rule.error
"Some other error"
{ message = "Some other error"
, details = [ "Some other details" ]
}
{ start = { row = 4, column = 1 }, end = { row = 5, column = 3 } }
in
ErrorMessage.wrongLocation
@ -288,7 +312,9 @@ tooManyErrorsTest =
extraErrors : List Rule.Error
extraErrors =
[ Rule.error
"Remove the use of `Debug` before shipping to production"
{ message = "Remove the use of `Debug` before shipping to production"
, details = [ "Some details about Debug" ]
}
{ start = { row = 2, column = 1 }, end = { row = 2, column = 5 } }
]
in
@ -307,10 +333,14 @@ I found 1 error too many:
extraErrors : List Rule.Error
extraErrors =
[ Rule.error
"Remove the use of `Debug` before shipping to production"
{ message = "Remove the use of `Debug` before shipping to production"
, details = [ "Some details about Debug" ]
}
{ start = { row = 2, column = 1 }, end = { row = 2, column = 5 } }
, Rule.error
"Remove the use of `Debug` before shipping to production"
{ message = "Remove the use of `Debug` before shipping to production"
, details = [ "Some details about Debug" ]
}
{ start = { row = 3, column = 1 }, end = { row = 3, column = 5 } }
]
in
@ -345,7 +375,9 @@ locationIsAmbiguousInSourceCodeTest =
error : Error
error =
Rule.error
"Some error"
{ message = "Some error"
, details = [ "Some details" ]
}
{ start = { row = 3, column = 1 }, end = { row = 3, column = 5 } }
in
ErrorMessage.locationIsAmbiguousInSourceCode
@ -387,7 +419,9 @@ Tip: I found them at:
error : Error
error =
Rule.error
"Some other error"
{ message = "Some other error"
, details = [ "Some other details" ]
}
{ start = { row = 3, column = 1 }, end = { row = 4, column = 3 } }
in
ErrorMessage.locationIsAmbiguousInSourceCode