Commit Graph

5 Commits

Author SHA1 Message Date
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 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 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 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