Adds ability to ignore false positives

[codyl-stripe]

Summary

This addresses #1 and now allows users to override a false positive by providing a comment //nolint:safesql.

When a statement is ignored, it will still be logged but will not cause the analyzer to exit with an exit code of 1 if all found statements are ignored.

Testing

I created a simple file of

package testdata

import (
	"database/sql"
	"fmt"
)

func main() {
	fmt.Println(query("'test' OR 1=1"))
}

const GetAllQuery = "SELECT COUNT(*) FROM t WHERE arg=%s"

func query(arg string) error {
	db, err := sql.Open("postgres", "postgresql://test:test@test")
	if err != nil {
		return err
	}

	query := fmt.Sprintf(GetAllQuery, arg)
	//nolint:safesql
	row := db.QueryRow(query)
	var count int
	if err := row.Scan(&count); err != nil {
		return err
	}

	row = db.QueryRow(fmt.Sprintf(GetAllQuery, "Catch me please?")) //nolint:safesql
	if err := row.Scan(&count); err != nil {
		return err
	}

	return nil
}

And ran the linter which now produces the following output and returns a status code of 1:

- /Users/codyl/go/src/github.com/stripe/safesql/testdata/injected.go:22:20 is potentially unsafe but ignored by comment
- /Users/codyl/go/src/github.com/stripe/safesql/testdata/injected.go:28:19 is potentially unsafe but ignored by comment

I also tested with removing 1 of the comments and confirmed that the analyzer does return an exit code of 1.

Further work

In a follow up PR I will split out the code to allow for easier testing, add some tests, and also allow it to be imported into more extensive linters.

[bobby-stripe]

directionally looks good! I had a suggestion about just reading in the whole file and splitting it into an array of lines to make checking for comments easier - LMK if I'm missing something or if you have questions!

[bobby-stripe]

can we make this simpler by not using a scanner? e.g.

contents, err := ioutil.ReadFile(file)
lines := strings.Split(string(contents), "\n")

and then just check HasIgnoreComment(line.Line) and BeginsWithComment(line.Line-1) && HasIgnoreComment(line.Line-1).

I think this would make the below for loop much much simpler

[codyl-stripe]

I actually did this to reduce the amount of memory the program could use, ie if a very large file was being processed.

[bobby-stripe]

sure, but we only process a single file at a time (each time through the loop, the previous file goes out of scope). Source code files are only 10s of KB in size, and it feels like it would be a big win in terms of readability + simplicity to just read the whole thing in at once.

[dcarney-stripe]

Great comment. 👍

[dcarney-stripe]

If you want, you can use the %q format string, which will quote the string automatically. I find it to be helpful when including strings in an output message.

[codyl-stripe]

I am hesitant to change this as I am not sure if anyone is parsing this statement

[dcarney-stripe]

Fair enough. I think in a future release (now that we'll have releases!), it would be nice to modify this to use the standard linter-style output format that you get with Go's analysis package. That way, it'll integrate nicely with other Go tooling.

[dcarney-stripe]

Looks pretty good. I think adding some test cases around the ignore comment handling would make this ready to go.

[bobby-stripe]

looks great, thank you :)

[dcarney-stripe]

👍