Writing Useful go/analysis Linter

· Read in about 25 min · (5128 words) ·

We’re going to write useful linter for Go using go/analysis. Then we will integrate it into go vet and golangci-lint.

What to lint

Let’s find all printf-like functions that don’t end with f. It’s the Go convention to name such functions with f at the end: fmt.Sprintf, fmt.Errorf, fmt.Printf, fmt.Fprintf, fmt.Scanf, fmt.Sscanf, log.Printf, log.Fatalf, log.Panicf.

Writing a simple linter

Sample program with an issue

Let’s use the following example program for explanation and testing. Here myLog should be myLogf:

package main

import "log"

func myLog(format string, args ...interface{}) {
	const prefix = "[my] "
	log.Printf(prefix + format, args...)
}

What is AST

Tree.
Tree. Image from Medium article.

Usually, such issues in code are detected by analyzing AST. AST - abstract syntax tree.

Let’s visualize AST of our program by this online visualizer.

     0  *ast.File {
     1  .  Package: 1:1
     2  .  Name: *ast.Ident {
     3  .  .  NamePos: 1:9
     4  .  .  Name: "p"
     5  .  }
     6  .  Decls: []ast.Decl (len = 2) {
     7  .  .  0: *ast.GenDecl {
     8  .  .  .  TokPos: 3:1
     9  .  .  .  Tok: import
    10  .  .  .  Lparen: -
    11  .  .  .  Specs: []ast.Spec (len = 1) {
    12  .  .  .  .  0: *ast.ImportSpec {
    13  .  .  .  .  .  Path: *ast.BasicLit {
    14  .  .  .  .  .  .  ValuePos: 3:8
    15  .  .  .  .  .  .  Kind: STRING
    16  .  .  .  .  .  .  Value: "\"log\""
    17  .  .  .  .  .  }
    18  .  .  .  .  .  EndPos: -
    19  .  .  .  .  }
    20  .  .  .  }
    21  .  .  .  Rparen: -
    22  .  .  }
    23  .  .  1: *ast.FuncDecl {
    24  .  .  .  Name: *ast.Ident {
    25  .  .  .  .  NamePos: 5:6
    26  .  .  .  .  Name: "myLog"
    27  .  .  .  .  Obj: *ast.Object {
    28  .  .  .  .  .  Kind: func
    29  .  .  .  .  .  Name: "myLog"
    30  .  .  .  .  .  Decl: *(obj @ 23)
    31  .  .  .  .  }
    32  .  .  .  }
    33  .  .  .  Type: *ast.FuncType {
    34  .  .  .  .  Func: 5:1
    35  .  .  .  .  Params: *ast.FieldList {
    36  .  .  .  .  .  Opening: 5:11
    37  .  .  .  .  .  List: []*ast.Field (len = 2) {
    38  .  .  .  .  .  .  0: *ast.Field {
    39  .  .  .  .  .  .  .  Names: []*ast.Ident (len = 1) {
    40  .  .  .  .  .  .  .  .  0: *ast.Ident {
    41  .  .  .  .  .  .  .  .  .  NamePos: 5:12
    42  .  .  .  .  .  .  .  .  .  Name: "format"
    43  .  .  .  .  .  .  .  .  .  Obj: *ast.Object {
    44  .  .  .  .  .  .  .  .  .  .  Kind: var
    45  .  .  .  .  .  .  .  .  .  .  Name: "format"
    46  .  .  .  .  .  .  .  .  .  .  Decl: *(obj @ 38)
    47  .  .  .  .  .  .  .  .  .  }
    48  .  .  .  .  .  .  .  .  }
    49  .  .  .  .  .  .  .  }
    50  .  .  .  .  .  .  .  Type: *ast.Ident {
    51  .  .  .  .  .  .  .  .  NamePos: 5:19
    52  .  .  .  .  .  .  .  .  Name: "string"
    53  .  .  .  .  .  .  .  }
    54  .  .  .  .  .  .  }
    55  .  .  .  .  .  .  1: *ast.Field {
    56  .  .  .  .  .  .  .  Names: []*ast.Ident (len = 1) {
    57  .  .  .  .  .  .  .  .  0: *ast.Ident {
    58  .  .  .  .  .  .  .  .  .  NamePos: 5:27
    59  .  .  .  .  .  .  .  .  .  Name: "args"
    60  .  .  .  .  .  .  .  .  .  Obj: *ast.Object {
    61  .  .  .  .  .  .  .  .  .  .  Kind: var
    62  .  .  .  .  .  .  .  .  .  .  Name: "args"
    63  .  .  .  .  .  .  .  .  .  .  Decl: *(obj @ 55)
    64  .  .  .  .  .  .  .  .  .  }
    65  .  .  .  .  .  .  .  .  }
    66  .  .  .  .  .  .  .  }
    67  .  .  .  .  .  .  .  Type: *ast.Ellipsis {
    68  .  .  .  .  .  .  .  .  Ellipsis: 5:32
    69  .  .  .  .  .  .  .  .  Elt: *ast.InterfaceType {
    70  .  .  .  .  .  .  .  .  .  Interface: 5:35
    71  .  .  .  .  .  .  .  .  .  Methods: *ast.FieldList {
    72  .  .  .  .  .  .  .  .  .  .  Opening: 5:44
    73  .  .  .  .  .  .  .  .  .  .  Closing: 5:45
    74  .  .  .  .  .  .  .  .  .  }
    75  .  .  .  .  .  .  .  .  .  Incomplete: false
    76  .  .  .  .  .  .  .  .  }
    77  .  .  .  .  .  .  .  }
    78  .  .  .  .  .  .  }
    79  .  .  .  .  .  }
    80  .  .  .  .  .  Closing: 5:46
    81  .  .  .  .  }
    82  .  .  .  }
    83  .  .  .  Body: *ast.BlockStmt {
    84  .  .  .  .  Lbrace: 5:48
    85  .  .  .  .  List: []ast.Stmt (len = 2) {
    86  .  .  .  .  .  0: *ast.DeclStmt {
    87  .  .  .  .  .  .  Decl: *ast.GenDecl {
    88  .  .  .  .  .  .  .  TokPos: 6:3
    89  .  .  .  .  .  .  .  Tok: const
    90  .  .  .  .  .  .  .  Lparen: -
    91  .  .  .  .  .  .  .  Specs: []ast.Spec (len = 1) {
    92  .  .  .  .  .  .  .  .  0: *ast.ValueSpec {
    93  .  .  .  .  .  .  .  .  .  Names: []*ast.Ident (len = 1) {
    94  .  .  .  .  .  .  .  .  .  .  0: *ast.Ident {
    95  .  .  .  .  .  .  .  .  .  .  .  NamePos: 6:9
    96  .  .  .  .  .  .  .  .  .  .  .  Name: "prefix"
    97  .  .  .  .  .  .  .  .  .  .  .  Obj: *ast.Object {
    98  .  .  .  .  .  .  .  .  .  .  .  .  Kind: const
    99  .  .  .  .  .  .  .  .  .  .  .  .  Name: "prefix"
   100  .  .  .  .  .  .  .  .  .  .  .  .  Decl: *(obj @ 92)
   101  .  .  .  .  .  .  .  .  .  .  .  .  Data: 0
   102  .  .  .  .  .  .  .  .  .  .  .  }
   103  .  .  .  .  .  .  .  .  .  .  }
   104  .  .  .  .  .  .  .  .  .  }
   105  .  .  .  .  .  .  .  .  .  Values: []ast.Expr (len = 1) {
   106  .  .  .  .  .  .  .  .  .  .  0: *ast.BasicLit {
   107  .  .  .  .  .  .  .  .  .  .  .  ValuePos: 6:18
   108  .  .  .  .  .  .  .  .  .  .  .  Kind: STRING
   109  .  .  .  .  .  .  .  .  .  .  .  Value: "\"[my] \""
   110  .  .  .  .  .  .  .  .  .  .  }
   111  .  .  .  .  .  .  .  .  .  }
   112  .  .  .  .  .  .  .  .  }
   113  .  .  .  .  .  .  .  }
   114  .  .  .  .  .  .  .  Rparen: -
   115  .  .  .  .  .  .  }
   116  .  .  .  .  .  }
   117  .  .  .  .  .  1: *ast.ExprStmt {
   118  .  .  .  .  .  .  X: *ast.CallExpr {
   119  .  .  .  .  .  .  .  Fun: *ast.SelectorExpr {
   120  .  .  .  .  .  .  .  .  X: *ast.Ident {
   121  .  .  .  .  .  .  .  .  .  NamePos: 7:3
   122  .  .  .  .  .  .  .  .  .  Name: "log"
   123  .  .  .  .  .  .  .  .  }
   124  .  .  .  .  .  .  .  .  Sel: *ast.Ident {
   125  .  .  .  .  .  .  .  .  .  NamePos: 7:7
   126  .  .  .  .  .  .  .  .  .  Name: "Printf"
   127  .  .  .  .  .  .  .  .  }
   128  .  .  .  .  .  .  .  }
   129  .  .  .  .  .  .  .  Lparen: 7:13
   130  .  .  .  .  .  .  .  Args: []ast.Expr (len = 2) {
   131  .  .  .  .  .  .  .  .  0: *ast.BinaryExpr {
   132  .  .  .  .  .  .  .  .  .  X: *ast.Ident {
   133  .  .  .  .  .  .  .  .  .  .  NamePos: 7:14
   134  .  .  .  .  .  .  .  .  .  .  Name: "prefix"
   135  .  .  .  .  .  .  .  .  .  .  Obj: *(obj @ 97)
   136  .  .  .  .  .  .  .  .  .  }
   137  .  .  .  .  .  .  .  .  .  OpPos: 7:21
   138  .  .  .  .  .  .  .  .  .  Op: +
   139  .  .  .  .  .  .  .  .  .  Y: *ast.Ident {
   140  .  .  .  .  .  .  .  .  .  .  NamePos: 7:23
   141  .  .  .  .  .  .  .  .  .  .  Name: "format"
   142  .  .  .  .  .  .  .  .  .  .  Obj: *(obj @ 43)
   143  .  .  .  .  .  .  .  .  .  }
   144  .  .  .  .  .  .  .  .  }
   145  .  .  .  .  .  .  .  .  1: *ast.Ident {
   146  .  .  .  .  .  .  .  .  .  NamePos: 7:31
   147  .  .  .  .  .  .  .  .  .  Name: "args"
   148  .  .  .  .  .  .  .  .  .  Obj: *(obj @ 60)
   149  .  .  .  .  .  .  .  .  }
   150  .  .  .  .  .  .  .  }
   151  .  .  .  .  .  .  .  Ellipsis: 7:35
   152  .  .  .  .  .  .  .  Rparen: 7:38
   153  .  .  .  .  .  .  }
   154  .  .  .  .  .  }
   155  .  .  .  .  }
   156  .  .  .  .  Rbrace: 8:1
   157  .  .  .  }
   158  .  .  }
   159  .  }
   160  .  Scope: *ast.Scope {
   161  .  .  Objects: map[string]*ast.Object (len = 1) {
   162  .  .  .  "myLog": *(obj @ 27)
   163  .  .  }
   164  .  }
   165  .  Imports: []*ast.ImportSpec (len = 1) {
   166  .  .  0: *(obj @ 12)
   167  .  }
   168  .  Unresolved: []*ast.Ident (len = 2) {
   169  .  .  0: *(obj @ 50)
   170  .  .  1: *(obj @ 120)
   171  .  }
   172  }
  

