Linters in Go. How to cook them. Denis Isaev

I propose to get acquainted with the transcript of the report of Denis Isaev jirfag "Linters in Go. How to prepare them."

In go 50+ linters: what is their profit and how to effectively integrate them into the development process? The report will be useful both to those who have not yet used linters, and to those who are already using them: I will reveal little-known tricks and practices of working with linders.

Who cares, I ask under the cat.

Hey. My name is Dennis Isaev. We will talk about how to cook linters in Go. The report will be of interest both to beginners who have not yet used linters, as well as to professionals. I will tell you about some obscure stunts.

Something about me. I am the author of the opensource project Golangci-lint. Worked in mail.ru. Now I work TeamLead in the backend in Yandex.Taxi. My report is based on the experience of communicating with hundreds of Golangci-lint users. How they used linter, what difficulties they had and how to implement Go linters in mail.ru and Yandex companies.

We will highlight 5 key issues in the report.

I quite often see that in general linter do not use. Raise your hands to those who use the linter in all projects without exception. Not all.

Let's talk about why they are not used. Most often, when I ask why you guys don't use it, they say that the linter interfere with us. They only slow down development. There is nothing good about them. In part, this is true. If you do not know the details of the settings, they can really interfere. We will talk about this a little later.

In addition, they often think that the linter finds only a small thing, something stylistic, some kind of non-critical bugs, and in fact it is not worth it. Is it easier to spend time?

Immediately counterexamples. A bug in Docker was found using go vet. Forgot call to the Cancel function. Because of this, the background gorutina may not complete.

Bug in Etcd project. A cool go-critic linter found that the strings.HasPrefix argument is confused. This means that the test for the unsafe HTTP protocol will not work.

In Go itself, the bug that i element is compared with the i-th, although it should be compared with the j-th. Also found by linders.

Three examples in large opensource projects found by linders. Some will ask: Ok and what? He finds some critical bugs. One critical bug finds 100 non-critical or false positives. I can give my empirical statistics: I usually have about 80 percent of all the problems that the linters have to do: some stylistic issues, for example, variables are not used and so on, 15 percent are real bugs and somewhere 5 percent are false positives.

Now let's talk about why linters are needed. The biggest bonus is that the linters save time. And time is money. The sooner you find bugs, the cheaper they are for your company. On the slide graph of the approximate cost of fixing the bug depending on the stage where it is found. Accordingly, from each stage of development to finishing production the cost increases threefold. Find bugs early, ideally in IDE and save company money.

It often happens that developers will have problems with CodeReview that some linters could find. Why do they do this is not clear. First of all, the author of the code needs to wait until he passes the CodeReview. Secondly, the reviewer himself needs to spend time to find some mechanical problems. He could trust that to the linter. When I notice this, I always force it and we agree on the team that all that can be found is a linter, we are not looking to review with our eyes. Do we trust all of this to the linter? Moreover, if we find some problems that are often in review and there are no linkers for them, we try to find linkers who could catch them in theory. So that we do not waste time on review.

Linters allow us to somehow guarantee and have a predictable quality of the code in the project. For example, in this case it is unused function arguments.

Linters allow you to find critical bugs as soon as possible, to save time CodeReview. At the same time guarantee some quality code for the project.

There are more than 50 linters in Go, but the most popular ones are 4. These are those that are on the slide. They are used simply because they are cool. With the rest usually do not want to understand. I now want to show with examples what kind of linters are there at all. I want to demonstrate 25 linter examples. Now will probably be the most important in the report.

Let's start with the linters checking formatting. Gofmt is essentially not a linter. But we can consider it as a linter. He knows how to say we lack the translation of strings, somewhere extra spaces. In general, it is a standard for testing and maintaining code formatting.

Also, gofmt has a little-known option -s, which allows you to simplify expressions.

Goimports has the same content that gofmt contains, but in addition it still knows how to reorder the import, remove and add the necessary imports.

Unindent is such a great linter that can lower the level of code nesting. In this case, he tells us that if we combine two if into one, then we will have to lower by one the level of nesting.

Consider the linters checking the complexity of the code. The coolest of them is gocyclo. He's the most boring. Many hate him. It checks the cyclomatic complexity of the code and swears when this function complexity exceeds a certain threshold. The threshold is configurable. If simplistic, cyclomatic complexity is the amount of if in the code. Here he is too big and linter swears.

