"go vet" doesn't catch printf formatting mistakes when using custom packages |
|||
Issue descriptionWhen I run "go vet" against the Tast source via the fast_build.sh script (https://chromium.googlesource.com/chromiumos/platform/tast/+/master/fast_build.sh), it doesn't appear to catch Log/Logf, Error/Errorf, etc. formatting mistakes that are made when using custom types that I've introduced, e.g. https://chromium.googlesource.com/chromiumos/platform/tast/+/master/src/chromiumos/tast/testing/state.go#85 https://chromium.googlesource.com/chromiumos/platform/tast/+/master/src/chromiumos/cmd/tast/logging/logger.go#19 I don't understand why this is. It works when I make mistakes when using go's log or testing package, and https://github.com/golang/go/issues/12294 suggests that vet should be checking functions with these names even when they're in custom packages. I tried switching to "go tool vet", which has slightly different behavior and is harder to use (i.e. it appears to want directories instead of packages), but it doesn't appear to catch this either.
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/tast/+/7d7d34b06d06e30abc8cac9de56195706be93b60 commit 7d7d34b06d06e30abc8cac9de56195706be93b60 Author: Daniel Erat <derat@chromium.org> Date: Wed Feb 21 20:39:28 2018 tast: Make "fast_build.sh -C" vet all in a single pass. Update the behavior of fast_build.sh's -C flag to use a single "go vet" invocation to vet all packages at once instead of running "go vet" for each package. I didn't realize that this works, but it cuts the running time on my workstation down from 12 seconds to 2 seconds. BUG= chromium:814327 TEST=manual: ran it Change-Id: I43019289db8922a812ddaa6443e72b722a9b63d2 Reviewed-on: https://chromium-review.googlesource.com/928941 Tested-by: Dan Erat <derat@chromium.org> Trybot-Ready: Dan Erat <derat@chromium.org> Reviewed-by: Rahul Chaudhry <rahulchaudhry@chromium.org> Commit-Queue: Dan Erat <derat@chromium.org> [modify] https://crrev.com/7d7d34b06d06e30abc8cac9de56195706be93b60/fast_build.sh
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/e17d6e12ed9b5a398cff1569c96e60a627fd861d commit e17d6e12ed9b5a398cff1569c96e60a627fd861d Author: Daniel Erat <derat@chromium.org> Date: Thu Feb 22 05:49:14 2018 tast-tests: Make security.LogPerms handle log rotation. Make the security.LogPerms local test avoid failure if /var/log/messages is missing or empty and owned by root. The file is briefly in both of these states while it's being rotated. Also update some errors in the security.LogPerms local test to be logged via Errorf rather than Error. BUG= chromium:813579 , chromium:814327 TEST=manual: test doesn't fail when messages is missing or empty and owned by root Change-Id: Iaae131234ee82051e4f07089876744cc28e18328 Reviewed-on: https://chromium-review.googlesource.com/928901 Tested-by: Dan Erat <derat@chromium.org> Reviewed-by: Eric Caruso <ejcaruso@chromium.org> [modify] https://crrev.com/e17d6e12ed9b5a398cff1569c96e60a627fd861d/src/chromiumos/tast/local/bundles/cros/security/log_perms.go
,
Sep 25
In https://crrev.com/c/1239657, I'm passing -printfuncs=Log,Logf,Error,Errorf,Fatal,Fatalf, to "go vet", which appears to make it catch mistakes in calls to testing.State.Logf etc. The documentation at https://golang.org/cmd/vet/ seems to suggest that I should be able to pass full names like chromiumos/tast/testing.State.Logf, but I haven't been able to get that to work, so I'm using the case-insensitive unqualified identifier format instead.
,
Sep 26
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/tast/+/c96b879a49f20ffe4ef6e542cb9d7ef0d0f3fbcb commit c96b879a49f20ffe4ef6e542cb9d7ef0d0f3fbcb Author: Daniel Erat <derat@chromium.org> Date: Wed Sep 26 01:57:04 2018 tast: Make fast_build.sh pass -printfuncs to "go vet". Make the "fast_build.sh -C" pass a -printfuncs flag to "go vet" instructing it to check calls to Log/Logf etc. methods on testing.State. BUG= chromium:888259 , chromium:814327 TEST=ran "./fast_build.sh -C" in the chroot Change-Id: I936636a4f15409ea3046da139946f7bbf38593ec Reviewed-on: https://chromium-review.googlesource.com/1244422 Tested-by: Dan Erat <derat@chromium.org> Trybot-Ready: Dan Erat <derat@chromium.org> Reviewed-by: Shuhei Takahashi <nya@chromium.org> [modify] https://crrev.com/c96b879a49f20ffe4ef6e542cb9d7ef0d0f3fbcb/fast_build.sh
,
Oct 23
I think that -printfuncs is probably the best we can do here. |
|||
►
Sign in to add a comment |
|||
Comment 1 by derat@chromium.org
, Feb 21 2018