Giter Site home page Giter Site logo

mgechev / revive Goto Github PK

View Code? Open in Web Editor NEW
4.6K 36.0 265.0 5.74 MB

🔥 ~6x faster, stricter, configurable, extensible, and beautiful drop-in replacement for golint

Home Page: https://revive.run

License: MIT License

Go 99.82% Makefile 0.16% Dockerfile 0.02%
go golang linter static-analysis golint static-code-analysis hacktoberfest

revive's Introduction

Build Status

revive

Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. Revive provides a framework for development of custom rules, and lets you define a strict preset for enhancing your development & code review processes.


Logo by Georgi Serev

Here's how revive is different from golint:

  • Allows to enable or disable rules using a configuration file.
  • Allows to configure the linting rules with a TOML file.
  • 2x faster running the same rules as golint.
  • Provides functionality for disabling a specific rule or the entire linter for a file or a range of lines.
    • golint allows this only for generated files.
  • Optional type checking. Most rules in golint do not require type checking. If you disable them in the config file, revive will run over 6x faster than golint.
  • Provides multiple formatters which let us customize the output.
  • Allows to customize the return code for the entire linter or based on the failure of only some rules.
  • Everyone can extend it easily with custom rules or formatters.
  • Revive provides more rules compared to golint.

Who uses Revive

  • tidb - TiDB is a distributed HTAP database compatible with the MySQL protocol
  • grafana - The tool for beautiful monitoring and metric analytics & dashboards for Graphite, InfluxDB & Prometheus & More
  • etcd - Distributed reliable key-value store for the most critical data of a distributed system
  • cadence - Cadence is a distributed, scalable, durable, and highly available orchestration engine by Uber to execute asynchronous long-running business logic in a scalable and resilient way
  • ferret - Declarative web scraping
  • gopass - The slightly more awesome standard unix password manager for teams
  • gitea - Git with a cup of tea, painless self-hosted git service
  • excelize - Go library for reading and writing Microsoft Excel™ (XLSX) files
  • aurora - aurora is a web-based Beanstalk queue server console written in Go
  • soar - SQL Optimizer And Rewriter
  • pyroscope - Continuous profiling platform
  • gorush - A push notification server written in Go (Golang).
  • dry - dry - A Docker manager for the terminal.
  • go-echarts - The adorable charts library for Golang
  • reviewdog - Automated code review tool integrated with any code analysis tools regardless of programming language
  • rudder-server - Privacy and Security focused Segment-alternative, in Golang and React.
  • sklearn - A partial port of scikit-learn written in Go.
  • protoc-gen-doc - Documentation generator plugin for Google Protocol Buffers.
  • llvm - Library for interacting with LLVM IR in pure Go.
  • jenkins-library - Jenkins shared library for Continuous Delivery pipelines by SAP.
  • pd - Placement driver for TiKV.
  • shellhub - ShellHub enables teams to easily access any Linux device behind firewall and NAT.
  • lorawan-stack - The Things Network Stack for LoRaWAN V3
  • gin-jwt - This is a JWT middleware for Gin framework.
  • gofight - Testing API Handler written in Golang.
  • Beaver - A Real Time Messaging Server.
  • ggz - An URL shortener service written in Golang
  • Codeac.io - Automated code review service integrates with GitHub, Bitbucket and GitLab (even self-hosted) and helps you fight technical debt.
  • DevLake - Apache DevLake is an open-source dev data platform to ingest, analyze, and visualize the fragmented data from DevOps tools,which can distill insights to improve engineering productivity.
  • checker - Checker helps validating user input through rules defined in struct tags or directly through functions.
  • milvus - A cloud-native vector database, storage for next generation AI applications.

Open a PR to add your project.

Installation

go install github.com/mgechev/revive@latest

or get a released executable from the Releases page.

You can install the main branch (including the last commit) with:

go install github.com/mgechev/revive@master

Usage

Since the default behavior of revive is compatible with golint, without providing any additional flags, the only difference you'd notice is faster execution.

