mgechev / revive Goto Github PK

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:

1. I updated revive go get -u github.com/mgechev/revive
2. I run it with the following flags & configuration file:

# 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):

• OS: macOS 10.13
• Go 1.11

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:

1. I updated revive go get -u github.com/mgechev/revive
2. I run it with the following flags & configuration file:

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):

• OS: mac os x
• Version of Go
  latest

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):

• linux/amd64
• go1.12.5

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.

1. We could provide a "whilelist" of allowed initialisms, such as 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.
2. Remove 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 here
3. Refactor initialisms into its own rule other than var-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:

1. I updated revive go get -u github.com/mgechev/revive
2. I run the rule indent-error-flow
3. I got a warning 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 ifblock 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

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:

1. I updated revive go get -u github.com/mgechev/revive
2. I run it with the following command line and configuration file:

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:

1. I updated revive go get -u github.com/mgechev/revive
2. I run it with the following flags & configuration file:

# 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):

• OS: Mint 19.1 (based on Ubuntu 18.04)
• Version of Go: 1.11.4

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

• Nate Finch's Go Nitpicks
• proposal: Go 2: remove bare return

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:

1. I updated revive go get -u github.com/mgechev/revive
2. Can be tested with docker only in isolated environment with default 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 -

3. Also can be tested on local machine for this file:

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):

• OS:
  • darwin
  • linux
• Version of Go:
  • go version go1.11.4 darwin/amd64
  • go version go1.11.4 linux/amd64

Additional context
Probably caused by new regexp:

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:

1. 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

2. 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:

1. I updated revive go get -u github.com/mgechev/revive (commit fa5acbc, from 31 Aug)
2. I run it with the following flags, configuration file, and Go source file:

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):

• OS: Ubuntu 19.04
• Tested with go1.10.4 and go1.13.

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:

• Dumping the results of my code at date t0, and use that file in revive to filter out preexisting bugs
  Example: revive -formatter friendly -config defaults.toml -ignore debt_19032019 src/...
• Let revive remove items that were part of the parent branch (if it can find it) or already present at a given commit / tag.
  Examples: 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:

1. I try to install revive on latest go go get github.com/mgechev/revive
2. I run it with the following configuration file:

# 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):

• OS: Ubuntu 16.04.6 LTS
• Version of Go 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?

• see #42
• Extensibility/customizability comparison
• Speed comparison
• Ease of integration with GH checks API comparison

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.