mgechev / revive Goto Github PK
View Code? Open in Web Editor NEW๐ฅ ~6x faster, stricter, configurable, extensible, and beautiful drop-in replacement for golint
Home Page: https://revive.run
License: MIT License
๐ฅ ~6x faster, stricter, configurable, extensible, and beautiful drop-in replacement for golint
Home Page: https://revive.run
License: MIT License
Is your feature request related to a problem? Please describe.
I'd like to see a clear order in imports between github packages, golang packages or custom packages
Describe the solution you'd like
Inspired by: https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/order.md
The idea is to have a code like this as valid:
import (
"os"
"strings"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"golang.org/x/crypto/bcrypt"
"my-app/model"
)
While this would be invalid:
import (
"os"
"strings"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"golang.org/x/crypto/bcrypt"
"my-app/model"
)
Additional context
Thoughts? I think ensuring some order in imports would help.
Sometimes I forget to handle an error. Go linters usually don't warn me. At least those I know. I don't think I am the only one who is bothered by this. By implementing this everyone's code will greatly improve.
For example:
Bad code:
os.Chdir("..")
But if we look into the docs, we can see that os.Chdir
returns error.
Good code:
err = os.Chdir("..")
if err != nil {
// something
}
Actually, we don't need to check for the if err != nil
part. It is handled by go compiler. We just need to check if err was assigned.
Solution I would like
Error like this:
os.Chdir error not handled
Thanks.
Is your feature request related to a problem? Please describe.
For security reason we sometime want to blacklist some packages (crypto/md5
, crypto/sha1
....).
Describe the solution you'd like
Add a rule named imports-blacklist
(inspired by https://palantir.github.io/tslint/rules/import-blacklist/) which takes a []string
as arguments and check that any of the imported packages are in the blacklist.
Additional context
I can implement this with pleasure, just let me know.
Is your feature request related to a problem? Please describe.
Having to pass a -config config.toml
without being able to simply -all
to get all of the lint rules.
Describe the solution you'd like
Add a -all
or something similar to the command line options so a TOML config file isn't needed.
Describe the bug
revive panics.
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
# flags
~/.gotools/bin/revive -config=$HOME/.revive.toml ./...
# config file
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0
[rule.blank-imports]
[rule.context-as-argument]
[rule.context-keys-type]
[rule.dot-imports]
[rule.error-return]
[rule.error-strings]
[rule.error-naming]
#[rule.exported]
[rule.if-return]
[rule.increment-decrement]
#[rule.var-naming]
[rule.var-declaration]
[rule.package-comments]
[rule.range]
#[rule.receiver-naming]
[rule.time-naming]
[rule.unexported-return]
[rule.indent-error-flow]
[rule.errorf]
Expected behavior
No panic.
Logs
[...]
vendor/github.com/apache/thrift/lib/go/thrift/server_socket.go:92:9: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)
panic: assertion failed [recovered]
panic: assertion failed
goroutine 1315 [running]:
go/types.(*Checker).handleBailout(0xc0005b6870, 0xc008cab800)
/usr/local/go/src/go/types/check.go:236 +0x98
panic(0x12b3600, 0x1370730)
/usr/local/go/src/runtime/panic.go:513 +0x1b9
go/types.assert(...)
/usr/local/go/src/go/types/errors.go:18
go/types.(*Checker).recordTypeAndValue(0xc0005b6870, 0x0, 0x0, 0x3, 0x1374960, 0xc02fead160, 0x0, 0x0)
/usr/local/go/src/go/types/check.go:281 +0x279
go/types.(*Checker).exprInternal(0xc0005b6870, 0xc02fed4e00, 0x13755a0, 0xc00a1ca800, 0x1374960, 0xc02fead160, 0x100b3c3)
/usr/local/go/src/go/types/expr.go:1164 +0x17c4
go/types.(*Checker).rawExpr(0xc0005b6870, 0xc02fed4e00, 0x13755a0, 0xc00a1ca800, 0x1374960, 0xc02fead160, 0x100ba38)
/usr/local/go/src/go/types/expr.go:969 +0x81
go/types.(*Checker).exprWithHint(0xc0005b6870, 0xc02fed4e00, 0x13755a0, 0xc00a1ca800, 0x1374960, 0xc02fead160)
/usr/local/go/src/go/types/expr.go:1597 +0x73
go/types.(*Checker).indexedElts(0xc0005b6870, 0xc005257a00, 0x1c, 0x20, 0x1374960, 0xc02fead160, 0xffffffffffffffff, 0x1374960)
/usr/local/go/src/go/types/expr.go:939 +0x1e2
go/types.(*Checker).exprInternal(0xc0005b6870, 0xc02fed4d00, 0x13755a0, 0xc00a1cb600, 0x0, 0x0, 0x101e728)
/usr/local/go/src/go/types/expr.go:1158 +0x1759
go/types.(*Checker).rawExpr(0xc0005b6870, 0xc02fed4d00, 0x13755a0, 0xc00a1cb600, 0x0, 0x0, 0xc00028cd80)
/usr/local/go/src/go/types/expr.go:969 +0x81
go/types.(*Checker).multiExpr(0xc0005b6870, 0xc02fed4d00, 0x13755a0, 0xc00a1cb600)
/usr/local/go/src/go/types/expr.go:1575 +0x58
go/types.(*Checker).expr(0xc0005b6870, 0xc02fed4d00, 0x13755a0, 0xc00a1cb600)
/usr/local/go/src/go/types/expr.go:1569 +0x49
go/types.(*Checker).varDecl(0xc0005b6870, 0xc011a03180, 0xc01ca5d008, 0x1, 0x1, 0x0, 0x0, 0x13755a0, 0xc00a1cb600)
/usr/local/go/src/go/types/decl.go:425 +0x1b7
go/types.(*Checker).objDecl(0xc0005b6870, 0x1378080, 0xc011a03180, 0x0, 0xc008cab700, 0x0, 0x8)
/usr/local/go/src/go/types/decl.go:244 +0x83c
go/types.(*Checker).packageObjects(0xc0005b6870)
/usr/local/go/src/go/types/resolver.go:542 +0x26f
go/types.(*Checker).checkFiles(0xc0005b6870, 0xc00d253000, 0x1e, 0x20, 0x0, 0x0)
/usr/local/go/src/go/types/check.go:250 +0xa5
go/types.(*Checker).Files(0xc0005b6870, 0xc00d253000, 0x1e, 0x20, 0xc00d0c47d0, 0xc00a68f030)
/usr/local/go/src/go/types/check.go:241 +0x49
go/types.(*Config).Check(0xc00d0c68c0, 0xc010c98d17, 0x5, 0xc0021ccd40, 0xc00d253000, 0x1e, 0x20, 0xc00d0c4730, 0x101e728, 0xc0094dd940, ...)
/usr/local/go/src/go/types/api.go:351 +0x11a
github.com/mgechev/revive/lint.(*Package).TypeCheck(0xc0021ccd80, 0xc0094dd940, 0xc011429880)
/Users/sandbox/.gotools/src/github.com/mgechev/revive/lint/package.go:80 +0x351
github.com/mgechev/revive/rule.(*TimeNamingRule).Apply(0x15813c0, 0xc01240f740, 0x0, 0x0, 0x0, 0x1581568, 0x0, 0x0)
/Users/sandbox/.gotools/src/github.com/mgechev/revive/rule/time-naming.go:25 +0xb6
github.com/mgechev/revive/lint.(*File).lint(0xc01240f740, 0xc00019e700, 0x10, 0x10, 0x0, 0x3fe999999999999a, 0xc0000146e0, 0x7, 0xc00000f710, 0x0, ...)
/Users/sandbox/.gotools/src/github.com/mgechev/revive/lint/file.go:100 +0x36a
github.com/mgechev/revive/lint.(*Package).lint.func1(0xc00019e700, 0x10, 0x10, 0x0, 0x3fe999999999999a, 0xc0000146e0, 0x7, 0xc00000f710, 0x0, 0x0, ...)
/Users/sandbox/.gotools/src/github.com/mgechev/revive/lint/package.go:157 +0x94
created by github.com/mgechev/revive/lint.(*Package).lint
/Users/sandbox/.gotools/src/github.com/mgechev/revive/lint/package.go:156 +0x173
Desktop (please complete the following information):
Additional context
The repository tested is https://github.com/pingcap/tidb.
Describe the bug
Error while running tool: /Users/elvizlai/.godeps/bin/revive -config=/Users/elvizlai/.revive.toml -exclude=vendor/... -formatter friendly
cannot read the config file
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
vscode user config
"go.lintTool": "revive",
"go.lintFlags": [
"-config=~/.revive.toml -exclude=vendor/... -formatter friendly"
]
# flags
revive ...
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0
[rule.blank-imports]
[rule.context-as-argument]
[rule.context-keys-type]
[rule.dot-imports]
[rule.error-return]
[rule.error-strings]
[rule.error-naming]
# [rule.exported]
[rule.if-return]
[rule.increment-decrement]
# [rule.var-naming]
[rule.var-declaration]
[rule.package-comments]
[rule.range]
[rule.receiver-naming]
[rule.time-naming]
[rule.unexported-return]
[rule.indent-error-flow]
[rule.errorf]
Expected behavior
I run the cmd manual, it works. but not the vscode
Logs
Error while running tool: /Users/elvizlai/.godeps/bin/revive -config=/Users/elvizlai/.revive.toml -exclude=vendor/... -formatter friendly
cannot read the config file
Desktop (please complete the following information):
Additional context
Nothing
Describe the bug
When using the following struct tag -,
, which is valid according the json.Marshal
documentation, to encode a field with -
as the name, the linter struct-tag
reports the following warning: unknown option '' in JSON tag
Expected behavior
No warnings.
Desktop (please complete the following information):
Additional context
Example code:
type x struct {
H string `json:"-,"` // use "-" as key
}
I assume that a new value for the switch case located at https://github.com/mgechev/revive/blob/master/rule/struct-tag.go#L167 should be added for the empty string when the field name is -
.
In GO it is possible to define a method that does not use its receiver:
func (ms *myStruct) Foo() { return }
The above method can be also written to explicitly state that the receiver is not used:
func (_ *myStruct) Foo() { return }
or
func (*myStruct) Foo() { return }
I propose to create a new rule (unused-receiver
?) to cope with this pattern
Is your feature request related to a problem? Please describe.
In short, I'm looking for alternatives to golint
, particularly because of this issue:
golang/lint#89
Describe the solution you'd like
There are some options to implement this feature.
Id
, in order to name variables such as projectId
via configuration. This is the most flexible option, but may lead to other initialisms than Id
to be whitelisted, spreading inconsistency across Go projects.ID
completely from the initialism list. The comment on source code, similarly to Golint, states that "Freudian code is rare". However this rule creates warnings for several projects, such as herevar-naming
, so it can be enabled/disabled as whole.Describe alternatives you've considered
I've tried disabling the var-naming
rule, but other effective idiomatic rules get disabled as well, so that's not an option.
Additional context
The lack of flexibility in Golint rules is something that this project can offer, along the speed.
I'd more than happy to give it a try at this myself, if applicable.
It's OK when linter tells me I should document a package or a method, or a a constant. But I do not like when it tells me how I should document my code.
Would be nice to have separate checks for missing comments and comments format
Expressions that have too many indirections are hard to understand.
For example (from Kubernetes):
o.s.GA.ZoneOperations.Get(o.projectID, o.key.Zone, o.key.Name).Context(ctx).Do()
f.Signature.Receiver.Elem.Name
new(RouteBuilder).typeNameHandler(w.typeNameHandleFunc).servicePath(w.rootPath).Method("DELETE").Path(subPath)
Other than hard to read, this kind of code is fragile (calls and slice accesses are chained without checking for nil returns nor empty slices)
I propose to create a rule that warns when an expression has too many indirections (too many must be configurable)
Howdy!
In #66 , it seems the determination was to tag releases w/o publishing release binaries. That said, it doesn't look like there's any tags as-yet.
I'm looking to use Revive in CI builds, where ideally I can lock the version of revive so that a given build is consistent. Right now, I'm locking to the git hash, but that's not ideal.
Is the intention to start tagging releases / provide a changelog, as per that issue?
Describe the bug
indent-error-flow
gives false positive for:
func h(f func() bool, x int) string {
if err == author.ErrCourseNotFound {
return
} else if err == author.ErrCourseAccess {
// side effect
} else if err == author.AnotherError {
return "okay"
} else {
// side effect
}
}
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
indent-error-flow
if block ends with a return statement, so drop this else and outdent its block
Expected behavior
Should not get a warning in the case above because of the else if
statements.
Just a quick drive-by comment after having glanced at:
https://blog.mgechev.com/2018/05/28/revive-golang-golint-linter/
you're using // revive:xxx
to feed intructions to revive
:
// revive:disable
type Expression struct {
Value string
IsStar bool
IsVariadic bool
IsWriter bool
Underlying string
}
// revive:enable
usually, in the Go ecosystem, comments that are meant for programs or machines do not have a leading space:
//+build ignore
package foo
import "C"
//go:noinline
func Foo() {}
//export MyLib
func MyLib(v *C.char) C.int { ... }
now, the consensus is to use //go:xyz
as a directive for programs.
Just my 2-cents.
In order to lint all files, the linter needs to be able to use build tags. It will actually fail to run if all the files in a directory have build tags.
The rule indent-error-flow
checks if the if
block ends with a return
statement. The rule could also check for if
blocks ending with branch statements like continue
,break
, or (ouch!) goto
[I've made the dev and I can make a pull request if you are interested]
Note: the rule is part of the golint
-related rules, if the goal is to be isofunctional with golint then instead of extending the rule, a new one can be created.
Is your feature request related to a problem? Please describe.
I want to be able to run revive from my text editor without having to make my text editor find revive.toml to use.
Describe the solution you'd like
Have revive look up directories until it finds a revive.toml rather than only looking in home.
Describe alternatives you've considered
I can make vim find a revive.toml to use, but I'd rather rely on revive itself for that.
Hi @mgechev, I would love to develop some rules that need package-wide analysis (e.g. unused-function
) but I do not see how to implement the analysis pattern required by these kind of rules.
I've developed one package-wide rule (confusing-naming
). A raw description of the rule is: create a global registry of names, and for each linted file: a) enrich that list, and b) check new names vs those in the list.
Because two names are similar (confusing) no matter the order in which we find them, checks are independent of: the order in which package files are analyzed, and the global progress of the analysis.
That is not the case of, for example, unused-function
: checks can be done only when all files in the package were analyzed (it is not possible to flag a function as unused
until all files of the package have been analyzed). These kind of rules need something like a reduce phase after been applied to all package's files.
My understanding is that rules are applied as follows:
for each file in the package
go {
for each rule
rule.Apply(file)
send failures
}
Files are concurrently analyzed (and that is good ๐ ), thus at rule-level it is impossible to have information of the global (package-level) progress of the analysis.
Is my understanding wrong? Do you see how to implement a rule like unused-function
without modifying the linter logic?
Is your feature request related to a problem? Please describe.
The idiomatic way to create software is to publish releases when required (fix, new feature, api break...).
There is currently no versioning for revive
.
Describe the solution you'd like
I would like to suggest https://github.com/astrocorp42/rocket to automate GitHub releases.
Thus We can automate releasing and binaries publishing.
Furthermore it would enable automated Docker publishing which is great for users of services like drone.io.
It would requires a Makefile
(for example: https://github.com/astrocorp42/rocket/blob/master/Makefile), to build revive
.
Describe alternatives you've considered
https://github.com/goreleaser/goreleaser but it mix too many things (build + publishing).
Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
a rule, rule.var-naming have two effect, packakge name lint and variable name lint, I want to set them in two rule.
Describe the solution you'd like
A clear and concise description of what you want to happen.
add [rule.package-naming] setting to lint
Line 44 in 50f3790
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
no
Additional context
Add any other context or screenshots about the feature request here.
Describe the bug
The rule empty-lines
warns on blocks starting or ending with comments
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
Command line
./revive -config=test.toml -formatter friendly test.go
Configuration file test.toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0
[rule.empty-lines]
Test file test.go
// Test of empty-lines.
package fixtures
func w() {
select {
case <-time.After(dur):
// TODO: Handle Ctrl-C is pressed in `mysql` client.
// return 1 when SLEEP() is KILLed
}
return 0, false, nil
}
func x() {
if tagArray[2] == "req" {
bit := len(u.reqFields)
u.reqFields = append(u.reqFields, name)
reqMask = uint64(1) << uint(bit)
// TODO: if we have more than 64 required fields, we end up
// not verifying that all required fields are present.
// Fix this, perhaps using a count of required fields?
}
if err == nil { // No need to refresh if the stream is over or failed.
// Consider any buffered body data (read from the conn but not
// consumed by the client) when computing flow control for this
// stream.
v := int(cs.inflow.available()) + cs.bufPipe.Len()
if v < transportDefaultStreamFlow-transportDefaultStreamMinRefresh {
streamAdd = int32(transportDefaultStreamFlow - v)
cs.inflow.add(streamAdd)
}
}
}
Expected behavior
revive must not emit any warning.
Logs
Actual output:
โ empty-lines extra empty line at the end of a block
fixtures/test.go:7:2
โ empty-lines extra empty line at the end of a block
fixtures/test.go:18:3
โ empty-lines extra empty line at the start of a block
fixtures/test.go:24:16
โ 3 problems (0 errors, 3 warnings)
Warnings:
3 empty-lines
Describe the bug
revive:disable-line
and revive:disable-next-line
do not work with rule line-length-limit
when applied to comments, although they work fine with lines of code.
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
# flags
revive -config myconfig.toml
# config file
[rule.line-length-limit]
arguments = [80]
package main
import "fmt"
// revive:disable-next-line:line-length-limit
// The length of this comment line is over 80 characters, this is bad for readability.
func main() {
// revive:disable-next-line:line-length-limit
fmt.Println("This line is way too long. In my opinion, it should be shortened.")
}
Expected behavior
revive
should output nothing, since the line length rule was disable for both offending lines.
Desktop (please complete the following information):
Rule atomic
has a method gofmt
(attached to an internal struct).
It could be interesting to make this method accessible to other rules (eg constant-logical-expr
)
I propose to move the method into rule/utils.go
.
Maybe I got the documentation wrong, but I think the logic evaluating the ignoreGeneratedHeader
option is wrong.
When I set ignoreGeneratedHeader = true
it will check my generated files, if I set it to ignoreGeneratedHader = false
it will not check my generated files.
IMHO there should be no !
in here: https://github.com/mgechev/revive/blob/master/lint/linter.go#L66
Is that correct?
I would like to have a simple method to disable one check.
Let's say "exported"
Ideally, with command-arg, but toml config is okay too.
I can't figure out how to do so from the docs.
Calling runtime.GC()
to force garbage collection is rarely a good idea. In very few occasions triggering the GC is justified (when benchmarking for example) but most of the time is a symptom of the developer trying (and failing) to be smarter than the runtime.
We can add a rule to warn on the presence of runtime.GC()
This would be essential to usage with github checks API and pre-hook linters
My organization would like to use revive instead of golangci-lint (mostly because of extensibility) but I cannot tell if revive has this feature. It is best described through this quote from golangci-lint docs
Integration into large codebases. A good way to start using linters in a large project is not to fix a plethora of existing issues, but to set up CI and fix only issues in new commits. You can use revgrep for it, but it's yet another utility to install and configure. With golangci-lint it's much easier: revgrep is already built into golangci-lint and you can use it with one option (-n, --new or --new-from-rev).
source: https://github.com/golangci/golangci-lint#golangci-lint-vs-gometalinter
In GO, when using named return values it is possible to use naked (aka bare) returns.
For example:
func ReturnId() (id int, err error) {
id = 10
return id, err
}
can be written as
func ReturnId() (id int, err error) {
id = 10
return
}
This feature is somewhat controversial, because it can lead to hard-to-understand code, see for example
Why do not add a rule that warns on bare returns?
It would be nice to support a global (possibly default) configuration for revive
.
I would like to have an option to exclude certain files for specific rules.
Example - revive.toml config file:
[rule.cyclomatic]
arguments = [10]
exclude= "path/to/the/file.go"
Now when revive runs it will not check "path/to/the/file.go" for cyclomatic rule.
Describe the bug
After commit c878d30 disabling multiple rules at once require additional newline between comments.
// disable:revive:rule1
/* there should be newline, otherwise rule2 will be still enabled*/
// disable:revive:rule2
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
config.toml
:echo '
FROM golang:alpine
RUN apk add --no-cache git
RUN go get -u -v github.com/mgechev/revive
RUN mkdir -p /go/src/test
RUN echo "package main" > /go/src/test/main.go && \
echo "" >> /go/src/test/main.go && \
echo "// revive:disable:var-naming" >> /go/src/test/main.go && \
echo "// revive:disable:increment-decrement" >> /go/src/test/main.go && \
echo "func main() {" >> /go/src/test/main.go && \
echo " var invalid_name = 0" >> /go/src/test/main.go && \
echo " invalid_name += 1" >> /go/src/test/main.go && \
echo " println(invalid_name)" >> /go/src/test/main.go && \
echo "}" >> /go/src/test/main.go
RUN revive test' | docker build --no-cache -
package main
// revive:disable:var-naming
// revive:disable:increment-decrement
func main() {
var invalid_name = 0
invalid_name += 1
println(invalid_name)
}
revive ...
Expected behavior
Revive should exit with 0 RC and with empty output to STDERR and STDOUT.
Logs
main.go:7:3: should replace invalid_name += 1 with invalid_name++
Desktop (please complete the following information):
Additional context
Probably caused by new regexp:
Line 130 in c878d30
In GO it is possible to declare identifiers (packages, structs, interfaces, params, receivers, variables, constants...) that conflict with the name of an imported package.
for example (from Kubernetes):
package app
import (
...
"k8s.io/kubernetes/pkg/controller/certificates/signer"
...
)
func startCSRSigningController(ctx ControllerContext) (http.Handler, bool, error) {
...
signer, err := signer.NewCSRSigningController(...)
...
go signer.Run(1, ctx.Stop)
return nil, true, nil
}
The variable signer
shadows the import name signer
. This kind of shadowing makes the code harder to understand.
I propose to add a rule to spot identifiers that shadow an import.
Is your feature request related to a problem? Please describe.
Some warning/error messages generated by rules are not enough to clearly understand what is wrong in the code or, more important, why.
Linters are a good source of knowledge for beginners, we can learn from reported warnings/errors.
For example, a warning use i++ instead of i += 1
does not provide any clue about why we should prefer i++
Describe the solution you'd like
Add a document that describes the spirit of rules. The document may also describe rule configuration details.
Describe alternatives you've considered
Java's FindBugs checks documentation is a good example.
Using the new unhandled-error
has revealed cumbersome because in some cases it generates too many failures.
I propose to add the possibility of configuring the rule with a blacklist of functions for whom te rule must not generate a failure.
Is your feature request related to a problem? Please describe.
The problem is dropping this in as a replacement to golint
.
Describe the solution you'd like
revive looks very close to being a drop-in replacement for golint, but support for the -set_exit_status
flag that golint
implements would really help. We use this exit status to drive automation.
-set_exit_status
set exit status to 1 if any issues are found
Is your feature request related to a problem? Please describe.
This passes with no warnings:
package main
import "fmt"
func main() {
x := ((((1+2))))
fmt.Println(x)
}
Describe the solution you'd like
A warning about useless parentheses.
revive -formatter friendly github.com/cockroachdb/cockroach/pkg/...
โ unexported-return exported func NewZeroNode returns unexported type sql.planNode, which can be annoying to use
pkg/sql/zero.go:36:49
The code in question: https://github.com/cockroachdb/cockroach/blob/97c3622c2a6f4ec579ed93655b9d8a83732ac654/pkg/sql/zero.go#L36
It's true that the returned object is of an unexported type, but it is being returned as an interface, which is exported.
I would like the ability to provide a reason for rule disabling (on the same line) as an option. Adding anything to the revive:
comment breaks it (regex I guess?), so currently I must do it on a separate line.
I propose two things:
Ability to write comments like
// revive:disable-line:RULE This is intentional
or even (for cleaner integration with other tools)
// revive:disable:var-naming TODO: fix it later
Global config flag (disabled by default) that would require to provide the reason (like staticcheck
does by default)
Describe the bug
revive reports "comment on exported type T should be of the form "T ..." (with optional leading article)" when the comment on that type is of the form "/* T ... */" (with a leading space). Likewise for exported functions, and presumably ValueSpec entries as well.
To Reproduce
Steps to reproduce the behavior:
go get -u github.com/mgechev/revive
(commit fa5acbc, from 31 Aug)revive -config exported.toml exported.go
[rule.exported]
package exported
/* Exported is an exported structure. */
type Exported struct {
p int
}
Expected behavior
I expected no warning. If I change the comment style to //, the warning does not occur.
Desktop (please complete the following information):
Additional context
It is easy to fix with strings.TrimSpace() in the obvious spots in exported.go, but I could not figure out how to add a regression test, so I did not file this as a PR. I would be happy to do that, but the problem is small enough that it may be easier to do it than to walk me through adding a suitable regression test.
When revive finds no errors it prints:
0 problems (0 errors) (0 warnings)
This violates the Rule of Silence:
The rule of silence, also referred to as the silence is golden rule, is an important part of the Unix philosophy that states that when a program has nothing surprising, interesting or useful to say, it should say nothing. It means that well-behaved programs should treat their users' attention and concentration as being valuable and thus perform their tasks as unobtrusively as possible. That is, silence in itself is a virtue. (http://www.linfo.org/rule_of_silence.html)
What would you say to a PR that made revive silent by default?
My project is rather big, and already has lots of warnings raised by revive. I am using git.
My goal is both to avoid creating new debt items, and to reduce current debt. However, "new debt items" are lost in the sea of "existing debt items", which makes them very difficult to spot.
Any of the following would help me identify new debt items:
revive -formatter friendly -config defaults.toml -ignore debt_19032019 src/...
revive -formatter friendly -config defaults.toml -ignore origin/dev src/...
revive -formatter friendly -config defaults.toml -ignore c458fe3a src/...
Describe the bug
It is not with a stable go version but the bug with the new version of go that is expected to be released end of this year.
To Reproduce
Steps to reproduce the behavior:
go get github.com/mgechev/revive
# config file
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0
[rule.blank-imports]
[rule.context-as-argument]
[rule.context-keys-type]
[rule.dot-imports]
[rule.error-return]
[rule.error-strings]
[rule.error-naming]
[rule.exported]
[rule.if-return]
[rule.increment-decrement]
[rule.var-naming]
[rule.var-declaration]
[rule.package-comments]
[rule.range]
[rule.receiver-naming]
[rule.time-naming]
[rule.unexported-return]
[rule.indent-error-flow]
[rule.errorf]
[rule.empty-block]
[rule.superfluous-else]
[rule.unused-parameter]
[rule.unreachable-code]
[rule.redefines-builtin-id]
Expected behavior
It should get installed, actually it used to work before mgechev/dots
got updated, i think it is failing because of version validation https://tip.golang.org/doc/go1.13#version-validation
Logs
14.50s$ make install_revive
>> ============= Install Revive ============= <<
go get github.com/mgechev/revive
go: finding github.com/mgechev/revive latest
go: downloading github.com/mgechev/revive v0.0.0-20190816211937-5806359b5998
go: extracting github.com/mgechev/revive v0.0.0-20190816211937-5806359b5998
go get: github.com/mgechev/[email protected] requires
github.com/mgechev/[email protected]: invalid pseudo-version: does not match version-control timestamp (2018-12-28T16:47:30Z)
Desktop (please complete the following information):
master (Latest Version)
Additional context
Build URL https://travis-ci.org/Clivern/Rabbit/jobs/572940769
Is your feature request related to a problem? Please describe.
In the https://github.com/mgechev/revive/blob/master/README.md length has the incorrect spelling.
line-lenght-limit
-> line-length-limit
Describe the solution you'd like
This should be spelled correctly.
Describe alternatives you've considered
n/a
Additional context
Is your feature request related to a problem? Please describe.
When I define my own revive.toml, it seems to disable all the rules. It'd be great if I could just tell it to use the rules from default.toml (with perhaps ability to turn off specific rules) so I don't have to change my config when revive adds new rules, but still have options like what error codes to return with.
To wit, golangci-lint. My org is leaning towards you but we're having trouble with the comparison process, and want input from the authors themselves.
Is there a comparison of your project vs golangci-lint somewhere?
Thank you! And anything that you point out that is positive in comparison should by all means go in your readme. For market share reasons.
Is your feature request related to a problem? Please describe.
Currently, we can enable/disable revive
with comments like
//revive:disable
//revive:disable-next-line:exported,random
But the enabling/disabling comment pattern does not allow to add a, sometimes useful, explanation of why the developer is enabling/disabling the linter.
Describe the solution you'd like
It could be nice if we can write enabling/disabling directives like
//revive:disable:cyclomatic - for the moment we accept this complex function
//revive:disable:deep-exit as discussed with Thom, in this case, it is not bad to exit from here
In GO it is possible to import the same package multiple times using different aliases.
For example
import (
"github.com/pkg/errors"
errs "github.com/pkg/errors"
)
Most of the time duplicated imports are generated when using goimports
.
Why do not create a rule that spots these cases?
Is there any policy about adding more rules to revive? Like "we want to keep them at minimum"? I am asking because I really like how fast revive works, but I am missing some checks that other linters have, like unused unexported symbols, empty branches or unchecked errors, and I would like to have those checks
i added an revive [file watchers] in goland
and it is not running...
Describe the bug
The following example fails:
func f10(*int) bool {
if x > 2 {
return true
}
}
This is because of the trailing rule taking the position if the if
statement, not the closing of the block into account.
Expected behavior
This is a false-positive and should not happen.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.