Nakedret is such a linter who can say that you use return without values ​​and at the same time you use it in a function that is too long. According to the official guide such return is not recommended.

There is a group of linter checking style. For example, gochecknoglobals checks that you are not using global variables. Of course they should not be used.

Golint swears at the same apiUrl variable. Says that url should be used in capital letters. Since this abbreviation.

Gochecknoinits makes sure that you do not use init functions. Init functions for certain reasons should not be used.

Gosimple cool linter. Part of staticheck or megacheck. Inside it contains a huge number of patterns to simplify the code. In this case, you may notice that strings.HasPrefix is ​​not needed, since strings.TrimPrefix already contains the necessary checks inside and you can remove if.

Goconst verifies that you do not have duplicate string literals in your code that you could put into constants. The number of these repetitions are customized. In this case, two.

Misspell linter, which checks that you have no typos in the code in the comments. In this case, on the slide there is a typo to the word else in the text of the comment. You can customize the dialect of English: American, British.

Unconvert a linter that checks that you do not do extra conversions. In this case, the variable is already of type string. It makes no sense to convert it.

Now let's take a look at the linters that check for unused code. First is the varcheck. It checks unused variables.

Unused is able to swear on unused fields of structures.

Deadcode tells us if the type is not used.

Or not used function.

Unparam can report when function arguments are not used in the function body itself.

Ineffassign reports when change is not used further in the code. This is either the result of some kind of refactoring. Somewhere they forgot to clean something, or a bug. In this example, count is incremented. In this case, no further use. This is very similar to the bug.

There is a group of linters checking performance. For example, maligned tells us that this testStruck structure can be compressed in size by reordering the fields. Moreover, if you run it as part of golangci-lint, it has the option to print the necessary order of fields at once so that you do not select them yourself.

There is such a cool gocritic linter. He has many checks inside. One of them is hugeParam. She can report on copying heavy data structures. In this case, the heavyStruct is copied by value and we just need to pass it as a pointer.

Prealloc is able to find us places in the code in which we can preallocate slice. He finds it so that he is looking for where we do constant hourly iterations on slice. And in them append affairs. In this case, you can preallocate the variable ret for the length of slice ss and save memory and CPU.

And finally the linter who find the bugs. Scopelint probably finds the most common mistake of newbies in go is capturing the variable of the loop cycle by reference. In this case, the loop variable arg is captured by reference. At the next iteration, there will be another value.

Staticcheck. Formerly called megacheck. Now it has been renamed. Because of this, there is so little confusion in the community. Staticcheck can find tons of various bugs. This is a very cool thing. Like go vet. One of them on the slide is a race. We need to increment sync.WaitGroup, of course, before entering the mountain.

Go vet finds mostly bugs. In this case, the variable i is compared so that the result will always be true. Therefore, there is obviously a bug. In general, you should always use go vet.

Gosec stands for go security. Finds potential security issues in Go. In this case, we may have user data in arg. Therefore, it can penetrate the rm shell command. And here there may be a shell in action for example. I note that go security quite often returns false positive. Therefore, I turn it off sometimes.

Errchek finds the places where we forgot error checking. A good, safe programming style is always everywhere to check all errors.

Separately, should note two linter is staticcheck and go-critic. Because inside each of them contains dozens, if not hundreds, of checks. So be sure to check them out.

Now we looked at 25 linter examples. And I also said that we have over 50 linters in Go. What to use? I advise usually to use everything to the maximum. Just turn on all the linters you can. And then spend an hour and start turning them off one by one. Turn off those that seem insignificant to you. For example, he finds you some performance optimizations that you are not interested in at all. You will spend an hour and create your own list of linters for yourself and you can live with it further.

The full catalog of all linters is on the link on the slide.

Let's talk about how to run linters. Sometimes the linters are run using such a makefile. The problem is that it works slowly. It is all done consistently.

We can do execution in parallel through xargs -P. Here too, the problem remains. First, it's just 4 linter. And already 10 lines of code. What will happen if we include 20 linters. Secondly, this parallelization is, to put it mildly, not the most optimal.

The gometalinter comes to the rescue. Gometalinter is such a linter aggregator that it can run literally in a couple of commands. On the slide, the command of launching the same linters is similar to the previous slide. But they do not need to be independently installed and there is no need for shamanism with a parallel launch. Gometalinter already under the hood all parallelizes. But he has one fundamental problem. It runs each linter as a separate process. Fork it. If we add to this the knowledge that each linter within himself spends 80 percent of the time on parsing the code and only 20 percent on the analysis itself, it turns out that 80 percent of the work we are wasting. And do not reuse the data. We could parse the program 1 time and then feed 50 linters.

