Skip to content

Commit 2c963f1

Browse files
committed
time-date: add rule to check for time.Date usage
This commit introduces a new rule to check for the usage of time.Date The rule is added to report the usage of time.Date with non-decimal literals Here the leading zeros that seems OK, forces the value to be octal literals. time.Date(2023, 01, 02, 03, 04, 05, 06, time.UTC) gofumpt formats the code like this when it encounters leading zeroes. time.Date(2023, 0o1, 0o2, 0o3, 0o4, 0o5, 0o6, time.UTC) The rule reports anything that is not a decimal literal.
1 parent f6cc22d commit 2c963f1

File tree

6 files changed

+409
-0
lines changed

6 files changed

+409
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
547547
| [`string-of-int`](./RULES_DESCRIPTIONS.md#string-of-int) | n/a | Warns on suspicious casts from int to string | no | yes |
548548
| [`struct-tag`](./RULES_DESCRIPTIONS.md#struct-tag) | []string | Checks common struct tags like `json`, `xml`, `yaml` | no | no |
549549
| [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else) | []string | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) | no | no |
550+
| [`time-date`](./RULES_DESCRIPTIONS.md#time-date) | n/a | Reports bad usage of `time.Date`. | no | yes |
550551
| [`time-equal`](./RULES_DESCRIPTIONS.md#time-equal) | n/a | Suggests to use `time.Time.Equal` instead of `==` and `!=` for equality check time. | no | yes |
551552
| [`time-naming`](./RULES_DESCRIPTIONS.md#time-naming) | n/a | Conventions around the naming of time variables. | yes | yes |
552553
| [`unchecked-type-assertions`](./RULES_DESCRIPTIONS.md#unchecked-type-assertions) | n/a | Disallows type assertions without checking the result. | no | yes |

RULES_DESCRIPTIONS.md

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ List of all available rules.
7171
- [string-of-int](#string-of-int)
7272
- [struct-tag](#struct-tag)
7373
- [superfluous-else](#superfluous-else)
74+
- [time-date](#time-date)
7475
- [time-equal](#time-equal)
7576
- [time-naming](#time-naming)
7677
- [unchecked-type-assertion](#unchecked-type-assertion)
@@ -957,6 +958,56 @@ Examples:
957958
arguments = ["preserve-scope"]
958959
```
959960

961+
## time-date
962+
963+
_Description_: Reports bad usage of `time.Date`.
964+
965+
_Configuration_: N/A
966+
967+
_Example_:
968+
969+
Here the leading zeros are defining integers with octal notation
970+
971+
```go
972+
var (
973+
// here we can imagine zeroes were used for padding purpose
974+
a = time.Date(2023, 1, 2, 3, 4, 0, 00000000, time.UTC) // 00000000 is octal and equals 0 in decimal
975+
b = time.Date(2023, 1, 2, 3, 4, 0, 00000006, time.UTC) // 00000006 is octal and equals 6 in decimal
976+
c = time.Date(2023, 1, 2, 3, 4, 0, 00123456, time.UTC) // 00123456 is octal and equals 42798 in decimal
977+
)
978+
```
979+
980+
But here, what was expected 123456 or 42798 ? It's a source of bugs.
981+
982+
So the rule will report this
983+
984+
```
985+
octal notation with padding zeroes found: choose between 123456 and 42798 (decimal value of 123456 octal value)
986+
```
987+
988+
This rule also reports strange notations used with time.Date.
989+
990+
Example:
991+
```go
992+
_ = time.Date(
993+
0x7e7, // hexadecimal notation: use 2023 instead of 0x7e7/
994+
0b1, // binary notation: use 1 instead of 0b1/
995+
0x_2, // hexadecimal notation: use 2 instead of 0x_2/
996+
1_3, // alternative notation: use 13 instead of 1_3/
997+
1e1, // exponential notation: use 10 instead of 1e1/
998+
0., // float literal: use 0 instead of 0./
999+
0x1.Fp+6, // float literal: use 124 instead of 0x1.Fp+6/
1000+
time.UTC)
1001+
```
1002+
1003+
All these are considered to be an uncommon usage of time.Date, are reported with a 0.8 confidence.
1004+
1005+
Note: even if 00, 01, 02, 03, 04, 05, 06, 07 are octal numbers, they can be considered as valid, and reported with 0.5 confidence.
1006+
1007+
```go
1008+
var _ = time.Date(2023, 01, 02, 03, 04, 00, 0, time.UTC)
1009+
```
1010+
9601011
## time-equal
9611012
9621013
_Description_: This rule warns when using `==` and `!=` for equality check `time.Time` and suggest to `time.time.Equal` method, for about information follow this [link](https://pkg.go.dev/time#Time)

config/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ var allRules = append([]lint.Rule{
8383
&rule.UselessBreak{},
8484
&rule.UncheckedTypeAssertionRule{},
8585
&rule.TimeEqualRule{},
86+
&rule.TimeDateRule{},
8687
&rule.BannedCharsRule{},
8788
&rule.OptimizeOperandsOrderRule{},
8889
&rule.UseAnyRule{},

rule/time_date.go

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
package rule
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"go/ast"
7+
"go/token"
8+
"strconv"
9+
"strings"
10+
11+
"github.com/mgechev/revive/lint"
12+
"github.com/mgechev/revive/logging"
13+
)
14+
15+
// TimeDateRule lints the way time.Date is used.
16+
type TimeDateRule struct{}
17+
18+
// Apply applies the rule to given file.
19+
func (*TimeDateRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
20+
var failures []lint.Failure
21+
22+
onFailure := func(failure lint.Failure) {
23+
failures = append(failures, failure)
24+
}
25+
26+
w := &lintTimeDate{file, onFailure}
27+
28+
ast.Walk(w, file.AST)
29+
return failures
30+
}
31+
32+
// Name returns the rule name.
33+
func (*TimeDateRule) Name() string {
34+
return "time-date"
35+
}
36+
37+
type lintTimeDate struct {
38+
file *lint.File
39+
onFailure func(lint.Failure)
40+
}
41+
42+
var (
43+
// timeDateArgumentNames are the names of the arguments of time.Date
44+
timeDateArgumentNames = []string{
45+
"year",
46+
"month",
47+
"day",
48+
"hour",
49+
"minute",
50+
"second",
51+
"nanosecond",
52+
"timezone",
53+
}
54+
55+
// timeDateArity is the number of arguments of time.Date
56+
timeDateArity = len(timeDateArgumentNames)
57+
)
58+
59+
func (w lintTimeDate) Visit(n ast.Node) ast.Visitor {
60+
ce, ok := n.(*ast.CallExpr)
61+
if !ok || len(ce.Args) != timeDateArity {
62+
return w
63+
}
64+
if !isPkgDot(ce.Fun, "time", "Date") {
65+
return w
66+
}
67+
68+
// The last argument is a timezone, the is no need to check it, also it has a different type
69+
for pos, arg := range ce.Args[:timeDateArity-1] {
70+
bl, ok := arg.(*ast.BasicLit)
71+
if !ok {
72+
continue
73+
}
74+
75+
replacedValue, err := parseDecimalInteger(bl)
76+
if err == nil {
77+
continue
78+
}
79+
80+
if errors.Is(err, errParsedInvalid) {
81+
// This is not supposed to happen, let's be defensive
82+
// log the error, but continue
83+
84+
logger, errLogger := logging.GetLogger()
85+
if errLogger != nil {
86+
// This is not supposed to happen, discard both errors
87+
continue
88+
}
89+
logger.With(
90+
"value", bl.Value,
91+
"kind", bl.Kind,
92+
"error", err.Error(),
93+
).Error("failed to parse time.Date argument")
94+
95+
continue
96+
}
97+
98+
confidence := 0.8 // default confidence
99+
errMessage := err.Error()
100+
instructions := fmt.Sprintf("use %s instead of %s", replacedValue, bl.Value)
101+
switch {
102+
case errors.Is(err, errParsedOctalWithZero):
103+
// people can use 00, 01, 02, 03, 04, 05, 06, and 07 if they want.
104+
confidence = 0.5
105+
106+
case errors.Is(err, errParsedOctalWithPaddingZeroes):
107+
// This is a clear mistake.
108+
// example with 000123456 (octal) is about 123456 or 42798 ?
109+
confidence = 1
110+
111+
strippedValue := strings.TrimLeft(bl.Value, "0")
112+
if strippedValue == "" {
113+
// avoid issue with 00000000
114+
strippedValue = "0"
115+
}
116+
117+
if strippedValue != replacedValue {
118+
instructions = fmt.Sprintf(
119+
"choose between %s and %s (decimal value of %s octal value)",
120+
strippedValue, replacedValue, strippedValue,
121+
)
122+
}
123+
}
124+
125+
w.onFailure(lint.Failure{
126+
Category: "time",
127+
Node: bl,
128+
Confidence: confidence,
129+
Failure: fmt.Sprintf(
130+
"use decimal digits for time.Date %s argument: %s found: %s",
131+
timeDateArgumentNames[pos], errMessage, instructions),
132+
})
133+
}
134+
135+
return w
136+
}
137+
138+
var (
139+
errParsedOctal = errors.New("octal notation")
140+
errParsedOctalWithZero = errors.New("octal notation with leading zero")
141+
errParsedOctalWithPaddingZeroes = errors.New("octal notation with padding zeroes")
142+
errParsedHexadecimal = errors.New("hexadecimal notation")
143+
errParseBinary = errors.New("binary notation")
144+
errParsedFloat = errors.New("float literal")
145+
errParsedExponential = errors.New("exponential notation")
146+
errParsedAlternative = errors.New("alternative notation")
147+
errParsedInvalid = errors.New("invalid notation")
148+
)
149+
150+
func parseDecimalInteger(bl *ast.BasicLit) (string, error) {
151+
currentValue := strings.ToLower(bl.Value)
152+
153+
switch currentValue {
154+
case "0":
155+
// skip 0 as it is a valid value for all the arguments
156+
return bl.Value, nil
157+
case "00", "01", "02", "03", "04", "05", "06", "07":
158+
// people can use 00, 01, 02, 03, 04, 05, 06, 07, if they want
159+
return bl.Value[1:], errParsedOctalWithZero
160+
}
161+
162+
switch bl.Kind {
163+
case token.FLOAT:
164+
// someone used a float literal, while they should have used an integer literal.
165+
parsedValue, err := strconv.ParseFloat(currentValue, 64)
166+
if err != nil {
167+
// This is not supposed to happen
168+
return bl.Value, fmt.Errorf(
169+
"%w: %s: %w",
170+
errParsedInvalid,
171+
"failed to parse number as float",
172+
err,
173+
)
174+
}
175+
176+
// this will convert back the number to a string
177+
cleanedValue := strconv.FormatFloat(parsedValue, 'f', -1, 64)
178+
if strings.Contains(currentValue, "e") {
179+
return cleanedValue, errParsedExponential
180+
}
181+
182+
return cleanedValue, errParsedFloat
183+
184+
case token.INT:
185+
// we expect this format
186+
187+
default:
188+
// This is not supposed to happen
189+
return bl.Value, fmt.Errorf(
190+
"%w: %s",
191+
errParsedInvalid,
192+
"unexpected kind of literal",
193+
)
194+
}
195+
196+
// Parse the number with base=0 that allows to accept all number formats and base
197+
parsedValue, err := strconv.ParseInt(currentValue, 0, 64)
198+
if err != nil {
199+
// This is not supposed to happen
200+
return bl.Value, fmt.Errorf(
201+
"%w: %s: %w",
202+
errParsedInvalid,
203+
"failed to parse number as integer",
204+
err,
205+
)
206+
}
207+
208+
cleanedValue := strconv.FormatInt(parsedValue, 10)
209+
210+
// Let's figure out the notation to return an error
211+
switch {
212+
case strings.HasPrefix(currentValue, "0b"):
213+
return cleanedValue, errParseBinary
214+
case strings.HasPrefix(currentValue, "0x"):
215+
return cleanedValue, errParsedHexadecimal
216+
case strings.HasPrefix(currentValue, "0"):
217+
// this matches both "0" and "0o" octal notation.
218+
219+
if strings.HasPrefix(currentValue, "00") {
220+
// 00123456 (octal) is about 123456 or 42798 ?
221+
return cleanedValue, errParsedOctalWithPaddingZeroes
222+
}
223+
224+
// 0006 is a valid octal notation, but we can use 6 instead.
225+
return cleanedValue, errParsedOctal
226+
}
227+
228+
// Convert back the number to a string, and compare it with the original one
229+
formattedValue := strconv.FormatInt(parsedValue, 10)
230+
if formattedValue != currentValue {
231+
// This can catch some edge cases like: 1_0 ...
232+
return cleanedValue, errParsedAlternative
233+
}
234+
235+
// The number is a decimal integer, we can use it as is.
236+
return bl.Value, nil
237+
}

test/time_date_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/mgechev/revive/rule"
7+
)
8+
9+
func TestTimeDate(t *testing.T) {
10+
testRule(t, "time_date_decimal_literal", &rule.TimeDateRule{})
11+
}

0 commit comments

Comments
 (0)