Several months ago, I started working on a Go linter (gomega-lint) for gomega. I’ve been using gomega at work for almost a year and love it now. I’ve formed some opinions on good practices for using gomega. But giving this kind of feedback during code reviews - it’s too late by that point. So, I wanted a Go linter to encourage these good practices.
Creating a new linter or linting rule in Go wasn’t immediately apparent. And I wanted to know how to test the linter. This post shares my lessons learned, and I hope it helps someone else in the future.
Let’s build a contrived rule to prevent invoking
fmt.Printf by using singlechecker.
Note: the final code can be found at go-lint-rule-demo
We’ll start by creating a new Go project that will have the following structure:
. ├── go.mod ├── go.sum └── internal └── rules ├── nofmtprintf.go ├── nofmtprintf_test.go └── testdata └── src └── nofmtprintf └── example.go
Run the following commands to initialize a new Go module and create the above file structure.
Afterward, you should have the same file structure as above.
Next, let’s download
golang.org/x/tools which contains singlechecker.
When I first started to build my rule, I took a naive approach to checking any function call matching “fmt.Printf”. Let’s start by doing that and then talk about the downsides.
I like to try to create failing tests before writing any source code, so let’s make
./internal/rules/nofmtfprintf_test.go with the following content:
This wires up analysistest, a tool used for testing rules. Ultimately, this will run our linting rule
testdata directory that we’ll now set up with an example code to lint.
Create a new file named
internal/rules/testdata/src/nofmtprintf/nofmtprintf.go with the following content:
This file is just a plain ol’ Go file. analysistest will execute our linting rule against this file.
analysistest will expect to get a linting error with a message of
Don't use fmt.Printf when linting
fmt.Printf("hello"). It will also
expect zero linting errors on all other lines.
We can execute our unit test via
go test ./....
Let’s now implement our naive rule. Let’s make a new file named
./internal/rules/nofmtprintf.go and populate it with:
A lot happens here if you’re unfamiliar with abstract syntax trees (AST). This code looks for function calls and then checks that the package name is “fmt” and that the function invoked is exactly named “Printf”.
So the naive implementation catches “fmt.Printf”. Unfortunately, there are several issues:
- dot imports of
fmtwon’t be caught
- aliasing the
fmtimport won’t be caught
- other packages or structs named fmt with a Printf function will be erroneously reported
Let’s update our example code first to prove this:
Our test will start failing as expected. So how do we avoid all of the false negatives and the permutations of aliases?
The parser has a nifty trick - it also identifies types! While analyzing a file, we can also look up information about the package’s imports.
So, what we can do is roughly:
- Look up the packages imported by the file under inspection
- Look for the fmt package in the list of imports
- Lookup the Printf function in fmt package when found
We can get the Printf function’s type once we’ve found the Printf function in the fmt package.
The magic ingredient here is that every usage of
fmt.Printf will have the same type. It’ll be the same for any alias and dot import.
Let’s update our rule to look up the “fmt.Printf” function and find matching function calls:
So, not only is our rule more accurate, but it’s simpler!
If you know an even better way, then please let me know!
Let’s build a binary so we can use this rule anywhere.
Create a new directory:
Note: there’s also golang.org/x/tools/go/analysis/multichecker if you want to include multiple rules in one binary.
Now we can build our executable with:
and run our executable like:
and we’ll see the same linting errors as our tests!
The singlechecker and multichecker come with great functionality out of the box. You can learn more via
./linter -h. One of the incredible options is the ability
to support auto fixes with
Linting is excellent for catching issues, but linters reach another level when they can automatically fix issues. Thankfully, the analysis package supports this as well. Our rule needs to provide suggested fixes for invalid code.
Our example linter rule that is going to replace
fmt.Printf arbitrarily with
Let’s start by updating our unit tests:
RunWithSuggestedFixes instead of
Run here. It looks the same as
RunWithSuggestedFixes does the following:
- Runs the linter to get a report of violations
- Applies any suggested fixes returned in the report
- Compares a fixed file to a
./internal/rules/testdata/src/nofmtfprintf/example.go.golden with the following content:
This file matches the content of our
./internal/rules/testdata/src/nofmtfprintf/example.go file, except the
fmt.Printf usages have been replaced with
TestNoFmtPrintfAutoFix test will start failing now.
One thing to note is the loop over
results is only so this test fails from the start. Without this logic, if a rule returns 0 suggested fixes, then
RunWithSuggestedFixes doesn’t compare to the
.golden file, so the unit test won’t fail.
Now let’s add support for suggested fixes by modifying
Suggested Fixes are a list of text edits to perform on the file. The text edit provides starting and end positions to replace with the new text.
In this case, we always replace it with
log.Printf. Once we build the linter, we can use
-fix for the suggested fixes to be automatically applied. Also,
at this point, our unit test will be passing.
There are a couple of gotchas with this approach to suggested fixes:
- missing imports aren’t added
- unused imports aren’t removed
Imagine the file wasn’t importing
log. Now we have a compilation issue.
On the flip side, imagine the file only imported
Printf. After all usages of
fmt.Printf are replaced with
log.Printf, we have an unused import causing another compilation issue.
I am still looking for a reliable way to modify the imports accordingly.
If you know a better way to implement rules or how to handle adding/removing imports, please connect on