Add Rule.filterErrorsForFiles (#115)

This commit is contained in:
Jie 2022-02-05 01:16:08 +09:00 committed by GitHub
parent 0355f30e36
commit 2bc6c2ec33
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 214 additions and 867 deletions

831
docs.json

File diff suppressed because one or more lines are too long

View File

@ -2,6 +2,7 @@ module Review.Exceptions exposing
( Exceptions
, init
, addDirectories, addFiles
, addFilter
, apply
)
@ -10,6 +11,7 @@ module Review.Exceptions exposing
@docs Exceptions
@docs init
@docs addDirectories, addFiles
@docs addFilter
@docs apply
-}
@ -18,22 +20,21 @@ import Set exposing (Set)
type Exceptions
= Exceptions
{ directories : List String
, files : Set String
}
= Exceptions (List (String -> Bool))
init : Exceptions
init =
Exceptions
{ directories = []
, files = Set.empty
}
Exceptions []
addFilter : (String -> Bool) -> Exceptions -> Exceptions
addFilter condition (Exceptions conditions) =
Exceptions (condition :: conditions)
addDirectories : List String -> Exceptions -> Exceptions
addDirectories directories (Exceptions exceptions) =
addDirectories directories =
let
cleanedDirectories : List String
cleanedDirectories =
@ -49,14 +50,11 @@ addDirectories directories (Exceptions exceptions) =
)
)
in
Exceptions
{ directories = cleanedDirectories ++ exceptions.directories
, files = exceptions.files
}
addFilter (isInAnIgnoredDirectory cleanedDirectories >> not)
addFiles : List String -> Exceptions -> Exceptions
addFiles files (Exceptions exceptions) =
addFiles files =
let
cleanedFiles : Set String
cleanedFiles =
@ -64,30 +62,17 @@ addFiles files (Exceptions exceptions) =
|> List.map makePathOSAgnostic
|> Set.fromList
in
Exceptions
{ files = Set.union cleanedFiles exceptions.files
, directories = exceptions.directories
}
addFilter (\file -> Set.member file cleanedFiles |> not)
apply : Exceptions -> (a -> String) -> List a -> List a
apply (Exceptions exceptions) getPath items =
if Set.isEmpty exceptions.files && List.isEmpty exceptions.directories then
items
else
List.filter (getPath >> shouldBeIgnored exceptions >> not) items
shouldBeIgnored : { directories : List String, files : Set String } -> String -> Bool
shouldBeIgnored exceptions path =
apply (Exceptions conditions) getPath items =
let
cleanedPath : String
cleanedPath =
makePathOSAgnostic path
allConditions : String -> Bool
allConditions path =
List.all (\condition -> condition path) conditions
in
Set.member cleanedPath exceptions.files
|| isInAnIgnoredDirectory exceptions.directories cleanedPath
List.filter (getPath >> makePathOSAgnostic >> allConditions) items
isInAnIgnoredDirectory : List String -> String -> Bool

View File

@ -17,7 +17,7 @@ module Review.Rule exposing
, Error, error, errorWithFix, ModuleKey, errorForModule, errorForModuleWithFix, ElmJsonKey, errorForElmJson, errorForElmJsonWithFix, ReadmeKey, errorForReadme, errorForReadmeWithFix
, globalError, configurationError
, ReviewError, errorRuleName, errorMessage, errorDetails, errorRange, errorFixes, errorFilePath, errorTarget
, ignoreErrorsForDirectories, ignoreErrorsForFiles
, ignoreErrorsForDirectories, ignoreErrorsForFiles, filterErrorsForFiles
, review, reviewV2, ProjectData, ruleName, getConfigurationError
, Required, Forbidden
)
@ -240,9 +240,10 @@ There are situations where you don't want review rules to report errors:
1. You copied and updated over an external library because one of your needs wasn't met, and you don't want to modify it more than necessary.
2. Your project contains generated source code, over which you have no control or for which you do not care that some rules are enforced (like the reports of unused variables).
3. You want to introduce a rule progressively, because there are too many errors in the project for you to fix in one go. You can then ignore the parts of the project where the problem has not yet been solved, and fix them as you go.
4. You wish to disable some rules for tests files (or enable some only for tests).
4. You wrote a rule that is very specific and should only be applied to a portion of your code.
5. You wish to disable some rules for tests files (or enable some only for tests).
You can use the following functions to ignore errors in directories or files.
You can use the following functions to ignore errors in directories or files, or only report errors found in specific directories or files.
**NOTE**: Even though they can be used to disable any errors, I **strongly recommend against**
doing so if you are not in the situations listed above. I highly recommend you
@ -250,7 +251,7 @@ leave a comment explaining the reason why you use these functions, or to
communicate with your colleagues if you see them adding exceptions without
reason or seemingly inappropriately.
@docs ignoreErrorsForDirectories, ignoreErrorsForFiles
@docs ignoreErrorsForDirectories, ignoreErrorsForFiles, filterErrorsForFiles
# Running rules
@ -3808,6 +3809,81 @@ ignoreErrorsForFiles files (Rule rule) =
}
{-| Ignore the errors reported for file paths that fail a condition.
Use it to control precisely which files the rule is applied to. For example, you
might have written a rule that should only be applied to one specific file.
config : List Rule
config =
[ Some.Rule.rule
|> Rule.filterErrorsForFiles (\path -> path == "src/Some/File.elm")
, Some.Other.Rule.rule
]
If you want to specify a condition for all of your rules, you can apply
`filterErrorsForFiles` like this:
config : List Rule
config =
[ Some.Rule.For.Tests.rule
, Some.Other.Rule.rule
]
|> List.map (Rule.filterErrorsForFiles (String.startsWith "tests/"))
The received paths will be relative to the `elm.json` file, just like the ones for the
`elm.json`'s `source-directories`, and will be formatted in the Unix style `src/Some/File.elm`.
You can apply `filterErrorsForFiles` several times for a rule, the conditions will get
compounded, following exactly the behavior of `List.filter`.
When "ignoreErrors" functions are used in combination, all constraints are observed.
You can also use it when writing a rule. We can hardcode in the rule that a rule
is only applicable to a folder, like `src/Api/` for instance. The following example
forbids using strings with hardcoded URLs, but only in the `src/Api/` folder.
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.Node as Node exposing (Node)
import Review.Rule as Rule exposing (Error, Rule)
rule : Rule
rule =
Rule.newModuleRuleSchema "NoHardcodedURLs" ()
|> Rule.withSimpleExpressionVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
|> Rule.filterErrorsForFiles (String.startsWith "src/Api/")
expressionVisitor : Node Expression -> List (Error {})
expressionVisitor node =
case Node.value node of
Expression.Literal string ->
if isUrl string then
[ Rule.error
{ message = "Do not use hardcoded URLs in the API modules"
, details = [ "Hardcoded URLs should never make it to production. Please refer to the documentation of the `Endpoint` module." ]
}
(Node.range node)
]
else
[]
_ ->
[]
-}
filterErrorsForFiles : (String -> Bool) -> Rule -> Rule
filterErrorsForFiles condition (Rule rule) =
Rule
{ name = rule.name
, exceptions = Exceptions.addFilter condition rule.exceptions
, requestedData = rule.requestedData
, ruleImplementation = rule.ruleImplementation
, configurationError = rule.configurationError
}
-- VISITOR
-- TODO BREAKING CHANGE Move this into a separate module later on

