From 88519c3704844e417266f63eeec05e1457e07020 Mon Sep 17 00:00:00 2001 From: Marcelo Cantos Date: Wed, 28 Feb 2018 17:11:18 +1100 Subject: [PATCH] Apply more golint recommendations (#201) * Apply more golint recommendations * Update dumpstdlibast.go to include StdAst comment * Improve dump.go package comment. --- ast/ast.go | 1 + ast/stdast.go | 9 +++++++++ ast/util.go | 9 +++++---- cmd/dumpstdlibast.go | 17 +++++++++++++++-- dump/dump.go | 29 ++++++++++++++++++++--------- dump/pointermap.go | 4 ++-- jsonnet/cmd.go | 39 ++++++++++++++++++--------------------- linter/linter.go | 1 + parser/parser.go | 1 + static_analyzer.go | 2 +- 10 files changed, 73 insertions(+), 39 deletions(-) diff --git a/ast/ast.go b/ast/ast.go index 57f0dd6..77c9c06 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package ast provides AST nodes and ancillary structures and algorithms. package ast import ( diff --git a/ast/stdast.go b/ast/stdast.go index 4ddeac9..e4ce691 100644 --- a/ast/stdast.go +++ b/ast/stdast.go @@ -1,3 +1,10 @@ +/////////////////////////////////////////////////////////// +// This file was auto-generated by cmd/dumpstdlibast.go. // +// https://github.com/google/go-jsonnet#generated-stdlib // +// // +// --------------- DO NOT EDIT BY HAND! --------------- // +/////////////////////////////////////////////////////////// + package ast var p3Var = "$" @@ -2768,6 +2775,8 @@ var p1 = &Source{ "\n", }, } + +// StdAst is the AST for the standard library. var StdAst = &DesugaredObject{ NodeBase: NodeBase{ loc: LocationRange{ diff --git a/ast/util.go b/ast/util.go index ec9b53f..f086b01 100644 --- a/ast/util.go +++ b/ast/util.go @@ -18,16 +18,17 @@ package ast import "sort" -func (i *IdentifierSet) Append(idents Identifiers) { +// AddIdentifiers adds a slice of identifiers to an identifier set. +func (i IdentifierSet) AddIdentifiers(idents Identifiers) { for _, ident := range idents { i.Add(ident) } } -// ToOrderedSlice returns the elements of the current set as a ordered slice -func (set IdentifierSet) ToOrderedSlice() []Identifier { +// ToOrderedSlice returns the elements of the current set as an ordered slice. +func (i IdentifierSet) ToOrderedSlice() []Identifier { var s []Identifier - for v := range set { + for v := range i { s = append(s, v) } sort.Sort(identifierSorter(s)) diff --git a/cmd/dumpstdlibast.go b/cmd/dumpstdlibast.go index 0af6a7b..9349f04 100644 --- a/cmd/dumpstdlibast.go +++ b/cmd/dumpstdlibast.go @@ -43,9 +43,22 @@ func main() { dump.Config.HidePrivateFields = false dump.Config.StripPackageNames = true dump.Config.VariableName = "StdAst" + dump.Config.VariableDescription = "StdAst is the AST for the standard library." ast := dump.Sdump(node) - file.WriteString("package ast\n\n") + file.WriteString(header) file.WriteString(ast) - file.Sync() + file.Close() } + +var header = ` +/////////////////////////////////////////////////////////// +// This file was auto-generated by cmd/dumpstdlibast.go. // +// https://github.com/google/go-jsonnet#generated-stdlib // +// // +// --------------- DO NOT EDIT BY HAND! --------------- // +/////////////////////////////////////////////////////////// + +package ast + +`[1:] diff --git a/dump/dump.go b/dump/dump.go index 8f18b96..128d205 100644 --- a/dump/dump.go +++ b/dump/dump.go @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package dump can dump a Go data structure to Go source file, so that it can +// be statically embedded into other code. package dump import ( @@ -31,17 +33,19 @@ var packageNameStripperRegexp = regexp.MustCompile("\\b[a-zA-Z_]+[a-zA-Z_0-9]+\\ // Options represents configuration option type Options struct { - StripPackageNames bool - HidePrivateFields bool - HomePackage string - VariableName string + StripPackageNames bool + HidePrivateFields bool + HomePackage string + VariableName string + VariableDescription string } // Config is the default config used when calling Dump var Config = Options{ - StripPackageNames: false, - HidePrivateFields: true, - VariableName: "Obj", + StripPackageNames: false, + HidePrivateFields: true, + VariableName: "Obj", + VariableDescription: "", } type dumpState struct { @@ -157,8 +161,15 @@ func (s *dumpState) dumpMap(v reflect.Value) { } func (s *dumpState) dump(value interface{}) { - if value == nil { + writeVar := func() { + if s.config.VariableDescription != "" { + fmt.Fprintf(s.w, "\n// %s\n", s.config.VariableDescription) + } s.w.Write([]byte("var " + s.config.VariableName + " = ")) + } + + if value == nil { + writeVar() printNil(s.w) s.newline() return @@ -170,7 +181,7 @@ func (s *dumpState) dump(value interface{}) { s.dumpReusedPointerVal(v) - s.w.Write([]byte("var " + s.config.VariableName + " = ")) + writeVar() if v.Kind() == reflect.Ptr && v.IsNil() { printNil(s.w) } else { diff --git a/dump/pointermap.go b/dump/pointermap.go index f17e54b..c8704d8 100644 --- a/dump/pointermap.go +++ b/dump/pointermap.go @@ -24,8 +24,8 @@ type pointerMap struct { primitivePointers []uintptr } -// GetPointers: Given a structure, it will recursively map all pointers mentioned in the tree, -// breaking circular references and provide list of: +// GetPointers recursively maps all pointers mentioned in a tree, breaking +// circular references, and provides a list of: // * all pointers // * all pointers that were referenced at least twice // * all pointers that point to primitive type diff --git a/jsonnet/cmd.go b/jsonnet/cmd.go index bf97eb9..b675791 100644 --- a/jsonnet/cmd.go +++ b/jsonnet/cmd.go @@ -120,13 +120,13 @@ func usage(o io.Writer) { func safeStrToInt(str string) (i int) { i, err := strconv.Atoi(str) if err != nil { - fmt.Fprintf(os.Stderr, "ERROR: Invalid integer \"%s\"\n", str) + fmt.Fprintf(os.Stderr, "Invalid integer \"%s\"\n", str) os.Exit(1) } return } -type Command int +type command int const ( commandEval = iota @@ -134,7 +134,7 @@ const ( ) type config struct { - cmd Command + cmd command inputFiles []string outputFile string filenameIsCode bool @@ -167,20 +167,18 @@ func getVarVal(s string) (string, string, error) { if exists { return name, content, nil } - return "", "", fmt.Errorf("ERROR: Environment variable %v was undefined.", name) - } else { - return name, parts[1], nil + return "", "", fmt.Errorf("environment variable %v was undefined", name) } + return name, parts[1], nil } func getVarFile(s string, imp string) (string, string, error) { parts := strings.SplitN(s, "=", 2) name := parts[0] if len(parts) == 1 { - return "", "", fmt.Errorf("ERROR: argument not in form = \"%s\".", s) - } else { - return name, fmt.Sprintf("%s @'%s'", imp, strings.Replace(parts[1], "'", "''", -1)), nil + return "", "", fmt.Errorf(`argument not in form = "%s"`, s) } + return name, fmt.Sprintf("%s @'%s'", imp, strings.Replace(parts[1], "'", "''", -1)), nil } type processArgsStatus int @@ -217,7 +215,7 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg } else if arg == "-o" || arg == "--output-file" { outputFile := nextArg(&i, args) if len(outputFile) == 0 { - return processArgsStatusFailure, fmt.Errorf("ERROR: -o argument was empty string") + return processArgsStatusFailure, fmt.Errorf("-o argument was empty string") } config.outputFile = outputFile } else if arg == "--" { @@ -231,13 +229,13 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg if arg == "-s" || arg == "--max-stack" { l := safeStrToInt(nextArg(&i, args)) if l < 1 { - return processArgsStatusFailure, fmt.Errorf("ERROR: Invalid --max-stack value: %d", l) + return processArgsStatusFailure, fmt.Errorf("invalid --max-stack value: %d", l) } vm.MaxStack = l } else if arg == "-J" || arg == "--jpath" { dir := nextArg(&i, args) if len(dir) == 0 { - return processArgsStatusFailure, fmt.Errorf("ERROR: -J argument was empty string") + return processArgsStatusFailure, fmt.Errorf("-J argument was empty string") } if dir[len(dir)-1] != '/' { dir += "/" @@ -302,14 +300,14 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg } else if arg == "-t" || arg == "--max-trace" { l := safeStrToInt(nextArg(&i, args)) if l < 0 { - return processArgsStatusFailure, fmt.Errorf("ERROR: Invalid --max-trace value: %d", l) + return processArgsStatusFailure, fmt.Errorf("invalid --max-trace value: %d", l) } vm.ErrorFormatter.SetMaxStackTraceSize(l) } else if arg == "-m" || arg == "--multi" { config.evalMulti = true outputDir := nextArg(&i, args) if len(outputDir) == 0 { - return processArgsStatusFailure, fmt.Errorf("ERROR: -m argument was empty string") + return processArgsStatusFailure, fmt.Errorf("-m argument was empty string") } if outputDir[len(outputDir)-1] != '/' { outputDir += "/" @@ -320,13 +318,12 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg } else if arg == "-S" || arg == "--string" { vm.StringOutput = true } else if len(arg) > 1 && arg[0] == '-' { - return processArgsStatusFailure, fmt.Errorf("ERROR: Unrecognized argument: %s", arg) + return processArgsStatusFailure, fmt.Errorf("unrecognized argument: %s", arg) } else { remainingArgs = append(remainingArgs, arg) } - } else { - return processArgsStatusFailure, fmt.Errorf("The Go implementation currently does not support jsonnet fmt.") + return processArgsStatusFailure, fmt.Errorf("the Go implementation currently does not support jsonnet fmt") } } @@ -335,7 +332,7 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg want = "code" } if len(remainingArgs) == 0 { - return processArgsStatusFailureUsage, fmt.Errorf("ERROR: Must give %s", want) + return processArgsStatusFailureUsage, fmt.Errorf("must give %s", want) } // TODO(dcunnin): Formatter allows multiple files in test and in-place mode. @@ -343,7 +340,7 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg if !multipleFilesAllowed { if len(remainingArgs) > 1 { - return processArgsStatusFailure, fmt.Errorf("ERROR: Only one %s is allowed", want) + return processArgsStatusFailure, fmt.Errorf("only one %s is allowed", want) } } @@ -390,7 +387,7 @@ func writeMultiOutputFiles(output map[string]string, outputDir, outputFile strin // Iterate through the map in order. keys := make([]string, 0, len(output)) - for k, _ := range output { + for k := range output { keys = append(keys, k) } sort.Strings(keys) @@ -511,7 +508,7 @@ func main() { status, err := processArgs(os.Args[1:], &config, vm) if err != nil { - fmt.Fprintln(os.Stderr, err.Error()) + fmt.Fprintln(os.Stderr, "ERROR: "+err.Error()) } switch status { case processArgsStatusContinue: diff --git a/linter/linter.go b/linter/linter.go index 1fde5f7..914133f 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -1,3 +1,4 @@ +// Package linter analyses Jsonnet code for code "smells". package linter import ( diff --git a/parser/parser.go b/parser/parser.go index 3436856..65384be 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package parser reads Jsonnet files and parses them into AST nodes. package parser import ( diff --git a/static_analyzer.go b/static_analyzer.go index d177448..88d9377 100644 --- a/static_analyzer.go +++ b/static_analyzer.go @@ -33,7 +33,7 @@ func visitNext(a ast.Node, inObject bool, vars ast.IdentifierSet, state *analysi return } state.err = analyzeVisit(a, inObject, vars) - state.freeVars.Append(a.FreeVariables()) + state.freeVars.AddIdentifiers(a.FreeVariables()) } func analyzeVisit(a ast.Node, inObject bool, vars ast.IdentifierSet) error {