revive supports a -config flag whose value should correspond to a TOML file describing which rules to use for revive's linting. If not provided, revive will try to use a global config file (assumed to be located at $HOME/revive.toml). Otherwise, if no configuration TOML file is found then revive uses a built-in set of default linting rules.

Docker

A volume needs to be mounted to share the current repository with the container. Please refer to the bind mounts Docker documentation

docker run -v "$(pwd)":/var/<repository> ghcr.io/mgechev/revive:v1.1.2-next -config /var/<repository>/revive.toml -formatter stylish ./var/kidle/...
  • -v is for the volume
  • ghcr.io/mgechev/revive:v1.1.2-next is the image name and its version corresponding to revive command
  • The provided flags are the same as the binary usage.

Bazel

If you want to use revive with Bazel, take a look at the rules that Atlassian maintains.

Text Editors

GitHub Actions

Continuous Integration

Codeac.io - Automated code review service integrates with GitHub, Bitbucket and GitLab (even self-hosted) and helps you fight technical debt. Check your pull-requests with revive automatically. (free for open-source projects)

Linter aggregators

golangci-lint

To enable revive in golangci-lint you need to add revive to the list of enabled linters:

# golangci-lint configuration file
linters:
   enable:
     - revive

Then revive can be configured by adding an entry to the linters-settings section of the configuration, for example:

# golangci-lint configuration file
linters-settings:
  revive:
    ignore-generated-header: true
    severity: warning
    rules:
      - name: atomic
      - name: line-length-limit
        severity: error
        arguments: [80]
      - name: unhandled-error
        arguments : ["fmt.Printf", "myFunction"]

The above configuration enables three rules of revive: atomic, line-length-limit and unhandled-error and pass some arguments to the last two. The Configuration section of this document provides details on how to configure revive. Note that while revive configuration is in TOML, that of golangci-lint is in YAML.

Please notice that if no particular configuration is provided, revive will behave as go-lint does, i.e. all go-lint rules are enabled (the Available Rules table details what are the go-lint rules). When a configuration is provided, only rules in the configuration are enabled.

Command Line Flags

revive accepts the following command line parameters:

  • -config [PATH] - path to config file in TOML format, defaults to $HOME/revive.toml if present.

  • -exclude [PATTERN] - pattern for files/directories/packages to be excluded for linting. You can specify the files you want to exclude for linting either as package name (i.e. github.com/mgechev/revive), list them as individual files (i.e. file.go), directories (i.e. ./foo/...), or any combination of the three.

  • -formatter [NAME] - formatter to be used for the output. The currently available formatters are:

    • default - will output the failures the same way that golint does.
    • json - outputs the failures in JSON format.
    • ndjson - outputs the failures as stream in newline delimited JSON (NDJSON) format.
    • friendly - outputs the failures when found. Shows summary of all the failures.
    • stylish - formats the failures in a table. Keep in mind that it doesn't stream the output so it might be perceived as slower compared to others.
    • checkstyle - outputs the failures in XML format compatible with that of Java's Checkstyle.
  • -max_open_files - maximum number of open files at the same time. Defaults to unlimited.

  • -set_exit_status - set exit status to 1 if any issues are found, overwrites errorCode and warningCode in config.

  • -version - get revive version.

Sample Invocations

revive -config revive.toml -exclude file1.go -exclude file2.go -formatter friendly github.com/mgechev/revive package/...
  • The command above will use the configuration from revive.toml
  • revive will ignore file1.go and file2.go
  • The output will be formatted with the friendly formatter
  • The linter will analyze github.com/mgechev/revive and the files in package

Comment Directives

Using comments, you can disable the linter for the entire file or only range of lines:

//revive:disable

func Public() {}
//revive:enable

The snippet above, will disable revive between the revive:disable and revive:enable comments. If you skip revive:enable, the linter will be disabled for the rest of the file.

With revive:disable-next-line and revive:disable-line you can disable revive on a particular code line.

You can do the same on a rule level. In case you want to disable only a particular rule, you can use:

//revive:disable:unexported-return
func Public() private {
  return private
}
//revive:enable:unexported-return