View File

@ -54,6 +54,121 @@ all =
, "src/File.elm"
, "src-other/Ignored/File.elm"
]
, test "should remove files using Exceptions.ignoreFiles and Exceptions.ignoreDirectories" <|
\() ->
let
exceptions : Exceptions
exceptions =
Exceptions.init
|> Exceptions.addFiles [ "src/addd/File.elm", "src/Foo/Thing.elm" ]
|> Exceptions.addDirectories [ "src/Ignored/", "src-other" ]
in
[ "src/Foo/Bar.elm"
, "src/Ignored/File.elm" -- should be removed
, "src/IgnoredFile.elm"
, "src/Foo/Thing.elm" -- should be removed
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
, "src-other/Ignored/File.elm" -- should be removed
]
|> Exceptions.apply exceptions identity
|> Expect.equal
[ "src/Foo/Bar.elm"
, "src/IgnoredFile.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
]
, test "should leave all files using Exceptions.addFilter with always True" <|
\() ->
let
exceptions : Exceptions
exceptions =
Exceptions.init
|> Exceptions.addFilter (always True)
in
[ "src/Foo/Bar.elm"
, "src/Ignored/File.elm"
, "src/IgnoredFile.elm"
, "src/Foo/Thing.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
, "src-other/Ignored/File.elm"
]
|> Exceptions.apply exceptions identity
|> Expect.equal
[ "src/Foo/Bar.elm"
, "src/Ignored/File.elm"
, "src/IgnoredFile.elm"
, "src/Foo/Thing.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
, "src-other/Ignored/File.elm"
]
, test "should remove all files using Exceptions.addFilter with always False" <|
\() ->
let
exceptions : Exceptions
exceptions =
Exceptions.init
|> Exceptions.addFilter (always False)
in
[ "src/Foo/Bar.elm"
, "src/Ignored/File.elm"
, "src/IgnoredFile.elm"
, "src/Foo/Thing.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
, "src-other/Ignored/File.elm"
]
|> Exceptions.apply exceptions identity
|> Expect.equal []
, test "should remove files using Exceptions.addFilter" <|
\() ->
let
exceptions : Exceptions
exceptions =
Exceptions.init
|> Exceptions.addFilter (String.contains "Ignored" >> not)
in
[ "src/Foo/Bar.elm"
, "src/Ignored/File.elm" -- should be removed
, "src/IgnoredFile.elm" -- should be removed
, "src/Foo/Thing.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
, "src-other/Ignored/File.elm" -- should be removed
]
|> Exceptions.apply exceptions identity
|> Expect.equal
[ "src/Foo/Bar.elm"
, "src/Foo/Thing.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
]
, test "should remove files using Exceptions.ignore* and Exceptions.addFilter" <|
\() ->
let
exceptions : Exceptions
exceptions =
Exceptions.init
|> Exceptions.addFiles [ "src/addd/File.elm", "src/Foo/Thing.elm" ]
|> Exceptions.addDirectories [ "src/Ignored/", "src-other" ]
|> Exceptions.addFilter (String.contains "Bar" >> not)
in
[ "src/Foo/Bar.elm" -- should be removed
, "src/Ignored/File.elm" -- should be removed
, "src/IgnoredFile.elm"
, "src/Foo/Thing.elm" -- should be removed
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
, "src-other/Ignored/File.elm" -- should be removed
]
|> Exceptions.apply exceptions identity
|> Expect.equal
[ "src/IgnoredFile.elm"
, "src/Foo/Thing/SubThing.elm"
, "src/File.elm"
]
, test "should be backslash-insensitive in the ignored paths" <|
\() ->
let