Skip to content

Commit

Permalink
checkers: follow-up for go-critic#814 (go-critic#818)
Browse files Browse the repository at this point in the history
- Using ast file (fPos) is not needed.
  It's possible to get Pos line by using a node and FileSet only.

- Trailing period in the Summary is redundant.
  We need to write tests for doc objects validation.

- Using plural forms in checker names is uncommon.
  s/dupImports/dupImport/

- Before/After examples should be as minimal, as possible.
  They are not required to be a complete Go program.
  Simplified them a bit.

- Code that emits a warning and builds up a message
  should be moved to "warn" function.
  This is a convention.

Signed-off-by: Iskander Sharipov <[email protected]>
  • Loading branch information
quasilyte authored Mar 2, 2019
1 parent f5dab9f commit 5379bdb
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 33 deletions.
53 changes: 20 additions & 33 deletions checkers/dupImports_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,18 @@ import (

func init() {
var info lintpack.CheckerInfo
info.Name = "dupImports"
info.Name = "dupImport"
info.Tags = []string{"style", "experimental"}
info.Summary = "Detects multiple imports of the same package under different aliases."
info.Summary = "Detects multiple imports of the same package under different aliases"
info.Before = `
import (
"fmt"
priting "fmt"
)
func helloWorld() {
fmt.Println("Hello")
printing.Println("World")
}`
priting "fmt" // Imported the second time
)`
info.After = `
import(
"fmt"
)
func helloWorld() {
fmt.Println("Hello")
fmt.Println("World")
}`
)`

collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker {
return &dupImportChecker{ctx: ctx}
Expand All @@ -48,29 +38,26 @@ func (c *dupImportChecker) WalkFile(f *ast.File) {
imports[pkg] = append(imports[pkg], importDcl)
}

fPos := c.ctx.FileSet.File(f.Pos())
if fPos == nil {
// Bail out if we can't, for any reason, determine
// the source file we are dealing with.
return
}
for _, importList := range imports {
if len(importList) == 1 {
continue
}
c.warn(importList)
}
}

msg := fmt.Sprintf("package is imported %d times under different aliases on lines", len(importList))
for idx, importDcl := range importList {
switch {
case idx == len(importList)-1:
msg += " and"
case idx > 0:
msg += ","
}
msg += fmt.Sprintf(" %d", fPos.Line(importDcl.Pos()))
}
for _, importDcl := range importList {
c.ctx.Warn(importDcl, msg)
func (c *dupImportChecker) warn(importList []*ast.ImportSpec) {
msg := fmt.Sprintf("package is imported %d times under different aliases on lines", len(importList))
for idx, importDcl := range importList {
switch {
case idx == len(importList)-1:
msg += " and"
case idx > 0:
msg += ","
}
msg += fmt.Sprintf(" %d", c.ctx.FileSet.Position(importDcl.Pos()).Line)
}
for _, importDcl := range importList {
c.ctx.Warn(importDcl, msg)
}
}

0 comments on commit 5379bdb

Please sign in to comment.