Skip to content

Commit 26b9be1

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 26b9be1

File tree

6 files changed

+413
-0
lines changed

6 files changed

+413
-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: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
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+
confidence := 0.8 // default confidence
81+
errMessage := err.Error()
82+
instructions := fmt.Sprintf("use %s instead of %s", replacedValue, bl.Value)
83+
if errors.Is(err, errParsedOctal) {
84+
switch {
85+
case errors.Is(err, errParsedOctalWithZero):
86+
// people are allowed to use 00, 01, 02, 03, 04, 05, 06, and 07
87+
confidence = 0.5
88+
89+
case errors.Is(err, errParsedOctalWithPaddingZeroes):
90+
confidence = 1 // this is a clear mistake
91+
// example with 000123456 (octal) is about 123456 or 42798 ?
92+
93+
strippedValue := strings.TrimLeft(bl.Value, "0")
94+
if strippedValue == "" {
95+
// avoid issue with 00000000
96+
strippedValue = "0"
97+
}
98+
99+
if strippedValue != replacedValue {
100+
instructions = fmt.Sprintf(
101+
"choose between %s and %s (decimal value of %s octal value)",
102+
strippedValue, replacedValue, strippedValue,
103+
)
104+
}
105+
}
106+
}
107+
108+
w.onFailure(lint.Failure{
109+
Category: "time",
110+
Node: bl,
111+
Confidence: confidence,
112+
Failure: fmt.Sprintf(
113+
"use decimal digits for time.Date %s argument: %s found: %s",
114+
timeDateArgumentNames[pos], errMessage, instructions),
115+
})
116+
}
117+
118+
return w
119+
}
120+
121+
var (
122+
errParsedOctal = errors.New("octal notation")
123+
errParsedOctalWithZero = fmt.Errorf("%w with leading zero", errParsedOctal)
124+
errParsedOctalWithPaddingZeroes = fmt.Errorf("%w with padding zeroes", errParsedOctal)
125+
errParsedHexadecimal = errors.New("hexadecimal notation")
126+
errParseBinary = errors.New("binary notation")
127+
errParsedFloat = errors.New("float literal")
128+
errParsedExponential = errors.New("exponential notation")
129+
errParsedAlternative = errors.New("alternative notation")
130+
)
131+
132+
func parseDecimalInteger(bl *ast.BasicLit) (string, error) {
133+
currentValue := strings.ToLower(bl.Value)
134+
135+
switch currentValue {
136+
case "0":
137+
// 0 is a valid value for all the arguments
138+
// let's skip it
139+
return bl.Value, nil
140+
// people can use 00, 01, 02, 03, 04, 05, 06, 07, if they want
141+
case "00", "01", "02", "03", "04", "05", "06", "07":
142+
return bl.Value[1:], errParsedOctalWithZero
143+
}
144+
145+
loggerError := func(message string, err error) {
146+
logger, errLogger := logging.GetLogger()
147+
if errLogger != nil {
148+
// this is not supposed to happen
149+
return
150+
}
151+
if err != nil {
152+
logger = logger.With(
153+
"error", err.Error(),
154+
)
155+
}
156+
logger.With(
157+
"value", bl.Value,
158+
"kind", bl.Kind.String(),
159+
).Error(message)
160+
}
161+
162+
switch bl.Kind {
163+
case token.FLOAT:
164+
// someone used a float literal, while they should have used a integer literal
165+
parsedValue, err := strconv.ParseFloat(currentValue, 64)
166+
if err != nil {
167+
// this is not supposed to happen
168+
// but let's be defensive and log it
169+
loggerError("failed to parse number as float", err)
170+
171+
// and return the original value, as if everything was ok
172+
return bl.Value, nil
173+
}
174+
175+
// this will convert back the number to a string
176+
cleanedValue := strconv.FormatFloat(parsedValue, 'f', -1, 64)
177+
if strings.Contains(currentValue, "e") {
178+
return cleanedValue, errParsedExponential
179+
}
180+
181+
return cleanedValue, errParsedFloat
182+
183+
case token.INT:
184+
// we expect this format
185+
186+
default:
187+
// this is not supposed to happen
188+
// but let's be defensive and log it
189+
loggerError("unexpected kind of literal", nil)
190+
191+
// let's be defensive and return nil
192+
return bl.Value, nil
193+
}
194+
195+
// Parse the number with base=0 that allows to accept all number formats and base
196+
parsedValue, err := strconv.ParseInt(currentValue, 0, 64)
197+
if err != nil {
198+
// this is not supposed to happen
199+
// but let's be defensive and log it
200+
201+
loggerError("failed to parse number as integer", err)
202+
203+
// and return the original value, as if everything was ok
204+
return bl.Value, nil
205+
}
206+
207+
cleanedValue := strconv.FormatInt(parsedValue, 10)
208+
209+
// The number is not in decimal notation
210+
// let's forge a failure message
211+
212+
// Let's figure out the notation
213+
switch {
214+
case strings.HasPrefix(currentValue, "0b"):
215+
return cleanedValue, errParseBinary
216+
case strings.HasPrefix(currentValue, "0x"):
217+
return cleanedValue, errParsedHexadecimal
218+
case strings.HasPrefix(currentValue, "0"):
219+
// this matches both "0" and "0o" octal notation.
220+
221+
if strings.HasPrefix(currentValue, "00") {
222+
// 00123456 (octal) is about 123456 or 42798 ?
223+
return cleanedValue, errParsedOctalWithPaddingZeroes
224+
}
225+
226+
// 0006 is a valid octal notation, but we can use 6 instead.
227+
return cleanedValue, errParsedOctal
228+
}
229+
230+
// anything else uses decimal notation
231+
// but let's validate it to be sure
232+
233+
// convert back the number to a string, and compare it with the original one
234+
formattedValue := strconv.FormatInt(parsedValue, 10)
235+
if formattedValue != currentValue {
236+
// this can catch some edge cases like: 1_0 ...
237+
return cleanedValue, errParsedAlternative
238+
}
239+
240+
return bl.Value, nil
241+
}

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)