Rewrite sqlvet using the analyzer framework.#8
Conversation
Use golang.org/x/tools/go/analysis framework. It's simpler, faster. It also
allows analyzing individual packages, not the whole program.
As a simple benchmark, running the sqlvet (old or new) on a 100kloc source takes
about 10s. Analyzing a single file in that source takes <1s.
This PR introduces a few incompatible changes:
- The commandline format changes. It's now
sqlvet [-f sqlvet.toml] packages...
The sqlvet.toml file must be in ".", or its location must be explicitly
specified by the new "-f" flag. This commandline format is compatible with by
most other analyzers, including govet and staticcheck.
The old `sqlvet <dir>` command becomes:
cd <dir> && sqlvet ./...
- It removes the support for string concatenation.
db.Query("SELECT" + " 1")
won't work any more. I personally think this isn't that big deal, we can
easily rewrite it using a `raw string`.
|
Thank you @yasushi-saito, the code simplification looks great 👍 I will take a closer look at it tomorrow :) |
|
Thanks @yasushi-saito again for taking a stab at this. I now remember why I went with the SSA form instead of AST in sqlvet. I think I even read the vet-analyzer article you linked at the time because it predates the project. I initially started with AST, but found out it was too restricted for more advanced code analysis. String concatenation is the simplest application of the SSA call graph, in my todo, I was also planning to do function tracing and recursively resolve variable type and value to reduce false positives. For example, the Analyzer approach is currently causing the following false positives to one of my projects: type Foo struct {
DbName string
}
func (s Foo) Dosomething() {
// ...
dbConn.Exec("QUERY ..." + s.DbName)
}Have you considered the following two workarounds?
|
| // log.Printf("QQQQ: %v err=%v", qs.Query, qs.Err) | ||
|
|
There was a problem hiding this comment.
| // log.Printf("QQQQ: %v err=%v", qs.Query, qs.Err) |
| if shouldIgnoreNode(pass, ignoredNodes, n.Pos()) { | ||
| return | ||
| } |
There was a problem hiding this comment.
I think the ignore check could be performed at the beginning of the callback?
|
Any word on updating this? |
|
@houqp I guess this is somewhat stale ? If we manage to update this MR or some other version of it; could we have this merged ? It would be a really nice addition to |
|
@adrianlungu yes, I am open to merging this as long as the limitations in my comments are addressed 👍 |
Use golang.org/x/tools/go/analysis framework. It's simpler, faster. It also
allows analyzing individual packages, not the whole program.
As a simple benchmark, running the sqlvet (old or new) on a 100kloc source takes
about 10s. Analyzing a single file in that source takes <1s.
This PR introduces a few incompatible changes:
The commandline format changes. It's now
sqlvet [-f sqlvet.toml] packages...
The sqlvet.toml file must be in ".", or its location must be explicitly
specified by the new "-f" flag. This commandline format is compatible with by
most other analyzers, including govet and staticcheck.
The old
sqlvet <dir>command becomes:cd
&& sqlvet ./...It removes the support for string concatenation.
db.Query("SELECT" + " 1")
won't work any more. I personally think this isn't that big deal, we can
easily rewrite it using a
raw string.