Skip to content

Commit 6bea9ed

Browse files
committed
Implement deprecation warning
fixes golang#238
1 parent c5fb716 commit 6bea9ed

9 files changed

+111
-25
lines changed

lint.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"go/printer"
1717
"go/token"
1818
"go/types"
19+
"log"
1920
"regexp"
2021
"sort"
2122
"strconv"
@@ -24,6 +25,7 @@ import (
2425
"unicode/utf8"
2526

2627
"golang.org/x/tools/go/gcexportdata"
28+
"golang.org/x/tools/go/loader"
2729
)
2830

2931
const styleGuideBase = "https://golang.org/wiki/CodeReviewComments"
@@ -140,6 +142,8 @@ type pkg struct {
140142
typesPkg *types.Package
141143
typesInfo *types.Info
142144

145+
prog *loader.Program
146+
143147
// sortable is the set of types in the package that implement sort.Interface.
144148
sortable map[string]bool
145149
// main is whether this is a "main" package.
@@ -168,6 +172,37 @@ func (p *pkg) lint() []Problem {
168172
*/
169173
}
170174

175+
// TODO(stapelberg): skip the p.typeCheck earlier in favor of using what the
176+
// loader gives us.
177+
var files []*ast.File
178+
for _, f := range p.files {
179+
files = append(files, f.f)
180+
}
181+
182+
conf := loader.Config{
183+
Fset: p.fset,
184+
ParserMode: parser.ParseComments, // for lintDeprecated
185+
CreatePkgs: []loader.PkgSpec{
186+
{Files: files},
187+
},
188+
TypeChecker: types.Config{
189+
// Enable FakeImportC so that we can load ad-hoc packages which
190+
// import "C".
191+
FakeImportC: true,
192+
Error: func(err error) {}, // ignore errors
193+
},
194+
// Skip type-checking the function bodies of dependencies:
195+
TypeCheckFuncBodies: func(path string) bool {
196+
return path == files[0].Name.Name
197+
},
198+
}
199+
prog, err := conf.Load()
200+
if err != nil {
201+
log.Printf("loading failed: %v", err)
202+
return p.problems
203+
}
204+
p.prog = prog
205+
171206
p.scanSortable()
172207
p.main = p.isMain()
173208

@@ -210,6 +245,7 @@ func (f *file) lint() {
210245
f.lintTimeNames()
211246
f.lintContextKeyTypes()
212247
f.lintContextArgs()
248+
f.lintDeprecated()
213249
}
214250

215251
type link string
@@ -1466,6 +1502,64 @@ func (f *file) lintContextArgs() {
14661502
})
14671503
}
14681504