Fortunately, there is golangci-lint, which does exactly that. He once parsit. Once gets types. Further, analyzers drive them away. Due to this, it works much faster. Similar launch command on slide.

You can see the graph on one of my projects 30 thousand lines of code. A small project and only 4 linter. You can see there a huge difference in the speed of work at times, both between the sequential launch, and between the gometalinter and golangci-lint. If these linters are not 4, but 20, then the difference will be much larger.

An important clarification about the gometalinter. C on April 7, the author of the project gometalinter announcing it deprecated. The repository is archived and everyone is advised to switch to golangci-lint, as it is faster, it has more buns. For example, support for go modules and so on.

And besides the performance and Go modules in golangci-lint, there are such advantages as the YAML configuration, the ability to somehow skip warnings, exclude them, and so on.

Golangci-lint is configured using the golangci-lint.yaml file. An example of this file with a description of all the options is on the link on the slide. Consider, for example, the linters-settings section. In this section, we consider the goimports configuration. It has such a rare local-prefixes option. In it you can specify the path to the current project. In this case, for example, github.com/local/repo.

When goimports will see local imports in github.com/local/repo, he will notice that they are in a separate section.

That they were at the very end. So that they are separate from all external imports. This makes it easier to visually distinguish external from internal imports. If he notices that this is not the case, he will curse.

And if you also use the golangci-lint run --fix option, then golangci-lint will also fix it for you automatically and re-sort the imports.

Let's talk about what linters are in the terminology of golangci-lint. Linters are divided into fast and slow. The fast ones are called fast, marked with the fast flag in help. They differ in that fast linters require a rather limited representation of the program, for example, an AST tree and some type information. While copper linters still additionally require SSA representation by the program and reuse the cache less. There are only six slow liners. They are marked on the slide. There are certain cases when it makes sense to run only fast linters.

You can notice the difference in speed. It is a whopping three times between a quick start and a slow one. Actually golangci-lint run - fast only fast linters are launched.

About build cache. There is such a thing as build cache. This is the cache that is built by the Go binary when compiling a program when loading types, so that the next time this compilation is faster. The same cache is reused by the linters to parse the program for building type information. You may notice that if the cache is cleared, the first fresh launch will be quite long. And the next one will be 3 times faster. Pay attention to your first launch linter on your project. It will always be much slower.

Here you can conclude that there is a point in CI between CI launches to reuse your cache. You will not only accelerate the linters, you will also accelerate the launch of tests, just a compilation and maybe something else. I advise everyone.

I can not talk about go analysis. This is a new framework that has appeared since Go 1.12. It unifies interfaces in such a way that linters become easy to write, linters are easy to use, run. Go vet starts 1.12 in general, moved entirely to go analysis. It is obvious that this is the future. That it will greatly change the whole Go ecosystem. But for now, it’s quite early to talk about it. So what will happen next? Because I saw only a few linters for go analysis and almost none of the existing ones have yet passed to it.

If you make a brief conclusion on the section on how to run linters, I advise everyone to use golangci-lint. You will quickly launch linters quickly. You do not need to shamanize with other instructions, commands.

Let's talk about how to implement linters in the project. I think everyone there who tried to implement linters faced such a problem. Here you have a project for a million lines of code with history. You convinced TeamLead to implement linters. Run and see a million messages. Understand that you sit all the time weeks to fix it all. What to do? You can just give up and throw it all. Or you can think of something.

First, the easiest option, you can try to exclude any comments from the linters in the text on the regular schedule using the golangci-lint.yaml config. If you see that the linkers scold on the comments, and you don't care about these comments in general, you can add to the exceptions.

You can exclude ways. For example, you have a third-party directory and your code is not there. You do not need to check it. You can exclude by file names.

If you do not want to exclude the entire file, you can exclude the function with nolint before the function. For nolint, you can specify a list of linters through the colon for which it acts as an exception. Either do not specify, then all the linters will be ignored.

When to use nolint? For example, I use nolint: deepguard, which knows how to blend imports, i.e. Imports cannot be used. I messed up the import of the logrus library in order not to accidentally use it instead of my desired logger. But in my very logger I use logrus. Therefore, I need only in one place in the project in only one file to do from import. I tag it with nolint.