The most interesting part of the AST for us is the myLog function declaration. We can simplify it to:

&ast.FuncDecl{
    Name: &ast.Ident{Name: "myLog"},
    Type: &ast.FuncType{
        Params: &ast.FieldList{
            List: []*ast.Field{
                &ast.Field{
                    Names: []*ast.Ident{&ast.Ident{Name: "format"}},
                    Type:  &ast.Ident{Name: "string"},
                },
                &ast.Field{
                    Names: []*ast.Ident{&ast.Ident{Name: "args"}},
                    Type: &ast.Ellipsis{
                        Elt: &ast.InterfaceType{
                            Methods: &ast.FieldList{
                                List: nil, // no methods in the interface{}
                            }, // end of Methods
                        }, // end of &ast.InterfaceType
                    }, // end of  &ast.Ellipsis
                }, // end of &ast.Field
            }, // end of []*ast.Field
        }, // end of &ast.FieldList
    }, // end of &ast.FuncType
} // end of &ast.FuncDecl

Let’s write it

To analyze AST we need to parse it first:

package main

import (
	"bytes"
	"fmt"
	"go/ast"
	"go/parser"
	"go/printer"
	"go/token"
	"log"
	"os"
)

func main() {
	v := visitor{fset: token.NewFileSet()}
	for _, filePath := range os.Args[1:] {
		if filePath == "--" { // to be able to run this like "go run main.go -- input.go"
			continue
		}

		f, err := parser.ParseFile(v.fset, filePath, nil, 0)
		if err != nil {
			log.Fatalf("Failed to parse file %s: %s", filePath, err)
		}

		ast.Walk(&v, f)
	}
}

