Skip to content

Commit 395f790

Browse files
alexandearalexandear
authored
refactor: replace failure Category raw string with constant (#1196)
* refactor: replace Category raw strings with constants * Add type FailureCategory; add comments for constants
1 parent 57fe5b6 commit 395f790

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+151
-107
lines changed

‎lint/failure.go

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,53 @@ import (
55
"go/token"
66
)
77

8+
const (
9+
// FailureCategoryArgOrder indicates argument order issues.
10+
FailureCategoryArgOrder FailureCategory = "arg-order"
11+
// FailureCategoryBadPractice indicates bad practice issues.
12+
FailureCategoryBadPractice FailureCategory = "bad practice"
13+
// FailureCategoryCodeStyle indicates code style issues.
14+
FailureCategoryCodeStyle FailureCategory = "code-style"
15+
// FailureCategoryComments indicates comment issues.
16+
FailureCategoryComments FailureCategory = "comments"
17+
// FailureCategoryComplexity indicates complexity issues.
18+
FailureCategoryComplexity FailureCategory = "complexity"
19+
// FailureCategoryContent indicates content issues.
20+
FailureCategoryContent FailureCategory = "content"
21+
// FailureCategoryErrors indicates error handling issues.
22+
FailureCategoryErrors FailureCategory = "errors"
23+
// FailureCategoryImports indicates import issues.
24+
FailureCategoryImports FailureCategory = "imports"
25+
// FailureCategoryLogic indicates logic issues.
26+
FailureCategoryLogic FailureCategory = "logic"
27+
// FailureCategoryMaintenance indicates maintenance issues.
28+
FailureCategoryMaintenance FailureCategory = "maintenance"
29+
// FailureCategoryNaming indicates naming issues.
30+
FailureCategoryNaming FailureCategory = "naming"
31+
// FailureCategoryOptimization indicates optimization issues.
32+
FailureCategoryOptimization FailureCategory = "optimization"
33+
// FailureCategoryStyle indicates style issues.
34+
FailureCategoryStyle FailureCategory = "style"
35+
// FailureCategoryTime indicates time-related issues.
36+
FailureCategoryTime FailureCategory = "time"
37+
// FailureCategoryTypeInference indicates type inference issues.
38+
FailureCategoryTypeInference FailureCategory = "type-inference"
39+
// FailureCategoryUnaryOp indicates unary operation issues.
40+
FailureCategoryUnaryOp FailureCategory = "unary-op"
41+
// FailureCategoryUnexportedTypeInAPI indicates unexported type in API issues.
42+
FailureCategoryUnexportedTypeInAPI FailureCategory = "unexported-type-in-api"
43+
// FailureCategoryZeroValue indicates zero value issues.
44+
FailureCategoryZeroValue FailureCategory = "zero-value"
45+
46+
// failureCategoryInternal indicates internal failures.
47+
failureCategoryInternal FailureCategory = "REVIVE_INTERNAL"
48+
// failureCategoryValidity indicates validity issues.
49+
failureCategoryValidity FailureCategory = "validity"
50+
)
51+
52+
// FailureCategory is the type for the failure categories.
53+
type FailureCategory string
54+
855
const (
956
// SeverityWarning declares failures of type warning
1057
SeverityWarning = "warning"
@@ -25,7 +72,7 @@ type FailurePosition struct {
2572
type Failure struct {
2673
Failure string
2774
RuleName string
28-
Category string
75+
Category FailureCategory
2976
Position FailurePosition
3077
Node ast.Node `json:"-"`
3178
Confidence float64
@@ -38,17 +85,15 @@ func (f *Failure) GetFilename() string {
3885
return f.Position.Start.Filename
3986
}
4087

41-
const internalFailure = "REVIVE_INTERNAL"
42-
4388
// IsInternal returns true if this failure is internal, false otherwise.
4489
func (f *Failure) IsInternal() bool {
45-
return f.Category == internalFailure
90+
return f.Category == failureCategoryInternal
4691
}
4792

4893
// NewInternalFailure yields an internal failure with the given message as failure message.
4994
func NewInternalFailure(message string) Failure {
5095
return Failure{
51-
Category: internalFailure,
96+
Category: failureCategoryInternal,
5297
Failure: message,
5398
}
5499
}

‎lint/linter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ func addInvalidFileFailure(filename, errStr string, failures chan Failure) {
223223
failures <- Failure{
224224
Confidence: 1,
225225
Failure: fmt.Sprintf("invalid file %s: %v", filename, errStr),
226-
Category: "validity",
226+
Category: failureCategoryValidity,
227227
Position: position,
228228
}
229229
}

‎rule/add_constant.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func (w *lintAddConstantRule) checkStrLit(n *ast.BasicLit) {
171171
w.onFailure(lint.Failure{
172172
Confidence: 1,
173173
Node: n,
174-
Category: "style",
174+
Category: lint.FailureCategoryStyle,
175175
Failure: fmt.Sprintf("string literal %s appears, at least, %d times, create a named constant for it", n.Value, w.strLits[n.Value]),
176176
})
177177
w.strLits[n.Value] = -1 // mark it to avoid failing again on the same literal
@@ -187,7 +187,7 @@ func (w *lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) {
187187
w.onFailure(lint.Failure{
188188
Confidence: 1,
189189
Node: n,
190-
Category: "style",
190+
Category: lint.FailureCategoryStyle,
191191
Failure: fmt.Sprintf("avoid magic numbers like '%s', create a named constant for it", n.Value),
192192
})
193193
}

‎rule/blank_imports.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu
2323

2424
const (
2525
message = "a blank import should be only in a main or test package, or have a comment justifying it"
26-
category = "imports"
2726
embedImportPath = `"embed"`
2827
)
2928

@@ -55,7 +54,7 @@ func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu
5554

5655
// This is the first blank import of a group.
5756
if imp.Doc == nil && imp.Comment == nil {
58-
failures = append(failures, lint.Failure{Failure: message, Category: category, Node: imp, Confidence: 1})
57+
failures = append(failures, lint.Failure{Failure: message, Category: lint.FailureCategoryImports, Node: imp, Confidence: 1})
5958
}
6059
}
6160

‎rule/bool_literal_in_expr.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,16 @@ func (w *lintBoolLiteral) Visit(node ast.Node) ast.Visitor {
5353
isConstant := (n.Op == token.LAND && lexeme == "false") || (n.Op == token.LOR && lexeme == "true")
5454

5555
if isConstant {
56-
w.addFailure(n, "Boolean expression seems to always evaluate to "+lexeme, "logic")
56+
w.addFailure(n, "Boolean expression seems to always evaluate to "+lexeme, lint.FailureCategoryLogic)
5757
} else {
58-
w.addFailure(n, "omit Boolean literal in expression", "style")
58+
w.addFailure(n, "omit Boolean literal in expression", lint.FailureCategoryStyle)
5959
}
6060
}
6161

6262
return w
6363
}
6464

65-
func (w lintBoolLiteral) addFailure(node ast.Node, msg, cat string) {
65+
func (w lintBoolLiteral) addFailure(node ast.Node, msg string, cat lint.FailureCategory) {
6666
w.onFailure(lint.Failure{
6767
Confidence: 1,
6868
Node: node,

‎rule/call_to_gc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (w lintCallToGC) Visit(node ast.Node) ast.Visitor {
6262
w.onFailure(lint.Failure{
6363
Confidence: 1,
6464
Node: node,
65-
Category: "bad practice",
65+
Category: lint.FailureCategoryBadPractice,
6666
Failure: "explicit call to the garbage collector",
6767
})
6868

‎rule/cognitive_complexity.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func (w cognitiveComplexityLinter) lintCognitiveComplexity() {
7373
if c > w.maxComplexity {
7474
w.onFailure(lint.Failure{
7575
Confidence: 1,
76-
Category: "maintenance",
76+
Category: lint.FailureCategoryMaintenance,
7777
Failure: fmt.Sprintf("function %s has cognitive complexity %d (> max enabled %d)", funcName(fn), c, w.maxComplexity),
7878
Node: fn,
7979
})

‎rule/comment_spacings.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func (r *CommentSpacingsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
5757
failures = append(failures, lint.Failure{
5858
Node: comment,
5959
Confidence: 1,
60-
Category: "style",
60+
Category: lint.FailureCategoryStyle,
6161
Failure: "no space between comment delimiter and comment text",
6262
})
6363
}

‎rule/confusing_naming.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func checkMethodName(holder string, id *ast.Ident, w *lintConfusingNames) {
102102
Failure: fmt.Sprintf("Method '%s' differs only by capitalization to %s '%s' in %s", id.Name, kind, refMethod.id.Name, fileName),
103103
Confidence: 1,
104104
Node: id,
105-
Category: "naming",
105+
Category: lint.FailureCategoryNaming,
106106
})
107107

108108
return
@@ -176,7 +176,7 @@ func checkStructFields(fields *ast.FieldList, structName string, w *lintConfusin
176176
Failure: fmt.Sprintf("Field '%s' differs only by capitalization to other field in the struct type %s", id.Name, structName),
177177
Confidence: 1,
178178
Node: id,
179-
Category: "naming",
179+
Category: lint.FailureCategoryNaming,
180180
})
181181
} else {
182182
bl[normName] = true

‎rule/confusing_results.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func (*ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fai
3434
failures = append(failures, lint.Failure{
3535
Node: result,
3636
Confidence: 1,
37-
Category: "naming",
37+
Category: lint.FailureCategoryNaming,
3838
Failure: "unnamed results of the same type may be confusing, consider using named results",
3939
})
4040

‎rule/constant_logical_expr.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func (w *lintConstantLogicalExpr) newFailure(node ast.Node, msg string) {
9595
w.onFailure(lint.Failure{
9696
Confidence: 1,
9797
Node: node,
98-
Category: "logic",
98+
Category: lint.FailureCategoryLogic,
9999
Failure: msg,
100100
})
101101
}

‎rule/context_as_argument.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func (r *ContextAsArgumentRule) Apply(file *lint.File, _ lint.Arguments) []lint.
3232
if argIsCtx && !isCtxStillAllowed {
3333
failures = append(failures, lint.Failure{
3434
Node: arg,
35-
Category: "arg-order",
35+
Category: lint.FailureCategoryArgOrder,
3636
Failure: "context.Context should be the first parameter of a function",
3737
Confidence: 0.9,
3838
})

‎rule/context_keys_type.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func checkContextKeyType(w lintContextKeyTypes, x *ast.CallExpr) {
7474
w.onFailure(lint.Failure{
7575
Confidence: 1,
7676
Node: x,
77-
Category: "content",
77+
Category: lint.FailureCategoryContent,
7878
Failure: fmt.Sprintf("should not use basic type %s as key in context.WithValue", key.Type),
7979
})
8080
}

‎rule/cyclomatic.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (r *CyclomaticRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure
4747
if c > r.maxComplexity {
4848
failures = append(failures, lint.Failure{
4949
Confidence: 1,
50-
Category: "maintenance",
50+
Category: lint.FailureCategoryMaintenance,
5151
Failure: fmt.Sprintf("function %s has cyclomatic complexity %d (> max enabled %d)",
5252
funcName(fn), c, r.maxComplexity),
5353
Node: fn,

‎rule/datarace.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,14 @@ func (w lintFunctionForDataRaces) Visit(node ast.Node) ast.Visitor {
122122
w.onFailure(lint.Failure{
123123
Confidence: 1,
124124
Node: id,
125-
Category: "logic",
125+
Category: lint.FailureCategoryLogic,
126126
Failure: fmt.Sprintf("datarace: range value %s is captured (by-reference) in goroutine", id.Name),
127127
})
128128
case isReturnID:
129129
w.onFailure(lint.Failure{
130130
Confidence: 0.8,
131131
Node: id,
132-
Category: "logic",
132+
Category: lint.FailureCategoryLogic,
133133
Failure: fmt.Sprintf("potential datarace: return value %s is captured (by-reference) in goroutine", id.Name),
134134
})
135135
}

‎rule/deep_exit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (w *lintDeepExit) Visit(node ast.Node) ast.Visitor {
6868
w.onFailure(lint.Failure{
6969
Confidence: 1,
7070
Node: ce,
71-
Category: "bad practice",
71+
Category: lint.FailureCategoryBadPractice,
7272
Failure: fmt.Sprintf("calls to %s.%s only in main() or init() functions", pkg, fn),
7373
})
7474
}

‎rule/defer.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
9494
return nil
9595
case *ast.ReturnStmt:
9696
if len(n.Results) != 0 && w.inADefer && w.inAFuncLit {
97-
w.newFailure("return in a defer function has no effect", n, 1.0, "logic", "return")
97+
w.newFailure("return in a defer function has no effect", n, 1.0, lint.FailureCategoryLogic, "return")
9898
}
9999
case *ast.CallExpr:
100100
isCallToRecover := isIdent(n.Fun, "recover")
@@ -103,13 +103,13 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
103103
// func fn() { recover() }
104104
//
105105
// confidence is not 1 because recover can be in a function that is deferred elsewhere
106-
w.newFailure("recover must be called inside a deferred function", n, 0.8, "logic", "recover")
106+
w.newFailure("recover must be called inside a deferred function", n, 0.8, lint.FailureCategoryLogic, "recover")
107107
case w.inADefer && !w.inAFuncLit && isCallToRecover:
108108
// defer helper(recover())
109109
//
110110
// confidence is not truly 1 because this could be in a correctly-deferred func,
111111
// but it is very likely to be a misunderstanding of defer's behavior around arguments.
112-
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, "logic", "immediate-recover")
112+
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, lint.FailureCategoryLogic, "immediate-recover")
113113
}
114114
return nil // no need to analyze the arguments of the function call
115115
case *ast.DeferStmt:
@@ -118,7 +118,7 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
118118
//
119119
// confidence is not truly 1 because this could be in a correctly-deferred func,
120120
// but normally this doesn't suppress a panic, and even if it did it would silently discard the value.
121-
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, "logic", "immediate-recover")
121+
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, lint.FailureCategoryLogic, "immediate-recover")
122122
}
123123
w.visitSubtree(n.Call.Fun, true, false, false)
124124
for _, a := range n.Call.Args {
@@ -131,17 +131,17 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
131131
}
132132

133133
if w.inALoop {
134-
w.newFailure("prefer not to defer inside loops", n, 1.0, "bad practice", "loop")
134+
w.newFailure("prefer not to defer inside loops", n, 1.0, lint.FailureCategoryBadPractice, "loop")
135135
}
136136

137137
switch fn := n.Call.Fun.(type) {
138138
case *ast.CallExpr:
139-
w.newFailure("prefer not to defer chains of function calls", fn, 1.0, "bad practice", "call-chain")
139+
w.newFailure("prefer not to defer chains of function calls", fn, 1.0, lint.FailureCategoryBadPractice, "call-chain")
140140
case *ast.SelectorExpr:
141141
if id, ok := fn.X.(*ast.Ident); ok {
142142
isMethodCall := id != nil && id.Obj != nil && id.Obj.Kind == ast.Typ
143143
if isMethodCall {
144-
w.newFailure("be careful when deferring calls to methods without pointer receiver", fn, 0.8, "bad practice", "method-call")
144+
w.newFailure("be careful when deferring calls to methods without pointer receiver", fn, 0.8, lint.FailureCategoryBadPractice, "method-call")
145145
}
146146
}
147147
}
@@ -163,7 +163,7 @@ func (w lintDeferRule) visitSubtree(n ast.Node, inADefer, inALoop, inAFuncLit bo
163163
ast.Walk(nw, n)
164164
}
165165

166-
func (w lintDeferRule) newFailure(msg string, node ast.Node, confidence float64, cat, subcase string) {
166+
func (w lintDeferRule) newFailure(msg string, node ast.Node, confidence float64, cat lint.FailureCategory, subcase string) {
167167
if !w.allow[subcase] {
168168
return
169169
}

‎rule/dot_imports.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (w lintImports) Visit(_ ast.Node) ast.Visitor {
8181
Confidence: 1,
8282
Failure: "should not use dot imports",
8383
Node: importSpec,
84-
Category: "imports",
84+
Category: lint.FailureCategoryImports,
8585
})
8686
}
8787
}

‎rule/duplicated_imports.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func (*DuplicatedImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
2222
Confidence: 1,
2323
Failure: fmt.Sprintf("Package %s already imported", path),
2424
Node: imp,
25-
Category: "imports",
25+
Category: lint.FailureCategoryImports,
2626
})
2727
continue
2828
}

‎rule/empty_block.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor {
5555
w.onFailure(lint.Failure{
5656
Confidence: 0.9,
5757
Node: n,
58-
Category: "logic",
58+
Category: lint.FailureCategoryLogic,
5959
Failure: "this block is empty, you can remove it",
6060
})
6161
return nil // skip visiting the range subtree (it will produce a duplicated failure)
@@ -65,7 +65,7 @@ func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor {
6565
w.onFailure(lint.Failure{
6666
Confidence: 1,
6767
Node: n,
68-
Category: "logic",
68+
Category: lint.FailureCategoryLogic,
6969
Failure: "this block is empty, you can remove it",
7070
})
7171
}

‎rule/empty_lines.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (w lintEmptyLines) checkStart(block *ast.BlockStmt) {
6060
w.onFailure(lint.Failure{
6161
Confidence: 1,
6262
Node: block,
63-
Category: "style",
63+
Category: lint.FailureCategoryStyle,
6464
Failure: "extra empty line at the start of a block",
6565
})
6666
}
@@ -79,7 +79,7 @@ func (w lintEmptyLines) checkEnd(block *ast.BlockStmt) {
7979
w.onFailure(lint.Failure{
8080
Confidence: 1,
8181
Node: block,
82-
Category: "style",
82+
Category: lint.FailureCategoryStyle,
8383
Failure: "extra empty line at the end of a block",
8484
})
8585
}

‎rule/enforce_map_style.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func (r *EnforceMapStyleRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
9191
failures = append(failures, lint.Failure{
9292
Confidence: 1,
9393
Node: v,
94-
Category: "style",
94+
Category: lint.FailureCategoryStyle,
9595
Failure: "use make(map[type]type) instead of map[type]type{}",
9696
})
9797
case *ast.CallExpr:
@@ -119,7 +119,7 @@ func (r *EnforceMapStyleRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
119119
failures = append(failures, lint.Failure{
120120
Confidence: 1,
121121
Node: v.Args[0],
122-
Category: "style",
122+
Category: lint.FailureCategoryStyle,
123123
Failure: "use map[type]type{} instead of make(map[type]type)",
124124
})
125125
}

0 commit comments

Comments
 (0)