Skip to content

Commit

Permalink
feat: configurable headers
Browse files Browse the repository at this point in the history
See: #12
  • Loading branch information
lasiar committed Nov 9, 2024
1 parent b58cea9 commit 74b78b3
Show file tree
Hide file tree
Showing 15 changed files with 335 additions and 118 deletions.
101 changes: 83 additions & 18 deletions analyzer.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package canonicalheader

import (
"flag"
"fmt"
"go/ast"
"go/types"
Expand All @@ -19,19 +20,87 @@ const (
)

//nolint:gochecknoglobals // struct is not big, can be skip.
var Analyzer = &analysis.Analyzer{
Name: "canonicalheader",
Doc: "canonicalheader checks whether net/http.Header uses canonical header",
Run: run,
Requires: []*analysis.Analyzer{inspect.Analyzer},
var Analyzer = New()

func New() *analysis.Analyzer {
c := &canonicalHeader{}

a := &analysis.Analyzer{
Name: "canonicalheader",
Doc: "canonicalheader checks whether net/http.Header uses canonical header",
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: c.run,
}

a.Flags.Init("canonicalheader", flag.ExitOnError)
a.Flags.Var(&c.exclusions, "exclusions", "comma-separated list of exclusion rules")
a.Flags.BoolVar(&c.useDefaultExclusion, "useDefaultExclusion", true, "use default exclusion rules")

return a
}

type canonicalHeader struct {
useDefaultExclusion bool
exclusions stringSet
}

type reasonReport uint8

const (
reasonReportNonCanonical reasonReport = iota + 1
reasonReportExclusion
reasonReportInitialism
)

func (r reasonReport) String() string {
switch r {
case reasonReportNonCanonical:
return "non-canonical"

case reasonReportExclusion:
return "exclusion"

case reasonReportInitialism:
return "initialism"

default:
return "unknown"
}
}

type argumenter interface {
diagnostic(canonicalHeader string) analysis.Diagnostic
diagnostic(canonicalHeader string, r reasonReport) analysis.Diagnostic
value() string
}

func run(pass *analysis.Pass) (any, error) {
type wellKnownHeader struct {
header string
reason reasonReport
}

func (c *canonicalHeader) buildExclusions() map[string]wellKnownHeader {
result := make(map[string]wellKnownHeader)

This comment has been minimized.

Copy link
@psyhatter

psyhatter Nov 11, 2024

Could be preallocated


if c.useDefaultExclusion {
for canonical, noCanonical := range initialism() {
result[canonical] = wellKnownHeader{
header: noCanonical,
reason: reasonReportInitialism,
}
}
}

for _, ex := range c.exclusions {
result[http.CanonicalHeaderKey(ex)] = wellKnownHeader{
header: ex,
reason: reasonReportExclusion,
}
}

return result
}

func (c *canonicalHeader) run(pass *analysis.Pass) (any, error) {
var headerObject types.Object
for _, object := range pass.TypesInfo.Uses {
if object.Pkg() != nil &&
Expand All @@ -52,7 +121,7 @@ func run(pass *analysis.Pass) (any, error) {
return nil, fmt.Errorf("want %T, got %T", spctor, pass.ResultOf[inspect.Analyzer])
}

wellKnownHeaders := initialism()
exclusions := c.buildExclusions()

nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
Expand Down Expand Up @@ -228,31 +297,27 @@ func run(pass *analysis.Pass) (any, error) {
}

argValue := arg.value()
headerKeyCanonical := http.CanonicalHeaderKey(argValue)
if argValue == headerKeyCanonical {
return
}

headerKeyCanonical, isWellKnown := canonicalHeaderKey(argValue, wellKnownHeaders)
if argValue == headerKeyCanonical || isWellKnown {
headerKeyCanonical, reason := canonicalHeaderKey(argValue, exclusions)
if argValue == headerKeyCanonical {
return
}

pass.Report(arg.diagnostic(headerKeyCanonical))
pass.Report(arg.diagnostic(headerKeyCanonical, reason))
})

return nil, outerErr
}

func canonicalHeaderKey(s string, m map[string]string) (string, bool) {
func canonicalHeaderKey(s string, m map[string]wellKnownHeader) (string, reasonReport) {
canonical := http.CanonicalHeaderKey(s)

wellKnown, ok := m[canonical]
if !ok {
return canonical, ok
return canonical, reasonReportNonCanonical
}

return wellKnown, ok
return wellKnown.header, wellKnown.reason
}

func isValidMethod(name string) bool {
Expand Down
62 changes: 45 additions & 17 deletions analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"github.com/stretchr/testify/require"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/analysistest"

"github.com/lasiar/canonicalheader"
Expand All @@ -17,45 +18,72 @@ const testValue = "hello_world"
func TestAnalyzer(t *testing.T) {
t.Parallel()

testCases := [...]string{
"alias",
"assigned",
"common",
"const",
"embedded",
"global",
"initialism",
"struct",
"underlying",
testCases := [...]struct {
pkg string
customAnalyzer func(t *testing.T) *analysis.Analyzer
}{
{pkg: "alias"},
{pkg: "assigned"},
{pkg: "common"},
{pkg: "const"},
{pkg: "embedded"},
{
pkg: "exclusions",
customAnalyzer: func(t *testing.T) *analysis.Analyzer {
t.Helper()

a := canonicalheader.New()
err := a.Flags.Set("exclusions", "exclusioN")
require.NoError(t, err)
return a
},
},
{pkg: "global"},
{pkg: "initialism"},
{pkg: "struct"},
{pkg: "underlying"},
}

for _, tt := range testCases {
tt := tt
t.Run(tt, func(t *testing.T) {

t.Run(tt.pkg, func(t *testing.T) {
t.Parallel()

var a *analysis.Analyzer
if tt.customAnalyzer != nil {
a = tt.customAnalyzer(t)
} else {
a = canonicalheader.New()
}

analysistest.RunWithSuggestedFixes(
t,
analysistest.TestData(),
canonicalheader.Analyzer,
tt,
a,
tt.pkg,
)
})
}

t.Run("are_test_cases_complete", func(t *testing.T) {
t.Parallel()

dirs, err := os.ReadDir(filepath.Join(analysistest.TestData(), "src"))
want, err := os.ReadDir(filepath.Join(analysistest.TestData(), "src"))
require.NoError(t, err)
require.Len(t, testCases, len(dirs))
require.Len(t, testCases, len(want))

got := [len(testCases)]string{}
for i, testCase := range testCases {
got[i] = testCase.pkg
}

require.EqualValues(
t,
transform(dirs, func(d os.DirEntry) string {
transform(want, func(d os.DirEntry) string {
return d.Name()
}),
testCases,
got,
)
})
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/canonicalheader/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ import (
)

func main() {
singlechecker.Main(canonicalheader.Analyzer)
singlechecker.Main(canonicalheader.New())
}
14 changes: 8 additions & 6 deletions cmd/initialismer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,26 +367,28 @@ func initialism() map[string]string {
}
`

const tmplTest = `// Code generated by initialismer; DO NOT EDIT.
//nolint:gochecknoglobals // maybe use file?
var tmplTest = fmt.Sprintf(`// Code generated by initialismer; DO NOT EDIT.
package initialism
import "net/http"
func _() {
h := http.Header{}
{{range .}}
h.Get("{{.Original}}"){{end}}
h.Get("{{.Canonical}}") // want %[1]sinitialism header "{{.Canonical}}", instead use: "{{.Original}}"%[1]s{{end}}
}
`
`, "`")

const tmplTestGolden = `// Code generated by initialismer; DO NOT EDIT.
//nolint:gochecknoglobals // maybe use file?
var tmplTestGolden = fmt.Sprintf(`// Code generated by initialismer; DO NOT EDIT.
package initialism
import "net/http"
func _() {
h := http.Header{}
{{range .}}
h.Get("{{.Original}}"){{end}}
h.Get("{{.Original}}") // want %[1]sinitialism header "{{.Canonical}}", instead use: "{{.Original}}"%[1]s{{end}}
}
`
`, "`")
28 changes: 28 additions & 0 deletions configure.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package canonicalheader

import (
"errors"
"strings"
)

// stringSet is a set-of-nonempty-strings-valued flag.
type stringSet []string

func (s *stringSet) String() string {
return strings.Join(*s, ",")
}

func (s *stringSet) Set(flag string) error {
list := strings.Split(flag, ",")

*s = make(stringSet, 0, len(list))

for _, element := range list {
if element == "" {
return errors.New("empty string")

This comment has been minimized.

Copy link
@psyhatter

psyhatter Nov 11, 2024

Maybe it's worth simply ignoring such an element?

}

*s = append(*s, element)
}
return nil
}
5 changes: 3 additions & 2 deletions constant_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ func newConstantKey(info *types.Info, ident *ast.Ident) (constantString, error)
}, nil
}

func (c constantString) diagnostic(canonicalHeader string) analysis.Diagnostic {
func (c constantString) diagnostic(canonicalHeader string, r reasonReport) analysis.Diagnostic {
return analysis.Diagnostic{
Pos: c.pos,
End: c.end,
Message: fmt.Sprintf(
"const %q used as a key at http.Header, but %q is not canonical, want %q",
"const %q used as a key at http.Header, but %q is %s, want %q",
c.nameOfConst,
c.originalValue,
r,
canonicalHeader,
),
}
Expand Down
4 changes: 2 additions & 2 deletions literal_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func newLiteralString(basicList *ast.BasicLit) (literalString, error) {
}, nil
}

func (l literalString) diagnostic(canonicalHeader string) analysis.Diagnostic {
func (l literalString) diagnostic(canonicalHeader string, r reasonReport) analysis.Diagnostic {
newText := make([]byte, 0, len(canonicalHeader)+2)
newText = append(newText, l.quote)
newText = append(newText, unsafe.Slice(unsafe.StringData(canonicalHeader), len(canonicalHeader))...)
Expand All @@ -59,7 +59,7 @@ func (l literalString) diagnostic(canonicalHeader string) analysis.Diagnostic {
return analysis.Diagnostic{
Pos: l.pos,
End: l.end,
Message: fmt.Sprintf("non-canonical header %q, instead use: %q", l.originalValue, canonicalHeader),
Message: fmt.Sprintf("%s header %q, instead use: %q", r, l.originalValue, canonicalHeader),
SuggestedFixes: []analysis.SuggestedFix{
{
Message: fmt.Sprintf("should replace %q with %q", l.originalValue, canonicalHeader),
Expand Down
3 changes: 2 additions & 1 deletion makefile
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ linter:
generate:
go run ./cmd/initialismer/*.go -target="mapping" > ./initialism.go
go run ./cmd/initialismer/*.go -target="test" > ./testdata/src/initialism/initialism.go
gofmt -w ./initialism.go ./testdata/src/initialism/initialism.go
go run ./cmd/initialismer/*.go -target="test-golden" > ./testdata/src/initialism/initialism.go.golden
gofmt -w ./initialism.go ./testdata/src/initialism/initialism.go ./testdata/src/initialism/initialism.go.golden
2 changes: 1 addition & 1 deletion testdata/src/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const testHeader = "testHeader"

func p() {
v := http.Header{}
v.Get(testHeader) // want `const "testHeader" used as a key at http.Header, but "testHeader" is not canonical, want "Testheader"`
v.Get(testHeader) // want `const "testHeader" used as a key at http.Header, but "testHeader" is non-canonical, want "Testheader"`

This comment has been minimized.

Copy link
@psyhatter

psyhatter Nov 11, 2024

I am not 100% sure, but in sounds more correct
const "testHeader" used as a key in http.Header, but "testHeader" is non-canonical, want "Testheader"


v.Get("Test-HEader") // want `non-canonical header "Test-HEader", instead use: "Test-Header"`
v.Set("Test-HEader", "value") // want `non-canonical header "Test-HEader", instead use: "Test-Header"`
Expand Down
2 changes: 1 addition & 1 deletion testdata/src/common/common.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const testHeader = "testHeader"

func p() {
v := http.Header{}
v.Get(testHeader) // want `const "testHeader" used as a key at http.Header, but "testHeader" is not canonical, want "Testheader"`
v.Get(testHeader) // want `const "testHeader" used as a key at http.Header, but "testHeader" is non-canonical, want "Testheader"`

v.Get("Test-Header") // want `non-canonical header "Test-HEader", instead use: "Test-Header"`
v.Set("Test-Header", "value") // want `non-canonical header "Test-HEader", instead use: "Test-Header"`
Expand Down
8 changes: 4 additions & 4 deletions testdata/src/const/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ func _() {
var mstr myString = "Tt"
http.Header{}.Get(string(mstr))

http.Header{}.Get(string(underlyingString)) // want `const "underlyingString" used as a key at http.Header, but "TT" is not canonical, want "Tt"`
http.Header{}.Get(string(underlyingString)) // want `const "underlyingString" used as a key at http.Header, but "TT" is not canonical, want "Tt"`
http.Header{}.Get(noCanonical) // want `const "noCanonical" used as a key at http.Header, but "TT" is not canonical, want "Tt"`
http.Header{}.Get(copiedFromNoCanonical) // want `const "copiedFromNoCanonical" used as a key at http.Header, but "TT" is not canonical, want "Tt"`
http.Header{}.Get(string(underlyingString)) // want `const "underlyingString" used as a key at http.Header, but "TT" is non-canonical, want "Tt"`
http.Header{}.Get(string(underlyingString)) // want `const "underlyingString" used as a key at http.Header, but "TT" is non-canonical, want "Tt"`
http.Header{}.Get(noCanonical) // want `const "noCanonical" used as a key at http.Header, but "TT" is non-canonical, want "Tt"`
http.Header{}.Get(copiedFromNoCanonical) // want `const "copiedFromNoCanonical" used as a key at http.Header, but "TT" is non-canonical, want "Tt"`
http.Header{}.Get(canonical)
}
Loading

0 comments on commit 74b78b3

Please sign in to comment.