From ded6a10f7b78f667302ade3706336c8d0bd74bf4 Mon Sep 17 00:00:00 2001 From: Angus Dippenaar Date: Wed, 5 Oct 2022 22:57:21 +0200 Subject: [PATCH] cleanup commented code, golangci-lint fixes --- endtoend_test.go | 4 +- go.mod | 1 + go.sum | 1 + golden_test.go | 24 ++++-- stringer.go | 203 +++++++++++++++++------------------------------ 5 files changed, 95 insertions(+), 138 deletions(-) diff --git a/endtoend_test.go b/endtoend_test.go index 203ecd6..fa65968 100644 --- a/endtoend_test.go +++ b/endtoend_test.go @@ -4,6 +4,7 @@ // go command is not available on android +//go:build !android // +build !android package main @@ -12,7 +13,6 @@ import ( "fmt" "go/build" "io" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -40,7 +40,7 @@ func init() { // binary panics if the String method for X is not correct, including for error cases. func TestEndToEnd(t *testing.T) { - dir, err := ioutil.TempDir("", "stringer") + dir, err := os.MkdirTemp("", "stringer") if err != nil { t.Fatal(err) } diff --git a/go.mod b/go.mod index b5924ed..491f7c3 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,7 @@ module github.com/dmarkham/enumer require ( github.com/pascaldekloe/name v1.0.0 + golang.org/x/text v0.3.7 golang.org/x/tools v0.1.12 ) diff --git a/go.sum b/go.sum index da8eb3f..77d7204 100644 --- a/go.sum +++ b/go.sum @@ -20,6 +20,7 @@ golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9sn golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= diff --git a/golden_test.go b/golden_test.go index 03479d2..52c688c 100644 --- a/golden_test.go +++ b/golden_test.go @@ -10,7 +10,7 @@ package main import ( - "io/ioutil" + "io" "os" "path/filepath" "strings" @@ -360,7 +360,7 @@ func runGoldenTest(t *testing.T, test Golden, file := test.name + ".go" input := "package test\n" + test.input - dir, err := ioutil.TempDir("", "stringer") + dir, err := os.MkdirTemp("", "stringer") if err != nil { t.Error(err) } @@ -372,7 +372,7 @@ func runGoldenTest(t *testing.T, test Golden, }() absFile := filepath.Join(dir, file) - err = ioutil.WriteFile(absFile, []byte(input), 0644) + err = os.WriteFile(absFile, []byte(input), 0644) if err != nil { t.Error(err) } @@ -382,12 +382,24 @@ func runGoldenTest(t *testing.T, test Golden, if len(tokens) != 3 { t.Fatalf("%s: need type declaration on first line", test.name) } - g.generate(tokens[1], generateJSON, generateYAML, generateSQL, generateText, generateGQLGen, "noop", trimPrefix, prefix, linecomment, generateValuesMethod) + g.generate(generateParams{ + AddPrefix: prefix, + IncludeGQLGen: generateGQLGen, + IncludeJSON: generateJSON, + IncludeSQL: generateSQL, + IncludeText: generateText, + IncludeValuesMethod: generateValuesMethod, + IncludeYAML: generateYAML, + LineComment: linecomment, + TransformMethod: "noop", + TrimPrefix: trimPrefix, + TypeName: tokens[1], + }) got := string(g.format()) if got != loadGolden(test.name) { // Use this to help build a golden text when changes are needed //goldenFile := fmt.Sprintf("./testdata/%v.golden", test.name) - //err = ioutil.WriteFile(goldenFile, []byte(got), 0644) + //err = os.WriteFile(goldenFile, []byte(got), 0644) //if err != nil { // t.Error(err) //} @@ -401,7 +413,7 @@ func loadGolden(name string) string { return "" } defer fh.Close() - b, err := ioutil.ReadAll(fh) + b, err := io.ReadAll(fh) if err != nil { return "" } diff --git a/stringer.go b/stringer.go index 49c87fd..6fb8b24 100644 --- a/stringer.go +++ b/stringer.go @@ -15,10 +15,8 @@ import ( "go/ast" exact "go/constant" "go/format" - "go/importer" "go/token" "go/types" - "io/ioutil" "log" "os" "path/filepath" @@ -27,6 +25,8 @@ import ( "unicode" "unicode/utf8" + "golang.org/x/text/cases" + "golang.org/x/text/language" "golang.org/x/tools/go/packages" "github.com/pascaldekloe/name" @@ -102,10 +102,8 @@ func main() { if len(args) == 1 && isDirectory(args[0]) { dir = args[0] - // g.parsePackageDir(args[0]) } else { dir = filepath.Dir(args[0]) - // g.parsePackageFiles(args) } g.parsePackage(args, []string{}) @@ -135,7 +133,19 @@ func main() { // Run generate for each type. for _, typeName := range typs { - g.generate(typeName, *json, *yaml, *sql, *text, *gqlgen, *transformMethod, *trimPrefix, *addPrefix, *linecomment, *altValuesFunc) + g.generate(generateParams{ + AddPrefix: *addPrefix, + IncludeGQLGen: *gqlgen, + IncludeJSON: *json, + IncludeSQL: *sql, + IncludeText: *text, + IncludeValuesMethod: *altValuesFunc, + IncludeYAML: *yaml, + LineComment: *linecomment, + TransformMethod: *transformMethod, + TrimPrefix: *trimPrefix, + TypeName: typeName, + }) } // Format the output. @@ -149,7 +159,7 @@ func main() { } // Write to tmpfile first - tmpFile, err := ioutil.TempFile(dir, fmt.Sprintf("%s_enumer_", typs[0])) + tmpFile, err := os.CreateTemp(dir, fmt.Sprintf("%s_enumer_", typs[0])) if err != nil { log.Fatalf("creating temporary file for output: %s", err) } @@ -196,58 +206,24 @@ type File struct { // These fields are reset for each type being generated. typeName string // Name of the constant type. values []Value // Accumulator for constant values of that type. - trimPrefix string lineComment bool } // Package holds information about a Go package type Package struct { - dir string - name string - defs map[*ast.Ident]types.Object - files []*File - typesPkg *types.Package + name string + defs map[*ast.Ident]types.Object + files []*File } -// // parsePackageDir parses the package residing in the directory. -// func (g *Generator) parsePackageDir(directory string) { -// pkg, err := build.Default.ImportDir(directory, 0) -// if err != nil { -// log.Fatalf("cannot process directory %s: %s", directory, err) -// } -// var names []string -// names = append(names, pkg.GoFiles...) -// names = append(names, pkg.CgoFiles...) -// // TODO: Need to think about constants in test files. Maybe write type_string_test.go -// // in a separate pass? For later. -// // names = append(names, pkg.TestGoFiles...) // These are also in the "foo" package. -// names = append(names, pkg.SFiles...) -// names = prefixDirectory(directory, names) -// g.parsePackage(directory, names, nil) -// } -// -// // parsePackageFiles parses the package occupying the named files. -// func (g *Generator) parsePackageFiles(names []string) { -// g.parsePackage(".", names, nil) -// } - -// // prefixDirectory places the directory name on the beginning of each name in the list. -// func prefixDirectory(directory string, names []string) []string { -// if directory == "." { -// return names -// } -// ret := make([]string, len(names)) -// for i, n := range names { -// ret[i] = filepath.Join(directory, n) -// } -// return ret -// } - // parsePackage analyzes the single package constructed from the patterns and tags. // parsePackage exits if there is an error. func (g *Generator) parsePackage(patterns []string, tags []string) { cfg := &packages.Config{ - Mode: packages.LoadSyntax, + Mode: packages.NeedName | + packages.NeedTypes | + packages.NeedSyntax | + packages.NeedTypesInfo, // TODO: Need to think about constants in test files. Maybe write type_string_test.go // in a separate pass? For later. Tests: false, @@ -278,52 +254,6 @@ func (g *Generator) addPackage(pkg *packages.Package) { } } -// parsePackage analyzes the single package constructed from the named files. -// If text is non-nil, it is a string to be used instead of the content of the file, -// to be used for testing. parsePackage exits if there is an error. -// func (g *Generator) parsePackagee(directory string, names []string, text interface{}) { -// var files []*File -// var astFiles []*ast.File -// g.pkg = new(Package) -// fs := token.NewFileSet() -// for _, n := range names { -// if !strings.HasSuffix(n, ".go") { -// continue -// } -// parsedFile, err := parser.ParseFile(fs, n, text, 0) -// if err != nil { -// log.Fatalf("parsing package: %s: %s", n, err) -// } -// astFiles = append(astFiles, parsedFile) -// files = append(files, &File{ -// file: parsedFile, -// pkg: g.pkg, -// }) -// } -// if len(astFiles) == 0 { -// log.Fatalf("%s: no buildable Go files", directory) -// } -// g.pkg.name = astFiles[0].Name.Name -// g.pkg.files = files -// g.pkg.dir = directory -// // Type check the package. -// g.pkg.check(fs, astFiles) -// } - -// check type-checks the package. The package must be OK to proceed. -func (pkg *Package) check(fs *token.FileSet, astFiles []*ast.File) { - pkg.defs = make(map[*ast.Ident]types.Object) - config := types.Config{Importer: importer.Default(), FakeImportC: true} - info := &types.Info{ - Defs: pkg.defs, - } - typesPkg, err := config.Check(pkg.dir, fs, astFiles, info) - if err != nil { - log.Fatalf("checking package: %s", err) - } - pkg.typesPkg = typesPkg -} - func (g *Generator) transformValueNames(values []Value, transformMethod string) { var fn func(src string) string switch transformMethod { @@ -353,11 +283,11 @@ func (g *Generator) transformValueNames(values []Value, transformMethod string) } case "title": fn = func(s string) string { - return strings.Title(s) + return cases.Title(language.English).String(s) } case "title-lower": fn = func(s string) string { - title := []rune(strings.Title(s)) + title := []rune(cases.Title(language.English).String(s)) title[0] = unicode.ToLower(title[0]) return string(title) } @@ -412,15 +342,27 @@ func (g *Generator) prefixValueNames(values []Value, prefix string) { } } +type generateParams struct { + AddPrefix string + IncludeGQLGen bool + IncludeJSON bool + IncludeSQL bool + IncludeText bool + IncludeValuesMethod bool + IncludeYAML bool + LineComment bool + TransformMethod string + TrimPrefix string + TypeName string +} + // generate produces the String method for the named type. -func (g *Generator) generate(typeName string, - includeJSON, includeYAML, includeSQL, includeText, includeGQLGen bool, - transformMethod string, trimPrefix string, addPrefix string, lineComment bool, includeValuesMethod bool) { +func (g *Generator) generate(params generateParams) { values := make([]Value, 0, 100) for _, file := range g.pkg.files { - file.lineComment = lineComment + file.lineComment = params.LineComment // Set the state for this run of the walker. - file.typeName = typeName + file.typeName = params.TypeName file.values = nil if file.file != nil { ast.Inspect(file.file, file.genDecl) @@ -429,16 +371,16 @@ func (g *Generator) generate(typeName string, } if len(values) == 0 { - log.Fatalf("no values defined for type %s", typeName) + log.Fatalf("no values defined for type %s", params.TypeName) } - for _, prefix := range strings.Split(trimPrefix, ",") { + for _, prefix := range strings.Split(params.TrimPrefix, ",") { g.trimValueNames(values, prefix) } - g.transformValueNames(values, transformMethod) + g.transformValueNames(values, params.TransformMethod) - g.prefixValueNames(values, addPrefix) + g.prefixValueNames(values, params.AddPrefix) runs := splitIntoRuns(values) // The decision of which pattern to use depends on the number of @@ -456,33 +398,33 @@ func (g *Generator) generate(typeName string, const runsThreshold = 10 switch { case len(runs) == 1: - g.buildOneRun(runs, typeName) + g.buildOneRun(runs, params.TypeName) case len(runs) <= runsThreshold: - g.buildMultipleRuns(runs, typeName) + g.buildMultipleRuns(runs, params.TypeName) default: - g.buildMap(runs, typeName) + g.buildMap(runs, params.TypeName) } - if includeValuesMethod { - g.buildAltStringValuesMethod(typeName) + if params.IncludeValuesMethod { + g.buildAltStringValuesMethod(params.TypeName) } - g.buildNoOpOrderChangeDetect(runs, typeName) + g.buildNoOpOrderChangeDetect(runs, params.TypeName) - g.buildBasicExtras(runs, typeName, runsThreshold) - if includeJSON { - g.buildJSONMethods(runs, typeName, runsThreshold) + g.buildBasicExtras(runs, params.TypeName, runsThreshold) + if params.IncludeJSON { + g.buildJSONMethods(runs, params.TypeName, runsThreshold) } - if includeText { - g.buildTextMethods(runs, typeName, runsThreshold) + if params.IncludeText { + g.buildTextMethods(runs, params.TypeName, runsThreshold) } - if includeYAML { - g.buildYAMLMethods(runs, typeName, runsThreshold) + if params.IncludeYAML { + g.buildYAMLMethods(runs, params.TypeName, runsThreshold) } - if includeSQL { - g.addValueAndScanMethod(typeName) + if params.IncludeSQL { + g.addValueAndScanMethod(params.TypeName) } - if includeGQLGen { - g.buildGQLGenMethods(runs, typeName) + if params.IncludeGQLGen { + g.buildGQLGenMethods(runs, params.TypeName) } } @@ -689,9 +631,8 @@ func (g *Generator) declareIndexAndNameVar(run []Value, typeName string) { index, n := g.createIndexAndNameDecl(run, typeName, "") g.Printf("const %s\n", n) g.Printf("var %s\n", index) - index, n = g.createLowerIndexAndNameDecl(run, typeName, "") + _, n = g.createLowerIndexAndNameDecl(run, typeName, "") g.Printf("const %s\n", n) - //g.Printf("var %s\n", index) } // createIndexAndNameDecl returns the pair of declarations for the run. The caller will add "const" and "var". @@ -774,9 +715,10 @@ func (g *Generator) buildOneRun(runs [][]Value, typeName string) { } // Arguments to format are: -// [1]: type name -// [2]: size of index element (8 for uint8 etc.) -// [3]: less than zero check (for signed types) +// +// [1]: type name +// [2]: size of index element (8 for uint8 etc.) +// [3]: less than zero check (for signed types) const stringOneRun = `func (i %[1]s) String() string { if %[3]si >= %[1]s(len(_%[1]sIndex)-1) { return fmt.Sprintf("%[1]s(%%d)", i) @@ -786,10 +728,11 @@ const stringOneRun = `func (i %[1]s) String() string { ` // Arguments to format are: -// [1]: type name -// [2]: lowest defined value for type, as a string -// [3]: size of index element (8 for uint8 etc.) -// [4]: less than zero check (for signed types) +// +// [1]: type name +// [2]: lowest defined value for type, as a string +// [3]: size of index element (8 for uint8 etc.) +// [4]: less than zero check (for signed types) const stringOneRunWithOffset = `func (i %[1]s) String() string { i -= %[2]s if %[4]si >= %[1]s(len(_%[1]sIndex)-1) {