Skip to content

Commit c13dc1b

Browse files
dominikhdominikh
committed
staticcheck: more tailored deprecation diagnostics
Print different diagnostics depending on the precise combination of "deprecated since" and "alternative available since".
1 parent 900aaa9 commit c13dc1b

File tree

6 files changed

+142
-86
lines changed

6 files changed

+142
-86
lines changed

‎knowledge/deprecated.go

Lines changed: 77 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
package knowledge
22

3+
const (
4+
DeprecatedNeverUse = -1
5+
DeprecatedUseNoLonger = -2
6+
)
7+
38
type Deprecation struct {
49
DeprecatedSince int
510
AlternativeAvailableSince int
@@ -8,90 +13,91 @@ type Deprecation struct {
813
var StdlibDeprecations = map[string]Deprecation{
914
// FIXME(dh): AllowBinary isn't being detected as deprecated
1015
// because the comment has a newline right after "Deprecated:"
11-
"go/build.AllowBinary": {7, 7},
12-
"(archive/zip.FileHeader).CompressedSize": {1, 1},
13-
"(archive/zip.FileHeader).UncompressedSize": {1, 1},
14-
"(archive/zip.FileHeader).ModifiedTime": {10, 10},
15-
"(archive/zip.FileHeader).ModifiedDate": {10, 10},
16-
"(*archive/zip.FileHeader).ModTime": {10, 10},
17-
"(*archive/zip.FileHeader).SetModTime": {10, 10},
18-
"(go/doc.Package).Bugs": {1, 1},
19-
"os.SEEK_SET": {7, 7},
20-
"os.SEEK_CUR": {7, 7},
21-
"os.SEEK_END": {7, 7},
22-
"(net.Dialer).Cancel": {7, 7},
23-
"runtime.CPUProfile": {9, 0},
24-
"compress/flate.ReadError": {6, 6},
25-
"compress/flate.WriteError": {6, 6},
26-
"path/filepath.HasPrefix": {0, 0},
27-
"(net/http.Transport).Dial": {7, 7},
28-
"(*net/http.Transport).CancelRequest": {6, 5},
29-
"net/http.ErrWriteAfterFlush": {7, 0},
30-
"net/http.ErrHeaderTooLong": {8, 0},
31-
"net/http.ErrShortBody": {8, 0},
32-
"net/http.ErrMissingContentLength": {8, 0},
33-
"net/http/httputil.ErrPersistEOF": {0, 0},
34-
"net/http/httputil.ErrClosed": {0, 0},
35-
"net/http/httputil.ErrPipeline": {0, 0},
36-
"net/http/httputil.ServerConn": {0, 0},
37-
"net/http/httputil.NewServerConn": {0, 0},
38-
"net/http/httputil.ClientConn": {0, 0},
39-
"net/http/httputil.NewClientConn": {0, 0},
40-
"net/http/httputil.NewProxyClientConn": {0, 0},
41-
"(net/http.Request).Cancel": {7, 7},
42-
"(text/template/parse.PipeNode).Line": {1, 1},
43-
"(text/template/parse.ActionNode).Line": {1, 1},
44-
"(text/template/parse.BranchNode).Line": {1, 1},
45-
"(text/template/parse.TemplateNode).Line": {1, 1},
46-
"database/sql/driver.ColumnConverter": {9, 9},
47-
"database/sql/driver.Execer": {8, 8},
48-
"database/sql/driver.Queryer": {8, 8},
49-
"(database/sql/driver.Conn).Begin": {8, 8},
50-
"(database/sql/driver.Stmt).Exec": {8, 8},
51-
"(database/sql/driver.Stmt).Query": {8, 8},
52-
"syscall.StringByteSlice": {1, 1},
53-
"syscall.StringBytePtr": {1, 1},
54-
"syscall.StringSlicePtr": {1, 1},
55-
"syscall.StringToUTF16": {1, 1},
56-
"syscall.StringToUTF16Ptr": {1, 1},
57-
"(*regexp.Regexp).Copy": {12, 12},
58-
"(archive/tar.Header).Xattrs": {10, 10},
59-
"archive/tar.TypeRegA": {11, 1},
60-
"go/types.NewInterface": {11, 11},
61-
"(*go/types.Interface).Embedded": {11, 11},
62-
"go/importer.For": {12, 12},
63-
"encoding/json.InvalidUTF8Error": {2, 2},
64-
"encoding/json.UnmarshalFieldError": {2, 2},
65-
"encoding/csv.ErrTrailingComma": {2, 2},
66-
"(encoding/csv.Reader).TrailingComma": {2, 2},
67-
"(net.Dialer).DualStack": {12, 12},
68-
"net/http.ErrUnexpectedTrailer": {12, 12},
69-
"net/http.CloseNotifier": {11, 7},
70-
"net/http.ProtocolError": {8, 8},
16+
"go/build.AllowBinary": {7, 7},
17+
"(archive/zip.FileHeader).CompressedSize": {1, 1},
18+
"(archive/zip.FileHeader).UncompressedSize": {1, 1},
19+
"(archive/zip.FileHeader).ModifiedTime": {10, 10},
20+
"(archive/zip.FileHeader).ModifiedDate": {10, 10},
21+
"(*archive/zip.FileHeader).ModTime": {10, 10},
22+
"(*archive/zip.FileHeader).SetModTime": {10, 10},
23+
"(go/doc.Package).Bugs": {1, 1},
24+
"os.SEEK_SET": {7, 7},
25+
"os.SEEK_CUR": {7, 7},
26+
"os.SEEK_END": {7, 7},
27+
"(net.Dialer).Cancel": {7, 7},
28+
"runtime.CPUProfile": {9, 0},
29+
"compress/flate.ReadError": {6, DeprecatedUseNoLonger},
30+
"compress/flate.WriteError": {6, DeprecatedUseNoLonger},
31+
"path/filepath.HasPrefix": {0, DeprecatedNeverUse},
32+
"(net/http.Transport).Dial": {7, 7},
33+
"(*net/http.Transport).CancelRequest": {6, 5},
34+
"net/http.ErrWriteAfterFlush": {7, DeprecatedUseNoLonger},
35+
"net/http.ErrHeaderTooLong": {8, DeprecatedUseNoLonger},
36+
"net/http.ErrShortBody": {8, DeprecatedUseNoLonger},
37+
"net/http.ErrMissingContentLength": {8, DeprecatedUseNoLonger},
38+
"net/http/httputil.ErrPersistEOF": {0, DeprecatedUseNoLonger},
39+
"net/http/httputil.ErrClosed": {0, DeprecatedUseNoLonger},
40+
"net/http/httputil.ErrPipeline": {0, DeprecatedUseNoLonger},
41+
"net/http/httputil.ServerConn": {0, 0},
42+
"net/http/httputil.NewServerConn": {0, 0},
43+
"net/http/httputil.ClientConn": {0, 0},
44+
"net/http/httputil.NewClientConn": {0, 0},
45+
"net/http/httputil.NewProxyClientConn": {0, 0},
46+
"(net/http.Request).Cancel": {7, 7},
47+
"(text/template/parse.PipeNode).Line": {1, DeprecatedUseNoLonger},
48+
"(text/template/parse.ActionNode).Line": {1, DeprecatedUseNoLonger},
49+
"(text/template/parse.BranchNode).Line": {1, DeprecatedUseNoLonger},
50+
"(text/template/parse.TemplateNode).Line": {1, DeprecatedUseNoLonger},
51+
"database/sql/driver.ColumnConverter": {9, 9},
52+
"database/sql/driver.Execer": {8, 8},
53+
"database/sql/driver.Queryer": {8, 8},
54+
"(database/sql/driver.Conn).Begin": {8, 8},
55+
"(database/sql/driver.Stmt).Exec": {8, 8},
56+
"(database/sql/driver.Stmt).Query": {8, 8},
57+
"syscall.StringByteSlice": {1, 1},
58+
"syscall.StringBytePtr": {1, 1},
59+
"syscall.StringSlicePtr": {1, 1},
60+
"syscall.StringToUTF16": {1, 1},
61+
"syscall.StringToUTF16Ptr": {1, 1},
62+
"(*regexp.Regexp).Copy": {12, DeprecatedUseNoLonger},
63+
"(archive/tar.Header).Xattrs": {10, 10},
64+
"archive/tar.TypeRegA": {11, 1},
65+
"go/types.NewInterface": {11, 11},
66+
"(*go/types.Interface).Embedded": {11, 11},
67+
"go/importer.For": {12, 12},
68+
"encoding/json.InvalidUTF8Error": {2, DeprecatedUseNoLonger},
69+
"encoding/json.UnmarshalFieldError": {2, DeprecatedUseNoLonger},
70+
"encoding/csv.ErrTrailingComma": {2, DeprecatedUseNoLonger},
71+
"(encoding/csv.Reader).TrailingComma": {2, DeprecatedUseNoLonger},
72+
"(net.Dialer).DualStack": {12, 12},
73+
"net/http.ErrUnexpectedTrailer": {12, DeprecatedUseNoLonger},
74+
"net/http.CloseNotifier": {11, 7},
75+
// This is hairy. The notice says "Not all errors in the http package related to protocol errors are of type ProtocolError", but doesn't that imply that
76+
"net/http.ProtocolError": {8, DeprecatedUseNoLonger},
7177
"(crypto/x509.CertificateRequest).Attributes": {5, 3},
7278

7379
// These functions have no direct alternative, but they are insecure and should no longer be used.
74-
"crypto/x509.IsEncryptedPEMBlock": {16, 0},
75-
"crypto/x509.DecryptPEMBlock": {16, 0},
76-
"crypto/x509.EncryptPEMBlock": {16, 0},
77-
"crypto/dsa": {16, 0},
80+
"crypto/x509.IsEncryptedPEMBlock": {16, DeprecatedNeverUse},
81+
"crypto/x509.DecryptPEMBlock": {16, DeprecatedNeverUse},
82+
"crypto/x509.EncryptPEMBlock": {16, DeprecatedNeverUse},
83+
"crypto/dsa": {16, DeprecatedNeverUse},
7884

7985
// This function has no alternative, but also no purpose.
80-
"(*crypto/rc4.Cipher).Reset": {12, 0},
86+
"(*crypto/rc4.Cipher).Reset": {12, DeprecatedNeverUse},
8187
"(net/http/httptest.ResponseRecorder).HeaderMap": {11, 7},
8288
"image.ZP": {13, 0},
8389
"image.ZR": {13, 0},
8490
"(*debug/gosym.LineTable).LineToPC": {2, 2},
8591
"(*debug/gosym.LineTable).PCToLine": {2, 2},
86-
"crypto/tls.VersionSSL30": {13, 0},
87-
"(crypto/tls.Config).NameToCertificate": {14, 14},
88-
"(*crypto/tls.Config).BuildNameToCertificate": {14, 14},
92+
"crypto/tls.VersionSSL30": {13, DeprecatedNeverUse},
93+
"(crypto/tls.Config).NameToCertificate": {14, DeprecatedUseNoLonger},
94+
"(*crypto/tls.Config).BuildNameToCertificate": {14, DeprecatedUseNoLonger},
8995
"(crypto/tls.Config).SessionTicketKey": {16, 5},
9096
// No alternative, no use
91-
"(crypto/tls.ConnectionState).NegotiatedProtocolIsMutual": {16, 0},
97+
"(crypto/tls.ConnectionState).NegotiatedProtocolIsMutual": {16, DeprecatedNeverUse},
9298
// No alternative, but insecure
93-
"(crypto/tls.ConnectionState).TLSUnique": {16, 0},
94-
"image/jpeg.Reader": {4, 0},
99+
"(crypto/tls.ConnectionState).TLSUnique": {16, DeprecatedNeverUse},
100+
"image/jpeg.Reader": {4, DeprecatedNeverUse},
95101

96102
// All of these have been deprecated in favour of external libraries
97103
"syscall.AttachLsf": {7, 0},

‎staticcheck/lint.go

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2929,14 +2929,37 @@ func CheckDeprecated(pass *analysis.Pass) (interface{}, error) {
29292929
return true
29302930
}
29312931
if depr, ok := deprs.Objects[obj]; ok {
2932-
// Look for the first available alternative, not the first
2933-
// version something was deprecated in. If a function was
2934-
// deprecated in Go 1.6, an alternative has been available
2935-
// already in 1.0, and we're targeting 1.2, it still
2936-
// makes sense to use the alternative from 1.0, to be
2937-
// future-proof.
2938-
minVersion := knowledge.StdlibDeprecations[code.SelectorName(pass, sel)].AlternativeAvailableSince
2939-
if !code.IsGoVersion(pass, minVersion) {
2932+
std, ok := knowledge.StdlibDeprecations[code.SelectorName(pass, sel)]
2933+
if ok {
2934+
switch std.AlternativeAvailableSince {
2935+
case knowledge.DeprecatedNeverUse:
2936+
// This should never be used, regardless of the
2937+
// targeted Go version. Examples include insecure
2938+
// cryptography or inherently broken APIs.
2939+
//
2940+
// We always want to flag these.
2941+
case knowledge.DeprecatedUseNoLonger:
2942+
// This should no longer be used. Using it with
2943+
// older Go versions might still make sense.
2944+
if !code.IsGoVersion(pass, std.DeprecatedSince) {
2945+
return true
2946+
}
2947+
default:
2948+
if std.AlternativeAvailableSince < 0 {
2949+
panic(fmt.Sprintf("unhandled case %d", std.AlternativeAvailableSince))
2950+
}
2951+
// Look for the first available alternative, not the first
2952+
// version something was deprecated in. If a function was
2953+
// deprecated in Go 1.6, an alternative has been available
2954+
// already in 1.0, and we're targeting 1.2, it still
2955+
// makes sense to use the alternative from 1.0, to be
2956+
// future-proof.
2957+
if !code.IsGoVersion(pass, std.AlternativeAvailableSince) {
2958+
return true
2959+
}
2960+
}
2961+
}
2962+
if ok && !code.IsGoVersion(pass, std.AlternativeAvailableSince) {
29402963
return true
29412964
}
29422965

@@ -2947,7 +2970,18 @@ func CheckDeprecated(pass *analysis.Pass) (interface{}, error) {
29472970
return true
29482971
}
29492972
}
2950-
report.Report(pass, sel, fmt.Sprintf("%s is deprecated: %s", report.Render(pass, sel), depr.Msg))
2973+
2974+
if ok {
2975+
if std.AlternativeAvailableSince == knowledge.DeprecatedNeverUse {
2976+
report.Report(pass, sel, fmt.Sprintf("%s has been deprecated since Go 1.%d because it shouldn't be used: %s", report.Render(pass, sel), std.DeprecatedSince, depr.Msg))
2977+
} else if std.AlternativeAvailableSince == std.DeprecatedSince || std.AlternativeAvailableSince == knowledge.DeprecatedUseNoLonger {
2978+
report.Report(pass, sel, fmt.Sprintf("%s has been deprecated since Go 1.%d: %s", report.Render(pass, sel), std.DeprecatedSince, depr.Msg))
2979+
} else {
2980+
report.Report(pass, sel, fmt.Sprintf("%s has been deprecated since Go 1.%d and an alternative has been available since Go 1.%d: %s", report.Render(pass, sel), std.DeprecatedSince, std.AlternativeAvailableSince, depr.Msg))
2981+
}
2982+
} else {
2983+
report.Report(pass, sel, fmt.Sprintf("%s is deprecated: %s", report.Render(pass, sel), depr.Msg))
2984+
}
29512985
return true
29522986
}
29532987
return true

‎staticcheck/lint_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ func TestAll(t *testing.T) {
2626
"SA1016": {{Dir: "CheckUntrappableSignal"}},
2727
"SA1017": {{Dir: "CheckUnbufferedSignalChan"}},
2828
"SA1018": {{Dir: "CheckStringsReplaceZero"}},
29-
"SA1019": {{Dir: "CheckDeprecated"}, {Dir: "CheckDeprecated_go14", Version: "1.4"}, {Dir: "CheckDeprecated_go18", Version: "1.8"}},
29+
"SA1019": {
30+
{Dir: "CheckDeprecated"},
31+
{Dir: "CheckDeprecated_go13", Version: "1.3"},
32+
{Dir: "CheckDeprecated_go14", Version: "1.4"},
33+
{Dir: "CheckDeprecated_go18", Version: "1.8"},
34+
},
3035
"SA1020": {{Dir: "CheckListenAddress"}},
3136
"SA1021": {{Dir: "CheckBytesEqualIP"}},
3237
"SA1023": {{Dir: "CheckWriterBufferModified"}},

‎staticcheck/testdata/src/CheckDeprecated_go13/CheckDeprecated.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package pkg
2+
3+
import (
4+
"crypto/x509"
5+
"net/http/httputil"
6+
"path/filepath"
7+
)
8+
9+
func fn() {
10+
filepath.HasPrefix("", "") // want `filepath.HasPrefix has been deprecated since Go 1.0 because it shouldn't be used:`
11+
_ = httputil.ErrPersistEOF // want `httputil.ErrPersistEOF has been deprecated since Go 1.0:`
12+
_ = httputil.ServerConn{} // want `httputil.ServerConn has been deprecated since Go 1.0:`
13+
_ = x509.CertificateRequest{}.Attributes // want `x509.CertificateRequest{}.Attributes has been deprecated since Go 1.5 and an alternative has been available since Go 1.3:`
14+
}

‎staticcheck/testdata/src/CheckDeprecated_go14/CheckDeprecated.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ func fn1(err error) {
1515
_ = r.Cancel // want `If a Request's Cancel field and context are both`
1616
_ = syscall.StringByteSlice("") // want `Use ByteSliceFromString instead`
1717
_ = os.SEEK_SET
18-
if err == http.ErrWriteAfterFlush { // want `ErrWriteAfterFlush is no longer`
19-
println()
20-
}
2118
var _ flate.ReadError
2219

2320
var tr *http.Transport

‎staticcheck/testdata/src/CheckDeprecated_go18/CheckDeprecated.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ func fn1(err error) {
2121
var _ flate.ReadError // want `No longer returned`
2222

2323
var tr *http.Transport
24-
tr.CancelRequest(nil) // want `CancelRequest is deprecated`
24+
tr.CancelRequest(nil) // want `CancelRequest has been deprecated`
2525

2626
var conn driver.Conn
27-
conn.Begin() // want `Begin is deprecated`
27+
conn.Begin() // want `Begin has been deprecated`
2828
}
2929

3030
// Deprecated: Don't use this.

0 commit comments

Comments
 (0)