如何快速写出高质量的 Go 代码?
阅读原文时间:2022年01月29日阅读:23

团队协作开发中,必然存在着不同的代码风格,并且诸如 http body closeunhandled error 等低级错误不能完全避免。通过使用 ci lint 能够及早的发现并修复问题,提高代码质量和幸福感。

目前主流的工具是 golangci-lint,里面集成了许多的 linters,安装及使用看官网文档即可,非常详细易读,本文主要是配合一些样例代码来分析一下相关 linters 的功能。

deadcode

检查未使用的代码

var a = 100

func foo() {
    println("hello,world")
}


main.go:3:5: `a` is unused (deadcode)
var a = 100
    ^
main.go:5:6: `foo` is unused (deadcode)
func foo() {
     ^

类似的工具还有 structcheckvarcheck

errcheck

检查未处理的错误

由于 Go 语言的错误处理机制,导致代码中遍地的 if err != nil ,因此有不耐烦的同学,在某些自认为不会出错的地方直接不处理,比如 json.Marshal(),虽然多数情况下没啥问题,不过一但出了问题加班啥的估计跑不了。

可以通过修改配置文件来定制不同情况是否报告

linters-settings:
  errcheck:
    # 检查类型断言
    check-type-assertions: true
    # 检查使用 _ 来处理错误
    check-blank: true


func main() {
    foo()

    var i interface{} = 1
    ii := i.(int)
    fmt.Println(ii)

    num, _ := strconv.Atoi("110")
    fmt.Println(num)
}

func foo() error {
    return errors.New("i am error")
}


$ golangci-lint run
main.go:10:5: Error return value is not checked (errcheck)
        foo()
           ^
main.go:13:8: Error return value is not checked (errcheck)
        ii := i.(int)
              ^
main.go:16:7: Error return value of `strconv.Atoi` is not checked (errcheck)
        num, _ := strconv.Atoi("110")
             ^

gosimple

简化代码

func main() {
    t := time.Now()
    fmt.Println(time.Now().Sub(t))
}


$ golangci-lint run
main.go:10:14: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
        fmt.Println(time.Now().Sub(t))
                    ^

原始仓库中还有许多别的测试用例,感兴趣的同学可以看看,可以修改配置文件来指定生效的规则,默认是 all。

govet

go vet 是官方提供的工具,可以检查出许多问题,如 printf 参数不匹配、unmarshall 时未传递指针或者接口、循环变量捕获问题等。

type AAA struct {
    A int `json:"a"`
}

func main() {
    fmt.Printf("%s", true)

    var a AAA
    if err := json.Unmarshal([]byte(`{"a":1}`), a); err != nil {
        panic(err)
    }

    var s []int
    for i, v := range s {
        go func() {
            fmt.Println(i)
            fmt.Println(v)
        }()
    }
}


$ golangci-lint run
main.go:23:16: loopclosure: loop variable i captured by func literal (govet)
                        fmt.Println(i)
                                    ^
main.go:24:16: loopclosure: loop variable v captured by func literal (govet)
                        fmt.Println(v)
                                    ^
main.go:13:2: printf: fmt.Printf format %s has arg true of wrong type bool (govet)
        fmt.Printf("%s", true)
        ^
main.go:16:26: unmarshal: call of Unmarshal passes non-pointer as second argument (govet)
        if err := json.Unmarshal([]byte(`{"a":1}`), a); err != nil {
                                ^

ineffassign

检查无效的赋值,即变量赋值后并未使用

func main() {
    a := os.Getenv("a")
    if a == "" {
        a = "unknown"
    }
}


$ golangci-lint run
main.go:8:3: ineffectual assignment to a (ineffassign)
                a = "unknown"
                ^

本以为这种情况应该不多,但是测试了一下我们组内的某个项目,发现还不少。

staticcheck

这个提供了更加多的检查项目,包括标准库的各种误用、并发问题、测试问题、无用的代码、代码正确性问题、性能问题、代码简化、代码风格等。

func main() {
    var m map[string]int
    m["a"] = 1

    if strings.ToLower("a") == strings.ToLower("A") {
        println(true)
    }
}


$ golangci-lint run
main.go:7:2: SA5000: assignment to nil map (staticcheck)
        m["a"] = 1
        ^
main.go:9:5: SA6005: should use strings.EqualFold instead (staticcheck)
        if strings.ToLower("a") == strings.ToLower("A") {
           ^

asciicheck

检查代码中是否存在非 ASCII 的字符。

type TestA struct{}
type TеstB struct{}

func main() {
    a := TestA{}
    b := TеstB{}
    println(a, b)
}


$ golangci-lint run --disable-all -E asciicheck
main.go:4:6: identifier "TеstB" contain non-ASCII character: U+0435 'е' (asciicheck)
type TеstB struct{}
     ^

这种的话肉眼根本无法识别,记得之前工作的时候就遇到过类似情况,你眼中的门他不一定同一个门。

bidichk

这个和安全有关,检查危险的 unicode 字符序列,细节内容可以看源代码特洛伊木马攻击

bodyclose

检查 http body 是否关闭,这种问题初学者还是非常容易犯的

func main() {
    resp, err := http.Get("https://example.com/")
    if err != nil {
        panic(err)
    }
    fmt.Println(resp.Status)
}


$ golangci-lint run --disable-all -E bodyclose
main.go:9:23: response body must be closed (bodyclose)
        resp, err := http.Get("https://example.com/")
                             ^

cyclop

检查代码的圈复杂度,cyclomatic complexity 是衡量代码复杂度的一种指标,值越高意味着代码复杂度越高,代码越难维护,bug 也可能更多。具体计算如下:

1 is the base complexity of a function

+1 for each 'if', 'for', 'case', '&&' or '||'

默认值是 10,不过一般业务代码稍微写着写着就超了,可以修改配置文件适当增大。

errorlint

Working with Errors in Go 1.13 引入了错误包裹机制,方便函数调用过程中形成错误链,更好的定位和处理问题。

这个工具可以检查错误处理是否遵循了其标准。

func main() {
    foo()
}

func foo() error {
    if _, err := os.Open("a.txt"); err != nil {
        return fmt.Errorf("call func one err: %s", err)
    }
    return nil
}


$ golangci-lint run --disable-all -E errorlint
main.go:14:46: non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
                return fmt.Errorf("call func one err: %s", err)
                                                           ^

forbidigo

这个工具可以检查出指定格式的语句,比如线上代码中不应该使用 fmt.Println() 这种临时的 debug 代码,而是应该使用统一的日志组件。

func main() {
    fmt.Println("debug message show remove or ues log.Println")
}


$ golangci-lint run --disable-all -E forbidigo
main.go:6:2: use of `fmt.Println` forbidden by pattern `^(fmt\.Print(|f|ln)|print|println)$` (forbidigo)
        fmt.Println("debug message show remove or ues log.Println")
        ^

funlen

相比于 cyclop 来说,该工具就比较直接了,可以判断函数的行数和语句是否超过某个值,

The default limits are 60 lines and 40 statements.

goconst

可以监测出多次使用的字符串,应该用 const 来优化

gofumpt

go fmt 更加严格,追求更加

goimports

对导入的包排序,这个挺重要的,避免你和同事互相打架

gomnd

可以检查出代码中的魔法数字(莫名奇妙出现的数字,如果没有注释,接手的人可能完全读不懂其含义)

func main() {
    var num int
    if num > 60 {
        //
    } else {
        //
    }
}


$ golangci-lint run --disable-all -E gomnd
main.go:7:11: mnd: Magic number: 60, in <condition> detected (gomnd)
        if num > 60 {
                 ^

一般使用常量或者枚举等可以优化,使得代码更加易读

const PassingScore = 60

func main() {
    var num int
    if num > PassingScore {
        //
    } else {
        //
    }
}

gosec

安全相关,检查代码中可能存在的安全风险

noctx

检查发送 http request 时是否指定了 context.Context,在涉及到网络请求时,都应该使用 context 机制将上下文串起来,这样方便做超时、链路追踪等。

prealloc

可以检查初始化 slice 时是否可以预估其容量,进而减少动态扩容时元素拷贝的性能开销

predeclared

检查代码中是否有 go 预先定义好的标识符

func copy(a string) string {
    return a
}

func foo() string {
    string := "x"
    return string
}


$ golangci-lint run --disable-all -E predeclared
main.go:7:6: function copy has same name as predeclared identifier (predeclared)
func copy(a string) string {
     ^
main.go:12:2: variable string has same name as predeclared identifier (predeclared)
        string := "x"
        ^

revive

内置了许多检查,用来替代 golint

task\message.go:9:6: exported: type name will be used as task.TaskMessage by other packages, and that stutters; consider calling this Message (revive)
type TaskMessage struct {
     ^
task\handler.go:16:6: exported: type name will be used as task.TaskDoneMsg by other packages, and that stutters; consider calling this DoneMsg (revive)
type TaskDoneMsg struct {
     ^
task\dispatch.go:30:6: exported: func name will be used as task.TaskDispatch by other packages, and that stutters; consider calling this Dispatch (revive)
func TaskDispatch(wg *sync.WaitGroup) {

检查了一下我们组某个项目的代码,发现不少类似问题,task.TaskDispath 明显比 task.Dispath 冗余了许多,就像口吃,结结巴巴的说话似的。

unconvert

检查没有必要的类型转化

做为工程师,我相信大家都喜欢阅读优美、高质量的代码,而不是 IDE 满篇高亮警示的代码。既然有这么多免费的 linter 帮你 review 代码,何愁代码质量无法提升?

关于静态分析的科普

https://github.com/fzipp/gocyclo