Be careful not to complain about math.Log and cmplx.Log.
Seems worthwhile since t.Log and t.Logf are often written but
rarely executed.
Nothing new turned up in the standard library.
Fixesgolang/go#8504.
LGTM=josharian, dsymonds
R=golang-codereviews, josharian, dsymonds
CC=golang-codereviews
https://golang.org/cl/130490043
s/enclosed by function/captured by func literal/
Users complained. They often do.
LGTM=josharian, adg
R=golang-codereviews, josharian, nightlyone, minux, adg
CC=golang-codereviews
https://golang.org/cl/132080043
This CL introduces two vet checks. Statistics and code below are from a recent 50mb corpus of public code.
1. Check for redundant conjunctions and disjunctions. This check caught 26 instances, of which 20 were clearly copy/paste bugs and 6 appeared to be mere duplication. A typical example:
if xResolution < 0 || xResolution < 0 {
panic("SetSize(): width < 0 || height < 0")
}
2. Check for expressions of the form 'x != c1 || x != c2' or 'x == c1 && x == c2', with c1 and c2 constant expressions. This check caught 16 instances, of which all were bugs. A typical example:
if rf.uri.Scheme != "http" || rf.uri.Scheme != "ftp" {
rf.uri.Scheme = "file"
}
Fixesgolang/go#7622.
LGTM=rsc, r
R=golang-codereviews, jscrockett01, r, gri, rsc
CC=golang-codereviews
https://golang.org/cl/98120043
Really two fixes: Don't panic on bad instructions and don't complain about commented out instructions.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/110070044
This is a common source of bugs, particularly for those new to Go. Running this on a corpus of public code flagged 114 instances.
This check may need to be updated once issue 7363 is resolved.
LGTM=r
R=golang-codereviews, r
CC=bradfitz, golang-codereviews
https://golang.org/cl/91010047
This removes much of the AST logic out of main.go,
and makes it easier to build custom vet binaries
The trade-off in this change is for flexibility.
There's very little change in the per-check files,
a lot less code in main.go (specifically the AST walking
logic has shrunk), and it makes it much easier to build
custom vet binaries simply by dropping new source files
in the directory.
LGTM=josharian, r
R=r, josharian, kamil.kisiel
CC=golang-codereviews
https://golang.org/cl/83400043
It was very ugly; a little tweaking helps godoc parse it better.
Also make unsafeptr.go not own the package doc (add a blank line)
and put one more sentence about that check into doc.go.
Fixesgolang/go#7925.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/98370044
Ignore calls to various flavours of atomic.AddInt with a wrong
number of arguments.
LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/91370045
It's pointless.
Also this fixes a crash, because the blank identifier no longer appears as a
defined object after CL 74190043 so we were getting nil pointer violations.
Even better, we get to re-enable a disabled test.
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/75140043
Now we can say
vet -printf=false
to disable the printf test but run all others.
Implemented by creating a tri-state boolean flag that records whether it has been
set explicitly; before this, -printf=false was not distinguishable from not having
mentioned the printf flag at all.
Fixesgolang/go#7422.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/72330043
Over time, a number of modules were added that used Warn instead of Bad
to report problems with the code, but the documentation states that
if there is a problem, the exit code must be 1, not 0. Warn does not set the
exit code and should be used only for internal errors and messages
triggered by the -v flag.
There's nothing substantive here except calling the other function in a few
places.
Fixesgolang/go#7017.
LGTM=crawshaw
R=golang-codereviews, crawshaw
CC=golang-codereviews
https://golang.org/cl/71860044
The old code was misleading in saying how many args were present.
Change the wording of the message to be unambiguous and change
the presentation of the format to include the full directive, making
it easier to correlate with the input (and fixing a silent bug).
Fixesgolang/go#6248.
LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=golang-codereviews
https://golang.org/cl/69120044
An identifier X in anonymous struct field struct{X} is both a
definition of a field (*Var) and reference to a type
(*TypeName). Now that we have split the map, we can capture
both of these aspects.
Interestingly, every client but one was going to extra effort
to iterate over just the uses or just the defs; this
simplifies them.
Also, fix two bug related to tagless switches:
- An entry was being recorded in the Object map for a piece of
synthetic syntax.
- The "true" identifier was being looked up in the current scope,
which allowed perverse users to locally redefine it. Now
we use the bool (not untyped boolean) constant true, per the
consequent clarification of the spec (issue 7404).
+ tests.
Fixesgolang/go#7276
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/68270044
How it handles packages vs. directories vs. files was not explained.
LGTM=rsc
R=golang-codereviews, gobot, rsc
CC=golang-codereviews
https://golang.org/cl/67150043
These are the simplest possible descriptions of each command.
They may be fleshed out later.
Fixesgolang/go#7298.
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/61480044
Method-set caching is now performed externally using a MethodSetCache (if desired), not by the Types themselves.
This a minor deoptimization due to the extra maps, but avoids a situation in which method-sets are computed and frozen prematurely. (See b/7114)
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/61430045
This results in significant improvement to type-checking time:
it reduces by 4% the entire running time of ssa/stdlib_test
(GOMAXPROCS=8, n=7).
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/57770043
Using a type containing a sync type directly
in a function call (whether as a receiver,
a param, or a return value) is an easy way
to accidentally copy a lock or other sync primitive.
Check for it.
The test as implemented does not provide 100%
coverage; see the discussion near the bottom of
testdata/copylock.go for shortcomings.
Fixesgolang/go#6729.
R=adg, r, dsymonds
CC=golang-dev
https://golang.org/cl/23420043
- fixed a couple of TODOs
- various cleanups along the way
- adjusted clients
Once submitted, clients of go/types that don't explicitly
specify Config.Import will need to add the extra import:
import _ "code.google.com/p/go.tools/go/gcimporter"
to install the default (gc) importer in go/types.
R=adonovan, gri
CC=golang-dev
https://golang.org/cl/26390043
The symbolic names such as NOSPLIT for annotations on the TEXT
directive appeared after vet started checking .s files. This CL tweaks
the regular expression to allow CAPITALS and the symbols | and +
as well as digits in that field, and interprets NOSPLIT as equivalent
to 7 in that field. All magic.
Fixesgolang/go#6695
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/18700044
- removed support for nil constants from go/exact
- instead define a singleton Nil Object (the nil _value_)
- in assignments, follow more closely spec wording
(pending spec CL 14415043)
- removed use of goto in checker.unary
- cleanup around handling of isRepresentable for
constants, with better error messages
- fix missing checks in checker.convertUntyped
- added isTyped (== !isUntyped) and isInterface predicates
- fixed hasNil predicate: unsafe.Pointer also has nil
- adjusted ssa per adonovan
- implememted types.Implements (wrapper arounfd types.MissingMethod)
- use types.Implements in vet (and fix a bug)
R=adonovan, r
CC=golang-dev
https://golang.org/cl/14438052
Update golang/go#6212
See issue 6259.
When that is resolved, we can do a better job. Until then, we just see if the
type has a method called Format and, if so, assume it's a Formatter and so
there's nothing to check.
R=golang-dev, dsymonds
CC=golang-dev
https://golang.org/cl/13267043
App Engine needs the whitelist file and it's cleaner if it's
in its own package where it can be imported directly, without
pushing composite.go through a pile of sed.
R=dsymonds, r
CC=golang-dev
https://golang.org/cl/12746045
TBR: gri
I cannot create an issue on the tracker for some reason, so here it is:
go vet contains this snippet:
if types.IsAssignableTo(typ, errorType) || types.IsAssignableTo(typ, stringerType) {
It's getting the wrong answer: It claims
interface {
f()
}
or even
interface {
f() float64
}
matches the Error and Stringer interfaces. Both of them. This causes a test failure:
$ go test code.google.com/p/go.tools/cmd/vet
BUG: errchk: testdata/print.go:124: missing expected error: '"for printf verb %s of wrong type"'
$
This worked until very recently.
R=gri
CC=golang-dev
https://golang.org/cl/12398043