This way, revive will not warn you for that you're returning an object of an unexported type, from an exported function.

You can document why you disable the linter by adding a trailing text in the directive, for example

//revive:disable Until the code is stable
//revive:disable:cyclomatic High complexity score but easy to understand

You can also configure revive to enforce documenting linter disabling directives by adding

[directive.specify-disable-reason]

in the configuration. You can set the severity (defaults to warning) of the violation of this directive

[directive.specify-disable-reason]
    severity = "error"

Configuration

revive can be configured with a TOML file. Here's a sample configuration with explanation for the individual properties:

# When set to false, ignores files with "GENERATED" header, similar to golint
ignoreGeneratedHeader = true

# Sets the default severity to "warning"
severity = "warning"

# Sets the default failure confidence. This means that linting errors
# with less than 0.8 confidence will be ignored.
confidence = 0.8

# Sets the error code for failures with severity "error"
errorCode = 0

# Sets the error code for failures with severity "warning"
warningCode = 0

# Configuration of the `cyclomatic` rule. Here we specify that
# the rule should fail if it detects code with higher complexity than 10.
[rule.cyclomatic]
  arguments = [10]

# Sets the severity of the `package-comments` rule to "error".
[rule.package-comments]
  severity = "error"

By default revive will enable only the linting rules that are named in the configuration file. For example, the previous configuration file makes revive to enable only cyclomatic and package-comments linting rules.

To enable all available rules you need to add:

enableAllRules = true

This will enable all available rules no matter of what rules are named in the configuration file.

To disable a rule, you simply mark it as disabled in the configuration. For example:

[rule.line-length-limit]
    Disabled = true

When enabling all rules you still need/can provide specific configurations for rules. The following files is an example configuration were all rules are enabled, with exception to those that are explicitly disabled, and some rules are configured with particular arguments:

severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

# Enable all available rules
enableAllRules = true

# Disabled rules
[rule.blank-imports]
    Disabled = true
[rule.file-header]
    Disabled = true
[rule.max-public-structs]
    Disabled = true
[rule.line-length-limit]
    Disabled = true
[rule.function-length]
    Disabled = true
[rule.banned-characters]
    Disabled = true

# Rule tuning
[rule.argument-limit]
    Arguments = [5]
[rule.cyclomatic]
    Arguments = [10]
[rule.cognitive-complexity]
    Arguments = [7]
[rule.function-result-limit]
    Arguments = [3]
[rule.error-strings]
    Arguments = ["mypackage.Error"]

Default Configuration

The default configuration of revive can be found at defaults.toml. This will enable all rules available in golint and use their default configuration (i.e. the way they are hardcoded in golint).

revive -config defaults.toml github.com/mgechev/revive

This will use the configuration file defaults.toml, the default formatter, and will run linting over the github.com/mgechev/revive package.

Custom Configuration

revive -config config.toml -formatter friendly github.com/mgechev/revive

This will use config.toml, the friendly formatter, and will run linting over the github.com/mgechev/revive package.

Recommended Configuration

The following snippet contains the recommended revive configuration that you can use in your project:

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

Rule-level file excludes

You also can setup custom excludes for each rule.

It's alternative for global -exclude program arg.

ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.blank-imports]
   Exclude=["**/*.pb.go"]
[rule.context-as-argument]
   Exclude=["src/somepkg/*.go", "TEST"]

