Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added static analyzer & linter #79

Merged
merged 1 commit into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/ci-staticcheck.yaml
Original file line number Diff line number Diff line change
@@ -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 ./...
27 changes: 4 additions & 23 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ package main

import (
"context"
crand "crypto/rand"
"encoding/binary"
"fmt"
"math/rand"
"os"
"strconv"
"strings"
Expand Down Expand Up @@ -69,7 +66,6 @@ const stdOutAndErrFlag = "--out-and-err"

func main() {
ctx := context.Background()
seedGlobalRand()

args := os.Args[1:]

Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Hydrocharged marked this conversation as resolved.
Show resolved Hide resolved
dataDir, hasDataDir := apr.GetValue(commands.DataDirFlag)
if hasDataDir {
// If a relative path was provided, this ensures we have an absolute path everywhere.
Expand All @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -515,5 +496,5 @@ func emitUsageEvent(dEnv *env.DoltEnv) {
return
}

err = emitter.LogEvents(server.Version, clientEvents)
_ = emitter.LogEvents(server.Version, clientEvents)
}
4 changes: 2 additions & 2 deletions postgres/connection/message_decode_encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 0 additions & 18 deletions postgres/parser/encoding/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ package encoding
import (
"bytes"
"fmt"
"math"
"math/big"
"unsafe"

Expand Down Expand Up @@ -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)))

Expand Down
22 changes: 2 additions & 20 deletions postgres/parser/encoding/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"bytes"
"encoding/binary"
"fmt"
"reflect"
"unsafe"

"github.com/cockroachdb/apd/v2"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 0 additions & 14 deletions postgres/parser/geo/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
36 changes: 10 additions & 26 deletions postgres/parser/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion postgres/parser/parser/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions postgres/parser/protoutil/marshaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ package protoutil

import (
"io"
"io/ioutil"

"github.com/cockroachdb/errors"
"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -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)
}
Expand Down
Loading