Merge pull request #79 from dolthub/daylon/staticcheck

Added static analyzer & linter

Author: Hydrocharged

.github/workflows/ci-staticcheck.yaml

@@ -0,0 +1,27 @@
+name: Static Analysis & Linter
+on: [pull_request]
+
+concurrency:
+  group: ci-staticcheck-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  ci:
+    name: Run Staticcheck
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Setup Go 1.x
+        uses: actions/setup-go@v3
+        with:
+          go-version: ^1.21
+      - uses: actions/checkout@v3
+        with:
+          submodules: true
+      - name: Build SQL Syntax
+        run: ./build.sh
+        working-directory: ./postgres/parser
+        shell: bash
+      - name: Run check
+        run: |
+          go install honnef.co/go/tools/cmd/[email protected]
+          staticcheck ./...

main.go

@@ -16,10 +16,7 @@ package main
 
 import (
 	"context"
-	crand "crypto/rand"
-	"encoding/binary"
 	"fmt"
-	"math/rand"
 	"os"
 	"strconv"
 	"strings"
@@ -69,7 +66,6 @@ const stdOutAndErrFlag = "--out-and-err"
 
 func main() {
 	ctx := context.Background()
-	seedGlobalRand()
 
 	args := os.Args[1:]
 
@@ -112,7 +108,7 @@ func main() {
 	}
 
 	// Otherwise, attempt to run the command indicated
-	cliCtx, err := configureCliCtx(subCommandName, apr, fs, dEnv, err, ctx)
+	cliCtx, err := configureCliCtx(subCommandName, apr, fs, dEnv, ctx)
 	if err != nil {
 		cli.PrintErrln(err.Error())
 		os.Exit(1)
@@ -122,7 +118,7 @@ func main() {
 	os.Exit(exitCode)
 }
 
-func configureCliCtx(subcommand string, apr *argparser.ArgParseResults, fs filesys.Filesys, dEnv *env.DoltEnv, err error, ctx context.Context) (cli.CliContext, error) {
+func configureCliCtx(subcommand string, apr *argparser.ArgParseResults, fs filesys.Filesys, dEnv *env.DoltEnv, ctx context.Context) (cli.CliContext, error) {
 	dataDir, hasDataDir := apr.GetValue(commands.DataDirFlag)
 	if hasDataDir {
 		// If a relative path was provided, this ensures we have an absolute path everywhere.
@@ -144,7 +140,7 @@ func configureCliCtx(subcommand string, apr *argparser.ArgParseResults, fs files
 			"To use the current directory as a database, start the server from the parent directory.")
 	}
 
-	err = reconfigIfTempFileMoveFails(dEnv)
+	err := reconfigIfTempFileMoveFails(dEnv)
 	if err != nil {
 		return nil, fmt.Errorf("failed to set up the temporary directory: %w", err)
 	}
@@ -297,15 +293,6 @@ func getProfile(apr *argparser.ArgParseResults, profileName, profiles string) (r
 	}
 }
 
-func seedGlobalRand() {
-	bs := make([]byte, 8)
-	_, err := crand.Read(bs)
-	if err != nil {
-		panic("failed to initial rand " + err.Error())
-	}
-	rand.Seed(int64(binary.LittleEndian.Uint64(bs)))
-}
-
 // buildLateBinder builds a LateBindQueryist for which is used to obtain the Queryist used for the length of the
 // command execution.
 func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, rootEnv *env.DoltEnv, mrEnv *env.MultiRepoEnv, creds *cli.UserPassword, apr *argparser.ArgParseResults, subcommandName string, verbose bool) (cli.LateBindQueryist, error) {
@@ -370,12 +357,6 @@ func buildLateBinder(ctx context.Context, cwdFS filesys.Filesys, rootEnv *env.Do
 		}, nil
 	}
 
-	// nil targetEnv will happen if the user ran a command in an empty directory or when there is a server running with
-	// no databases. CLI will try to connect to the server in this case.
-	if targetEnv == nil {
-		targetEnv = rootEnv
-	}
-
 	if verbose {
 		cli.Println("verbose: starting local mode")
 	}
@@ -515,5 +496,5 @@ func emitUsageEvent(dEnv *env.DoltEnv) {
 		return
 	}
 
-	err = emitter.LogEvents(server.Version, clientEvents)
+	_ = emitter.LogEvents(server.Version, clientEvents)
 }

postgres/connection/message_decode_encode.go

@@ -102,15 +102,15 @@ func decode(buffer *decodeBuffer, fields []FieldGroup, iterations int32) error {
 					}
 					data := make([]byte, byteCount)
 					copy(data, buffer.data)
-					if field.Flags&StaticData != 0 && bytes.Compare(field.Data.([]byte), data) != 0 {
+					if field.Flags&StaticData != 0 && !bytes.Equal(field.Data.([]byte), data) {
 						return errors.New("static data differs from the buffer data")
 					}
 					field.Data = data
 					buffer.advance(byteCount)
 				} else {
 					data := make([]byte, len(buffer.data))
 					copy(data, buffer.data)
-					if field.Flags&StaticData != 0 && bytes.Compare(field.Data.([]byte), data) != 0 {
+					if field.Flags&StaticData != 0 && !bytes.Equal(field.Data.([]byte), data) {
 						return errors.New("static data differs from the buffer data")
 					}
 					field.Data = data

postgres/parser/encoding/decimal.go

@@ -31,7 +31,6 @@ package encoding
 import (
 	"bytes"
 	"fmt"
-	"math"
 	"math/big"
 	"unsafe"
 
@@ -730,23 +729,6 @@ func UpperBoundNonsortingDecimalSize(d *apd.Decimal) int {
 	return 1 + MaxVarintLen + WordLen(d.Coeff.Bits())
 }
 
-// upperBoundNonsortingDecimalUnscaledSize is the same as
-// UpperBoundNonsortingDecimalSize but for a decimal with the given unscaled
-// length. The upper bound here may not be as tight as the one returned by
-// UpperBoundNonsortingDecimalSize.
-func upperBoundNonsortingDecimalUnscaledSize(unscaledLen int) int {
-	// The number of digits needed to represent a base 10 number of length
-	// unscaledLen in base 2.
-	unscaledLenBase2 := float64(unscaledLen) * math.Log2(10)
-	unscaledLenBase2Words := math.Ceil(unscaledLenBase2 / 8 / float64(bigWordSize))
-	unscaledLenWordRounded := int(unscaledLenBase2Words) * bigWordSize
-	// Makeup of upper bound size:
-	// - 1 byte for the prefix
-	// - MaxVarintLen for the exponent
-	// - unscaledLenWordRounded for the big.Int bytes
-	return 1 + MaxVarintLen + unscaledLenWordRounded
-}
-
 // Taken from math/big/arith.go.
 const bigWordSize = int(unsafe.Sizeof(big.Word(0)))
 

postgres/parser/encoding/encoding.go

@@ -28,7 +28,6 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
-	"reflect"
 	"unsafe"
 
 	"github.com/cockroachdb/apd/v2"
@@ -161,8 +160,6 @@ func DecodeUint32Ascending(b []byte) ([]byte, uint32, error) {
 	return b[4:], v, nil
 }
 
-const uint64AscendingEncodedLength = 8
-
 // EncodeUint64Ascending encodes the uint64 value using a big-endian 8 byte
 // representation. The bytes are appended to the supplied buffer and
 // the final buffer is returned.
@@ -461,10 +458,10 @@ func UnsafeConvertStringToBytes(s string) []byte {
 	// kosher because we know that EncodeBytes{,Descending} does
 	// not keep a reference to the value it encodes. The first
 	// step is getting access to the string internals.
-	hdr := (*reflect.StringHeader)(unsafe.Pointer(&s))
+	data := unsafe.StringData(s)
 	// Next we treat the string data as a maximally sized array which we
 	// slice. This usage is safe because the pointer value remains in the string.
-	return (*[0x7fffffff]byte)(unsafe.Pointer(hdr.Data))[:len(s):len(s)]
+	return (*[0x7fffffff]byte)(unsafe.Pointer(data))[:len(s):len(s)]
 }
 
 // EncodeStringAscending encodes the string value using an escape-based encoding. See
@@ -694,19 +691,6 @@ func GetMultiVarintLen(b []byte, num int) (int, error) {
 	return p, nil
 }
 
-// getMultiNonsortingVarintLen finds the length of <num> encoded nonsorting varints.
-func getMultiNonsortingVarintLen(b []byte, num int) (int, error) {
-	p := 0
-	for i := 0; i < num && p < len(b); i++ {
-		_, len, _, err := DecodeNonsortingStdlibVarint(b[p:])
-		if err != nil {
-			return 0, err
-		}
-		p += len
-	}
-	return p, nil
-}
-
 // getArrayLength returns the length of a key encoded array. The input
 // must have had the array type marker stripped from the front.
 func getArrayLength(buf []byte, dir Direction) (int, error) {
@@ -883,8 +867,6 @@ func DecodeNonsortingStdlibUvarint(
 	return buf[n:], n, i, nil
 }
 
-const floatValueEncodedLength = uint64AscendingEncodedLength
-
 // EncodeUntaggedDecimalValue encodes an apd.Decimal value, appends it to the supplied
 // buffer, and returns the final buffer.
 func EncodeUntaggedDecimalValue(appendTo []byte, d *apd.Decimal) []byte {

postgres/parser/geo/parse.go

@@ -35,8 +35,6 @@ import (
 	"github.com/twpayne/go-geom/encoding/ewkb"
 	"github.com/twpayne/go-geom/encoding/ewkbhex"
 	"github.com/twpayne/go-geom/encoding/geojson"
-	"github.com/twpayne/go-geom/encoding/wkb"
-	"github.com/twpayne/go-geom/encoding/wkbcommon"
 
 	"github.com/dolthub/doltgresql/postgres/parser/geo/geopb"
 	"github.com/dolthub/doltgresql/postgres/parser/geo/geos"
@@ -109,18 +107,6 @@ func parseEWKB(
 	return spatialObjectFromGeomT(t, soType)
 }
 
-// parseWKB takes given bytes assumed to be WKB and transforms it into a SpatialObject.
-func parseWKB(
-	soType geopb.SpatialObjectType, b []byte, defaultSRID geopb.SRID,
-) (geopb.SpatialObject, error) {
-	t, err := wkb.Unmarshal(b, wkbcommon.WKBOptionEmptyPointHandling(wkbcommon.EmptyPointHandlingNaN))
-	if err != nil {
-		return geopb.SpatialObject{}, err
-	}
-	AdjustGeomTSRID(t, defaultSRID)
-	return spatialObjectFromGeomT(t, soType)
-}
-
 // parseGeoJSON takes given bytes assumed to be GeoJSON and transforms it into a SpatialObject.
 func parseGeoJSON(
 	soType geopb.SpatialObjectType, b []byte, defaultSRID geopb.SRID,

postgres/parser/json/json.go

@@ -64,8 +64,8 @@ const (
 const (
 	wordSize          = unsafe.Sizeof(big.Word(0))
 	decimalSize       = unsafe.Sizeof(apd.Decimal{})
-	stringHeaderSize  = unsafe.Sizeof(reflect.StringHeader{})
-	sliceHeaderSize   = unsafe.Sizeof(reflect.SliceHeader{})
+	stringHeaderSize  = unsafe.Sizeof(reflect.StringHeader{}) //lint:ignore SA1019 Still useful here
+	sliceHeaderSize   = unsafe.Sizeof(reflect.SliceHeader{})  //lint:ignore SA1019 Still useful here
 	keyValuePairSize  = unsafe.Sizeof(jsonKeyValuePair{})
 	jsonInterfaceSize = unsafe.Sizeof((JSON)(nil))
 )
@@ -1423,12 +1423,8 @@ func (j jsonObject) RemoveString(s string) (JSON, bool, error) {
 	}
 
 	newVal := make([]jsonKeyValuePair, len(j)-1)
-	for i, elem := range j[:idx] {
-		newVal[i] = elem
-	}
-	for i, elem := range j[idx+1:] {
-		newVal[idx+i] = elem
-	}
+	copy(newVal, j[:idx])
+	copy(newVal[idx:], j[idx+1:])
 	return jsonObject(newVal), true, nil
 }
 
@@ -1487,9 +1483,7 @@ func scalarConcat(left, other JSON) (JSON, error) {
 		right := decoded.(jsonArray)
 		result := make(jsonArray, len(right)+1)
 		result[0] = left
-		for i := range right {
-			result[i+1] = right[i]
-		}
+		copy(result[1:], right)
 		return result, nil
 	case ObjectJSONType:
 		return nil, errInvalidConcat
@@ -1514,18 +1508,12 @@ func (j jsonArray) Concat(other JSON) (JSON, error) {
 		}
 		right := decoded.(jsonArray)
 		result := make(jsonArray, len(left)+len(right))
-		for i := range left {
-			result[i] = left[i]
-		}
-		for i := range right {
-			result[len(left)+i] = right[i]
-		}
+		copy(result, left)
+		copy(result[len(left):], right)
 		return result, nil
 	default:
 		result := make(jsonArray, len(left)+1)
-		for i := range left {
-			result[i] = left[i]
-		}
+		copy(result, left)
 		result[len(left)] = other
 		return result, nil
 	}
@@ -1833,9 +1821,7 @@ func (j jsonArray) doRemovePath(path []string) (JSON, bool, error) {
 	}
 
 	result := make(jsonArray, len(j))
-	for i := range j {
-		result[i] = j[i]
-	}
+	copy(result, j)
 	result[idx] = newVal
 
 	return result, true, nil
@@ -1862,9 +1848,7 @@ func (j jsonObject) doRemovePath(path []string) (JSON, bool, error) {
 	}
 
 	result := make(jsonObject, len(j))
-	for i := range j {
-		result[i] = j[i]
-	}
+	copy(result, j)
 	result[idx].v = newVal
 
 	return result, true, nil

postgres/parser/parser/help.go

@@ -33,6 +33,8 @@ import (
 	"text/tabwriter"
 
 	"github.com/cockroachdb/errors"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 
 	"github.com/dolthub/doltgresql/postgres/parser/pgcode"
 	"github.com/dolthub/doltgresql/postgres/parser/pgerror"
@@ -240,7 +242,7 @@ var AllHelp = func(h map[string]HelpMessageBody) string {
 	var buf bytes.Buffer
 	w := tabwriter.NewWriter(&buf, 0, 0, 1, ' ', 0)
 	for _, cat := range categories {
-		fmt.Fprintf(w, "%s:\n", strings.Title(cat))
+		fmt.Fprintf(w, "%s:\n", cases.Title(language.English).String(cat))
 		for _, item := range cmds[cat] {
 			fmt.Fprintf(w, "\t\t%s\t%s\n", item, h[item].ShortDescription)
 		}

postgres/parser/protoutil/marshaler.go

@@ -26,7 +26,6 @@ package protoutil
 
 import (
 	"io"
-	"io/ioutil"
 
 	"github.com/cockroachdb/errors"
 	"github.com/gogo/protobuf/proto"
@@ -72,7 +71,7 @@ func (*ProtoPb) NewDecoder(r io.Reader) gwruntime.Decoder {
 		// NB: we use proto.Message here because grpc-gateway passes us protos that
 		// we don't control and thus don't implement protoutil.Message.
 		if p, ok := v.(proto.Message); ok {
-			bytes, err := ioutil.ReadAll(r)
+			bytes, err := io.ReadAll(r)
 			if err == nil {
 				err = proto.Unmarshal(bytes, p)
 			}