Skip to content

Commit

Permalink
code cleaning
Browse files Browse the repository at this point in the history
  • Loading branch information
chavacava committed Sep 29, 2024
1 parent 55ff7ae commit ed77479
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 88 deletions.
46 changes: 26 additions & 20 deletions rule/struct-tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,27 @@ type StructTagRule struct {
func (r *StructTagRule) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()
if r.userDefined == nil && len(arguments) > 0 {
checkNumberOfArguments(1, arguments, r.Name())
r.userDefined = make(map[string][]string, len(arguments))
for _, arg := range arguments {
item, ok := arg.(string)
if !ok {
panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string, got %v (of type %T)", r.Name(), arg, arg))
}
parts := strings.Split(item, ",")
if len(parts) < 2 {
panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string of the form key[,option]+, got %s", r.Name(), item))
}
key := strings.TrimSpace(parts[0])
for i := 1; i < len(parts); i++ {
option := strings.TrimSpace(parts[i])
r.userDefined[key] = append(r.userDefined[key], option)
}

mustConfigure := r.userDefined == nil && len(arguments) > 0
if !mustConfigure {
return
}

checkNumberOfArguments(1, arguments, r.Name())
r.userDefined = make(map[string][]string, len(arguments))
for _, arg := range arguments {
item, ok := arg.(string)
if !ok {
panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string, got %v (of type %T)", r.Name(), arg, arg))
}
parts := strings.Split(item, ",")
if len(parts) < 2 {
panic(fmt.Sprintf("Invalid argument to the %s rule. Expecting a string of the form key[,option]+, got %s", r.Name(), item))
}
key := strings.TrimSpace(parts[0])
for i := 1; i < len(parts); i++ {
option := strings.TrimSpace(parts[i])
r.userDefined[key] = append(r.userDefined[key], option)
}
}
}
Expand Down Expand Up @@ -75,11 +79,13 @@ type lintStructTagRule struct {
func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor {
switch n := node.(type) {
case *ast.StructType:
if n.Fields == nil || n.Fields.NumFields() < 1 {
isEmptyStruct := n.Fields == nil || n.Fields.NumFields() < 1
if isEmptyStruct {
return nil // skip empty structs
}
w.usedTagNbr = map[int]bool{} // init
w.usedTagName = map[string]bool{} // init

w.usedTagNbr = map[int]bool{}
w.usedTagName = map[string]bool{}
for _, f := range n.Fields.List {
if f.Tag != nil {
w.checkTaggedField(f)
Expand Down
19 changes: 8 additions & 11 deletions rule/time-equal.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,23 @@ func (l *lintTimeEqual) Visit(node ast.Node) ast.Visitor {
return l
}

xtyp := l.file.Pkg.TypeOf(expr.X)
ytyp := l.file.Pkg.TypeOf(expr.Y)

if !isNamedType(xtyp, "time", "Time") || !isNamedType(ytyp, "time", "Time") {
typeOfX := l.file.Pkg.TypeOf(expr.X)
typeOfY := l.file.Pkg.TypeOf(expr.Y)
bothAreOfTimeType := isNamedType(typeOfX, "time", "Time") && isNamedType(typeOfY, "time", "Time")
if !bothAreOfTimeType {
return l
}

var failure string
switch expr.Op {
case token.EQL:
failure = fmt.Sprintf("use %s.Equal(%s) instead of %q operator", gofmt(expr.X), gofmt(expr.Y), expr.Op)
case token.NEQ:
failure = fmt.Sprintf("use !%s.Equal(%s) instead of %q operator", gofmt(expr.X), gofmt(expr.Y), expr.Op)
negateStr := ""
if token.NEQ == expr.Op {
negateStr = "!"
}

l.onFailure(lint.Failure{
Category: "time",
Confidence: 1,
Node: node,
Failure: failure,
Failure: fmt.Sprintf("use %s%s.Equal(%s) instead of %q operator", negateStr, gofmt(expr.X), gofmt(expr.Y), expr.Op),
})

return l
Expand Down
5 changes: 3 additions & 2 deletions rule/time-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func isNamedType(typ types.Type, importPath, name string) bool {
if !ok {
return false
}
tn := n.Obj()
return tn != nil && tn.Pkg() != nil && tn.Pkg().Path() == importPath && tn.Name() == name

typeName := n.Obj()
return typeName != nil && typeName.Pkg() != nil && typeName.Pkg().Path() == importPath && typeName.Name() == name
}
26 changes: 13 additions & 13 deletions rule/unchecked-type-assertion.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (u *UncheckedTypeAssertionRule) Apply(file *lint.File, args lint.Arguments)

var failures []lint.Failure

walker := &lintUnchekedTypeAssertion{
walker := &lintUncheckedTypeAssertion{
onFailure: func(failure lint.Failure) {
failures = append(failures, failure)
},
Expand All @@ -71,7 +71,7 @@ func (*UncheckedTypeAssertionRule) Name() string {
return "unchecked-type-assertion"
}

type lintUnchekedTypeAssertion struct {
type lintUncheckedTypeAssertion struct {
onFailure func(lint.Failure)
acceptIgnoredTypeAssertionResult bool
}
Expand All @@ -89,42 +89,42 @@ func isTypeSwitch(e *ast.TypeAssertExpr) bool {
return e.Type == nil
}

func (w *lintUnchekedTypeAssertion) requireNoTypeAssert(expr ast.Expr) {
func (w *lintUncheckedTypeAssertion) requireNoTypeAssert(expr ast.Expr) {
e, ok := expr.(*ast.TypeAssertExpr)
if ok && !isTypeSwitch(e) {
w.addFailure(e, ruleUTAMessagePanic)
}
}

func (w *lintUnchekedTypeAssertion) handleIfStmt(n *ast.IfStmt) {
func (w *lintUncheckedTypeAssertion) handleIfStmt(n *ast.IfStmt) {
ifCondition, ok := n.Cond.(*ast.BinaryExpr)
if ok {
w.requireNoTypeAssert(ifCondition.X)
w.requireNoTypeAssert(ifCondition.Y)
}
}

func (w *lintUnchekedTypeAssertion) requireBinaryExpressionWithoutTypeAssertion(expr ast.Expr) {
func (w *lintUncheckedTypeAssertion) requireBinaryExpressionWithoutTypeAssertion(expr ast.Expr) {
binaryExpr, ok := expr.(*ast.BinaryExpr)
if ok {
w.requireNoTypeAssert(binaryExpr.X)
w.requireNoTypeAssert(binaryExpr.Y)
}
}

func (w *lintUnchekedTypeAssertion) handleCaseClause(n *ast.CaseClause) {
func (w *lintUncheckedTypeAssertion) handleCaseClause(n *ast.CaseClause) {
for _, expr := range n.List {
w.requireNoTypeAssert(expr)
w.requireBinaryExpressionWithoutTypeAssertion(expr)
}
}

func (w *lintUnchekedTypeAssertion) handleSwitch(n *ast.SwitchStmt) {
func (w *lintUncheckedTypeAssertion) handleSwitch(n *ast.SwitchStmt) {
w.requireNoTypeAssert(n.Tag)
w.requireBinaryExpressionWithoutTypeAssertion(n.Tag)
}

func (w *lintUnchekedTypeAssertion) handleAssignment(n *ast.AssignStmt) {
func (w *lintUncheckedTypeAssertion) handleAssignment(n *ast.AssignStmt) {
if len(n.Rhs) == 0 {
return
}
Expand All @@ -148,21 +148,21 @@ func (w *lintUnchekedTypeAssertion) handleAssignment(n *ast.AssignStmt) {
}

// handles "return foo(.*bar)" - one of them is enough to fail as golang does not forward the type cast tuples in return statements
func (w *lintUnchekedTypeAssertion) handleReturn(n *ast.ReturnStmt) {
func (w *lintUncheckedTypeAssertion) handleReturn(n *ast.ReturnStmt) {
for _, r := range n.Results {
w.requireNoTypeAssert(r)
}
}

func (w *lintUnchekedTypeAssertion) handleRange(n *ast.RangeStmt) {
func (w *lintUncheckedTypeAssertion) handleRange(n *ast.RangeStmt) {
w.requireNoTypeAssert(n.X)
}

func (w *lintUnchekedTypeAssertion) handleChannelSend(n *ast.SendStmt) {
func (w *lintUncheckedTypeAssertion) handleChannelSend(n *ast.SendStmt) {
w.requireNoTypeAssert(n.Value)
}

func (w *lintUnchekedTypeAssertion) Visit(node ast.Node) ast.Visitor {
func (w *lintUncheckedTypeAssertion) Visit(node ast.Node) ast.Visitor {
switch n := node.(type) {
case *ast.RangeStmt:
w.handleRange(n)
Expand All @@ -183,7 +183,7 @@ func (w *lintUnchekedTypeAssertion) Visit(node ast.Node) ast.Visitor {
return w
}

func (w *lintUnchekedTypeAssertion) addFailure(n *ast.TypeAssertExpr, why string) {
func (w *lintUncheckedTypeAssertion) addFailure(n *ast.TypeAssertExpr, why string) {
s := fmt.Sprintf("type cast result is unchecked in %v - %s", gofmt(n), why)
w.onFailure(lint.Failure{
Category: "bad practice",
Expand Down
7 changes: 4 additions & 3 deletions rule/unconditional-recursion.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,10 @@ func (lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool {
return false
}

fn := se.Sel.Name
pkg := id.Name
if exitFunctions[pkg] != nil && exitFunctions[pkg][fn] { // it's a call to an exit function
functionName := se.Sel.Name
pkgName := id.Name
isCallToExitFunction := exitFunctions[pkgName] != nil && exitFunctions[pkgName][functionName]
if isCallToExitFunction {
return true
}
}
Expand Down
41 changes: 22 additions & 19 deletions rule/unhandled-error.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,30 @@ type UnhandledErrorRule struct {

func (r *UnhandledErrorRule) configure(arguments lint.Arguments) {
r.Lock()
if r.ignoreList == nil {
for _, arg := range arguments {
argStr, ok := arg.(string)
if !ok {
panic(fmt.Sprintf("Invalid argument to the unhandled-error rule. Expecting a string, got %T", arg))
}
defer r.Unlock()

argStr = strings.Trim(argStr, " ")
if argStr == "" {
panic("Invalid argument to the unhandled-error rule, expected regular expression must not be empty.")
}
if r.ignoreList != nil {
return // already configured
}

exp, err := regexp.Compile(argStr)
if err != nil {
panic(fmt.Sprintf("Invalid argument to the unhandled-error rule: regexp %q does not compile: %v", argStr, err))
}
for _, arg := range arguments {
argStr, ok := arg.(string)
if !ok {
panic(fmt.Sprintf("Invalid argument to the unhandled-error rule. Expecting a string, got %T", arg))
}

r.ignoreList = append(r.ignoreList, exp)
argStr = strings.Trim(argStr, " ")
if argStr == "" {
panic("Invalid argument to the unhandled-error rule, expected regular expression must not be empty.")
}

exp, err := regexp.Compile(argStr)
if err != nil {
panic(fmt.Sprintf("Invalid argument to the unhandled-error rule: regexp %q does not compile: %v", argStr, err))
}

r.ignoreList = append(r.ignoreList, exp)
}
r.Unlock()
}

// Apply applies the rule to given file.
Expand Down Expand Up @@ -130,9 +133,9 @@ func (w *lintUnhandledErrors) funcName(call *ast.CallExpr) string {
}

name := fn.FullName()
name = strings.Replace(name, "(", "", -1)
name = strings.Replace(name, ")", "", -1)
name = strings.Replace(name, "*", "", -1)
name = strings.ReplaceAll(name, "(", "")
name = strings.ReplaceAll(name, ")", "")
name = strings.ReplaceAll(name, "*", "")

return name
}
Expand Down
16 changes: 9 additions & 7 deletions rule/var-declarations.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ type lintVarDeclarations struct {
func (w *lintVarDeclarations) Visit(node ast.Node) ast.Visitor {
switch v := node.(type) {
case *ast.GenDecl:
if v.Tok != token.CONST && v.Tok != token.VAR {
isVarOrConstDeclaration := v.Tok == token.CONST || v.Tok == token.VAR
if !isVarOrConstDeclaration {
return nil
}
w.lastGen = v
return w
case *ast.ValueSpec:
if w.lastGen.Tok == token.CONST {
isConstDeclaration := w.lastGen.Tok == token.CONST
if isConstDeclaration {
return nil
}
if len(v.Names) > 1 || v.Type == nil || len(v.Values) == 0 {
Expand All @@ -64,14 +66,14 @@ func (w *lintVarDeclarations) Visit(node ast.Node) ast.Visitor {
if isIdent(v.Names[0], "_") {
return nil
}
// If the RHS is a zero value, suggest dropping it.
zero := false
// If the RHS is a isZero value, suggest dropping it.
isZero := false
if lit, ok := rhs.(*ast.BasicLit); ok {
zero = zeroLiteral[lit.Value]
isZero = zeroLiteral[lit.Value]
} else if isIdent(rhs, "nil") {
zero = true
isZero = true
}
if zero {
if isZero {
w.onFailure(lint.Failure{
Confidence: 0.9,
Node: rhs,
Expand Down
24 changes: 12 additions & 12 deletions rule/var-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ var upperCaseConstRE = regexp.MustCompile(`^_?[A-Z][A-Z\d]*(_[A-Z\d]+)*$`)
// VarNamingRule lints given else constructs.
type VarNamingRule struct {
configured bool
allowlist []string
blocklist []string
upperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants
allowList []string
blockList []string
allowUpperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants
skipPackageNameChecks bool
sync.Mutex
}
Expand All @@ -35,11 +35,11 @@ func (r *VarNamingRule) configure(arguments lint.Arguments) {

r.configured = true
if len(arguments) >= 1 {
r.allowlist = getList(arguments[0], "allowlist")
r.allowList = getList(arguments[0], "allowlist")
}

if len(arguments) >= 2 {
r.blocklist = getList(arguments[1], "blocklist")
r.blockList = getList(arguments[1], "blocklist")
}

if len(arguments) >= 3 {
Expand All @@ -56,7 +56,7 @@ func (r *VarNamingRule) configure(arguments lint.Arguments) {
if !ok {
panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, with map, but %T", "options", asSlice[0]))
}
r.upperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true"
r.allowUpperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true"
r.skipPackageNameChecks = fmt.Sprint(args["skipPackageNameChecks"]) == "true"
}
}
Expand Down Expand Up @@ -93,12 +93,12 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint.
walker := lintNames{
file: file,
fileAst: fileAst,
allowlist: r.allowlist,
blocklist: r.blocklist,
allowList: r.allowList,
blockList: r.blockList,
onFailure: func(failure lint.Failure) {
failures = append(failures, failure)
},
upperCaseConst: r.upperCaseConst,
upperCaseConst: r.allowUpperCaseConst,
}

if !r.skipPackageNameChecks {
Expand Down Expand Up @@ -151,7 +151,7 @@ func (w *lintNames) check(id *ast.Ident, thing string) {
return
}

should := lint.Name(id.Name, w.allowlist, w.blocklist)
should := lint.Name(id.Name, w.allowList, w.blockList)
if id.Name == should {
return
}
Expand All @@ -177,8 +177,8 @@ type lintNames struct {
file *lint.File
fileAst *ast.File
onFailure func(lint.Failure)
allowlist []string
blocklist []string
allowList []string
blockList []string
upperCaseConst bool
}

Expand Down
Loading

0 comments on commit ed77479

Please sign in to comment.