Skip to content

Commit

Permalink
SA1019: account for all combinations of test packages using each other
Browse files Browse the repository at this point in the history
A single Go package results in three packages when testing: the package
under test with its internal tests, a package for external tests, and a
package main that runs the tests.

We need to allow external tests to import the package under test, and
the test's main to import both the package under test as well as its
external tests, and to use the objects defined in them.

Closes gh-1285
  • Loading branch information
dominikh committed Jan 26, 2023
1 parent 016be07 commit 52a0144
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 2 deletions.
37 changes: 35 additions & 2 deletions staticcheck/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Deprecated: this package is deprecated.
package pkg

import _ "example.com/CheckDeprecated.assist" //@ diag(`Alas, it is deprecated.`)
Expand All @@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 52a0144

Please sign in to comment.