1505+
func docText(n ast.Node) string {
1506+
switch n := n.(type) {
1507+
case *ast.FuncDecl:
1508+
return n.Doc.Text()
1509+
1510+
case *ast.Field:
1511+
return n.Doc.Text()
1512+
1513+
case *ast.ValueSpec:
1514+
return n.Doc.Text()
1515+
1516+
case *ast.TypeSpec:
1517+
return n.Doc.Text()
1518+
1519+
default:
1520+
return ""
1521+
}
1522+
}
1523+
1524+
// deprecated returns the deprecation message of a doc comment, or "".
1525+
func deprecated(doc string) string {
1526+
paragraphs := strings.Split(doc, "\n\n")
1527+
last := paragraphs[len(paragraphs)-1]
1528+
if !strings.HasPrefix(last, "Deprecated: ") {
1529+
return ""
1530+
}
1531+
return strings.Replace(strings.TrimPrefix(last, "Deprecated: "), "\n", " ", -1)
1532+
}
1533+
1534+
func (f *file) lintDeprecated() {
1535+
f.walk(func(n ast.Node) bool {
1536+
sel, ok := n.(*ast.SelectorExpr)
1537+
if !ok {
1538+
return true
1539+
}
1540+
1541+
obj := f.pkg.prog.Created[0].ObjectOf(sel.Sel)
1542+
if obj == nil || obj.Pkg() == nil {
1543+
return false
1544+
}
1545+
// TODO(stapelberg): verify obj is in a different package, add a test
1546+
1547+
_, path, _ := f.pkg.prog.PathEnclosingInterval(obj.Pos(), obj.Pos())
1548+
// Expectation:
1549+
// path[0] holds an *ast.Ident
1550+
// path[1] holds the declaration we are interested in
1551+
if len(path) <= 2 {
1552+
return false
1553+
}
1554+
1555+
if dep := deprecated(docText(path[1])); dep != "" {
1556+
f.errorf(sel, 1.0, category("deprecation"), "deprecated: %s", dep)
1557+
}
1558+
1559+
return false
1560+
})
1561+
}
1562+
14691563
// receiverType returns the named type of the method receiver, sans "*",
14701564
// or "invalid-type" if fn.Recv is ill formed.
14711565
func receiverType(fn *ast.FuncDecl) string {

testdata/broken.go

Lines changed: 0 additions & 9 deletions
This file was deleted.

testdata/context.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ func x(ctx context.Context) { // ok
1212
}
1313

1414
// A proper context.Context location
15-
func x(ctx context.Context, s string) { // ok
15+
func x2(ctx context.Context, s string) { // ok
1616
}
1717

1818
// An invalid context.Context location
1919
func y(s string, ctx context.Context) { // MATCH /context.Context should be the first parameter.*/
2020
}
2121

2222
// An invalid context.Context location with more than 2 args
23-
func y(s string, r int, ctx context.Context, x int) { // MATCH /context.Context should be the first parameter.*/
23+
func y2(s string, r int, ctx context.Context, x int) { // MATCH /context.Context should be the first parameter.*/
2424
}

testdata/contextkeytypes.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,4 @@ func contextKeyTypeTests() {
3434
context.WithValue(c, complex128(1i), "bar") // MATCH /should not use basic type complex128 as key in context.WithValue/
3535
context.WithValue(c, ctxKey{}, "bar") // ok
3636
context.WithValue(c, &ctxKey{}, "bar") // ok
37-
context.WithValue(c, invalid{}, "bar") // ok
3837
}

testdata/deprecated.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Test for deprecation warnings.
2+
3+
// Package main ...
4+
package main
5+
6+
import "net/http/httputil"
7+
8+
func main() {
9+
httputil.NewClientConn(nil, nil) // MATCH /deprecated/
10+
}

testdata/errorf.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ func f(x int) error {
2626
func TestF(t *testing.T) error {
2727
x := 1
2828
if x > 10 {
29-
return t.Error(fmt.Sprintf("something %d", x)) // MATCH /should replace.*t\.Error\(fmt\.Sprintf\(\.\.\.\)\).*t\.Errorf\(\.\.\.\)/
29+
t.Error(fmt.Sprintf("something %d", x)) // MATCH /should replace.*t\.Error\(fmt\.Sprintf\(\.\.\.\)\).*t\.Errorf\(\.\.\.\)/
3030
}
3131
if x > 5 {
32-
return t.Error(g("blah")) // ok
32+
t.Error(g("blah")) // ok
3333
}
3434
if x > 4 {
35-
return t.Error("something else") // ok
35+
t.Error("something else") // ok
3636
}
3737
return nil
3838
}

testdata/receiver-names.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,3 @@ type multiError struct{}
4444

4545
func (me multiError) f8() {
4646
}
47-
48-
// Regression test for a panic caused by ill-formed receiver type.
49-
func (recv []*x.y) f()

testdata/unexp-return.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
// Package foo ...
44
package foo
55

6-
import ()
7-
86
type hidden struct{}
97

108
// Exported returns a hidden type, which is annoying.
@@ -14,9 +12,11 @@ func Exported() hidden { // MATCH /Exported.*unexported.*hidden/
1412

1513
// ExpErr returns a builtin type.
1614
func ExpErr() error { // ok
15+
return nil
1716
}
1817

1918
func (hidden) ExpOnHidden() hidden { // ok
19+
return hidden{}
2020
}
2121

2222
// T is another test type.

testdata/var-decl.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"io"
99
"net/http"
10-
"nosuchpkg" // export data unavailable
1110
"os"
1211
)
1312

@@ -75,10 +74,6 @@ var out2 io.Writer = newWriter() // MATCH /should omit.*io\.Writer/
7574

7675
func newWriter() io.Writer { return nil }
7776

78-
// We don't figure out the true types of LHS and RHS here,
79-
// so we suppress the check.
80-
var ni nosuchpkg.Interface = nosuchpkg.NewConcrete()
81-
8277
var y string = q(1).String() // MATCH /should.*string/
8378

8479
type q int

0 commit comments

Comments
 (0)