type visitor struct {
	fset *token.FileSet
}

func (v *visitor) Visit(node ast.Node) ast.Visitor {
	if node == nil {
		return nil
	}

	var buf bytes.Buffer
	printer.Fprint(&buf, v.fset, node)
	fmt.Printf("%s | %#v\n", buf.String(), node)

 	return v
}

parser.ParseFile parses source file and builds AST. ast.Walk visits nodes of AST with our visitor: it continues visiting nodes until the Visit method returns nil or all nodes were visited. In this simple implementation, we parse source files and visit all AST nodes.

Let’s run it on the example file with myLog function:

go run ./main.go -- ./example.go

import "log"

func myLog(format string, args ...interface{}) {
        const prefix = "[my] "
        log.Printf(prefix+format, args...)
}
 | &ast.File{Doc:(*ast.CommentGroup)(nil), Package:1, Name:(*ast.Ident)(0xc000096020), Decls:[]ast.Decl{(*ast.GenDecl)(0xc0000a2040), (*ast.FuncDecl)(0xc0000882a0)}, Scope:(*ast.Scope)(0xc00009a050), Imports:[]*ast.ImportSpec{(*ast.ImportSpec)(0xc000088150)}, Unresolved:[]*ast.Ident{(*ast.Ident)(0xc0000960a0), (*ast.Ident)(0xc000096180)}, Comments:[]*ast.CommentGroup(nil)}
