Commit Graph

9 Commits

Author SHA1 Message Date
Daniel Martí e5f3ab76ea go/analysis: fix more printf pointer bugs
The first issue is that %b and %o can print pointers, but printVerbs
didn't reflect that:

	The %b, %d, %o, %x and %X verbs also work with pointers,
	formatting the value exactly as if it were an integer.

The second issue is that arrays can never be printed as pointers. This
was previously reported as part of #27672.

The third issue is that only %p can print all slices, maps, and
functions as if they were pointers. This differs from verbs like %b or
%o, which can't print these types as pointers.

Fix all of the issues above, and add extensive test cases covering all
the combinations. Verified all of them with an executed program. The
amount of test cases is perhaps overkill, but this is not the first time
we've gotten the printf pointer logic wrong.

Updates #27672.
Fixes #28858.

Change-Id: I62eb79d505fd1e250a16b90bda3c68b702f35a29
Reviewed-on: https://go-review.googlesource.com/c/149979
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
2018-12-03 21:00:56 +00:00
Alan Donovan f62bfb5415 go/analysis/passes/printf: fix regression in "recursive stringer" logic
The recursive stringer check should report cases such as

   func (x T) String() string { return fmt.Sprint(x) }

in which the receiver x (or possibly &x) was passed into a fmt print call.
However, in translating it from the go/ast to the go/types representation,
I inadvertently made it report any situation in which a value of type T
was passed to fmt, even when the value is not x, as in:

   func (cons *cons) String() string {
	... fmt.Sprint(cons.cdr) ...
   }

Fixed and tested.

Change-Id: I57e88755c9989deaaad45cc306a604f3db4ee269
Reviewed-on: https://go-review.googlesource.com/c/149616
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
2018-11-15 16:22:56 +00:00
Alan Donovan 29e82b56d9 go/analysis/passes/printf: actually use doc constant
Change-Id: If33697d77a86e4b29089f3a1ba096477a34062be
Reviewed-on: https://go-review.googlesource.com/c/149740
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2018-11-15 15:56:23 +00:00
Alan Donovan 0a8e63141f go/analysis/passes/printf: fix false negative with nested pointers
Pointers to compound objects (structs, slices, arrays, maps) are only
followed by fmt if the pointer is at the top level of an argument. This
is to minimise the chances of fmt running into loops.

However, vet did not follow this rule. It likely doesn't help that fmt
does not document that restriction well, which is being tracked in
 #28625.

This change was originally made to cmd/vet as
https://go-review.googlesource.com/c/147997.

Updates #27672.

Change-Id: I65944cf355baedb4578af57046e2bbfd3fe6a9dc
Reviewed-on: https://go-review.googlesource.com/c/149319
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
2018-11-13 19:37:33 +00:00
Alan Donovan ea84011da2 go/analysis/passes/printf: fix some pointer false positives
fmt's godoc reads:

	For compound objects, the elements are printed using these
	rules, recursively, laid out like this:

		struct:             {field0 field1 ...}
		array, slice:       [elem0 elem1 ...]
		maps:               map[key1:value1 key2:value2 ...]
		pointer to above:   &{}, &[], &map[]

That is, a pointer to a struct, array, slice, or map, can be correctly
printed by fmt if the type pointed to can be printed without issues.

vet was only following this rule for pointers to structs, omitting
arrays, slices, and maps. Fix that, and add tests for all the
combinations.

This change was originally made to cmd/vet in
https://go-review.googlesource.com/c/147758

Updates #27672.

Change-Id: I7e25ecaeed619ae8b6ada79bccacba6b67171733
Reviewed-on: https://go-review.googlesource.com/c/149318
Reviewed-by: Michael Matloob <matloob@golang.org>
2018-11-13 18:24:24 +00:00
Alan Donovan 92d8274bd7 go/analysis/passes/printf: preload with facts for std lib
This change adds to the list of standard library functions known to be
print or printf wrappers.

Although the printf Analyzer is capable of identifying wrapper
functions in the standard library, some drivers (e.g. Bazel) do not
apply analyzers to the standard packages. Really this is a  bug
in those drivers but it is not likely to be fixed for a while.

Change-Id: I2032d0cb5fcb50e7b9933a75809becdd680380ec
Reviewed-on: https://go-review.googlesource.com/c/148572
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
2018-11-09 20:29:20 +00:00
Alan Donovan 3a0c4deef1 go/analysis/passes/printf: changes for analysis API
Guide to changes:

- The -printfuncs flag is renamed -printf.funcs.
  It no longer supports "pkg.T.f" form, requiring instead
  "(dir/pkg.T).f" or "(*dir/pkg.T).f".
  The legacy ":%d" suffix is no longer supported.

- (*testing.T).Errorf and friends are removed from the isPrint map
  because they are discovered by induction while analyzing package
  "testing".

- localPrintfLike map operations are replaced by the Fact mechanism.

- The go/types representation is used instead of strings/ast.Nodes in
  various places. For example:

       pkgpath, name string -> *types.Func  (to identify a function)
    format, args *ast.Field -> *types.Var   (to identify format/args params)

  This was required to fix a latent bug in maybePrintfWrapper's
  handling of format string parameters` declared using "factored"
  syntax, such as: func f(foo, format string, args...interface{}).
  See L253 of the original testdata file for a testcase that ensured
  the buggy (?) behavior.

- func printfLike is removed as it was deadcode.

- isFormatter is rewritten to avoid a global variable.

- "if verbose { warn }" is replaced by "if false { report }" for now.

- recursive stringer is rewritten more simply in term of go/types.

Change-Id: Ia6ee827117b611c686e38207916a21fe1fc296e2
Reviewed-on: https://go-review.googlesource.com/c/142239
Reviewed-by: Michael Matloob <matloob@golang.org>
2018-10-16 19:44:49 +00:00
Alan Donovan cdd5f6199c go/analysis/passes/printf: add types.go too
This file is nominally part of the core vet tool but in reality it is
only used by the printf checker.

This is just a rename; the actual change will come in a follow-up.

Change-Id: I5497d6888228e6781cf0d3ae62b7c6b5b723e151
Reviewed-on: https://go-review.googlesource.com/c/142240
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
2018-10-15 19:52:40 +00:00
Alan Donovan 104357fe96 go/analysis/passes/printf: move printf to correct subdirectory
Change-Id: I9c510fe042461707a47d8ba4fb4f70c33efbc8cc
Reviewed-on: https://go-review.googlesource.com/c/142238
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
2018-10-15 19:19:59 +00:00