This commit is contained in:
Jeroen Engels 2019-10-30 19:51:57 +01:00
parent 841430be60
commit a879710561
5 changed files with 109 additions and 89 deletions

View File

@ -8,6 +8,7 @@
"Review", "Review",
"Review.Rule", "Review.Rule",
"Review.Project", "Review.Project",
"Review.File",
"Review.Fix", "Review.Fix",
"Review.Test" "Review.Test"
], ],

View File

@ -95,7 +95,7 @@ fileKeyVisitor fileKey context =
error : ( List String, { fileKey : Rule.FileKey, moduleNameLocation : Range } ) -> Error error : ( List String, { fileKey : Rule.FileKey, moduleNameLocation : Range } ) -> Error
error ( moduleName, { fileKey, moduleNameLocation } ) = error ( moduleName, { fileKey, moduleNameLocation } ) =
Rule.errorForFile fileKey Rule.errorForFile fileKey
{ message = "`" ++ String.join "." moduleName ++ "` is never used." { message = "Module `" ++ String.join "." moduleName ++ "` is never used."
, details = [ "This module is never used. You may want to remove it to keep your project clean, and maybe detect some unused dependencies in your project." ] , details = [ "This module is never used. You may want to remove it to keep your project clean, and maybe detect some unused dependencies in your project." ]
} }
moduleNameLocation moduleNameLocation

View File

