diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index d3781c9..27a9f30 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -20,6 +20,7 @@ const ( CryptoHashFlag = "crypto-hash" HTTPMethodFlag = "http-method" HTTPStatusCodeFlag = "http-status-code" + HTTPNoBodyFlag = "http-no-body" DefaultRPCPathFlag = "default-rpc-path" ) @@ -38,6 +39,7 @@ func flags() flag.FlagSet { flags := flag.NewFlagSet("", flag.ExitOnError) flags.Bool(HTTPMethodFlag, true, "suggest the use of http.MethodXX") flags.Bool(HTTPStatusCodeFlag, true, "suggest the use of http.StatusXX") + flags.Bool(HTTPNoBodyFlag, false, "suggest the use of http.NoBody") flags.Bool(TimeWeekdayFlag, false, "suggest the use of time.Weekday") flags.Bool(TimeMonthFlag, false, "suggest the use of time.Month") flags.Bool(TimeLayoutFlag, false, "suggest the use of time.Layout") @@ -74,21 +76,29 @@ func run(pass *analysis.Pass) (interface{}, error) { } case "NewRequest": - if !lookupFlag(pass, HTTPMethodFlag) { - return + if lookupFlag(pass, HTTPMethodFlag) { + if basicLit := getBasicLitFromArgs(n.Args, 3, 0, token.STRING); basicLit != nil { + checkHTTPMethod(pass, basicLit) + } } - if basicLit := getBasicLitFromArgs(n.Args, 3, 0, token.STRING); basicLit != nil { - checkHTTPMethod(pass, basicLit) + if lookupFlag(pass, HTTPNoBodyFlag) { + if ident := getIdentFromArgs(n.Args, 3, 2); ident != nil { + checkHTTPNoBody(pass, ident) + } } case "NewRequestWithContext": - if !lookupFlag(pass, HTTPMethodFlag) { - return + if lookupFlag(pass, HTTPMethodFlag) { + if basicLit := getBasicLitFromArgs(n.Args, 4, 1, token.STRING); basicLit != nil { + checkHTTPMethod(pass, basicLit) + } } - if basicLit := getBasicLitFromArgs(n.Args, 4, 1, token.STRING); basicLit != nil { - checkHTTPMethod(pass, basicLit) + if lookupFlag(pass, HTTPNoBodyFlag) { + if ident := getIdentFromArgs(n.Args, 4, 3); ident != nil { + checkHTTPNoBody(pass, ident) + } } } @@ -173,6 +183,14 @@ func checkHTTPStatusCode(pass *analysis.Pass, basicLit *ast.BasicLit) { } } +func checkHTTPNoBody(pass *analysis.Pass, ident *ast.Ident) { + currentVal := ident.Name + + if newVal, ok := mapping.HTTPNoBody[currentVal]; ok { + report(pass, ident.Pos(), currentVal, newVal) + } +} + func checkTimeWeekday(pass *analysis.Pass, pos token.Pos, currentVal string) { if newVal, ok := mapping.TimeWeekday[currentVal]; ok { report(pass, pos, currentVal, newVal) @@ -226,6 +244,24 @@ func getBasicLitFromArgs(args []ast.Expr, count, idx int, typ token.Token) *ast. return basicLit } +// getIdentFromArgs gets the *ast.Ident of a function argument. +// +// Arguments: +// - count - expected number of argument in function +// - idx - index of the argument to get the *ast.Ident +func getIdentFromArgs(args []ast.Expr, count, idx int) *ast.Ident { + if len(args) != count { + return nil + } + + ident, ok := args[idx].(*ast.Ident) + if !ok { + return nil + } + + return ident +} + // getBasicLitFromElts gets the *ast.BasicLit of a struct elements. // // Arguments: diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 8f13385..cc9f3dc 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -33,6 +33,9 @@ func TestUseStdlibVars(t *testing.T) { if err := a.Flags.Set(analyzer.DefaultRPCPathFlag, "true"); err != nil { t.Error(err) } + if err := a.Flags.Set(analyzer.HTTPNoBodyFlag, "true"); err != nil { + t.Error(err) + } analysistest.Run(t, analysistest.TestData(), a, pkgs...) } diff --git a/pkg/analyzer/internal/gen.go b/pkg/analyzer/internal/gen.go index 45d5117..961de79 100644 --- a/pkg/analyzer/internal/gen.go +++ b/pkg/analyzer/internal/gen.go @@ -83,6 +83,12 @@ func main() { templateName: "test-issue32.go.tmpl", fileName: "pkg/analyzer/testdata/src/a/http/issue32.go", }, + { + mapping: mapping.HTTPNoBody, + packageName: "http_test", + templateName: "test-httpnobody.go.tmpl", + fileName: "pkg/analyzer/testdata/src/a/http/nobody.go", + }, } for _, operation := range operations { diff --git a/pkg/analyzer/internal/mapping/mapping.go b/pkg/analyzer/internal/mapping/mapping.go index 56fae1b..7f53c35 100644 --- a/pkg/analyzer/internal/mapping/mapping.go +++ b/pkg/analyzer/internal/mapping/mapping.go @@ -159,3 +159,7 @@ var TimeLayout = map[string]string{ time.StampMicro: "time.StampMicro", time.StampNano: "time.StampNano", } + +var HTTPNoBody = map[string]string{ + "nil": "http.NoBody", +} diff --git a/pkg/analyzer/internal/template/test-httpmethod.go.tmpl b/pkg/analyzer/internal/template/test-httpmethod.go.tmpl index f2229d2..d039f38 100644 --- a/pkg/analyzer/internal/template/test-httpmethod.go.tmpl +++ b/pkg/analyzer/internal/template/test-httpmethod.go.tmpl @@ -18,13 +18,13 @@ const ( func _() { {{- range $key, $value := .Mapping }} - _, _ = http.NewRequest("{{ $key }}", "", nil) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` + _, _ = http.NewRequest("{{ $key }}", "", http.NoBody) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` {{- end }} } func _() { {{- range $key, $value := .Mapping }} - _, _ = http.NewRequestWithContext(nil, "{{ $key }}", "", nil) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` + _, _ = http.NewRequestWithContext(nil, "{{ $key }}", "", http.NoBody) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` {{- end }} } diff --git a/pkg/analyzer/internal/template/test-httpnobody.go.tmpl b/pkg/analyzer/internal/template/test-httpnobody.go.tmpl new file mode 100644 index 0000000..c0b2843 --- /dev/null +++ b/pkg/analyzer/internal/template/test-httpnobody.go.tmpl @@ -0,0 +1,17 @@ +// Code generated by usestdlibvars, DO NOT EDIT. + +package {{ .PackageName }} + +import "net/http" + +func _() { +{{- range $key, $value := .Mapping }} + _, _ = http.NewRequest(http.MethodGet, "", {{ $key }}) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` +{{- end }} +} + +func _() { +{{- range $key, $value := .Mapping }} + _, _ = http.NewRequestWithContext(nil, http.MethodGet, "", {{ $key }}) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` +{{- end }} +} diff --git a/pkg/analyzer/internal/template/test-issue32.go.tmpl b/pkg/analyzer/internal/template/test-issue32.go.tmpl index d1ea87a..b195e84 100644 --- a/pkg/analyzer/internal/template/test-issue32.go.tmpl +++ b/pkg/analyzer/internal/template/test-issue32.go.tmpl @@ -18,13 +18,13 @@ const ( func _() { {{- range $key, $value := .Mapping }} - _, _ = http.NewRequest("{{ lower $key }}", "", nil) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` + _, _ = http.NewRequest("{{ lower $key }}", "", http.NoBody) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` {{- end }} } func _() { {{- range $key, $value := .Mapping }} - _, _ = http.NewRequestWithContext(nil, "{{ lower $key }}", "", nil) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` + _, _ = http.NewRequestWithContext(nil, "{{ lower $key }}", "", http.NoBody) // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}` {{- end }} } diff --git a/pkg/analyzer/testdata/src/a/http/issue32.go b/pkg/analyzer/testdata/src/a/http/issue32.go index f3aa091..8046415 100755 --- a/pkg/analyzer/testdata/src/a/http/issue32.go +++ b/pkg/analyzer/testdata/src/a/http/issue32.go @@ -29,27 +29,27 @@ const ( ) func _() { - _, _ = http.NewRequest("connect", "", nil) // want `"CONNECT" can be replaced by http\.MethodConnect` - _, _ = http.NewRequest("delete", "", nil) // want `"DELETE" can be replaced by http\.MethodDelete` - _, _ = http.NewRequest("get", "", nil) // want `"GET" can be replaced by http\.MethodGet` - _, _ = http.NewRequest("head", "", nil) // want `"HEAD" can be replaced by http\.MethodHead` - _, _ = http.NewRequest("options", "", nil) // want `"OPTIONS" can be replaced by http\.MethodOptions` - _, _ = http.NewRequest("patch", "", nil) // want `"PATCH" can be replaced by http\.MethodPatch` - _, _ = http.NewRequest("post", "", nil) // want `"POST" can be replaced by http\.MethodPost` - _, _ = http.NewRequest("put", "", nil) // want `"PUT" can be replaced by http\.MethodPut` - _, _ = http.NewRequest("trace", "", nil) // want `"TRACE" can be replaced by http\.MethodTrace` + _, _ = http.NewRequest("connect", "", http.NoBody) // want `"CONNECT" can be replaced by http\.MethodConnect` + _, _ = http.NewRequest("delete", "", http.NoBody) // want `"DELETE" can be replaced by http\.MethodDelete` + _, _ = http.NewRequest("get", "", http.NoBody) // want `"GET" can be replaced by http\.MethodGet` + _, _ = http.NewRequest("head", "", http.NoBody) // want `"HEAD" can be replaced by http\.MethodHead` + _, _ = http.NewRequest("options", "", http.NoBody) // want `"OPTIONS" can be replaced by http\.MethodOptions` + _, _ = http.NewRequest("patch", "", http.NoBody) // want `"PATCH" can be replaced by http\.MethodPatch` + _, _ = http.NewRequest("post", "", http.NoBody) // want `"POST" can be replaced by http\.MethodPost` + _, _ = http.NewRequest("put", "", http.NoBody) // want `"PUT" can be replaced by http\.MethodPut` + _, _ = http.NewRequest("trace", "", http.NoBody) // want `"TRACE" can be replaced by http\.MethodTrace` } func _() { - _, _ = http.NewRequestWithContext(nil, "connect", "", nil) // want `"CONNECT" can be replaced by http\.MethodConnect` - _, _ = http.NewRequestWithContext(nil, "delete", "", nil) // want `"DELETE" can be replaced by http\.MethodDelete` - _, _ = http.NewRequestWithContext(nil, "get", "", nil) // want `"GET" can be replaced by http\.MethodGet` - _, _ = http.NewRequestWithContext(nil, "head", "", nil) // want `"HEAD" can be replaced by http\.MethodHead` - _, _ = http.NewRequestWithContext(nil, "options", "", nil) // want `"OPTIONS" can be replaced by http\.MethodOptions` - _, _ = http.NewRequestWithContext(nil, "patch", "", nil) // want `"PATCH" can be replaced by http\.MethodPatch` - _, _ = http.NewRequestWithContext(nil, "post", "", nil) // want `"POST" can be replaced by http\.MethodPost` - _, _ = http.NewRequestWithContext(nil, "put", "", nil) // want `"PUT" can be replaced by http\.MethodPut` - _, _ = http.NewRequestWithContext(nil, "trace", "", nil) // want `"TRACE" can be replaced by http\.MethodTrace` + _, _ = http.NewRequestWithContext(nil, "connect", "", http.NoBody) // want `"CONNECT" can be replaced by http\.MethodConnect` + _, _ = http.NewRequestWithContext(nil, "delete", "", http.NoBody) // want `"DELETE" can be replaced by http\.MethodDelete` + _, _ = http.NewRequestWithContext(nil, "get", "", http.NoBody) // want `"GET" can be replaced by http\.MethodGet` + _, _ = http.NewRequestWithContext(nil, "head", "", http.NoBody) // want `"HEAD" can be replaced by http\.MethodHead` + _, _ = http.NewRequestWithContext(nil, "options", "", http.NoBody) // want `"OPTIONS" can be replaced by http\.MethodOptions` + _, _ = http.NewRequestWithContext(nil, "patch", "", http.NoBody) // want `"PATCH" can be replaced by http\.MethodPatch` + _, _ = http.NewRequestWithContext(nil, "post", "", http.NoBody) // want `"POST" can be replaced by http\.MethodPost` + _, _ = http.NewRequestWithContext(nil, "put", "", http.NoBody) // want `"PUT" can be replaced by http\.MethodPut` + _, _ = http.NewRequestWithContext(nil, "trace", "", http.NoBody) // want `"TRACE" can be replaced by http\.MethodTrace` } func _() { diff --git a/pkg/analyzer/testdata/src/a/http/method.go b/pkg/analyzer/testdata/src/a/http/method.go index b472bf4..4579cab 100644 --- a/pkg/analyzer/testdata/src/a/http/method.go +++ b/pkg/analyzer/testdata/src/a/http/method.go @@ -29,27 +29,27 @@ const ( ) func _() { - _, _ = http.NewRequest("CONNECT", "", nil) // want `"CONNECT" can be replaced by http\.MethodConnect` - _, _ = http.NewRequest("DELETE", "", nil) // want `"DELETE" can be replaced by http\.MethodDelete` - _, _ = http.NewRequest("GET", "", nil) // want `"GET" can be replaced by http\.MethodGet` - _, _ = http.NewRequest("HEAD", "", nil) // want `"HEAD" can be replaced by http\.MethodHead` - _, _ = http.NewRequest("OPTIONS", "", nil) // want `"OPTIONS" can be replaced by http\.MethodOptions` - _, _ = http.NewRequest("PATCH", "", nil) // want `"PATCH" can be replaced by http\.MethodPatch` - _, _ = http.NewRequest("POST", "", nil) // want `"POST" can be replaced by http\.MethodPost` - _, _ = http.NewRequest("PUT", "", nil) // want `"PUT" can be replaced by http\.MethodPut` - _, _ = http.NewRequest("TRACE", "", nil) // want `"TRACE" can be replaced by http\.MethodTrace` + _, _ = http.NewRequest("CONNECT", "", http.NoBody) // want `"CONNECT" can be replaced by http\.MethodConnect` + _, _ = http.NewRequest("DELETE", "", http.NoBody) // want `"DELETE" can be replaced by http\.MethodDelete` + _, _ = http.NewRequest("GET", "", http.NoBody) // want `"GET" can be replaced by http\.MethodGet` + _, _ = http.NewRequest("HEAD", "", http.NoBody) // want `"HEAD" can be replaced by http\.MethodHead` + _, _ = http.NewRequest("OPTIONS", "", http.NoBody) // want `"OPTIONS" can be replaced by http\.MethodOptions` + _, _ = http.NewRequest("PATCH", "", http.NoBody) // want `"PATCH" can be replaced by http\.MethodPatch` + _, _ = http.NewRequest("POST", "", http.NoBody) // want `"POST" can be replaced by http\.MethodPost` + _, _ = http.NewRequest("PUT", "", http.NoBody) // want `"PUT" can be replaced by http\.MethodPut` + _, _ = http.NewRequest("TRACE", "", http.NoBody) // want `"TRACE" can be replaced by http\.MethodTrace` } func _() { - _, _ = http.NewRequestWithContext(nil, "CONNECT", "", nil) // want `"CONNECT" can be replaced by http\.MethodConnect` - _, _ = http.NewRequestWithContext(nil, "DELETE", "", nil) // want `"DELETE" can be replaced by http\.MethodDelete` - _, _ = http.NewRequestWithContext(nil, "GET", "", nil) // want `"GET" can be replaced by http\.MethodGet` - _, _ = http.NewRequestWithContext(nil, "HEAD", "", nil) // want `"HEAD" can be replaced by http\.MethodHead` - _, _ = http.NewRequestWithContext(nil, "OPTIONS", "", nil) // want `"OPTIONS" can be replaced by http\.MethodOptions` - _, _ = http.NewRequestWithContext(nil, "PATCH", "", nil) // want `"PATCH" can be replaced by http\.MethodPatch` - _, _ = http.NewRequestWithContext(nil, "POST", "", nil) // want `"POST" can be replaced by http\.MethodPost` - _, _ = http.NewRequestWithContext(nil, "PUT", "", nil) // want `"PUT" can be replaced by http\.MethodPut` - _, _ = http.NewRequestWithContext(nil, "TRACE", "", nil) // want `"TRACE" can be replaced by http\.MethodTrace` + _, _ = http.NewRequestWithContext(nil, "CONNECT", "", http.NoBody) // want `"CONNECT" can be replaced by http\.MethodConnect` + _, _ = http.NewRequestWithContext(nil, "DELETE", "", http.NoBody) // want `"DELETE" can be replaced by http\.MethodDelete` + _, _ = http.NewRequestWithContext(nil, "GET", "", http.NoBody) // want `"GET" can be replaced by http\.MethodGet` + _, _ = http.NewRequestWithContext(nil, "HEAD", "", http.NoBody) // want `"HEAD" can be replaced by http\.MethodHead` + _, _ = http.NewRequestWithContext(nil, "OPTIONS", "", http.NoBody) // want `"OPTIONS" can be replaced by http\.MethodOptions` + _, _ = http.NewRequestWithContext(nil, "PATCH", "", http.NoBody) // want `"PATCH" can be replaced by http\.MethodPatch` + _, _ = http.NewRequestWithContext(nil, "POST", "", http.NoBody) // want `"POST" can be replaced by http\.MethodPost` + _, _ = http.NewRequestWithContext(nil, "PUT", "", http.NoBody) // want `"PUT" can be replaced by http\.MethodPut` + _, _ = http.NewRequestWithContext(nil, "TRACE", "", http.NoBody) // want `"TRACE" can be replaced by http\.MethodTrace` } func _() { diff --git a/pkg/analyzer/testdata/src/a/http/nobody.go b/pkg/analyzer/testdata/src/a/http/nobody.go new file mode 100755 index 0000000..9d3a43f --- /dev/null +++ b/pkg/analyzer/testdata/src/a/http/nobody.go @@ -0,0 +1,13 @@ +// Code generated by usestdlibvars, DO NOT EDIT. + +package http_test + +import "net/http" + +func _() { + _, _ = http.NewRequest(http.MethodGet, "", nil) // want `"nil" can be replaced by http\.NoBody` +} + +func _() { + _, _ = http.NewRequestWithContext(nil, http.MethodGet, "", nil) // want `"nil" can be replaced by http\.NoBody` +}