diff --git a/go/analysis/passes/timeformat/testdata/src/a/a.go b/go/analysis/passes/timeformat/testdata/src/a/a.go new file mode 100644 index 0000000000..98481446e5 --- /dev/null +++ b/go/analysis/passes/timeformat/testdata/src/a/a.go @@ -0,0 +1,50 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the timeformat checker. + +package a + +import ( + "time" + + "b" +) + +func hasError() { + a, _ := time.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00") // want `2006-02-01 should be 2006-01-02` + a.Format(`2006-02-01`) // want `2006-02-01 should be 2006-01-02` + a.Format("2006-02-01 15:04:05") // want `2006-02-01 should be 2006-01-02` + + const c = "2006-02-01" + a.Format(c) // want `2006-02-01 should be 2006-01-02` +} + +func notHasError() { + a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00") + a.Format("2006-01-02") + + const c = "2006-01-02" + a.Format(c) + + v := "2006-02-01" + a.Format(v) // Allowed though variables. + + m := map[string]string{ + "y": "2006-02-01", + } + a.Format(m["y"]) + + s := []string{"2006-02-01"} + a.Format(s[0]) + + a.Format(badFormat()) + + o := b.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00") + o.Format("2006-02-01") +} + +func badFormat() string { + return "2006-02-01" +} diff --git a/go/analysis/passes/timeformat/testdata/src/a/a.go.golden b/go/analysis/passes/timeformat/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..9eccded63b --- /dev/null +++ b/go/analysis/passes/timeformat/testdata/src/a/a.go.golden @@ -0,0 +1,50 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests for the timeformat checker. + +package a + +import ( + "time" + + "b" +) + +func hasError() { + a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00") // want `2006-02-01 should be 2006-01-02` + a.Format(`2006-01-02`) // want `2006-02-01 should be 2006-01-02` + a.Format("2006-01-02 15:04:05") // want `2006-02-01 should be 2006-01-02` + + const c = "2006-02-01" + a.Format(c) // want `2006-02-01 should be 2006-01-02` +} + +func notHasError() { + a, _ := time.Parse("2006-01-02 15:04:05", "2021-01-01 00:00:00") + a.Format("2006-01-02") + + const c = "2006-01-02" + a.Format(c) + + v := "2006-02-01" + a.Format(v) // Allowed though variables. + + m := map[string]string{ + "y": "2006-02-01", + } + a.Format(m["y"]) + + s := []string{"2006-02-01"} + a.Format(s[0]) + + a.Format(badFormat()) + + o := b.Parse("2006-02-01 15:04:05", "2021-01-01 00:00:00") + o.Format("2006-02-01") +} + +func badFormat() string { + return "2006-02-01" +} diff --git a/go/analysis/passes/timeformat/testdata/src/b/b.go b/go/analysis/passes/timeformat/testdata/src/b/b.go new file mode 100644 index 0000000000..de5690863c --- /dev/null +++ b/go/analysis/passes/timeformat/testdata/src/b/b.go @@ -0,0 +1,11 @@ +package b + +type B struct { +} + +func Parse(string, string) B { + return B{} +} + +func (b B) Format(string) { +} diff --git a/go/analysis/passes/timeformat/timeformat.go b/go/analysis/passes/timeformat/timeformat.go new file mode 100644 index 0000000000..9147826b4a --- /dev/null +++ b/go/analysis/passes/timeformat/timeformat.go @@ -0,0 +1,131 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package timeformat defines an Analyzer that checks for the use +// of time.Format or time.Parse calls with a bad format. +package timeformat + +import ( + "fmt" + "go/ast" + "go/constant" + "go/token" + "go/types" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/types/typeutil" +) + +const badFormat = "2006-02-01" +const goodFormat = "2006-01-02" + +const Doc = `check for calls of (time.Time).Format or time.Parse with 2006-02-01 + +The timeformat checker looks for time formats with the 2006-02-01 (yyyy-dd-mm) +format. Internationally, "yyyy-dd-mm" does not occur in common calendar date +standards, and so it is more likely that 2006-01-02 (yyyy-mm-dd) was intended. +` + +var Analyzer = &analysis.Analyzer{ + Name: "timeformat", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + call := n.(*ast.CallExpr) + fn, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Func) + if !ok { + return + } + if !isTimeDotFormat(fn) && !isTimeDotParse(fn) { + return + } + if len(call.Args) > 0 { + arg := call.Args[0] + badAt := badFormatAt(pass.TypesInfo, arg) + + if badAt > -1 { + // Check if it's a literal string, otherwise we can't suggest a fix. + if _, ok := arg.(*ast.BasicLit); ok { + fmt.Printf("%#v\n", arg) + pos := int(arg.Pos()) + badAt + 1 // +1 to skip the " or ` + end := pos + len(badFormat) + + pass.Report(analysis.Diagnostic{ + Pos: token.Pos(pos), + End: token.Pos(end), + Message: badFormat + " should be " + goodFormat, + SuggestedFixes: []analysis.SuggestedFix{{ + Message: "Replace " + badFormat + " with " + goodFormat, + TextEdits: []analysis.TextEdit{{ + Pos: token.Pos(pos), + End: token.Pos(end), + NewText: []byte(goodFormat), + }}, + }}, + }) + } else { + pass.Reportf(arg.Pos(), badFormat+" should be "+goodFormat) + } + } + } + }) + return nil, nil +} + +func isTimeDotFormat(f *types.Func) bool { + if f.Name() != "Format" || f.Pkg().Path() != "time" { + return false + } + sig, ok := f.Type().(*types.Signature) + if !ok { + return false + } + // Verify that the receiver is time.Time. + recv := sig.Recv() + if recv == nil { + return false + } + named, ok := recv.Type().(*types.Named) + return ok && named.Obj().Name() == "Time" +} + +func isTimeDotParse(f *types.Func) bool { + if f.Name() != "Parse" || f.Pkg().Path() != "time" { + return false + } + // Verify that there is no receiver. + sig, ok := f.Type().(*types.Signature) + return ok && sig.Recv() == nil +} + +// badFormatAt return the start of a bad format in e or -1 if no bad format is found. +func badFormatAt(info *types.Info, e ast.Expr) int { + tv, ok := info.Types[e] + if !ok { // no type info, assume good + return -1 + } + + t, ok := tv.Type.(*types.Basic) + if !ok || t.Info()&types.IsString == 0 { + return -1 + } + + if tv.Value == nil { + return -1 + } + + return strings.Index(constant.StringVal(tv.Value), badFormat) +} diff --git a/go/analysis/passes/timeformat/timeformat_test.go b/go/analysis/passes/timeformat/timeformat_test.go new file mode 100644 index 0000000000..86bbe1bb3f --- /dev/null +++ b/go/analysis/passes/timeformat/timeformat_test.go @@ -0,0 +1,17 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package timeformat_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/timeformat" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, timeformat.Analyzer, "a") +} diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 861be1ca62..90a5118a4b 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -495,6 +495,17 @@ identifiers. Please see the documentation for package testing in golang.org/pkg/testing for the conventions that are enforced for Tests, Benchmarks, and Examples. +**Enabled by default.** + +## **timeformat** + +check for calls of (time.Time).Format or time.Parse with 2006-02-01 + +The timeformat checker looks for time formats with the 2006-02-01 (yyyy-dd-mm) +format. Internationally, "yyyy-dd-mm" does not occur in common calendar date +standards, and so it is more likely that 2006-01-02 (yyyy-mm-dd) was intended. + + **Enabled by default.** ## **unmarshal** diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index 807f496a6b..e20b8a671d 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -378,6 +378,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for common mistaken usages of tests and examples\n\nThe tests checker walks Test, Benchmark and Example functions checking\nmalformed names, wrong signatures and examples documenting non-existent\nidentifiers.\n\nPlease see the documentation for package testing in golang.org/pkg/testing\nfor the conventions that are enforced for Tests, Benchmarks, and Examples.", Default: "true", }, + { + Name: "\"timeformat\"", + Doc: "check for calls of (time.Time).Format or time.Parse with 2006-02-01\n\nThe timeformat checker looks for time formats with the 2006-02-01 (yyyy-dd-mm)\nformat. Internationally, \"yyyy-dd-mm\" does not occur in common calendar date\nstandards, and so it is more likely that 2006-01-02 (yyyy-mm-dd) was intended.\n", + Default: "true", + }, { Name: "\"unmarshal\"", Doc: "report passing non-pointer or non-interface values to unmarshal\n\nThe unmarshal analysis reports calls to functions such as json.Unmarshal\nin which the argument type is not a pointer or an interface.", @@ -966,6 +971,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for common mistaken usages of tests and examples\n\nThe tests checker walks Test, Benchmark and Example functions checking\nmalformed names, wrong signatures and examples documenting non-existent\nidentifiers.\n\nPlease see the documentation for package testing in golang.org/pkg/testing\nfor the conventions that are enforced for Tests, Benchmarks, and Examples.", Default: true, }, + { + Name: "timeformat", + Doc: "check for calls of (time.Time).Format or time.Parse with 2006-02-01\n\nThe timeformat checker looks for time formats with the 2006-02-01 (yyyy-dd-mm)\nformat. Internationally, \"yyyy-dd-mm\" does not occur in common calendar date\nstandards, and so it is more likely that 2006-01-02 (yyyy-mm-dd) was intended.\n", + Default: true, + }, { Name: "unmarshal", Doc: "report passing non-pointer or non-interface values to unmarshal\n\nThe unmarshal analysis reports calls to functions such as json.Unmarshal\nin which the argument type is not a pointer or an interface.", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 8abcc7452c..6881a7cb82 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -43,6 +43,7 @@ import ( "golang.org/x/tools/go/analysis/passes/structtag" "golang.org/x/tools/go/analysis/passes/testinggoroutine" "golang.org/x/tools/go/analysis/passes/tests" + "golang.org/x/tools/go/analysis/passes/timeformat" "golang.org/x/tools/go/analysis/passes/unmarshal" "golang.org/x/tools/go/analysis/passes/unreachable" "golang.org/x/tools/go/analysis/passes/unsafeptr" @@ -1393,6 +1394,7 @@ func defaultAnalyzers() map[string]*Analyzer { useany.Analyzer.Name: {Analyzer: useany.Analyzer, Enabled: false}, infertypeargs.Analyzer.Name: {Analyzer: infertypeargs.Analyzer, Enabled: true}, embeddirective.Analyzer.Name: {Analyzer: embeddirective.Analyzer, Enabled: true}, + timeformat.Analyzer.Name: {Analyzer: timeformat.Analyzer, Enabled: true}, // gofmt -s suite: simplifycompositelit.Analyzer.Name: {