From 98e91bae517f82bc87889a15c79641f668549201 Mon Sep 17 00:00:00 2001 From: Neil O'Toole Date: Sun, 12 Sep 2021 16:14:30 -0600 Subject: [PATCH] Bunch of linting issues (#96) * linting * linting * linting * linting * linting * cleaned up readme trailing newlines --- .golangci.yml | 5 +-- README.md | 2 -- libsq/ast/parser.go | 50 +++++++++++++++++++++-------- libsq/ast/parser_test.go | 17 +++++----- libsq/ast/segment.go | 7 ++++ libsq/ast/sqlbuilder/basebuilder.go | 3 +- libsq/ast/sqlbuilder/sqlbuilder.go | 2 +- libsq/core/kind/kind.go | 5 ++- libsq/core/sqlz/record.go | 4 +-- libsq/core/stringz/stringz_test.go | 1 - libsq/source/fetcher/fetcher.go | 16 ++++----- libsq/source/files.go | 10 +++--- libsq/source/files_test.go | 8 ++--- libsq/source/handle.go | 4 +-- libsq/source/metadata.go | 6 ++-- main.go | 17 +++++----- 16 files changed, 93 insertions(+), 64 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f4ec69a3..d7ef11ec 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -66,6 +66,7 @@ linters: - dogsled - dupl - errcheck + - exportloopref - funlen - gochecknoinits - goconst @@ -73,7 +74,7 @@ linters: - gocyclo - gofmt - goimports - - golint +# - golint #- gomnd - goprintffuncname - gosec @@ -83,8 +84,8 @@ linters: - lll # - misspell - nakedret + - revive - rowserrcheck - - scopelint - staticcheck - structcheck - stylecheck diff --git a/README.md b/README.md index 60ce43b7..93df8879 100644 --- a/README.md +++ b/README.md @@ -357,5 +357,3 @@ xlsx Microsoft Excel XLSX false https://en.wikip - [golang-migrate](https://github.com/golang-migrate/migrate) - [octosql](https://github.com/cube2222/octosql) - [rq](https://github.com/dflemstr/rq) - - diff --git a/libsq/ast/parser.go b/libsq/ast/parser.go index 0b5d742d..1c4d5147 100644 --- a/libsq/ast/parser.go +++ b/libsq/ast/parser.go @@ -59,28 +59,38 @@ func (el *antlrErrorListener) String() string { return fmt.Sprintf("%s: no issues", el.name) } - str := append(el.errs, el.warnings...) - return strings.Join(str, "\n") + strs := make([]string, 0, len(el.errs)+len(el.warnings)) + strs = append(strs, el.errs...) + strs = append(strs, el.warnings...) + + return strings.Join(strs, "\n") } +// SyntaxError implements antlr.ErrorListener. func (el *antlrErrorListener) SyntaxError(recognizer antlr.Recognizer, offendingSymbol interface{}, line, column int, msg string, e antlr.RecognitionException) { text := fmt.Sprintf("%s: syntax error: [%d:%d] %s", el.name, line, column, msg) el.errs = append(el.errs, text) } -func (el *antlrErrorListener) ReportAmbiguity(recognizer antlr.Parser, dfa *antlr.DFA, startIndex, stopIndex int, exact bool, ambigAlts *antlr.BitSet, configs antlr.ATNConfigSet) { +// ReportAmbiguity implements antlr.ErrorListener. +func (el *antlrErrorListener) ReportAmbiguity(recognizer antlr.Parser, dfa *antlr.DFA, startIndex, stopIndex int, + exact bool, ambigAlts *antlr.BitSet, configs antlr.ATNConfigSet) { tok := recognizer.GetCurrentToken() text := fmt.Sprintf("%s: syntax ambiguity: [%d:%d]", el.name, startIndex, stopIndex) text = text + " >>" + tok.GetText() + "<<" el.warnings = append(el.warnings, text) } -func (el *antlrErrorListener) ReportAttemptingFullContext(recognizer antlr.Parser, dfa *antlr.DFA, startIndex, stopIndex int, conflictingAlts *antlr.BitSet, configs antlr.ATNConfigSet) { +// ReportAttemptingFullContext implements antlr.ErrorListener. +func (el *antlrErrorListener) ReportAttemptingFullContext(recognizer antlr.Parser, dfa *antlr.DFA, startIndex, + stopIndex int, conflictingAlts *antlr.BitSet, configs antlr.ATNConfigSet) { text := fmt.Sprintf("%s: attempting full context: [%d:%d]", el.name, startIndex, stopIndex) el.warnings = append(el.warnings, text) } -func (el *antlrErrorListener) ReportContextSensitivity(recognizer antlr.Parser, dfa *antlr.DFA, startIndex, stopIndex, prediction int, configs antlr.ATNConfigSet) { +// ReportContextSensitivity implements antlr.ErrorListener. +func (el *antlrErrorListener) ReportContextSensitivity(recognizer antlr.Parser, dfa *antlr.DFA, startIndex, stopIndex, + prediction int, configs antlr.ATNConfigSet) { text := fmt.Sprintf("%s: context sensitivity: [%d:%d]", el.name, startIndex, stopIndex) el.warnings = append(el.warnings, text) } @@ -106,6 +116,7 @@ type parseTreeVisitor struct { AST *AST } +// Visit implements antlr.ParseTreeVisitor. func (v *parseTreeVisitor) Visit(ctx antlr.ParseTree) interface{} { v.log.Debugf("visiting %T: %v: ", ctx, ctx.GetText()) @@ -146,6 +157,7 @@ func (v *parseTreeVisitor) Visit(ctx antlr.ParseTree) interface{} { return errorf("unknown node type: %T", ctx) } +// VisitChildren implements antlr.ParseTreeVisitor. func (v *parseTreeVisitor) VisitChildren(ctx antlr.RuleNode) interface{} { for _, child := range ctx.GetChildren() { tree, ok := child.(antlr.ParseTree) @@ -161,6 +173,7 @@ func (v *parseTreeVisitor) VisitChildren(ctx antlr.RuleNode) interface{} { return nil } +// VisitQuery implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitQuery(ctx *slq.QueryContext) interface{} { v.AST = &AST{} v.AST.ctx = ctx @@ -176,6 +189,7 @@ func (v *parseTreeVisitor) VisitQuery(ctx *slq.QueryContext) interface{} { return nil } +// VisitDsElement implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitDsElement(ctx *slq.DsElementContext) interface{} { ds := &Datasource{} ds.parent = v.cur @@ -183,6 +197,7 @@ func (v *parseTreeVisitor) VisitDsElement(ctx *slq.DsElementContext) interface{} return v.cur.AddChild(ds) } +// VisitDsTblElement implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitDsTblElement(ctx *slq.DsTblElementContext) interface{} { tblSel := &TblSelector{} tblSel.parent = v.cur @@ -194,25 +209,19 @@ func (v *parseTreeVisitor) VisitDsTblElement(ctx *slq.DsTblElementContext) inter return v.cur.AddChild(tblSel) } +// VisitSegment implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitSegment(ctx *slq.SegmentContext) interface{} { seg := &Segment{} seg.bn.ctx = ctx seg.bn.parent = v.AST - if v == nil { - panic("v is nil") - } - - if v.AST == nil { - panic("v.AST is nil") - } - v.AST.AddSegment(seg) v.cur = seg return v.VisitChildren(ctx) } +// VisitSelElement implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitSelElement(ctx *slq.SelElementContext) interface{} { selector := &Selector{} selector.parent = v.cur @@ -220,10 +229,12 @@ func (v *parseTreeVisitor) VisitSelElement(ctx *slq.SelElementContext) interface return v.cur.AddChild(selector) } +// VisitElement implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitElement(ctx *slq.ElementContext) interface{} { return v.VisitChildren(ctx) } +// VisitFn implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitFn(ctx *slq.FnContext) interface{} { v.log.Debugf("visiting function: %v", ctx.GetText()) @@ -245,6 +256,7 @@ func (v *parseTreeVisitor) VisitFn(ctx *slq.FnContext) interface{} { return v.cur.AddChild(fn) } +// VisitExpr implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitExpr(ctx *slq.ExprContext) interface{} { v.log.Debugf("visiting expr: %v", ctx.GetText()) @@ -279,14 +291,17 @@ func (v *parseTreeVisitor) VisitExpr(ctx *slq.ExprContext) interface{} { return v.cur.AddChild(ex) } +// VisitCmpr implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitCmpr(ctx *slq.CmprContext) interface{} { return v.VisitChildren(ctx) } +// VisitStmtList implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitStmtList(ctx *slq.StmtListContext) interface{} { return nil // not using StmtList just yet } +// VisitLiteral implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitLiteral(ctx *slq.LiteralContext) interface{} { v.log.Debugf("visiting literal: %q", ctx.GetText()) @@ -297,13 +312,17 @@ func (v *parseTreeVisitor) VisitLiteral(ctx *slq.LiteralContext) interface{} { return err } +// VisitUnaryOperator implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitUnaryOperator(ctx *slq.UnaryOperatorContext) interface{} { return nil } + +// VisitFnName implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitFnName(ctx *slq.FnNameContext) interface{} { return nil } +// VisitGroup implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitGroup(ctx *slq.GroupContext) interface{} { // parent node must be a segment _, ok := v.cur.(*Segment) @@ -333,6 +352,7 @@ func (v *parseTreeVisitor) VisitGroup(ctx *slq.GroupContext) interface{} { return nil } +// VisitJoin implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitJoin(ctx *slq.JoinContext) interface{} { // parent node must be a segment seg, ok := v.cur.(*Segment) @@ -362,6 +382,7 @@ func (v *parseTreeVisitor) VisitJoin(ctx *slq.JoinContext) interface{} { return nil } +// VisitJoinConstraint implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitJoinConstraint(ctx *slq.JoinConstraintContext) interface{} { joinNode, ok := v.cur.(*Join) if !ok { @@ -433,6 +454,7 @@ func (v *parseTreeVisitor) VisitJoinConstraint(ctx *slq.JoinConstraintContext) i return join.AddChild(joinCondition) } +// VisitTerminal implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitTerminal(ctx antlr.TerminalNode) interface{} { v.log.Debugf("visiting terminal: %q", ctx.GetText()) @@ -459,6 +481,7 @@ func (v *parseTreeVisitor) VisitTerminal(ctx antlr.TerminalNode) interface{} { return nil } +// VisitRowRange implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitRowRange(ctx *slq.RowRangeContext) interface{} { // [] select all rows (no range) // [1] select row[1] @@ -517,6 +540,7 @@ func (v *parseTreeVisitor) VisitRowRange(ctx *slq.RowRangeContext) interface{} { return v.cur.AddChild(rr) } +// VisitErrorNode implements slq.SLQVisitor. func (v *parseTreeVisitor) VisitErrorNode(ctx antlr.ErrorNode) interface{} { v.log.Debugf("error node: %v", ctx.GetText()) return nil diff --git a/libsq/ast/parser_test.go b/libsq/ast/parser_test.go index 0e9ca304..c7ae328e 100644 --- a/libsq/ast/parser_test.go +++ b/libsq/ast/parser_test.go @@ -12,15 +12,14 @@ import ( ) const ( - fixtRowRange1 = `@mydb1 | .user | .uid, .username | .[]` - fixtRowRange2 = `@mydb1 | .user | .uid, .username | .[2]` - fixtRowRange3 = `@mydb1 | .user | .uid, .username | .[1:3]` - fixtRowRange4 = `@mydb1 | .user | .uid, .username | .[0:3]` - fixtRowRange5 = `@mydb1 | .user | .uid, .username | .[:3]` - fixtRowRange6 = `@mydb1 | .user | .uid, .username | .[2:]` - fixtJoinRowRange = `@my1 |.user, .address | join(.uid) | .[0:4] | .user.uid, .username, .country` - fixtJoinQuery1 = `@mydb1 | .user, .address | join(.user.uid == .address.uid) | .uid, .username, .country` - fixtSelect1 = `@mydb1 | .user | .uid, .username` + fixtRowRange1 = `@mydb1 | .user | .uid, .username | .[]` + fixtRowRange2 = `@mydb1 | .user | .uid, .username | .[2]` + fixtRowRange3 = `@mydb1 | .user | .uid, .username | .[1:3]` + fixtRowRange4 = `@mydb1 | .user | .uid, .username | .[0:3]` + fixtRowRange5 = `@mydb1 | .user | .uid, .username | .[:3]` + fixtRowRange6 = `@mydb1 | .user | .uid, .username | .[2:]` + fixtJoinQuery1 = `@mydb1 | .user, .address | join(.user.uid == .address.uid) | .uid, .username, .country` + fixtSelect1 = `@mydb1 | .user | .uid, .username` ) var slqInputs = map[string]string{ diff --git a/libsq/ast/segment.go b/libsq/ast/segment.go index 48a14ad2..9c0e5aa4 100644 --- a/libsq/ast/segment.go +++ b/libsq/ast/segment.go @@ -18,10 +18,12 @@ type Segment struct { bn baseNode } +// Parent implements ast.Node. func (s *Segment) Parent() Node { return s.bn.Parent() } +// SetParent implements ast.Node. func (s *Segment) SetParent(parent Node) error { ast, ok := parent.(*AST) if !ok { @@ -30,24 +32,29 @@ func (s *Segment) SetParent(parent Node) error { return s.bn.SetParent(ast) } +// Children implements ast.Node. func (s *Segment) Children() []Node { return s.bn.Children() } +// AddChild implements ast.Node. func (s *Segment) AddChild(child Node) error { s.bn.addChild(child) return child.SetParent(s) } +// SetChildren implements ast.Node. func (s *Segment) SetChildren(children []Node) error { s.bn.setChildren(children) return nil } +// Context implements ast.Node. func (s *Segment) Context() antlr.ParseTree { return s.bn.Context() } +// SetContext implements ast.Node. func (s *Segment) SetContext(ctx antlr.ParseTree) error { segCtx, ok := ctx.(*slq.SegmentContext) if !ok { diff --git a/libsq/ast/sqlbuilder/basebuilder.go b/libsq/ast/sqlbuilder/basebuilder.go index 5adde04b..8b215451 100644 --- a/libsq/ast/sqlbuilder/basebuilder.go +++ b/libsq/ast/sqlbuilder/basebuilder.go @@ -96,7 +96,6 @@ func (fb *BaseFragmentBuilder) SelectAll(tblSel *ast.TblSelector) (string, error // Function implements FragmentBuilder. func (fb *BaseFragmentBuilder) Function(fn *ast.Func) (string, error) { - buf := &bytes.Buffer{} children := fn.Children() @@ -225,7 +224,7 @@ func sqlAppend(existing, add string) string { // .table.col --> "table"."col" // // Thus, the selector must have exactly one or two periods. -func quoteTableOrColSelector(quote string, selector string) (string, error) { +func quoteTableOrColSelector(quote, selector string) (string, error) { if len(selector) < 2 || selector[0] != '.' { return "", errz.Errorf("invalid selector: %s", selector) } diff --git a/libsq/ast/sqlbuilder/sqlbuilder.go b/libsq/ast/sqlbuilder/sqlbuilder.go index 2bf397b4..9203db4f 100644 --- a/libsq/ast/sqlbuilder/sqlbuilder.go +++ b/libsq/ast/sqlbuilder/sqlbuilder.go @@ -1,4 +1,4 @@ -// Package sqlbuilder contains functionality building SQL from +// Package sqlbuilder contains functionality for building SQL from // the AST. package sqlbuilder diff --git a/libsq/core/kind/kind.go b/libsq/core/kind/kind.go index 165c93ac..d8b5b539 100644 --- a/libsq/core/kind/kind.go +++ b/libsq/core/kind/kind.go @@ -212,8 +212,11 @@ func (d *Detector) Sample(v interface{}) { // We're dealing with a string value, which could a variety // of things, such as: "1", "1.0", "true", "11:30". - s := v.(string) + d.doSampleString(v.(string)) +} +//nolint:dupl,gocognit +func (d *Detector) doSampleString(s string) { if s == "" { // Can't really do anything useful with this return diff --git a/libsq/core/sqlz/record.go b/libsq/core/sqlz/record.go index caa79f51..85ccd40f 100644 --- a/libsq/core/sqlz/record.go +++ b/libsq/core/sqlz/record.go @@ -205,12 +205,12 @@ type ColumnTypeData struct { // NewColumnTypeData returns a new instance with field values // taken from col, supplemented with the kind param. -func NewColumnTypeData(col *sql.ColumnType, kind kind.Kind) *ColumnTypeData { +func NewColumnTypeData(col *sql.ColumnType, knd kind.Kind) *ColumnTypeData { ct := &ColumnTypeData{ Name: col.Name(), DatabaseTypeName: col.DatabaseTypeName(), ScanType: col.ScanType(), - Kind: kind, + Kind: knd, } ct.Nullable, ct.HasNullable = col.Nullable() diff --git a/libsq/core/stringz/stringz_test.go b/libsq/core/stringz/stringz_test.go index e6fd1e2b..21b989e0 100644 --- a/libsq/core/stringz/stringz_test.go +++ b/libsq/core/stringz/stringz_test.go @@ -279,5 +279,4 @@ func TestLineCount(t *testing.T) { require.Equal(t, tc.skipEmpty, count) }) } - } diff --git a/libsq/source/fetcher/fetcher.go b/libsq/source/fetcher/fetcher.go index be079dd5..e1374558 100644 --- a/libsq/source/fetcher/fetcher.go +++ b/libsq/source/fetcher/fetcher.go @@ -33,9 +33,9 @@ type Fetcher struct { Config *Config } -// Fetch writes the body of the document at url to w. -func (f *Fetcher) Fetch(ctx context.Context, url string, w io.Writer) error { - return fetchHTTP(ctx, f.Config, url, w) +// Fetch writes the body of the document at fileURL to w. +func (f *Fetcher) Fetch(ctx context.Context, fileURL string, w io.Writer) error { + return fetchHTTP(ctx, f.Config, fileURL, w) } func httpClient(cfg *Config) *http.Client { @@ -49,7 +49,7 @@ func httpClient(cfg *Config) *http.Client { } if tr.TLSClientConfig == nil { - tr.TLSClientConfig = &tls.Config{} + tr.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} } else { tr.TLSClientConfig = tr.TLSClientConfig.Clone() } @@ -64,9 +64,9 @@ func httpClient(cfg *Config) *http.Client { return &client } -func fetchHTTP(ctx context.Context, cfg *Config, url string, w io.Writer) error { +func fetchHTTP(ctx context.Context, cfg *Config, fileURL string, w io.Writer) error { c := httpClient(cfg) - req, err := http.NewRequest("GET", url, nil) + req, err := http.NewRequest("GET", fileURL, nil) if err != nil { return err } @@ -78,13 +78,13 @@ func fetchHTTP(ctx context.Context, cfg *Config, url string, w io.Writer) error if resp.StatusCode != http.StatusOK { _ = resp.Body.Close() - return errz.Errorf("http: returned non-200 status code (%s) from: %s", resp.Status, url) + return errz.Errorf("http: returned non-200 status code (%s) from: %s", resp.Status, fileURL) } _, err = io.Copy(w, resp.Body) if err != nil { _ = resp.Body.Close() - return errz.Wrapf(err, "http: failed to read body from: %s", url) + return errz.Wrapf(err, "http: failed to read body from: %s", fileURL) } return errz.Err(resp.Body.Close()) diff --git a/libsq/source/files.go b/libsq/source/files.go index 882fb5df..f38a917b 100644 --- a/libsq/source/files.go +++ b/libsq/source/files.go @@ -403,13 +403,13 @@ func (fs *Files) detectType(ctx context.Context, loc string) (typ Type, ok bool, default: } - typ, score, err := detectFn(gctx, fs.log, openFn) - if err != nil { - return err + gTyp, gScore, gErr := detectFn(gctx, fs.log, openFn) + if gErr != nil { + return gErr } - if score > 0 { - resultCh <- result{typ: typ, score: score} + if gScore > 0 { + resultCh <- result{typ: gTyp, score: gScore} } return nil }) diff --git a/libsq/source/files_test.go b/libsq/source/files_test.go index d999ac62..c169258f 100644 --- a/libsq/source/files_test.go +++ b/libsq/source/files_test.go @@ -158,11 +158,11 @@ func TestFiles_NewReader(t *testing.T) { for i := 0; i < 1000; i++ { g.Go(func() error { - r, err := fs.Open(src) - require.NoError(t, err) + r, gErr := fs.Open(src) + require.NoError(t, gErr) - b, err := ioutil.ReadAll(r) - require.NoError(t, err) + b, gErr := ioutil.ReadAll(r) + require.NoError(t, gErr) require.Equal(t, wantBytes, b) return nil diff --git a/libsq/source/handle.go b/libsq/source/handle.go index 71bffb03..ff6cdfff 100644 --- a/libsq/source/handle.go +++ b/libsq/source/handle.go @@ -12,7 +12,7 @@ import ( ) var ( - handlePattern = regexp.MustCompile(`\A[@][a-zA-Z][a-zA-Z0-9_]*$`) + handlePattern = regexp.MustCompile(`\A@[a-zA-Z][a-zA-Z0-9_]*$`) tablePattern = regexp.MustCompile(`\A[a-zA-Z_][a-zA-Z0-9_]*$`) ) @@ -20,7 +20,7 @@ var ( // not an acceptable source handle value. // Valid input must match: // -// \A[@][a-zA-Z][a-zA-Z0-9_]*$ +// \A@[a-zA-Z][a-zA-Z0-9_]*$ func VerifyLegalHandle(handle string) error { matches := handlePattern.MatchString(handle) if !matches { diff --git a/libsq/source/metadata.go b/libsq/source/metadata.go index 13cf62a5..9ee8208a 100644 --- a/libsq/source/metadata.go +++ b/libsq/source/metadata.go @@ -52,9 +52,9 @@ type Metadata struct { // TableNames is a convenience method that returns md's table names. func (md *Metadata) TableNames() []string { - var names []string - for _, tblDef := range md.Tables { - names = append(names, tblDef.Name) + names := make([]string, 0, len(md.Tables)) + for i, tblDef := range md.Tables { + names[i] = tblDef.Name } return names } diff --git a/main.go b/main.go index d3bb8940..52bce99d 100644 --- a/main.go +++ b/main.go @@ -10,8 +10,14 @@ import ( ) func main() { + var err error ctx, cancelFn := context.WithCancel(context.Background()) - defer cancelFn() + defer func() { + cancelFn() + if err != nil { + os.Exit(1) + } + }() go func() { stopCh := make(chan os.Signal, 1) @@ -21,12 +27,5 @@ func main() { cancelFn() }() - err := cli.Execute(ctx, os.Stdin, os.Stdout, os.Stderr, os.Args[1:]) - if err != nil { - // invoke cancelFn before os.Exit, because deferred funcs don't - // run before os.Exit. - cancelFn() - - os.Exit(1) - } + err = cli.Execute(ctx, os.Stdin, os.Stdout, os.Stderr, os.Args[1:]) }