Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1696)

Issue 12811043: code review 12811043: cmd/vet: flag redundant invocations of String and Error... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by dsymonds
Modified:
10 years, 7 months ago
Reviewers:
r
CC:
r, golang-dev
Visibility:
Public.

Description

cmd/vet: flag redundant invocations of String and Error methods in printf calls.

Patch Set 1 #

Patch Set 2 : diff -r 93e7f796eeab https://code.google.com/p/go.tools #

Patch Set 3 : diff -r 93e7f796eeab https://code.google.com/p/go.tools #

Total comments: 4

Patch Set 4 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Patch Set 5 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Total comments: 4

Patch Set 6 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Total comments: 2

Patch Set 7 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Patch Set 8 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Total comments: 3

Patch Set 9 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Patch Set 10 : diff -r 400755f06d55 https://code.google.com/p/go.tools #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -5 lines) Patch
M cmd/vet/print.go View 1 2 3 4 5 1 chunk +26 lines, -0 lines 0 comments Download
M cmd/vet/testdata/print.go View 1 2 3 4 5 2 chunks +4 lines, -0 lines 0 comments Download
M cmd/vet/types.go View 1 2 3 4 5 6 7 8 1 chunk +20 lines, -5 lines 0 comments Download

Messages

Total messages: 16
dsymonds
Hello r (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.tools
10 years, 7 months ago (2013-08-12 22:22:56 UTC) #1
r
https://codereview.appspot.com/12811043/diff/6001/cmd/vet/print.go File cmd/vet/print.go (right): https://codereview.appspot.com/12811043/diff/6001/cmd/vet/print.go#newcode396 cmd/vet/print.go:396: func (f *File) checkIfArgIsRedundant(arg ast.Expr) { comment https://codereview.appspot.com/12811043/diff/6001/cmd/vet/print.go#newcode405 cmd/vet/print.go:405: ...
10 years, 7 months ago (2013-08-13 01:07:41 UTC) #2
dsymonds
https://codereview.appspot.com/12811043/diff/6001/cmd/vet/print.go File cmd/vet/print.go (right): https://codereview.appspot.com/12811043/diff/6001/cmd/vet/print.go#newcode396 cmd/vet/print.go:396: func (f *File) checkIfArgIsRedundant(arg ast.Expr) { On 2013/08/13 01:07:41, ...
10 years, 7 months ago (2013-08-13 01:24:41 UTC) #3
r
if type checking fails, you can still complain, but if it doesn't you should check. ...
10 years, 7 months ago (2013-08-13 01:33:55 UTC) #4
r
matchArgType will complain if it can. it might not be able to.
10 years, 7 months ago (2013-08-13 01:34:53 UTC) #5
dsymonds
Hello r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 7 months ago (2013-08-13 01:49:40 UTC) #6
r
https://codereview.appspot.com/12811043/diff/16001/cmd/vet/testdata/print.go File cmd/vet/testdata/print.go (right): https://codereview.appspot.com/12811043/diff/16001/cmd/vet/testdata/print.go#newcode192 cmd/vet/testdata/print.go:192: Printf("%s", stringerv.String()) // ERROR "redundant invocation of String method ...
10 years, 7 months ago (2013-08-13 02:18:54 UTC) #7
dsymonds
PTAL https://codereview.appspot.com/12811043/diff/16001/cmd/vet/testdata/print.go File cmd/vet/testdata/print.go (right): https://codereview.appspot.com/12811043/diff/16001/cmd/vet/testdata/print.go#newcode192 cmd/vet/testdata/print.go:192: Printf("%s", stringerv.String()) // ERROR "redundant invocation of String ...
10 years, 7 months ago (2013-08-13 02:27:26 UTC) #8
r
https://codereview.appspot.com/12811043/diff/21001/cmd/vet/types.go File cmd/vet/types.go (right): https://codereview.appspot.com/12811043/diff/21001/cmd/vet/types.go#newcode287 cmd/vet/types.go:287: func (f *File) returnsString(sel *ast.SelectorExpr) bool { should just ...
10 years, 7 months ago (2013-08-13 03:26:35 UTC) #9
dsymonds
https://codereview.appspot.com/12811043/diff/21001/cmd/vet/types.go File cmd/vet/types.go (right): https://codereview.appspot.com/12811043/diff/21001/cmd/vet/types.go#newcode287 cmd/vet/types.go:287: func (f *File) returnsString(sel *ast.SelectorExpr) bool { On 2013/08/13 ...
10 years, 7 months ago (2013-08-13 03:32:32 UTC) #10
r
that's not what i meant. sorry for being unclear. i meant, take that horrible final ...
10 years, 7 months ago (2013-08-13 03:39:09 UTC) #11
dsymonds
Hello r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 7 months ago (2013-08-13 03:40:53 UTC) #12
r
https://codereview.appspot.com/12811043/diff/23007/cmd/vet/types.go File cmd/vet/types.go (right): https://codereview.appspot.com/12811043/diff/23007/cmd/vet/types.go#newcode276 cmd/vet/types.go:276: // Finally the real questions. s/s// https://codereview.appspot.com/12811043/diff/23007/cmd/vet/types.go#newcode280 cmd/vet/types.go:280: // ...
10 years, 7 months ago (2013-08-13 03:44:19 UTC) #13
dsymonds
Hello r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
10 years, 7 months ago (2013-08-13 03:45:55 UTC) #14
r
LGTM
10 years, 7 months ago (2013-08-13 03:47:18 UTC) #15
dsymonds
10 years, 7 months ago (2013-08-13 03:48:06 UTC) #16
*** Submitted as
https://code.google.com/p/go/source/detail?r=44387735a77d&repo=tools ***

cmd/vet: flag redundant invocations of String and Error methods in printf calls.

R=r
CC=golang-dev
https://codereview.appspot.com/12811043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b