Skip to content

refactor: replace failure Category raw string with constant #1196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions lint/failure.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,30 @@ import (
"go/token"
)

const (
FailureCategoryArgOrder = "arg-order"
FailureCategoryBadPractice = "bad practice"
FailureCategoryCodeStyle = "code-style"
FailureCategoryComments = "comments"
FailureCategoryComplexity = "complexity"
FailureCategoryContent = "content"
FailureCategoryErrors = "errors"
FailureCategoryImports = "imports"
FailureCategoryLogic = "logic"
FailureCategoryMaintenance = "maintenance"
FailureCategoryNaming = "naming"
FailureCategoryOptimization = "optimization"
FailureCategoryStyle = "style"
FailureCategoryTime = "time"
FailureCategoryTypeInference = "type-inference"
FailureCategoryUnaryOp = "unary-op"
FailureCategoryUnexportedTypeInAPI = "unexported-type-in-api"
FailureCategoryZeroValue = "zero-value"

failureCategoryInternal = "REVIVE_INTERNAL"
failureCategoryValidity = "validity"
)

const (
// SeverityWarning declares failures of type warning
SeverityWarning = "warning"
Expand Down Expand Up @@ -38,17 +62,15 @@ func (f *Failure) GetFilename() string {
return f.Position.Start.Filename
}

const internalFailure = "REVIVE_INTERNAL"

// IsInternal returns true if this failure is internal, false otherwise.
func (f *Failure) IsInternal() bool {
return f.Category == internalFailure
return f.Category == failureCategoryInternal
}

// NewInternalFailure yields an internal failure with the given message as failure message.
func NewInternalFailure(message string) Failure {
return Failure{
Category: internalFailure,
Category: failureCategoryInternal,
Failure: message,
}
}
2 changes: 1 addition & 1 deletion lint/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func addInvalidFileFailure(filename, errStr string, failures chan Failure) {
failures <- Failure{
Confidence: 1,
Failure: fmt.Sprintf("invalid file %s: %v", filename, errStr),
Category: "validity",
Category: failureCategoryValidity,
Position: position,
}
}
Expand Down
4 changes: 2 additions & 2 deletions rule/add_constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (w *lintAddConstantRule) checkStrLit(n *ast.BasicLit) {
w.onFailure(lint.Failure{
Confidence: 1,
Node: n,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: fmt.Sprintf("string literal %s appears, at least, %d times, create a named constant for it", n.Value, w.strLits[n.Value]),
})
w.strLits[n.Value] = -1 // mark it to avoid failing again on the same literal
Expand All @@ -187,7 +187,7 @@ func (w *lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) {
w.onFailure(lint.Failure{
Confidence: 1,
Node: n,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: fmt.Sprintf("avoid magic numbers like '%s', create a named constant for it", n.Value),
})
}
Expand Down
3 changes: 1 addition & 2 deletions rule/blank_imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu

const (
message = "a blank import should be only in a main or test package, or have a comment justifying it"
category = "imports"
embedImportPath = `"embed"`
)

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

// This is the first blank import of a group.
if imp.Doc == nil && imp.Comment == nil {
failures = append(failures, lint.Failure{Failure: message, Category: category, Node: imp, Confidence: 1})
failures = append(failures, lint.Failure{Failure: message, Category: lint.FailureCategoryImports, Node: imp, Confidence: 1})
}
}

Expand Down
4 changes: 2 additions & 2 deletions rule/bool_literal_in_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func (w *lintBoolLiteral) Visit(node ast.Node) ast.Visitor {
isConstant := (n.Op == token.LAND && lexeme == "false") || (n.Op == token.LOR && lexeme == "true")

if isConstant {
w.addFailure(n, "Boolean expression seems to always evaluate to "+lexeme, "logic")
w.addFailure(n, "Boolean expression seems to always evaluate to "+lexeme, lint.FailureCategoryLogic)
} else {
w.addFailure(n, "omit Boolean literal in expression", "style")
w.addFailure(n, "omit Boolean literal in expression", lint.FailureCategoryStyle)
}
}

Expand Down
2 changes: 1 addition & 1 deletion rule/call_to_gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (w lintCallToGC) Visit(node ast.Node) ast.Visitor {
w.onFailure(lint.Failure{
Confidence: 1,
Node: node,
Category: "bad practice",
Category: lint.FailureCategoryBadPractice,
Failure: "explicit call to the garbage collector",
})

Expand Down
2 changes: 1 addition & 1 deletion rule/cognitive_complexity.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (w cognitiveComplexityLinter) lintCognitiveComplexity() {
if c > w.maxComplexity {
w.onFailure(lint.Failure{
Confidence: 1,
Category: "maintenance",
Category: lint.FailureCategoryMaintenance,
Failure: fmt.Sprintf("function %s has cognitive complexity %d (> max enabled %d)", funcName(fn), c, w.maxComplexity),
Node: fn,
})
Expand Down
2 changes: 1 addition & 1 deletion rule/comment_spacings.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (r *CommentSpacingsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
failures = append(failures, lint.Failure{
Node: comment,
Confidence: 1,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "no space between comment delimiter and comment text",
})
}
Expand Down
4 changes: 2 additions & 2 deletions rule/confusing_naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func checkMethodName(holder string, id *ast.Ident, w *lintConfusingNames) {
Failure: fmt.Sprintf("Method '%s' differs only by capitalization to %s '%s' in %s", id.Name, kind, refMethod.id.Name, fileName),
Confidence: 1,
Node: id,
Category: "naming",
Category: lint.FailureCategoryNaming,
})

return
Expand Down Expand Up @@ -176,7 +176,7 @@ func checkStructFields(fields *ast.FieldList, structName string, w *lintConfusin
Failure: fmt.Sprintf("Field '%s' differs only by capitalization to other field in the struct type %s", id.Name, structName),
Confidence: 1,
Node: id,
Category: "naming",
Category: lint.FailureCategoryNaming,
})
} else {
bl[normName] = true
Expand Down
2 changes: 1 addition & 1 deletion rule/confusing_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (*ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fai
failures = append(failures, lint.Failure{
Node: result,
Confidence: 1,
Category: "naming",
Category: lint.FailureCategoryNaming,
Failure: "unnamed results of the same type may be confusing, consider using named results",
})

Expand Down
2 changes: 1 addition & 1 deletion rule/constant_logical_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (w lintConstantLogicalExpr) newFailure(node ast.Node, msg string) {
w.onFailure(lint.Failure{
Confidence: 1,
Node: node,
Category: "logic",
Category: lint.FailureCategoryLogic,
Failure: msg,
})
}
2 changes: 1 addition & 1 deletion rule/context_as_argument.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (r *ContextAsArgumentRule) Apply(file *lint.File, _ lint.Arguments) []lint.
if argIsCtx && !isCtxStillAllowed {
failures = append(failures, lint.Failure{
Node: arg,
Category: "arg-order",
Category: lint.FailureCategoryArgOrder,
Failure: "context.Context should be the first parameter of a function",
Confidence: 0.9,
})
Expand Down
2 changes: 1 addition & 1 deletion rule/context_keys_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func checkContextKeyType(w lintContextKeyTypes, x *ast.CallExpr) {
w.onFailure(lint.Failure{
Confidence: 1,
Node: x,
Category: "content",
Category: lint.FailureCategoryContent,
Failure: fmt.Sprintf("should not use basic type %s as key in context.WithValue", key.Type),
})
}
Expand Down
2 changes: 1 addition & 1 deletion rule/cyclomatic.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (r *CyclomaticRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure
if c > r.maxComplexity {
failures = append(failures, lint.Failure{
Confidence: 1,
Category: "maintenance",
Category: lint.FailureCategoryMaintenance,
Failure: fmt.Sprintf("function %s has cyclomatic complexity %d (> max enabled %d)",
funcName(fn), c, r.maxComplexity),
Node: fn,
Expand Down
4 changes: 2 additions & 2 deletions rule/datarace.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ func (w lintFunctionForDataRaces) Visit(node ast.Node) ast.Visitor {
w.onFailure(lint.Failure{
Confidence: 1,
Node: id,
Category: "logic",
Category: lint.FailureCategoryLogic,
Failure: fmt.Sprintf("datarace: range value %s is captured (by-reference) in goroutine", id.Name),
})
case isReturnID:
w.onFailure(lint.Failure{
Confidence: 0.8,
Node: id,
Category: "logic",
Category: lint.FailureCategoryLogic,
Failure: fmt.Sprintf("potential datarace: return value %s is captured (by-reference) in goroutine", id.Name),
})
}
Expand Down
2 changes: 1 addition & 1 deletion rule/deep_exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (w *lintDeepExit) Visit(node ast.Node) ast.Visitor {
w.onFailure(lint.Failure{
Confidence: 1,
Node: ce,
Category: "bad practice",
Category: lint.FailureCategoryBadPractice,
Failure: fmt.Sprintf("calls to %s.%s only in main() or init() functions", pkg, fn),
})
}
Expand Down
14 changes: 7 additions & 7 deletions rule/defer.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
return nil
case *ast.ReturnStmt:
if len(n.Results) != 0 && w.inADefer && w.inAFuncLit {
w.newFailure("return in a defer function has no effect", n, 1.0, "logic", "return")
w.newFailure("return in a defer function has no effect", n, 1.0, lint.FailureCategoryLogic, "return")
}
case *ast.CallExpr:
isCallToRecover := isIdent(n.Fun, "recover")
Expand All @@ -103,13 +103,13 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
// func fn() { recover() }
//
// confidence is not 1 because recover can be in a function that is deferred elsewhere
w.newFailure("recover must be called inside a deferred function", n, 0.8, "logic", "recover")
w.newFailure("recover must be called inside a deferred function", n, 0.8, lint.FailureCategoryLogic, "recover")
case w.inADefer && !w.inAFuncLit && isCallToRecover:
// defer helper(recover())
//
// confidence is not truly 1 because this could be in a correctly-deferred func,
// but it is very likely to be a misunderstanding of defer's behavior around arguments.
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, "logic", "immediate-recover")
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, lint.FailureCategoryLogic, "immediate-recover")
}
return nil // no need to analyze the arguments of the function call
case *ast.DeferStmt:
Expand All @@ -118,7 +118,7 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
//
// confidence is not truly 1 because this could be in a correctly-deferred func,
// but normally this doesn't suppress a panic, and even if it did it would silently discard the value.
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, "logic", "immediate-recover")
w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, lint.FailureCategoryLogic, "immediate-recover")
}
w.visitSubtree(n.Call.Fun, true, false, false)
for _, a := range n.Call.Args {
Expand All @@ -131,17 +131,17 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor {
}

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

switch fn := n.Call.Fun.(type) {
case *ast.CallExpr:
w.newFailure("prefer not to defer chains of function calls", fn, 1.0, "bad practice", "call-chain")
w.newFailure("prefer not to defer chains of function calls", fn, 1.0, lint.FailureCategoryBadPractice, "call-chain")
case *ast.SelectorExpr:
if id, ok := fn.X.(*ast.Ident); ok {
isMethodCall := id != nil && id.Obj != nil && id.Obj.Kind == ast.Typ
if isMethodCall {
w.newFailure("be careful when deferring calls to methods without pointer receiver", fn, 0.8, "bad practice", "method-call")
w.newFailure("be careful when deferring calls to methods without pointer receiver", fn, 0.8, lint.FailureCategoryBadPractice, "method-call")
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion rule/dot_imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (w lintImports) Visit(_ ast.Node) ast.Visitor {
Confidence: 1,
Failure: "should not use dot imports",
Node: importSpec,
Category: "imports",
Category: lint.FailureCategoryImports,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion rule/duplicated_imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (*DuplicatedImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
Confidence: 1,
Failure: fmt.Sprintf("Package %s already imported", path),
Node: imp,
Category: "imports",
Category: lint.FailureCategoryImports,
})
continue
}
Expand Down
4 changes: 2 additions & 2 deletions rule/empty_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor {
w.onFailure(lint.Failure{
Confidence: 0.9,
Node: n,
Category: "logic",
Category: lint.FailureCategoryLogic,
Failure: "this block is empty, you can remove it",
})
return nil // skip visiting the range subtree (it will produce a duplicated failure)
Expand All @@ -65,7 +65,7 @@ func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor {
w.onFailure(lint.Failure{
Confidence: 1,
Node: n,
Category: "logic",
Category: lint.FailureCategoryLogic,
Failure: "this block is empty, you can remove it",
})
}
Expand Down
4 changes: 2 additions & 2 deletions rule/empty_lines.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (w lintEmptyLines) checkStart(block *ast.BlockStmt) {
w.onFailure(lint.Failure{
Confidence: 1,
Node: block,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "extra empty line at the start of a block",
})
}
Expand All @@ -79,7 +79,7 @@ func (w lintEmptyLines) checkEnd(block *ast.BlockStmt) {
w.onFailure(lint.Failure{
Confidence: 1,
Node: block,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "extra empty line at the end of a block",
})
}
Expand Down
4 changes: 2 additions & 2 deletions rule/enforce_map_style.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (r *EnforceMapStyleRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
failures = append(failures, lint.Failure{
Confidence: 1,
Node: v,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "use make(map[type]type) instead of map[type]type{}",
})
case *ast.CallExpr:
Expand Down Expand Up @@ -119,7 +119,7 @@ func (r *EnforceMapStyleRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa
failures = append(failures, lint.Failure{
Confidence: 1,
Node: v.Args[0],
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "use map[type]type{} instead of make(map[type]type)",
})
}
Expand Down
8 changes: 4 additions & 4 deletions rule/enforce_repeated_arg_type_style.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, _ lint.Argument
failures = append(failures, lint.Failure{
Confidence: 1,
Node: field,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "argument types should not be omitted",
})
}
Expand All @@ -137,7 +137,7 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, _ lint.Argument
failures = append(failures, lint.Failure{
Confidence: 1,
Node: prevType,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: fmt.Sprintf("repeated argument type %q can be omitted", prevTypeStr),
})
}
Expand All @@ -154,7 +154,7 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, _ lint.Argument
failures = append(failures, lint.Failure{
Confidence: 1,
Node: field,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: "return types should not be omitted",
})
}
Expand All @@ -170,7 +170,7 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, _ lint.Argument
failures = append(failures, lint.Failure{
Confidence: 1,
Node: prevType,
Category: "style",
Category: lint.FailureCategoryStyle,
Failure: fmt.Sprintf("repeated return type %q can be omitted", prevTypeStr),
})
}
Expand Down
Loading
Loading