main | &ast.Ident{NamePos:9, Name:"main", Obj:(*ast.Object)(nil)}
import "log" | &ast.GenDecl{Doc:(*ast.CommentGroup)(nil), TokPos:15, Tok:75, Lparen:0, Specs:[]ast.Spec{(*ast.ImportSpec)(0xc000088150)}, Rparen:0}
"log" | &ast.ImportSpec{Doc:(*ast.CommentGroup)(nil), Name:(*ast.Ident)(nil), Path:(*ast.BasicLit)(0xc000096040), Comment:(*ast.CommentGroup)(nil), EndPos:0}
"log" | &ast.BasicLit{ValuePos:22, Kind:9, Value:"\"log\""}
func myLog(format string, args ...interface{}) {
        const prefix = "[my] "
        log.Printf(prefix+format, args...)
} | &ast.FuncDecl{Doc:(*ast.CommentGroup)(nil), Recv:(*ast.FieldList)(nil), Name:(*ast.Ident)(0xc000096060), Type:(*ast.FuncType)(0xc000096280), Body:(*ast.BlockStmt)(0xc000088270)}
myLog | &ast.Ident{NamePos:34, Name:"myLog", Obj:(*ast.Object)(0xc00009c1e0)}
func(format string, args ...interface{}) | &ast.FuncType{Func:29, Params:(*ast.FieldList)(0xc0000881e0), Results:(*ast.FieldList)(nil)}
 | &ast.FieldList{Opening:39, List:[]*ast.Field{(*ast.Field)(0xc0000a2080), (*ast.Field)(0xc0000a20c0)}, Closing:74}
 | &ast.Field{Doc:(*ast.CommentGroup)(nil), Names:[]*ast.Ident{(*ast.Ident)(0xc000096080)}, Type:(*ast.Ident)(0xc0000960a0), Tag:(*ast.BasicLit)(nil), Comment:(*ast.CommentGroup)(nil)}
format | &ast.Ident{NamePos:40, Name:"format", Obj:(*ast.Object)(0xc00009c0a0)}
string | &ast.Ident{NamePos:47, Name:"string", Obj:(*ast.Object)(nil)}
 | &ast.Field{Doc:(*ast.CommentGroup)(nil), Names:[]*ast.Ident{(*ast.Ident)(0xc0000960c0)}, Type:(*ast.Ellipsis)(0xc000096100), Tag:(*ast.BasicLit)(nil), Comment:(*ast.CommentGroup)(nil)}
args | &ast.Ident{NamePos:55, Name:"args", Obj:(*ast.Object)(0xc00009c0f0)}
...interface{} | &ast.Ellipsis{Ellipsis:60, Elt:(*ast.InterfaceType)(0xc0000960e0)}
interface{} | &ast.InterfaceType{Interface:63, Methods:(*ast.FieldList)(0xc0000881b0), Incomplete:false}
 | &ast.FieldList{Opening:72, List:[]*ast.Field(nil), Closing:73}
{
        const prefix = "[my] "
        log.Printf(prefix+format, args...)
} | &ast.BlockStmt{Lbrace:76, List:[]ast.Stmt{(*ast.DeclStmt)(0xc00009a0e0), (*ast.ExprStmt)(0xc00009a130)}, Rbrace:138}
const prefix = "[my] " | &ast.DeclStmt{Decl:(*ast.GenDecl)(0xc0000a2100)}
const prefix = "[my] " | &ast.GenDecl{Doc:(*ast.CommentGroup)(nil), TokPos:79, Tok:64, Lparen:0, Specs:[]ast.Spec{(*ast.ValueSpec)(0xc00009c140)}, Rparen:0}
prefix = "[my] " | &ast.ValueSpec{Doc:(*ast.CommentGroup)(nil), Names:[]*ast.Ident{(*ast.Ident)(0xc000096140)}, Type:ast.Expr(nil), Values:[]ast.Expr{(*ast.BasicLit)(0xc000096160)}, Comment:(*ast.CommentGroup)(nil)}
prefix | &ast.Ident{NamePos:85, Name:"prefix", Obj:(*ast.Object)(0xc00009c190)}
"[my] " | &ast.BasicLit{ValuePos:94, Kind:9, Value:"\"[my] \""}
log.Printf(prefix+format, args...) | &ast.ExprStmt{X:(*ast.CallExpr)(0xc0000a2140)}
log.Printf(prefix+format, args...) | &ast.CallExpr{Fun:(*ast.SelectorExpr)(0xc0000961c0), Lparen:113, Args:[]ast.Expr{(*ast.BinaryExpr)(0xc000088240), (*ast.Ident)(0xc000096220)}, Ellipsis:133, Rparen:136}
log.Printf | &ast.SelectorExpr{X:(*ast.Ident)(0xc000096180), Sel:(*ast.Ident)(0xc0000961a0)}
log | &ast.Ident{NamePos:103, Name:"log", Obj:(*ast.Object)(nil)}
Printf | &ast.Ident{NamePos:107, Name:"Printf", Obj:(*ast.Object)(nil)}
prefix + format | &ast.BinaryExpr{X:(*ast.Ident)(0xc0000961e0), OpPos:120, Op:12, Y:(*ast.Ident)(0xc000096200)}
prefix | &ast.Ident{NamePos:114, Name:"prefix", Obj:(*ast.Object)(0xc00009c190)}
format | &ast.Ident{NamePos:121, Name:"format", Obj:(*ast.Object)(0xc00009c0a0)}
args | &ast.Ident{NamePos:129, Name:"args", Obj:(*ast.Object)(0xc00009c0f0)}
  

You can see that it visits and prints all nodes. Now let’s write linting in visitor.Visit:

