Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

Commit

Permalink
Hook up typechecker.
Browse files Browse the repository at this point in the history
This does nothing with the extra information except warn if there's an error
returned by the typechecker (e.g. var not used).

This is the first step towards addressing #7.
  • Loading branch information
dsymonds committed Sep 1, 2014
1 parent 4b1924a commit 54857da
Show file tree
Hide file tree
Showing 12 changed files with 123 additions and 19 deletions.
41 changes: 41 additions & 0 deletions lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import (
"strings"
"unicode"
"unicode/utf8"

"code.google.com/p/go.tools/go/types"

_ "code.google.com/p/go.tools/go/gcimporter" // use gcimporter as types.DefaultImport
)

const styleGuideBase = "http://golang.org/s/comments"
Expand Down Expand Up @@ -61,6 +65,9 @@ type file struct {
src []byte
filename string

typesPkg *types.Package
typesInfo *types.Info

// sortable is the set of types in the file that implement sort.Interface.
sortable map[string]bool
// main is whether this file is in a "main" package.
Expand All @@ -72,6 +79,23 @@ type file struct {
func (f *file) isTest() bool { return strings.HasSuffix(f.filename, "_test.go") }

func (f *file) lint() []Problem {
if err := f.typeCheck(); err != nil {
if e, ok := err.(types.Error); ok {
p := f.fset.Position(e.Pos)
conf := 1.0
if strings.Contains(e.Msg, "can't find import: ") {
// Golint is probably being run in a context that doesn't support
// typechecking (e.g. package files aren't found), so don't warn about it.
conf = 0
}
if conf > 0 {
f.errorfAt(p, conf, category("typechecking"), e.Msg)
}

// TODO(dsymonds): Abort if !e.Soft?
}
}

f.scanSortable()
f.main = f.isMain()

Expand Down Expand Up @@ -101,6 +125,10 @@ type category string
// and must end with a format string and any arguments.
func (f *file) errorf(n ast.Node, confidence float64, args ...interface{}) {
p := f.fset.Position(n.Pos())
f.errorfAt(p, confidence, args...)
}

func (f *file) errorfAt(p token.Position, confidence float64, args ...interface{}) {
problem := Problem{
Position: p,
Confidence: confidence,
Expand All @@ -125,6 +153,19 @@ argLoop:
f.problems = append(f.problems, problem)
}

func (f *file) typeCheck() error {
// Do typechecking without errors so we do as much as possible.
config := &types.Config{}
f.typesInfo = &types.Info{
Types: make(map[ast.Expr]types.TypeAndValue),
Defs: make(map[*ast.Ident]types.Object),
Uses: make(map[*ast.Ident]types.Object),
}
var err error
f.typesPkg, err = config.Check(f.f.Name.Name, f.fset, []*ast.File{f.f}, f.typesInfo)
return err
}

func (f *file) scanSortable() {
f.sortable = make(map[string]bool)

Expand Down
5 changes: 5 additions & 0 deletions testdata/blank-import-lib.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@ import (
_ "go/scanner" // Don't gripe about this or the following line.
_ "go/token"
)

var (
_ fmt.Stringer // for "fmt"
_ ast.Node // for "go/ast"
)
5 changes: 5 additions & 0 deletions testdata/blank-import-lib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,8 @@ import (
_ "net/http"
_ "path"
)

var (
_ fmt.Stringer // for "fmt"
_ testing.T // for "testing"
)
2 changes: 2 additions & 0 deletions testdata/blank-import-main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ import (
"os"
_ "path"
)

var _ os.File // for "os"
8 changes: 4 additions & 4 deletions testdata/error-return.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@ func i() (int, error) { // ok
}

// Check for multiple return but error at end with named variables.
func i() (x int, err error) { // ok
func j() (x int, err error) { // ok
return 0, nil
}

// Check for error in the wrong location on 2 types
func j() (error, int) { // MATCH /error should be the last type/
func k() (error, int) { // MATCH /error should be the last type/
return nil, 0
}

// Check for error in the wrong location for > 2 types
func k() (int, error, int) { // MATCH /error should be the last type/
func l() (int, error, int) { // MATCH /error should be the last type/
return 0, nil, 0
}

// Check for error in the wrong location with named variables.
func l() (x int, err error, y int) { // MATCH /error should be the last type/
func m() (x int, err error, y int) { // MATCH /error should be the last type/
return 0, nil, 0
}
2 changes: 2 additions & 0 deletions testdata/errorf.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ func f(x int) error {
}
return nil
}

func g(s string) string { return "prefix: " + s }
1 change: 1 addition & 0 deletions testdata/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var (

func f() {
var whatever = errors.New("ok") // ok
_ = whatever
}

// Check for the error strings themselves.
Expand Down
2 changes: 2 additions & 0 deletions testdata/import-dot.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
package pkg

import . "fmt" // MATCH /dot import/

var _ Stringer // from "fmt"
15 changes: 12 additions & 3 deletions testdata/make.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,17 @@
// Package pkg ...
package pkg

import "net/http"

// T is a test type.
type T int

var z []T

func f() {
x := make([]T, 0) // MATCH /var x \[\]T/
y := make([]somepkg.Foo_Bar, 0) // MATCH /var y \[\]somepkg.Foo_Bar/
z = make([]T, 0) // ok, because we don't know where z is declared
x := make([]T, 0) // MATCH /var x \[\]T/
y := make([]http.Request, 0) // MATCH /var y \[\]http\.Request/
z = make([]T, 0) // ok, because we don't know where z is declared

_, _ = x, y
}
33 changes: 26 additions & 7 deletions testdata/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
// Package pkg_with_underscores ...
package pkg_with_underscores // MATCH /underscore.*package name/

import (
"io"
"net"
net_http "net/http" // renamed deliberately
"net/url"
)

var var_name int // MATCH /underscore.*var.*var_name/

type t_wow struct { // MATCH /underscore.*type.*t_wow/
Expand All @@ -13,23 +20,35 @@ type t_wow struct { // MATCH /underscore.*type.*t_wow/
const fooId = "blah" // MATCH /fooId.*fooID/

func f_it() { // MATCH /underscore.*func.*f_it/
more_underscore := 4 // MATCH /underscore.*var.*more_underscore/
more_underscore := 4 // MATCH /underscore.*var.*more_underscore/
_ = more_underscore
var err error
if isEof := (err == io.EOF); isEof { // MATCH /var.*isEof.*isEOF/
more_underscore = 7 // should be okay
}

x := foo_proto.Blah{} // should be okay
x := net_http.Request{} // should be okay
_ = x

var ips []net.IP
for _, theIp := range ips { // MATCH /range var.*theIp.*theIP/
_ = theIp
}

switch myJson := g(); { // MATCH /var.*myJson.*myJSON/
default:
_ = myJson
}
switch tApi := x.(type) { // MATCH /var.*tApi.*tAPI/
var y net_http.ResponseWriter // an interface
switch tApi := y.(type) { // MATCH /var.*tApi.*tAPI/
default:
_ = tApi
}

var c chan int
select {
case qId := <-c: // MATCH /var.*qId.*qID/
_ = qId
}
}

Expand All @@ -42,10 +61,10 @@ const (
X509B = 4 // ditto
)

func f(bad_name int) {} // MATCH /underscore.*func parameter.*bad_name/
func g() (no_way int) {} // MATCH /underscore.*func result.*no_way/
func (t *t_wow) f(more_under string) {} // MATCH /underscore.*method parameter.*more_under/
func (t *t_wow) g() (still_more string) {} // MATCH /underscore.*method result.*still_more/
func f(bad_name int) {} // MATCH /underscore.*func parameter.*bad_name/
func g() (no_way int) { return 0 } // MATCH /underscore.*func result.*no_way/
func (t *t_wow) f(more_under string) {} // MATCH /underscore.*method parameter.*more_under/
func (t *t_wow) g() (still_more string) { return "" } // MATCH /underscore.*method result.*still_more/

type i interface {
CheckHtml() string // okay; interface method names are often constrained by the concrete types' method names
Expand Down
16 changes: 13 additions & 3 deletions testdata/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,34 @@
package foo

func f() {
var m map[string]int

// with :=
for x, _ := range m { // MATCH /should omit 2nd value.*range.*equivalent.*for x := range/
_ = x
}
// with =
var y string
_ = y
for y, _ = range m { // MATCH /should omit 2nd value.*range.*equivalent.*for y = range/
}

// all OK:
for x := range m {
_ = x
}
for x, y := range m {
_, _ = x, y
}
for _, y := range m {
_ = y
}
for x = range m {
var x int
_ = x
for y = range m {
}
for x, y = range m {
for y, x = range m {
}
for _, y = range m {
for _, x = range m {
}
}
12 changes: 10 additions & 2 deletions testdata/var-decl.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ package foo
import "fmt"
import "net/http"

// Q is a test type.
type Q bool

var mux *http.ServeMux = http.NewServeMux() // MATCH /should.*\*http\.ServeMux.*inferred/
var myInt int = 7 // MATCH /should.*int.*myInt.*inferred/

var myZeroInt int = 0 // MATCH /should.*= 0.*myZeroInt.*zero value/
var myZeroFlt float32 = 0. // MATCH /should.*= 0\..*myZeroFlt.*zero value/
var myZeroF64 float64 = 0.0 // MATCH /should.*= 0\..*myZeroF64.*zero value/
var myZeroImg complex = 0i // MATCH /should.*= 0i.*myZeroImg.*zero value/
var myZeroImg complex64 = 0i // MATCH /should.*= 0i.*myZeroImg.*zero value/
var myZeroStr string = "" // MATCH /should.*= "".*myZeroStr.*zero value/
var myZeroRaw string = `` // MATCH /should.*= ``.*myZeroRaw.*zero value/
var myZeroPtr *Q = nil // MATCH /should.*= nil.*myZeroPtr.*zero value/
Expand All @@ -26,7 +29,7 @@ var x = 0
var str fmt.Stringer

// No warning because this is a const.
const x uint64 = 7
const k uint64 = 7

// No warnings because the RHS is an ideal int, and the LHS is a different int type.
var userID int64 = 1235
Expand All @@ -43,6 +46,11 @@ var floatV floatT = 123.45

// No warning because the LHS names an interface type.
var data interface{} = googleIPs
var googleIPs []int

// No warning because it's a common idiom for interface satisfaction.
var _ Server = (*serverImpl)(nil)

// Server is a test type.
type Server interface{}
type serverImpl struct{}

0 comments on commit 54857da

Please sign in to comment.