You can use following exclude patterns

  1. full paths to files src/pkg/mypkg/some.go
  2. globs src/**/*.pb.go
  3. regexes (should have prefix ~) ~\.(pb|auto|generated)\.go$
  4. well-known TEST (same as **/*_test.go)
  5. special cases: a. * and ~ patterns exclude all files (same effect than disabling the rule) b. "" (empty) pattern excludes nothing

NOTE: do not mess with exclude that can be used at top level of TOML file, that mean "exclude package patterns", not "exclude file patterns"

Available Rules

List of all available rules. The rules ported from golint are left unchanged and indicated in the golint column.

Name Config Description golint Typed
context-keys-type n/a Disallows the usage of basic types in context.WithValue. yes yes
time-equal n/a Suggests to use time.Time.Equal instead of == and != for equality check time. no yes
time-naming n/a Conventions around the naming of time variables. yes yes
unchecked-type-assertions n/a Disallows type assertions without checking the result. no yes
var-declaration n/a Reduces redundancies around variable declaration. yes yes
unexported-return n/a Warns when a public return is from unexported type. yes yes
errorf n/a Should replace errors.New(fmt.Sprintf()) with fmt.Errorf() yes yes
blank-imports n/a Disallows blank imports yes no
context-as-argument n/a context.Context should be the first argument of a function. yes no
dot-imports n/a Forbids . imports. yes no
error-return n/a The error return parameter should be last. yes no
error-strings []string Conventions around error strings. yes no
error-naming n/a Naming of error variables. yes no
exported []string Naming and commenting conventions on exported symbols. yes no
if-return n/a Redundant if when returning an error. no no
increment-decrement n/a Use i++ and i-- instead of i += 1 and i -= 1. yes no
var-naming allowlist & blocklist of initialisms Naming rules. yes no
package-comments n/a Package commenting conventions. yes no
range n/a Prevents redundant variables when iterating over a collection. yes no
receiver-naming n/a Conventions around the naming of receivers. yes no
indent-error-flow []string Prevents redundant else statements. yes no
argument-limit int (defaults to 8) Specifies the maximum number of arguments a function can receive no no
cyclomatic int (defaults to 10) Sets restriction for maximum Cyclomatic complexity. no no
max-public-structs int (defaults to 5) The maximum number of public structs in a file. no no
file-header string (defaults to none) Header which each file should have. no no
empty-block n/a Warns on empty code blocks no yes
superfluous-else []string Prevents redundant else statements (extends indent-error-flow) no no
confusing-naming n/a Warns on methods with names that differ only by capitalization no no
get-return n/a Warns on getters that do not yield any result no no
modifies-parameter n/a Warns on assignments to function parameters no no
confusing-results n/a Suggests to name potentially confusing function results no no
deep-exit n/a Looks for program exits in funcs other than main() or init() no no
unused-parameter n/a Suggests to rename or remove unused function parameters no no
unreachable-code n/a Warns on unreachable code no no
add-constant map Suggests using constant for magic numbers and string literals no no
flag-parameter n/a Warns on boolean parameters that create a control coupling no no
unnecessary-stmt n/a Suggests removing or simplifying unnecessary statements no no
struct-tag []string Checks common struct tags like json, xml, yaml no no
modifies-value-receiver n/a Warns on assignments to value-passed method receivers no yes
constant-logical-expr n/a Warns on constant logical expressions no no
bool-literal-in-expr n/a Suggests removing Boolean literals from logic expressions no no
redefines-builtin-id n/a Warns on redefinitions of builtin identifiers no no
function-result-limit int (defaults to 3) Specifies the maximum number of results a function can return no no
imports-blocklist []string Disallows importing the specified packages no no
range-val-in-closure n/a Warns if range value is used in a closure dispatched as goroutine no no
range-val-address n/a Warns if address of range value is used dangerously no yes
waitgroup-by-value n/a Warns on functions taking sync.WaitGroup as a by-value parameter no no
atomic n/a Check for common mistaken usages of the sync/atomic package no no
empty-lines n/a Warns when there are heading or trailing newlines in a block no no
line-length-limit int (defaults to 80) Specifies the maximum number of characters in a line no no
call-to-gc n/a Warns on explicit call to the garbage collector no no
duplicated-imports n/a Looks for packages that are imported two or more times no no
import-shadowing n/a Spots identifiers that shadow an import no no
bare-return n/a Warns on bare returns no no
unused-receiver n/a Suggests to rename or remove unused method receivers no no
unhandled-error []string Warns on unhandled errors returned by function calls no yes
cognitive-complexity int (defaults to 7) Sets restriction for maximum Cognitive complexity. no no
string-of-int n/a Warns on suspicious casts from int to string no yes
string-format map Warns on specific string literals that fail one or more user-configured regular expressions no no
early-return []string Spots if-then-else statements where the predicate may be inverted to reduce nesting no no
unconditional-recursion n/a Warns on function calls that will lead to (direct) infinite recursion no no
identical-branches n/a Spots if-then-else statements with identical then and else branches no no
defer map Warns on some defer gotchas no no
unexported-naming n/a Warns on wrongly named un-exported symbols no no
function-length int, int (defaults to 50 statements, 75 lines) Warns on functions exceeding the statements or lines max no no
nested-structs n/a Warns on structs within structs no no
useless-break n/a Warns on useless break statements in case clauses no no
banned-characters []string (defaults to []string{}) Checks banned characters in identifiers no no
optimize-operands-order n/a Checks inefficient conditional expressions no no
use-any n/a Proposes to replace interface{} with its alias any no no
datarace n/a Spots potential dataraces no no
comment-spacings []string Warns on malformed comments no no
redundant-import-alias n/a Warns on import aliases matching the imported package name no no
import-alias-naming string or map[string]string (defaults to allow regex pattern ^[a-z][a-z0-9]{0,}$) Conventions around the naming of import aliases. no no
enforce-map-style string (defaults to "any") Enforces consistent usage of make(map[type]type) or map[type]type{} for map initialization. Does not affect make(map[type]type, size) constructions. no no
enforce-slice-style string (defaults to "any") Enforces consistent usage of make([]type, 0) or []type{} for slice initialization. Does not affect make(map[type]type, non_zero_len, or_non_zero_cap) constructions. no no
enforce-repeated-arg-type-style string (defaults to "any") Enforces consistent style for repeated argument and/or return value types. no no
max-control-nesting int (defaults to 5) Sets restriction for maximum nesting of control structures. no no
comments-density int (defaults to 0) Enforces a minumum comment / code relation no no

Configurable rules

Here you can find how you can configure some existing rules:

var-naming

This rule accepts two slices of strings, an allowlist and a blocklist of initialisms. By default, the rule behaves exactly as the alternative in golint but optionally, you can relax it (see golint/lint/issues/89)

[rule.var-naming]
  arguments = [["ID"], ["VM"]]

This way, revive will not warn for identifier called customId but will warn that customVm should be called customVM.

Available Formatters

This section lists all the available formatters and provides a screenshot for each one.

Friendly

Friendly formatter

Stylish

Stylish formatter

Default

The default formatter produces the same output as golint.

Default formatter

Plain

The plain formatter produces the same output as the default formatter and appends URL to the rule description.

Plain formatter

Unix

The unix formatter produces the same output as the default formatter but surrounds the rules in [].

Unix formatter

SARIF

The sarif formatter produces outputs in SARIF, for Static Analysis Results Interchange Format, a standard JSON-based format for the output of static analysis tools defined and promoted by OASIS.

Current supported version of the standard is SARIF-v2.1.0.

Extensibility

The tool can be extended with custom rules or formatters. This section contains additional information on how to implement such.

To extend the linter with a custom rule you can push it to this repository or use revive as a library (see below)

To add a custom formatter you'll have to push it to this repository or fork it. This is due to the limited -buildmode=plugin support which works only on Linux (with known issues).

Writing a Custom Rule

Each rule needs to implement the lint.Rule interface:

type Rule interface {
	Name() string
	Apply(*File, Arguments) []Failure
}

The Arguments type is an alias of the type []interface{}. The arguments of the rule are passed from the configuration file.

Example

Let's suppose we have developed a rule called BanStructNameRule which disallow us to name a structure with given identifier. We can set the banned identifier by using the TOML configuration file:

[rule.ban-struct-name]
  arguments = ["Foo"]

With the snippet above we:

  • Enable the rule with name ban-struct-name. The Name() method of our rule should return a string which matches ban-struct-name.
  • Configure the rule with the argument Foo. The list of arguments will be passed to Apply(*File, Arguments) together with the target file we're linting currently.

A sample rule implementation can be found here.

Using revive as a library

If a rule is specific to your use case (i.e. it is not a good candidate to be added to revive's rule set) you can add it to your own linter using revive as linting engine.

The following code shows how to use revive in your own application. In the example only one rule is added (myRule), of course, you can add as many as you need to. Your rules can be configured programmatically or with the standard revive configuration file. The full rule set of revive is also actionable by your application.

package main

import (
	"github.com/mgechev/revive/cli"
	"github.com/mgechev/revive/lint"
	"github.com/mgechev/revive/revivelib"
)

func main() {
	cli.RunRevive(revivelib.NewExtraRule(&myRule{}, lint.RuleConfig{}))
}

type myRule struct{}

func (f myRule) Name() string {
	return "myRule"
}

func (f myRule) Apply(*lint.File, lint.Arguments) []lint.Failure { ... }

You can still go further and use revive without its cli, as part of your library, or your own cli:

package mylib

import (
	"github.com/mgechev/revive/cli"
	"github.com/mgechev/revive/revivelib"
	"github.com/mgechev/revive/lint"
)

// Error checking removed for clarity
func LintMyFile(file string) {
	conf, _:= config.GetConfig("../defaults.toml")

	revive, _ := revivelib.New(
		conf,  // Configuration file
		true,  // Set exit status
		2048,  // Max open files

		// Then add as many extra rules as you need
		revivelib.NewExtraRule(&myRule{}, lint.RuleConfig{}),
	)

	failuresChan, err := revive.Lint(
 		revivelib.Include(file),
 		revivelib.Exclude("./fixtures"),
 		// You can use as many revivelib.Include or revivelib.Exclude as required
 	)
  	if err != nil {
  	 	panic("Shouldn't have failed: " + err.Error)
  	}

  	// Now let's return the formatted errors
	failures, exitCode, _ := revive.Format("stylish", failuresChan)

  	// failures is the string with all formatted lint error messages
  	// exit code is 0 if no errors, 1 if errors (unless config options change it)
  	// ... do something with them
}

type myRule struct{}

func (f myRule) Name() string {
	return "myRule"
}

func (f myRule) Apply(*lint.File, lint.Arguments) []lint.Failure { ... }

Custom Formatter

Each formatter needs to implement the following interface:

type Formatter interface {
	Format(<-chan Failure, Config) (string, error)
	Name() string
}

The Format method accepts a channel of Failure instances and the configuration of the enabled rules. The Name() method should return a string different from the names of the already existing rules. This string is used when specifying the formatter when invoking the revive CLI tool.

For a sample formatter, take a look at this file.

Speed Comparison

Compared to golint, revive performs better because it lints the files for each individual rule into a separate goroutine. Here's a basic performance benchmark on MacBook Pro Early 2013 run on kubernetes:

golint

time golint kubernetes/... > /dev/null

real    0m54.837s
user    0m57.844s
sys     0m9.146s

revive

# no type checking
time revive -config untyped.toml kubernetes/... > /dev/null

real    0m8.471s
user    0m40.721s
sys     0m3.262s

Keep in mind that if you use rules which require type checking, the performance may drop to 2x faster than golint:

# type checking enabled
time revive kubernetes/... > /dev/null

real    0m26.211s
user    2m6.708s
sys     0m17.192s

Currently, type checking is enabled by default. If you want to run the linter without type checking, remove all typed rules from the configuration file.

Overriding colorization detection

By default, revive determines whether or not to colorize its output based on whether it's connected to a TTY or not. This works for most use cases, but may not behave as expected if you use revive in a pipeline of commands, where STDOUT is being piped to another command.

To force colorization, add REVIVE_FORCE_COLOR=1 to the environment you're running in. For example:

REVIVE_FORCE_COLOR=1 revive -formatter friendly ./... | tee revive.log

Contributors

renovate[bot] mgechev chavacava renovate-bot xuri denisvmedia
renovate[bot] mgechev chavacava renovate-bot xuri denisvmedia
mfederowicz doniacld Clivern morphy2k bernhardreisenberger alexandear
mfederowicz doniacld Clivern morphy2k bernhardreisenberger alexandear
dshemin butuzov rawen17 sina-devel tymonx mdelah
dshemin butuzov rawen17 sina-devel tymonx mdelah
ldez gsamokovarov comdiv heynemann cce mapreal19
ldez gsamokovarov comdiv heynemann cce mapreal19
zimmski shmsr git-hulk ccoVeille tamird markelog
zimmski shmsr git-hulk ccoVeille tamird markelog
mihaitodor dvejmz damif94 abeltay zeripath Groxx
mihaitodor dvejmz damif94 abeltay zeripath Groxx
StephenBrown2 qascade ridvansumset rliebz rdeusser rmarku
StephenBrown2 qascade ridvansumset rliebz rdeusser rmarku
rnikoopour rafamadriz paco0x weastur pa-m cinar
rnikoopour rafamadriz paco0x weastur pa-m cinar
natefinch flesser y-yagi techknowlogick okhowang meanguy
natefinch flesser y-yagi techknowlogick okhowang meanguy
likyh kerneltravel jmckenzieark haya14busa fregin ydah
likyh kerneltravel jmckenzieark haya14busa fregin ydah
WillAbides heyvito scop vkrol Jarema tartale
WillAbides heyvito scop vkrol Jarema tartale
tmzane felipedavid euank Juneezee echoix EXHades
tmzane felipedavid euank Juneezee echoix EXHades
petethepig Dirk007 yangdiangzb derekperkins bboreham attiss
petethepig Dirk007 yangdiangzb derekperkins bboreham attiss
Aragur kulti Abirdcfly abhinav r-ricci nunnatsa
Aragur kulti Abirdcfly abhinav r-ricci nunnatsa
michalhisim mathieu-aubin martinsirbe avorima very-amused johnrichardrinehart
michalhisim mathieu-aubin martinsirbe avorima very-amused johnrichardrinehart
walles jefersonf jan-xyz jamesmaidment grongor tie
walles jefersonf jan-xyz jamesmaidment grongor tie
quasilyte davidhsingyuchen gburanov ginglis13
quasilyte davidhsingyuchen gburanov ginglis13

License

MIT

revive's People

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

revive's Issues

config file discovery

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.

add package-naming setting rather var-naming

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

Failure: "don't use an underscore in package name",

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.

Separate checks for missing comments and comments format

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

Extending indent-error-flow to check for other branch statements

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.

Add a rule to blacklist imports (imports-blacklist)

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.

Detect duplicated imports

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?

Imports order

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.

Warn on unused method receivers

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

Implement -set_exit_status

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

revive: unidiomatic `// revive:` syntax

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.

New rule: warn on bare/naked returns

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?

Tagged version

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?

Support comments when enabling/disabling

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 

Add option to enable all lints in the command line

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.

panic: assertion failed [recovered]

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.

Add SemVer releases

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

indent-error-flow reports a false positive

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.

Feature Request: Customize initialisms or remove "ID" from the common list

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.

Warn on presence of too many indirections

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)

Not clear how to disable one check

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.

Version validation failure for mgechev/dots

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

Ignore preexisting technical debt

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

Warn on unhandled error.

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.

Ability to provide a reason for rule disabling

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)

Inheriting rules from default.toml

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.

Can revive only report problems on git changes?

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

Rules documentation

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.

support build tags

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.

vscode error

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

Detect import name shadowing

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.

Package-wide analysis

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?

Add blacklist to unhandled-error rule

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.

Warn about unneeded parentheses

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.

lint: Disabling multiple rules require additional newline

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

re := regexp.MustCompile(`^\s*revive:(enable|disable)(?:-(line|next-line))?(:)?([^\s]*)?(\s|$)`)

Exclude files for specific rules

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.

Adding more rules?

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

struct-tag linter false positive for empty options

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

Revive violates the Rule of Silence

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?

empty-lines, false positives in presence of comments

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

Comparison with other linters

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.

Add a rule to warn on explicit triggers of the garbage collector

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

False positives for "exported" rule

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.

Edge cases in empty-lines rule

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.

disable-(next-)-line does not work for long comment 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

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.