func (v *visitor) Visit(node ast.Node) ast.Visitor {
	funcDecl, ok := node.(*ast.FuncDecl)
	if !ok {
		return v
	}

	params := funcDecl.Type.Params.List
	if len(params) != 2 { // [0] must be format (string), [1] must be args (...interface{})
		return v
	}

	firstParamType, ok := params[0].Type.(*ast.Ident)
	if !ok { // first param type isn't identificator so it can't be of type "string"
		return v
	}

	if firstParamType.Name != "string" { // first param (format) type is not string
		return v
	}

	secondParamType, ok := params[1].Type.(*ast.Ellipsis)
	if !ok { // args are not ellipsis (...args)
		return v
	}

	elementType, ok := secondParamType.Elt.(*ast.InterfaceType)
	if !ok { // args are not of interface type, but we need interface{}
		return v
	}

	if elementType.Methods != nil && len(elementType.Methods.List) != 0 {
		return v // has >= 1 method in interface, but we need an empty interface "interface{}"
	}

	if strings.HasSuffix(funcDecl.Name.Name, "f") {
		return v
	}

	fmt.Printf("%s: printf-like formatting function '%s' should be named '%sf'\n",
		v.fset.Position(node.Pos()), funcDecl.Name.Name, funcDecl.Name.Name)
	return v
}

Run it and ensure that it works:

$ go run ./inspect.go -- ./example.go
./example.go:5:1: printf-like formatting function 'myLog' should be named 'myLogf'

We’ve implemented a simple linter without go/analysis or any other frameworks. Let’s move on to how we can use go/analysis to improve it.

Using go/analysis

Detective looks for Go bugs. Maybe go/analysis can help him.
Detective looks for Go bugs. Maybe go/analysis can help him.

Go/analysis is an API for modular analysis. Also, it’s a common interface for all linters. This API simplifies developing a new linter. It forces using best practices for static analysis, for example, working with one package at a time.

The primary type in the API is analysis.Analyzer. An Analyzer statically describes an analysis function: its name, documentation, flags, relationship to other analyzers, and of course, it’s logic.

To define an analysis, a user declares a variable of type *analysis.Analyzer:

type Analyzer struct {
	Name			string
	Doc			string
	Flags			flag.FlagSet
	Run			func(*Pass) (interface{}, error)
	RunDespiteErrors	bool
	ResultType		reflect.Type
	Requires		[]*Analyzer
	FactTypes		[]Fact
}

The most interesting field is Run: it’s where linting logic lives. It takes *analysis.Pass as an argument:

type Pass struct {
	Fset   		*token.FileSet
	Files		[]*ast.File
	OtherFiles	[]string
	Pkg		*types.Package
	TypesInfo	*types.Info
	ResultOf	map[*Analyzer]interface{}
	Report		func(Diagnostic)
	...
}

A Pass describes a single unit of work: the application of a particular Analyzer to a particular package of Go code. The Pass provides information to the Analyzer’s Run function about the package being analyzed. It provides operations to the Run function for reporting diagnostics and other information back to the driver.

The Fset, Files, Pkg, and TypesInfo fields provide source positions, the syntax trees, type information for a single package of Go code. We don’t need types information, so Pkg and TypesInfo aren’t needed. Wait, but do we need to know that the first argument of printf-like function has type string? Yes, we need. But AST contains enough information about that, therefore we don’t need type-checking.

Why do we need go/analysis

There are 2 main reasons to use go/analysis: unified interface and code reuse.

Go/analysis provides a unified interface for linters. It simplifies their integration into IDE, metalinters, code review tools, etc. For example, any go/analysis linter can be effectively run by go vet. We will talk about integration later in this article.

In our simple linter, we require a user to pass a list of all files to analyze. But the convenient way is to run linter without arguments or with ./... to run it recursively on all files in all packages. We can implement it ourselves: I already did it in the first version of golangci-lint. It requires 100-500 lines of code. Or we can use the go/analysis API to do this in 10 lines of code.

Let’s use go/analysis

Let’s port our simple linter to use go/analysis API. I’ve made the repository go-printf-func-name for that.

First, let’s define the project structure by following popular Go project layout:

.
├── README.md
├── cmd
│   └── go-printf-func-name
│       └── main.go
├── go.mod
├── go.sum
└── pkg
    └── analyzer
        └── analyzer.go

Let’s start from the main function in cmd/go-printf-func-name/main.go:

package main

import (
	"github.com/jirfag/go-printf-func-name/pkg/analyzer"
	"golang.org/x/tools/go/analysis/singlechecker"
)

func main() {
	singlechecker.Main(analyzer.Analyzer)
}

That’s all! singlechecker.Main generates CLI for running linter.

Let’s define Analyzer in pkg/analyzer/analyzer.go:

package analyzer

import (
	"go/ast"
	"strings"

	"golang.org/x/tools/go/analysis"
)

var Analyzer = &analysis.Analyzer{
	Name: "goprintffuncname",
	Doc:  "Checks that printf-like functions are named with `f` at the end.",
	Run:  run,
}