Suppose you did it all, exclude added, nolint put down. We see that there are still thousands of messages left. Fix it a few days. There is a cool hack. Consider an example. There is a main.go file in which the 5th line was added a long time ago, and the 6th line was added only today. What can we do?

We can use revgrep. Revgrep allows us to specify a git revision, after which you need to search for bugs. That is, leave a message linter only after a given revision. If the 6th line is changed after the origin master, then he will only replenish it. And all the previous posts, line 5, he will not report. This is a very cool trick. It is with the help of it that you can embed a linter in any project in an hour. How we do it. We take to run there golangci-lint on a project of a million lines of code. He issues a thousand warnings. We have built a little bit, podshamanit. Then we agree with the team that right now we are doing a git revision or using hash commit. After which we do not make linter errors. But up to which we leave all the mistakes of the linter and until we rule them or rule slowly. We specify this this hash commit or tag in revgrep and start running CI. From now on, the linter will report to us only errors in the new code. With this old code, they will not react to errors. and so it’s possible to inject an linter into any project in an hour. This is exactly what I did in mail.ru when I implemented linters in huge projects.

Moreover, revgrep is already embedded in golangci-lint. Simply specify the option --new-from-rev or --new. Everyone must advise.

There is one more subtlety. Suppose we gradually, over time, fix all errors in the old code and remove the option --new altogether. We now have 20 linters, we run them. No new errors. At one point, a new linter is added. We want to launch this linter too. But he gives a lot of mistakes. What to do? If we add --new-from to all the linters, it will not be cool. We want to run all past linters on the entire project.

The solution is simple. We can run golangci-lint twice. Run it once on a new code with a new linter. The second time to run it entirely with all the old linters on the entire project. Such a trick helps greatly to introduce new linters when they go out.

We talked about the introduction of linter in any project. Now let's talk about the convenience of work. First, you need to achieve reproducibility in CI. As soon as you add a linter to the CI, you need it to be stable. Never do go get. Because it is not versioned. Linter at any time has changed, updated, and all your CI builds have started to fail. This I have seen dozens of times. Always use specific versions. Better with wget put it. She will be even faster. In addition, I do not recommend using the option --- enable-all for linters, because one day you update golangci-lint, for example, you add 5 new linters and you have all the builds starting to fail. Because you accidentally turned on these linters. It is better to explicitly prescribe what linters include.

Cool pre-commit hook. Who uses the pre-commit hook raise your hands? Pretty little. A pre-commit hook is a file in git that allows you to execute arbitrary code after you want to commit. But before this commit completes successfully. If the pre-commit hook returns an error to you, the commit will fail. There it is usually convenient to embed quick tests, static analysis, and so on. I advise everyone to embed golangci-lint. You can do it manually through a shell script. It is possible through the pre-commit utility. An example of how to configure on the slide. Set pre-commit with pip, a utility for installing Python packages. pip install pre-commit install config. Golangci-lint pre-commit.

--fast. . IDE . IDE . IDE --fast.

. CI . , : " , , ". . . CI. , CI, build fail build log, . , ? .

. , reviewer. github.com, gitlab.com Pull request. . . . build log. , . reviewdog. opensource. . .

reviewdog GolangCI, Code Climate, Hound. opensouce inline Pull Request. SonarQube.

goreportcard. . , , . .

- . . - golangci-lint. . 1 . , . golangci-lint CI, IDE pre-commit hook. C --new-from-rev . . reviewdog, github gitlab. . .

Thank you all for your attention. .

: , golangci-lint? .

: . golangci-lint golangci-lint.yaml, . .

: build cache . . .cache/downloads : 400 10. . .

: go dep - ?

: . go packages. . . .

: Travis, ?

: golangci-lint . CI golangci-lint --run.

: - , html-.

: junit, csv, json xml. .

: gometalinter . revive. . . , . revive.

: revive golint. . . , . Golangci-lint 30-50 . revive . Revive golint . Revive golint. . Revive golangci-lint.

: gocritic: hugeParam, . HEAP. , ? , , .

: . . , . , performance . .

: . . . , go package , , . . .

: idssue , golangci-lint.

: , , CI ? , .

: , 3 4 30. ? Unclear. , - . . . . .

: . C++ . . . , , . , . , , . , , , ? .

Answer: I had a plan. Thanks for the suggestion. I had plans to write a comprehensive article about this presentation, but there it’s more detailed and extensive throughout the year. Perhaps I will write in Russian and in English.