@ -1,11 +1,17 @@
module Review exposing module Review exposing
( review, reviewFiles ( parseFile, parseFiles
, Error, errorModuleName, errorRuleName, errorMessage, errorDetails, errorRange, errorFixes , review, reviewFiles
, Error, errorModuleName, errorRuleName, errorMessage, errorDetails, errorRange, errorFixes, errorFilePath
) )
{-| Module to configure your review configuration and run it on a source file. {-| Module to configure your review configuration and run it on a source file.
# Parsing files
@docs parseFile, parseFiles
# Reviewing # Reviewing
@docs review, reviewFiles @docs review, reviewFiles
@ -13,7 +19,7 @@ module Review exposing
# Errors # Errors
@docs Error, errorModuleName, errorRuleName, errorMessage, errorDetails, errorRange, errorFixes @docs Error, errorModuleName, errorRuleName, errorMessage, errorDetails, errorRange, errorFixes, errorFilePath
-} -}
@ -23,6 +29,7 @@ import Elm.Syntax.File exposing (File)
import Elm.Syntax.Module as Module import Elm.Syntax.Module as Module
import Elm.Syntax.Node as Node import Elm.Syntax.Node as Node
import Elm.Syntax.Range exposing (Range) import Elm.Syntax.Range exposing (Range)
import Review.File exposing (ParsedFile, RawFile)
import Review.Fix exposing (Fix) import Review.Fix exposing (Fix)
import Review.Project exposing (Project) import Review.Project exposing (Project)
import Review.Rule as Rule exposing (Rule) import Review.Rule as Rule exposing (Rule)
@ -38,6 +45,7 @@ like the name of the rule that emitted it and the file name.
type Error type Error
= Error = Error
{ moduleName : Maybe String { moduleName : Maybe String
, filePath : String
, ruleName : String , ruleName : String
, message : String , message : String
, details : List String , details : List String
@ -78,6 +86,7 @@ review config project { path, source } =
Err _ -> Err _ ->
[ Error [ Error
{ moduleName = Nothing { moduleName = Nothing
, filePath = path
, ruleName = "ParsingError" , ruleName = "ParsingError"
, message = path ++ " is not a correct Elm file" , message = path ++ " is not a correct Elm file"
, details = , details =
@ -105,12 +114,8 @@ type alias RawFile =
{-| TODO documentation {-| TODO documentation
-} -}
reviewFiles : List Rule -> Project -> List RawFile -> List Error reviewFiles : List Rule -> Project -> List ParsedFile -> List Error
reviewFiles rules project files = reviewFiles rules project files =
let
( parsedFiles, errors ) =
parseFiles files
in
rules rules
|> List.concatMap |> List.concatMap
(\rule -> (\rule ->
@ -121,16 +126,15 @@ reviewFiles rules project files =
fn project file fn project file
|> List.map (ruleErrorToReviewError rule) |> List.map (ruleErrorToReviewError rule)
) )
parsedFiles files
Rule.Multi fn -> Rule.Multi fn ->
fn project parsedFiles fn project files
|> List.map (ruleErrorToReviewError rule) |> List.map (ruleErrorToReviewError rule)
) )
|> List.append errors
parseFiles : List RawFile -> ( List File, List Error ) parseFiles : List RawFile -> ( List ParsedFile, List Error )
parseFiles files = parseFiles files =
files files
|> List.map parseFile |> List.map parseFile
@ -146,13 +150,21 @@ parseFiles files =
( [], [] ) ( [], [] )
parseFile : RawFile -> Result Error File parseFile : RawFile -> Result Error ParsedFile
parseFile rawFile = parseFile rawFile =
parseSource rawFile.source case parseSource rawFile.source of
|> Result.mapError Ok ast ->
(\_ -> Ok
{ path = rawFile.path
, source = rawFile.source
, ast = ast
}
Err _ ->
Err <|
Error Error
{ moduleName = Nothing { moduleName = Nothing
, filePath = rawFile.path
, ruleName = "ParsingError" , ruleName = "ParsingError"
, message = rawFile.path ++ " is not a correct Elm file" , message = rawFile.path ++ " is not a correct Elm file"
, details = , details =
@ -162,7 +174,6 @@ parseFile rawFile =
, range = { start = { row = 0, column = 0 }, end = { row = 0, column = 0 } } , range = { start = { row = 0, column = 0 }, end = { row = 0, column = 0 } }
, fixes = Nothing , fixes = Nothing
} }
)
compareErrorPositions : Error -> Error -> Order compareErrorPositions : Error -> Error -> Order
@ -216,6 +227,7 @@ ruleErrorToReviewError : Rule -> Rule.Error -> Error
ruleErrorToReviewError rule error = ruleErrorToReviewError rule error =
Error Error
{ moduleName = Nothing { moduleName = Nothing
, filePath = Rule.errorFilePath error
, ruleName = Rule.name rule , ruleName = Rule.name rule
, message = Rule.errorMessage error , message = Rule.errorMessage error
, details = Rule.errorDetails error , details = Rule.errorDetails error
@ -278,3 +290,10 @@ errorRange (Error error) =
errorFixes : Error -> Maybe (List Fix) errorFixes : Error -> Maybe (List Fix)
errorFixes (Error error) = errorFixes (Error error) =
error.fixes error.fixes
{-| TODO
-}
errorFilePath : Error -> String
errorFilePath (Error error) =
error.filePath

19
src/Review/File.elm Normal file
View File

@ -0,0 +1,19 @@
module Review.File exposing (ParsedFile, RawFile)
{-| TODO Documentation
-}
import Elm.Syntax.File
type alias RawFile =
{ path : String
, source : String
}
type alias ParsedFile =
{ path : String
, source : String
, ast : Elm.Syntax.File.File
}

View File

@ -5,7 +5,7 @@ module Review.Rule exposing
, withInitialContext, withModuleDefinitionVisitor, withImportVisitor, Direction(..), withDeclarationVisitor, withDeclarationListVisitor, withExpressionVisitor, withFinalEvaluation , withInitialContext, withModuleDefinitionVisitor, withImportVisitor, Direction(..), withDeclarationVisitor, withDeclarationListVisitor, withExpressionVisitor, withFinalEvaluation
, withElmJsonVisitor , withElmJsonVisitor
, withFixes , withFixes
, Error, error, errorMessage, errorDetails, errorRange, errorFixes , Error, error, errorMessage, errorDetails, errorRange, errorFixes, errorFilePath
, name, analyzer , name, analyzer
, Analyzer(..), newMultiSchema, fromMultiSchema, newFileVisitorSchema , Analyzer(..), newMultiSchema, fromMultiSchema, newFileVisitorSchema
, FileKey, withFileKeyVisitor, errorForFile , FileKey, withFileKeyVisitor, errorForFile
@ -180,7 +180,7 @@ For more information on automatic fixing, read the documentation for [`Review.Fi
## Errors ## Errors
@docs Error, error, errorMessage, errorDetails, errorRange, errorFixes @docs Error, error, errorMessage, errorDetails, errorRange, errorFixes, errorFilePath
# ACCESS # ACCESS
@ -204,6 +204,7 @@ import Elm.Syntax.Infix exposing (InfixDirection(..))
import Elm.Syntax.Module exposing (Module) import Elm.Syntax.Module exposing (Module)
import Elm.Syntax.Node as Node exposing (Node) import Elm.Syntax.Node as Node exposing (Node)
import Elm.Syntax.Range exposing (Range) import Elm.Syntax.Range exposing (Range)
import Review.File exposing (ParsedFile)
import Review.Fix exposing (Fix) import Review.Fix exposing (Fix)
import Review.Project exposing (Project) import Review.Project exposing (Project)
@ -226,8 +227,8 @@ May need to move the Rule type in there too?
type Analyzer type Analyzer
= -- TODO Can't Single also be (Project -> List File -> List Error)? = -- TODO Can't Single also be (Project -> List File -> List Error)?
-- Have the file processing be done in this file rather than in Review.elm -- Have the file processing be done in this file rather than in Review.elm
Single (Project -> File -> List Error) Single (Project -> ParsedFile -> List Error)
| Multi (Project -> List File -> List Error) | Multi (Project -> List ParsedFile -> List Error)
{-| Represents a Schema for a [`Rule`](#Rule). Create one using [`newSchema`](#newSchema). {-| Represents a Schema for a [`Rule`](#Rule). Create one using [`newSchema`](#newSchema).
@ -389,19 +390,30 @@ fromSchema (Schema schema) =
{ name = schema.name { name = schema.name
, analyzer = , analyzer =
Single Single
(\project file -> (\project { path, ast } ->
schema.initialContext schema.initialContext
|> schema.elmJsonVisitor (Review.Project.elmJson project) |> schema.elmJsonVisitor (Review.Project.elmJson project)
|> schema.moduleDefinitionVisitor file.moduleDefinition |> schema.moduleDefinitionVisitor ast.moduleDefinition
|> accumulateList schema.importVisitor file.imports |> accumulateList schema.importVisitor ast.imports
|> accumulate (schema.declarationListVisitor file.declarations) |> accumulate (schema.declarationListVisitor ast.declarations)
|> accumulateList (visitDeclaration schema.declarationVisitor schema.expressionVisitor) file.declarations |> accumulateList (visitDeclaration schema.declarationVisitor schema.expressionVisitor) ast.declarations
|> makeFinalEvaluation schema.finalEvaluationFn |> makeFinalEvaluation schema.finalEvaluationFn
|> List.map (\(Error err) -> Error { err | filePath = path })
|> List.reverse |> List.reverse
) )
} }
maybeApply : Maybe (b -> a -> a) -> b -> a -> a
maybeApply maybeFn argument =
case maybeFn of
Just fn ->
fn argument
Nothing ->
identity
type MultiSchema context type MultiSchema context
= MultiSchema = MultiSchema
{ name : String { name : String
@ -434,17 +446,7 @@ newMultiSchema name_ { initialContext, elmJsonVisitor, fileVisitor, mergeContext
} }
{-| TODO documentation multiAnalyzer : MultiSchema context -> Project -> List ParsedFile -> List Error
-}
fromMultiSchema : MultiSchema context -> Rule
fromMultiSchema ((MultiSchema schema) as multiSchema) =
Rule
{ name = schema.name
, analyzer = Multi (multiAnalyzer multiSchema)
}
multiAnalyzer : MultiSchema context -> Project -> List File -> List Error
multiAnalyzer (MultiSchema schema) project = multiAnalyzer (MultiSchema schema) project =
let let
initialContext : context initialContext : context
@ -476,13 +478,24 @@ multiAnalyzer (MultiSchema schema) project =
] ]
visitFileForMulti : Schema { multiFile : () } { hasAtLeastOneVisitor : () } context -> context -> File -> ( List Error, context ) visitFileForMulti : Schema { multiFile : () } { hasAtLeastOneVisitor : () } context -> context -> ParsedFile -> ( List Error, context )
visitFileForMulti (Schema schema) initialContext file = visitFileForMulti (Schema schema) initialContext { path, ast } =
initialContext initialContext
|> schema.moduleDefinitionVisitor file.moduleDefinition |> maybeApply schema.fileKeyVisitor (FileKey path)
|> accumulateList schema.importVisitor file.imports |> schema.moduleDefinitionVisitor ast.moduleDefinition
|> accumulate (schema.declarationListVisitor file.declarations) |> accumulateList schema.importVisitor ast.imports
|> accumulateList (visitDeclaration schema.declarationVisitor schema.expressionVisitor) file.declarations |> accumulate (schema.declarationListVisitor ast.declarations)
|> accumulateList (visitDeclaration schema.declarationVisitor schema.expressionVisitor) ast.declarations
{-| TODO documentation
-}
fromMultiSchema : MultiSchema context -> Rule
fromMultiSchema ((MultiSchema schema) as multiSchema) =
Rule
{ name = schema.name
, analyzer = Multi (multiAnalyzer multiSchema)
}
{-| Concatenate the errors of the previous step and of the last step. {-| Concatenate the errors of the previous step and of the last step.
@ -1263,51 +1276,9 @@ withFinalEvaluation visitor (Schema schema) =
Schema { schema | finalEvaluationFn = visitor } Schema { schema | finalEvaluationFn = visitor }
{-| Add a function that makes a final evaluation based only on the data that was {-| TODO
collected in the `context`. This can be useful if you can't or if it is hard to
determine something as you traverse the file.
The following example forbids importing both `Element` (`elm-ui`) and
`Html.Styled` (`elm-css`). Note that this is the same one written in the example
for [`withImportVisitor`](#withImportVisitor), but using [`withFinalEvaluation`](#withFinalEvaluation).
import Dict as Dict exposing (Dict)
import Elm.Syntax.Import exposing (Import)
import Elm.Syntax.Node as Node exposing (Node)
import Elm.Syntax.Range exposing (Range)
import Review.Rule as Rule exposing (Error, Rule)
type alias Context =
Dict (List String) Range
rule : Rule
rule =
Rule.newSchema "NoUsingBothHtmlAndHtmlStyled"
|> Rule.withInitialContext Dict.empty
|> Rule.withImportVisitor importVisitor
|> Rule.withFinalEvaluation finalEvaluation
|> Rule.fromSchema
importVisitor : Node Import -> Context -> ( List Error, Context )
importVisitor node context =
( [], Dict.insert (Node.value node |> .moduleName |> Node.value) (Node.range node) context )
finalEvaluation : Context -> List Error
finalEvaluation context =
case ( Dict.get [ "Element" ] context, Dict.get [ "Html", "Styled" ] context ) of
( Just elmUiRange, Just _ ) ->
[ Rule.error
{ message = "Do not use both `elm-ui` and `elm-css`"
, details = [ "At fruits.com, we use `elm-ui` in the dashboard application, and `elm-css` in the rest of the code. We want to use `elm-ui` in our new projects, but in projects using `elm-css`, we don't want to use both libraries to keep things simple." ]
}
elmUiRange
]
_ ->
[]
-} -}
withFileKeyVisitor : (FileKey -> context -> context) -> Schema anytype { hasNoVisitor : () } context -> Schema anytype { hasNoVisitor : () } context withFileKeyVisitor : (FileKey -> context -> context) -> Schema { multiFile : () } { hasNoVisitor : () } context -> Schema { multiFile : () } { hasNoVisitor : () } context
withFileKeyVisitor visitor (Schema schema) = withFileKeyVisitor visitor (Schema schema) =
Schema { schema | fileKeyVisitor = Just visitor } Schema { schema | fileKeyVisitor = Just visitor }
@ -1327,6 +1298,7 @@ name of the rule that emitted it and the file name.
type Error type Error
= Error = Error
{ message : String { message : String
, filePath : String
, details : List String , details : List String
, range : Range , range : Range
, fixes : Maybe (List Fix) , fixes : Maybe (List Fix)
@ -1359,6 +1331,7 @@ error : { message : String, details : List String } -> Range -> Error
error { message, details } range = error { message, details } range =
Error Error
{ message = message { message = message
, filePath = ""
, details = details , details = details
, range = range , range = range
, fixes = Nothing , fixes = Nothing
@ -1384,12 +1357,13 @@ by the tests automatically.
-} -}
errorForFile : FileKey -> { message : String, details : List String } -> Range -> Error errorForFile : FileKey -> { message : String, details : List String } -> Range -> Error
errorForFile fileKey { message, details } range = errorForFile (FileKey path) { message, details } range =
-- TODO Use fileKey -- TODO Use fileKey
Error Error
{ message = message { message = message
, details = details , details = details
, range = range , range = range
, filePath = path
, fixes = Nothing , fixes = Nothing
} }
@ -1457,6 +1431,13 @@ errorFixes (Error err) =
err.fixes err.fixes
{-| TODO
-}
errorFilePath : Error -> String
errorFilePath (Error err) =
err.filePath
-- ACCESS -- ACCESS