revive
revive copied to clipboard
[rule.import-blacklist] should allow check tests
Is your feature request related to a problem? Please describe. Hi, I' using revive to check my team's project, it's great.
I want to disable an unmaintained and archived package, but it doesn't work:
[rule.imports-blacklist]
arguments = ["bou.ke/monkey"]
So I checked the source code and found that:
if file.IsTest() {
return failures // skip, test file
}
Describe the solution you'd like I don't know why this design, but at least it should be configurable.
Thanks!
Hi @chen3feng, thanks for filling the issue. I do not remember why this exception was added. As you said, it could be configurable. For backward compatibility reasons, if we add a configuration, the default behavior must be the current one (ignore test files)
I'd like to take a look at doing this. @chavacava do you think this is satisfied by a global option to include tests? Or should it be an option to only this rule? Or should it be per argument (in the case where a user would want to include tests for package foo, but not package bar? Or something else?
Some options here...
Global
include_tests = true
For the entire rule One option is to treat certain strings as config options, something like with a special prefix like so...
[rule.imports-blacklist]
arguments=[
"config:include_tests",
"bou.ke/monkey",
]
or
allow either a []string (for backwards compatibility) or a []map[string]interface{} and DTRT in either case like so...
[rule.imports-blacklist]
arguments=[ { config = "include_tests", imports = ["bou.ke/monkey"] } ]
the above extends out well if different imports apply a different config....
[rule.imports-blacklist]
arguments=[
{ config = "include_tests", imports = ["bou.ke/monkey"] }, #includes tests
{ imports = ["foo/bar"] }, #doesn't include tests
]
This issue is fixed here: #862.
cc @chavacava
fixed by #862