dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.01k stars 361 forks source link

quickfix: replace manual slicing with `strings.HasPrefix` #1557

Open ccoVeille opened 4 weeks ago

ccoVeille commented 4 weeks ago

I faced to following piece of advice in recent code review

Please consider the following pseudocode


func foo(path string) {
    if path[0] == '/' { // here we are checking if first character is a slash
        // whatever
    }

    // ...
}

foo("/path/foo/bar)
foo("path/foo/bar)

The following code should be preferred and suggested


func foo(path string) {
    if strings.HasPrefix(path, "/") {
        // whatever
    }

    // ...
}

foo("/path/bar")
foo("path/bar")

This rule someone completes S1017

The pattern is very common:

ccoVeille commented 3 weeks ago

The opposite may also be detected

func foo(path string) {
    if path[len(path)-1] == '/' { // here we are checking if last character is a slash
        // whatever
    }

    // ...
}

foo("/path/bar/")
foo("/path/bar")
func foo(path string) {
    if strings.HasSuffix(path, "/") {
        // whatever
    }

    // ...
}
ccoVeille commented 3 weeks ago

patterns like this could also be detected, with the same checker maybe


    path := "foo"

    if path[0:2] == "fo" {
        // whatever
    }

    // 

This pattern is pretty common https://github.com/search?q=lang%3Ago+%22%5B0%3A2%5D+%3D%3D%22&type=code https://github.com/search?q=lang%3Ago+%22%5B0%3A2%5D+%21%3D%22&type=code

example https://github.com/grafana/k6/blob/93649df0939a4cb89828da2be40c98100e92fab1/lib/archive.go#L161