Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

quickfix: replace manual slicing with strings.HasPrefix #1557

Open
ccoVeille opened this issue Jun 3, 2024 · 2 comments
Open

quickfix: replace manual slicing with strings.HasPrefix #1557

ccoVeille opened this issue Jun 3, 2024 · 2 comments

Comments

@ccoVeille
Copy link

ccoVeille commented Jun 3, 2024

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 ccoVeille added the needs-triage Newly filed issue that needs triage label Jun 3, 2024
@ccoVeille
Copy link
Author

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
Copy link
Author

ccoVeille commented Jun 3, 2024

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

@dominikh dominikh added new-check and removed needs-triage Newly filed issue that needs triage labels Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants