diff --git a/staticcheck/lint.go b/staticcheck/lint.go index 2677bcc8d..77e42b423 100644 --- a/staticcheck/lint.go +++ b/staticcheck/lint.go @@ -3166,6 +3166,8 @@ func CheckDeprecated(pass *analysis.Pass) (interface{}, error) { if fn, ok := node.(*ast.FuncDecl); ok { tfn = pass.TypesInfo.ObjectOf(fn.Name) } + + // FIXME(dh): this misses dot-imported objects sel, ok := node.(*ast.SelectorExpr) if !ok { return true @@ -3178,8 +3180,26 @@ func CheckDeprecated(pass *analysis.Pass) (interface{}, error) { if obj.Pkg() == nil { return true } - if pass.Pkg == obj.Pkg() || obj.Pkg().Path()+"_test" == pass.Pkg.Path() { - // Don't flag stuff in our own package + + if obj.Pkg() == pass.Pkg { + // A package is allowed to use its own deprecated objects + return true + } + + // A package "foo" has two related packages "foo_test" and "foo.test", for external tests and the package main + // generated by 'go test' respectively. "foo_test" can import and use "foo", "foo.test" imports and uses "foo" + // and "foo_test". + + if strings.TrimSuffix(pass.Pkg.Path(), "_test") == obj.Pkg().Path() { + // foo_test (the external tests of foo) can use objects from foo. + return true + } + if strings.TrimSuffix(pass.Pkg.Path(), ".test") == obj.Pkg().Path() { + // foo.test (the main package of foo's tests) can use objects from foo. + return true + } + if strings.TrimSuffix(pass.Pkg.Path(), ".test") == strings.TrimSuffix(obj.Pkg().Path(), "_test") { + // foo.test (the main package of foo's tests) can use objects from foo's external tests. return true } @@ -3208,6 +3228,19 @@ func CheckDeprecated(pass *analysis.Pass) (interface{}, error) { } } + if strings.TrimSuffix(pass.Pkg.Path(), "_test") == path { + // foo_test can import foo + return + } + if strings.TrimSuffix(pass.Pkg.Path(), ".test") == path { + // foo.test can import foo + return + } + if strings.TrimSuffix(pass.Pkg.Path(), ".test") == strings.TrimSuffix(path, "_test") { + // foo.test can import foo_test + return + } + handleDeprecation(depr, spec.Path, path, path, nil) } } diff --git a/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go b/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go index 7a8225f2f..a28673353 100644 --- a/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go +++ b/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated.go @@ -1,3 +1,4 @@ +// Deprecated: this package is deprecated. package pkg import _ "example.com/CheckDeprecated.assist" //@ diag(`Alas, it is deprecated.`) @@ -8,4 +9,14 @@ import "example.com/AnotherCheckDeprecated.assist" //@ diag(`Alas, it is dep func init() { foo.Fn() AnotherCheckDeprecatedassist.Fn() + + // Field is deprecated, but we're using it from the same package, which is fine. + var s S + _ = s.Field +} + + +type S struct { + // Deprecated: this is deprecated. + Field int } diff --git a/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated_test.go b/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated_test.go new file mode 100644 index 000000000..dc95d3e29 --- /dev/null +++ b/staticcheck/testdata/src/example.com/CheckDeprecated/CheckDeprecated_test.go @@ -0,0 +1,18 @@ +package pkg + +import "testing" + +// Deprecated: deprecating tests is silly but possible. +func TestFoo(t *testing.T) { + var s S + // Internal tests can use deprecated objects from the package they test. + _ = s.Field +} + +// This test isn't deprecated, to test that s.Field doesn't get flagged because it's from the package under test. If +// TestBar was itself deprecated, it could use any deprecated objects it wanted. +func TestBar(t *testing.T) { + var s S + // Internal tests can use deprecated objects from the package under test + _ = s.Field +} diff --git a/staticcheck/testdata/src/example.com/CheckDeprecated/external_test.go b/staticcheck/testdata/src/example.com/CheckDeprecated/external_test.go new file mode 100644 index 000000000..601887981 --- /dev/null +++ b/staticcheck/testdata/src/example.com/CheckDeprecated/external_test.go @@ -0,0 +1,21 @@ +// Deprecated: deprecating external test packages is silly but possible. +package pkg_test + +import ( + "testing" + + // External tests can import deprecated packages under test. + pkg "example.com/CheckDeprecated" +) + +// Deprecated: deprecating tests is silly but possible. +func TestFoo(t *testing.T) { +} + +// This test isn't deprecated, to test that s.Field doesn't get flagged because it's from the package under test. If +// TestBar was itself deprecated, it could use any deprecated objects it wanted. +func TestBar(t *testing.T) { + var s pkg.S + // External tests can use deprecated objects from the package under test + _ = s.Field +}