func run(pass *analysis.Pass) (interface{}, error) {
	inspect := func(node ast.Node) bool {
		funcDecl, ok := node.(*ast.FuncDecl)
		if !ok {
			return true
		}

		params := funcDecl.Type.Params.List
		if len(params) != 2 { // [0] must be format (string), [1] must be args (...interface{})
			return true
		}

		firstParamType, ok := params[0].Type.(*ast.Ident)
		if !ok { // first param type isn't identificator so it can't be of type "string"
			return true
		}

		if firstParamType.Name != "string" { // first param (format) type is not string
			return true
		}

		secondParamType, ok := params[1].Type.(*ast.Ellipsis)
		if !ok { // args are not ellipsis (...args)
			return true
		}

		elementType, ok := secondParamType.Elt.(*ast.InterfaceType)
		if !ok { // args are not of interface type, but we need interface{}
			return true
		}

		if elementType.Methods != nil && len(elementType.Methods.List) != 0 {
			return true // has >= 1 method in interface, but we need an empty interface "interface{}"
		}

		if strings.HasSuffix(funcDecl.Name.Name, "f") {
			return true
		}

		pass.Reportf(node.Pos(), "printf-like formatting function '%s' should be named '%sf'",
			funcDecl.Name.Name, funcDecl.Name.Name)
		return true
	}

	for _, f := range pass.Files {
		ast.Inspect(f, inspect)
	}
	return nil, nil
}

Notice what changed:

  1. We use ast.Inspect instead of ast.Walk: it requires less code. To do that we just need to remove the visitor and return true instead of the visitor.
  2. We iterate over pass.Files to get syntax trees (AST) for the current package. We don’t load them manually by parser.ParseFile anymore.
  3. We report about found issues by pass.Reportf instead of fmt.Printf. This method from go/analysis abstracts reporting logic from us. Also, we’ve removed \n from the message.

Run it

Let’s run it:

$ go run ./cmd/go-printf-func-name/main.go
goprintffuncname: Checks that printf-like functions are named with `f` at the end.

Usage: goprintffuncname [-flag] [package]


Flags:  -V      print version and exit
  -all
        no effect (deprecated)
  -c int
        display offending line with this many lines of context (default -1)
  -cpuprofile string
        write CPU profile to this file
  -debug string
        debug flags, any subset of "fpstv"
  -fix
        apply all suggested fixes
  -flags
        print analyzer flags in JSON
  -json
        emit JSON output
  -memprofile string
        write memory profile to this file
  -source
        no effect (deprecated)
  -tags string
        no effect (deprecated)
  -trace string
        write trace log to this file
  -v    no effect (deprecated)
exit status 1

Thanks to go/analysis we got a lot of command-line options. Now it’s time to test it again on example source code:

$ go run ./cmd/go-printf-func-name/main.go -- ./example.go
/Users/denis/go-printf-func-name/example.go:5:1: printf-like formatting function 'myLog' should be named 'myLogf'

exit status 3

Yes, it works!

ast.Inspect vs Inspector

Tree inspector
Tree inspector. Image from Saskatoon website.

Did you notice ineffectiveness in ast.Inspect and ast.Walk? We visit every AST node: binary expressions, variables, constants, etc. But we need only function declarations. Can we visit only them to improve performance?

Yes, we can. Go/analysis contains inspect.Analyzer. This analyzer filters AST nodes by types. First, we should require it as a dependency for our analyzer:

var Analyzer = &analysis.Analyzer{
	Name:     "goprintffuncname",
	Doc:      "Checks that printf-like functions are named with `f` at the end.",
	Run:      run,
	Requires: []*analysis.Analyzer{inspect.Analyzer},
}

After that change the run function to:

func run(pass *analysis.Pass) (interface{}, error) {
  // pass.ResultOf[inspect.Analyzer] will be set if we've added inspect.Analyzer to Requires.
  inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

	nodeFilter := []ast.Node{ // filter needed nodes: visit only them
		(*ast.FuncDecl)(nil),
	}

	inspector.Preorder(nodeFilter, func(node ast.Node) {
		funcDecl := node.(*ast.FuncDecl)

		params := funcDecl.Type.Params.List
		if len(params) != 2 { // [0] must be format (string), [1] must be args (...interface{})
			return
		}

		firstParamType, ok := params[0].Type.(*ast.Ident)
		if !ok { // first param type isn't identificator so it can't be of type "string"
			return
		}

		if firstParamType.Name != "string" { // first param (format) type is not string
			return
		}

		secondParamType, ok := params[1].Type.(*ast.Ellipsis)
		if !ok { // args are not ellipsis (...args)
			return
		}

		elementType, ok := secondParamType.Elt.(*ast.InterfaceType)
		if !ok { // args are not of interface type, but we need interface{}
			return
		}

		if elementType.Methods != nil && len(elementType.Methods.List) != 0 {
			return // has >= 1 method in interface, but we need an empty interface "interface{}"
		}

		if strings.HasSuffix(funcDecl.Name.Name, "f") {
			return
		}

		pass.Reportf(node.Pos(), "printf-like formatting function '%s' should be named '%sf'",
			funcDecl.Name.Name, funcDecl.Name.Name)
	})

	return nil, nil
}

Here we’ve changed AST walking a little bit. Now we use inspector.Preorder and don’t return anything from visiting function. The excerpt from the inspector’s comments explains the profit from its usage:

// During construction, the inspector does a complete traversal and
// builds a list of push/pop events and their node type. Subsequent
// method calls that request a traversal scan this list, rather than walk
// the AST, and perform type filtering using efficient bit sets.
//
// Experiments suggest the inspector's traversals are about 2.5x faster
// than ast.Inspect, but it may take around 5 traversals for this
// benefit to amortize the inspector's construction cost.
// If efficiency is the primary concern, do not use Inspector for
// one-off traversals.

We expect our linter to be used from linters runner like golangci-lint. Such runners contain dozens of go/analysis analyzers, therefore, we are sure that inspector construction time will be amortized. And even if it’s slower - it’s just a simpler API to traverse syntax trees.

Testing

Automated tests

It’s time to test our linter. Go/analysis provides a framework for this. Let’s create files analyzer_test.go and testdata/src/p/p.go:

.
├── README.md
├── cmd
│   └── go-printf-func-name
│       └── main.go
├── example.go
├── go.mod
├── go.sum
├── pkg
│   └── analyzer
│       ├── analyzer.go
│       └── analyzer_test.go
└── testdata
    └── src
        └── p
            └── p.go

In the file pkg/analyzer/analyzer_test.go we write:

func TestAll(t *testing.T) {
	wd, err := os.Getwd()
	if err != nil {
		t.Fatalf("Failed to get wd: %s", err)
	}

	testdata := filepath.Join(filepath.Dir(filepath.Dir(wd)), "testdata")
	analysistest.Run(t, testdata, analyzer.Analyzer, "p")
}

Now we can write test functions in testdata/src/p/p.go. If linter should report about a function - we leave a comment // want "printf-like formatting function". The go/analysis finds such comments and ensures that they match found issues on these lines.

Let’s start from simple tests:

package p

func notPrintfFuncAtAll() {}

func funcWithEllipsis(args ...interface{}) {}

func printfLikeButWithStrings(format string, args ...string) {}

func printfLikeButWithBadFormat(format int, args ...interface{}) {}

func prinfLikeFunc(format string, args ...interface{}) {} // want "printf-like formatting function"

func prinfLikeFuncWithReturnValue(format string, args ...interface{}) string { // want "printf-like formatting function"
	return ""
}

If we run tests they pass:

$ go test -v ./...
?       github.com/jirfag/go-printf-func-name   [no test files]
?       github.com/jirfag/go-printf-func-name/cmd/go-printf-func-name   [no test files]
=== RUN   TestAll
--- PASS: TestAll (0.29s)
PASS
ok      github.com/jirfag/go-printf-func-name/pkg/analyzer      0.387s

Did we finish with tests? No. Let’s check our code coverage. I’m using VS Code with the following configuration for code coverage:

    "go.coverOnSave": true,
    "go.coverageDecorator": {
        "type": "gutter",
        "coveredHighlightColor": "rgba(64,128,128,0.5)",
        "uncoveredHighlightColor": "rgba(128,64,64,0.25)",
        "coveredGutterStyle": "blockgreen",
        "uncoveredGutterStyle": "blockred"
    },
    "go.coverOnSingleTest": true,
}

I’ve run the test from VS Code and have checked the coverage in pkg/analyzer/analyzer.go:

Code coverage of our linter
Code coverage of our linter

Some code blocks are collapsed to reduce the screenshot height. Covered lines are marked with green strips to the left of the line numbers. You can see that we didn’t cover 4 cases, let’s fix it. I’ve added these test cases:

func secondArgIsNotEllipsis(arg1 string, arg2 int) {}

func printfLikeButWithExtraInterfaceMethods(format string, args ...interface {
	String() string
}) {
}

func prinfLikeFuncf(format string, args ...interface{}) {}

I’ve checked coverage again: all cases except one are covered. The uncovered one is:

firstParamType, ok := params[0].Type.(*ast.Ident)
if !ok { // first param type isn't identificator so it can't be of type "string"
	return
}

I couldn’t find a test case for it.

Did we finish with tests? No. We’ve forgotten about important test case:

func prinfLikeFuncWithExtraArgs1(extraArg, format string, args ...interface{}) {} // want "printf-like formatting function"

func prinfLikeFuncWithExtraArgs2(extraArg int, format string, args ...interface{}) {} // want "printf-like formatting function"

I’ve added these test cases and tests have failed. Let’s fix our code:

params := funcDecl.Type.Params.List
if len(params) < 2 { // [0] must be format (string), [1] must be args (...interface{})
	return
}

formatParamType, ok := params[len(params)-2].Type.(*ast.Ident)
if !ok { // first param type isn't identificator so it can't be of type "string"
	return
}

if formatParamType.Name != "string" { // first param (format) type is not string
	return
}

argsParamType, ok := params[len(params)-1].Type.(*ast.Ellipsis)
if !ok { // args are not ellipsis (...args)
	return
}

elementType, ok := argsParamType.Elt.(*ast.InterfaceType)
if !ok { // args are not of interface type, but we need interface{}
	return
}

Now tests pass and we can move on.

Run on the real and large repos

It’s important to test on real code on large codebases. Let’s run it on Go source code:

