Skip to content

Commit

Permalink
refactor: replace Category raw strings with constants
Browse files Browse the repository at this point in the history
  • Loading branch information
alexandear committed Dec 25, 2024
1 parent 4ca2c11 commit 9805dde
Show file tree
Hide file tree
Showing 63 changed files with 125 additions and 104 deletions.
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

0 comments on commit 9805dde

Please sign in to comment.