go install ./cmd/...
cd $GOROOT/src
go-printf-func-name ./...

It reports about ~30 issues. Some of them are valid:

func (check *Checker) trace(pos token.Pos, format string, args ...interface{}) {
        fmt.Printf("%s:\t%s%s\n",
                check.fset.Position(pos),
                strings.Repeat(".  ", check.indent),
                check.sprintf(format, args...),
        )
}

Some of them are false-positives:

func Sscanf(str string, format string, a ...interface{}) (n int, err error) {
        return Fscanf((*stringReader)(&str), format, a...)
}

func (tx *Tx) Query(query string, args ...interface{}) (*Rows, error) {
        return tx.QueryContext(context.Background(), query, args...)
}

Let’s fix it. The proper fix is to trace all calls inside such functions and find out whether format argument goes to formatting function like fmt.Sprintf. Maybe we will do it in the next post. Now, let’s make a simple fix: require format argument name to be equal "format" and don’t allow return values. It will increase precision of our analysis but will decrease recall.

You can find fixed code and tests in linter’s GitHub repo.

I’ve run on $GOROOT again and got only 8 issues: all of them are valid. Also, I’ve run the linter on istio source code and it reported 8 valid issues.

We’ve tested our linter. The final step is to integrate it into linter runners.

Integrating

Integrate with go vet

It’s very simple:

go install ./cmd/...
go vet -vettool=$(which go-printf-func-name) ./...

If you see flag provided but not defined: -unsafeptr add these lines to the main function:

// Don't use it: just to not crash on -unsafeptr flag from go vet
flag.Bool("unsafeptr", false, "")

Such usage isn’t useful in practice: we’ve replaced default go vet checks by one our check. Instead, we can save go vet checks and add our linter. To do this copy the sample go vet main and add your analyzer to it.

Integrate with golangci-lint

Logo of golangci-lint
Logo of golangci-lint

Golangci-lint is a linters runner. It contains dozes of linters. Let’s add our linter to it. The main part of adding is:

func NewGoPrintfFuncName() *goanalysis.Linter {
	return goanalysis.NewLinter(
		"goprintffuncname",
		"Checks that printf-like functions are named with `f` at the end",
		[]*analysis.Analyzer{analyzer.Analyzer},
		nil,
	).WithLoadMode(goanalysis.LoadModeSyntax)
}

In the end, don’t forget to do make vendor README.md.

The whole pull request to golangci-lint is small:

---
 README.md                         |  3 +++
 go.mod                            |  3 ++-
 go.sum                            |  2 ++
 pkg/golinters/goprintffuncname.go | 17 +++++++++++++++++
 pkg/lint/lintersdb/manager.go     |  3 +++
 test/testdata/goprintffuncname.go |  5 +++++
 vendor/modules.txt                |  4 +++-
 7 files changed, 35 insertions(+), 2 deletions(-)

Let’s check it on istio repo:

~/istio ((1.3.5))$ golangci-lint run --no-config --disable-all -E goprintffuncname
pkg/test/framework/components/prometheus/kube.go:203:1: printf-like formatting function 'WaitForOneOrMoreOrFail' should be named 'WaitForOneOrMoreOrFailf' (goprintffuncname)
func (c *kubeComponent) WaitForOneOrMoreOrFail(t test.Failer, format string, args ...interface{}) {
^
mixer/pkg/runtime/config/ephemeral.go:914:1: printf-like formatting function 'appendErr' should be named 'appendErrf' (goprintffuncname)
func appendErr(errs *multierror.Error, field string, format string, a ...interface{}) {
^
mixer/pkg/lang/compiler/compiler.go:754:1: printf-like formatting function 'internalError' should be named 'internalErrorf' (goprintffuncname)
func (g *generator) internalError(format string, args ...interface{}) {
^
mixer/tools/codegen/pkg/modelgen/diag.go:80:1: printf-like formatting function 'addError' should be named 'addErrorf' (goprintffuncname)
func (m *Model) addError(file string, line string, format string, a ...interface{}) {
^
mixer/pkg/il/text/read.go:321:1: printf-like formatting function 'fail' should be named 'failf' (goprintffuncname)
func (p *parser) fail(format string, args ...interface{}) {
^
mixer/pkg/il/text/read.go:325:1: printf-like formatting function 'failLoc' should be named 'failLocf' (goprintffuncname)
func (p *parser) failLoc(l location, format string, args ...interface{}) {
^
mixer/pkg/runtime/testing/data/logger.go:35:1: printf-like formatting function 'WriteFormat' should be named 'WriteFormatf' (goprintffuncname)
func (l *Logger) WriteFormat(name string, format string, args ...interface{}) {
^
galley/pkg/testing/common/mocklog.go:29:1: printf-like formatting function 'Append' should be named 'Appendf' (goprintffuncname)
func (e *MockLog) Append(format string, args ...interface{}) {
^

It works. The pull request to golangci-lint is here.

What’s next

We’ve written a real-world linter. It’s code is here. The linter has found issues in Go and Istio source code. We’ve made a pull request to include it into golangci-lint.

The next step can be improving its recall by using facts and not checking that param name equals format.