diff --git a/Makefile b/Makefile index 24cf720f10..6ed5b46d19 100644 --- a/Makefile +++ b/Makefile @@ -142,20 +142,25 @@ lint-fix: getdeps @$(INSTALL_PATH)/gofumpt -l -w internal/ @$(INSTALL_PATH)/gofumpt -l -w cmd/ @$(INSTALL_PATH)/gofumpt -l -w pkg/ + @$(INSTALL_PATH)/gofumpt -l -w client/ @$(INSTALL_PATH)/gofumpt -l -w tests/integration/ @echo "Running gci fix" @$(INSTALL_PATH)/gci write cmd/ --skip-generated -s standard -s default -s "prefix(github.com/milvus-io)" --custom-order @$(INSTALL_PATH)/gci write internal/ --skip-generated -s standard -s default -s "prefix(github.com/milvus-io)" --custom-order @$(INSTALL_PATH)/gci write pkg/ --skip-generated -s standard -s default -s "prefix(github.com/milvus-io)" --custom-order + @$(INSTALL_PATH)/gci write client/ --skip-generated -s standard -s default -s "prefix(github.com/milvus-io)" --custom-order @$(INSTALL_PATH)/gci write tests/ --skip-generated -s standard -s default -s "prefix(github.com/milvus-io)" --custom-order @echo "Running golangci-lint auto-fix" - @source $(PWD)/scripts/setenv.sh && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --fix --timeout=30m --config $(PWD)/.golangci.yml; cd pkg && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --fix --timeout=30m --config $(PWD)/.golangci.yml + @source $(PWD)/scripts/setenv.sh && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --fix --timeout=30m --config $(PWD)/.golangci.yml; + @source $(PWD)/scripts/setenv.sh && cd pkg && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --fix --timeout=30m --config $(PWD)/.golangci.yml + @source $(PWD)/scripts/setenv.sh && cd client && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --fix --timeout=30m --config $(PWD)/client/.golangci.yml #TODO: Check code specifications by golangci-lint static-check: getdeps @echo "Running $@ check" @source $(PWD)/scripts/setenv.sh && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --timeout=30m --config $(PWD)/.golangci.yml @source $(PWD)/scripts/setenv.sh && cd pkg && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --timeout=30m --config $(PWD)/.golangci.yml + @source $(PWD)/scripts/setenv.sh && cd client && GO111MODULE=on $(INSTALL_PATH)/golangci-lint run --timeout=30m --config $(PWD)/client/.golangci.yml verifiers: build-cpp getdeps cppcheck fmt static-check diff --git a/client/.golangci.yml b/client/.golangci.yml new file mode 100644 index 0000000000..5c90b6d694 --- /dev/null +++ b/client/.golangci.yml @@ -0,0 +1,172 @@ +run: + go: "1.20" + skip-dirs: + - build + - configs + - deployments + - docs + - scripts + - internal/core + - cmake_build + skip-files: + - partial_search_test.go + +linters: + disable-all: true + enable: + - gosimple + - govet + - ineffassign + - staticcheck + - decorder + - depguard + - gofmt + - goimports + - gosec + - revive + - unconvert + - misspell + - typecheck + - durationcheck + - forbidigo + - gci + - whitespace + - gofumpt + - gocritic + +linters-settings: + gci: + sections: + - standard + - default + - prefix(github.com/milvus-io) + custom-order: true + gofumpt: + lang-version: "1.18" + module-path: github.com/milvus-io + goimports: + local-prefixes: github.com/milvus-io + revive: + rules: + - name: unused-parameter + disabled: true + - name: var-naming + severity: warning + disabled: false + arguments: + - ["ID"] # Allow list + - name: context-as-argument + severity: warning + disabled: false + arguments: + - allowTypesBefore: "*testing.T" + - name: datarace + severity: warning + disabled: false + - name: duplicated-imports + severity: warning + disabled: false + - name: waitgroup-by-value + severity: warning + disabled: false + - name: indent-error-flow + severity: warning + disabled: false + arguments: + - "preserveScope" + - name: range-val-in-closure + severity: warning + disabled: false + - name: range-val-address + severity: warning + disabled: false + - name: string-of-int + severity: warning + disabled: false + misspell: + locale: US + gocritic: + enabled-checks: + - ruleguard + settings: + ruleguard: + failOnError: true + rules: "ruleguard/rules.go" + depguard: + rules: + main: + deny: + - pkg: "errors" + desc: not allowed, use github.com/cockroachdb/errors + - pkg: "github.com/pkg/errors" + desc: not allowed, use github.com/cockroachdb/errors + - pkg: "github.com/pingcap/errors" + desc: not allowed, use github.com/cockroachdb/errors + - pkg: "golang.org/x/xerrors" + desc: not allowed, use github.com/cockroachdb/errors + - pkg: "github.com/go-errors/errors" + desc: not allowed, use github.com/cockroachdb/errors + - pkg: "io/ioutil" + desc: ioutil is deprecated after 1.16, 1.17, use os and io package instead + - pkg: "github.com/tikv/client-go/rawkv" + desc: not allowed, use github.com/tikv/client-go/v2/txnkv + - pkg: "github.com/tikv/client-go/v2/rawkv" + desc: not allowed, use github.com/tikv/client-go/v2/txnkv + forbidigo: + forbid: + - '^time\.Tick$' + - 'return merr\.Err[a-zA-Z]+' + - 'merr\.Wrap\w+\(\)\.Error\(\)' + - '\.(ErrorCode|Reason) = ' + - 'Reason:\s+\w+\.Error\(\)' + - 'errors.New\((.+)\.GetReason\(\)\)' + - 'commonpb\.Status\{[\s\n]*ErrorCode:[\s\n]*.+[\s\S\n]*?\}' + - 'os\.Open\(.+\)' + - 'os\.ReadFile\(.+\)' + - 'os\.WriteFile\(.+\)' + - "runtime.NumCPU" + - "runtime.GOMAXPROCS(0)" + #- 'fmt\.Print.*' WIP + +issues: + exclude-use-default: false + exclude-rules: + - path: .+_test\.go + linters: + - forbidigo + exclude: + - should have a package comment + - should have comment + - should be of the form + - should not use dot imports + - which can be annoying to use + # Binds to all network interfaces + - G102 + # Use of unsafe calls should be audited + - G103 + # Errors unhandled + - G104 + # file/folder Permission + - G301 + - G302 + # Potential file inclusion via variable + - G304 + # Deferring unsafe method like *os.File Close + - G307 + # TLS MinVersion too low + - G402 + # Use of weak random number generator math/rand + - G404 + # Unused parameters + - SA1019 + # defer return errors + - SA5001 + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 + +service: + # use the fixed version to not introduce new linters unexpectedly + golangci-lint-version: 1.55.2 diff --git a/client/client_config.go b/client/client_config.go index 63a4f6d2b8..01f82877f7 100644 --- a/client/client_config.go +++ b/client/client_config.go @@ -10,10 +10,11 @@ import ( "time" "github.com/cockroachdb/errors" - "github.com/milvus-io/milvus/pkg/util/crypto" "google.golang.org/grpc" "google.golang.org/grpc/backoff" "google.golang.org/grpc/keepalive" + + "github.com/milvus-io/milvus/pkg/util/crypto" ) const ( diff --git a/client/client_test.go b/client/client_test.go index f23a9d9941..c6d0867ee8 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -31,7 +31,7 @@ func (s *ClientSuite) TestNewClient() { s.NotNil(c) }) - s.Run("emtpy_addr", func() { + s.Run("empty_addr", func() { _, err := New(ctx, &ClientConfig{}) s.Error(err) s.T().Log(err) diff --git a/client/column/columns.go b/client/column/columns.go index 8a2a52d879..a30b064e15 100644 --- a/client/column/columns.go +++ b/client/column/columns.go @@ -239,7 +239,7 @@ func FieldDataColumn(fd *schemapb.FieldData, begin, end int) (Column, error) { data := x.FloatVector.GetData() dim := int(vectors.GetDim()) if end < 0 { - end = int(len(data) / dim) + end = len(data) / dim } vector := make([][]float32, 0, end-begin) // shall not have remanunt for i := begin; i < end; i++ { @@ -262,7 +262,7 @@ func FieldDataColumn(fd *schemapb.FieldData, begin, end int) (Column, error) { dim := int(vectors.GetDim()) blen := dim / 8 if end < 0 { - end = int(len(data) / blen) + end = len(data) / blen } vector := make([][]byte, 0, end-begin) for i := begin; i < end; i++ { @@ -281,7 +281,7 @@ func FieldDataColumn(fd *schemapb.FieldData, begin, end int) (Column, error) { data := x.Float16Vector dim := int(vectors.GetDim()) if end < 0 { - end = int(len(data) / dim) + end = len(data) / dim } vector := make([][]byte, 0, end-begin) for i := begin; i < end; i++ { @@ -300,7 +300,7 @@ func FieldDataColumn(fd *schemapb.FieldData, begin, end int) (Column, error) { data := x.Bfloat16Vector dim := int(vectors.GetDim()) if end < 0 { - end = int(len(data) / dim) + end = len(data) / dim } vector := make([][]byte, 0, end-begin) // shall not have remanunt for i := begin; i < end; i++ { diff --git a/client/column/sparse.go b/client/column/sparse.go index b9d20fd616..cc02e3ee2f 100644 --- a/client/column/sparse.go +++ b/client/column/sparse.go @@ -22,6 +22,7 @@ import ( "math" "github.com/cockroachdb/errors" + "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" "github.com/milvus-io/milvus/client/v2/entity" ) diff --git a/client/column/sparse_test.go b/client/column/sparse_test.go index 387df9efe7..564f223ff1 100644 --- a/client/column/sparse_test.go +++ b/client/column/sparse_test.go @@ -21,9 +21,10 @@ import ( "math/rand" "testing" - "github.com/milvus-io/milvus/client/v2/entity" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/milvus-io/milvus/client/v2/entity" ) func TestColumnSparseEmbedding(t *testing.T) { diff --git a/client/column/varchar.go b/client/column/varchar.go index 9ed1646450..63aff96ae9 100644 --- a/client/column/varchar.go +++ b/client/column/varchar.go @@ -17,9 +17,10 @@ package column import ( - "errors" "fmt" + "github.com/cockroachdb/errors" + "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" "github.com/milvus-io/milvus/client/v2/entity" ) @@ -70,7 +71,7 @@ func (c *ColumnVarChar) FieldData() *schemapb.FieldData { } data := make([]string, 0, c.Len()) for i := 0; i < c.Len(); i++ { - data = append(data, string(c.values[i])) + data = append(data, c.values[i]) } fd.Field = &schemapb.FieldData_Scalars{ Scalars: &schemapb.ScalarField{ diff --git a/client/database_test.go b/client/database_test.go index f46a0cafb8..d7555d7d5a 100644 --- a/client/database_test.go +++ b/client/database_test.go @@ -5,11 +5,12 @@ import ( "fmt" "testing" + mock "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus/pkg/util/merr" - mock "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" ) type DatabaseSuite struct { diff --git a/client/entity/sparse.go b/client/entity/sparse.go index 00f41c60d3..2bded8f6e8 100644 --- a/client/entity/sparse.go +++ b/client/entity/sparse.go @@ -56,7 +56,7 @@ func (e sliceSparseEmbedding) FieldType() FieldType { } func (e sliceSparseEmbedding) Get(idx int) (uint32, float32, bool) { - if idx < 0 || idx >= int(e.len) { + if idx < 0 || idx >= e.len { return 0, 0, false } return e.positions[idx], e.values[idx], true diff --git a/client/example/database/main.go b/client/example/database/main.go index 5b978b6261..0069923d9a 100644 --- a/client/example/database/main.go +++ b/client/example/database/main.go @@ -5,6 +5,7 @@ import ( "log" milvusclient "github.com/milvus-io/milvus/client/v2" + "github.com/milvus-io/milvus/client/v2/entity" ) const ( @@ -34,4 +35,55 @@ func main() { log.Fatal("failed to list databases", err.Error()) } log.Println("=== Databases: ", dbNames) + + schema := entity.NewSchema().WithName("hello_milvus"). + WithField(entity.NewField().WithName("ID").WithDataType(entity.FieldTypeInt64).WithIsPrimaryKey(true)). + WithField(entity.NewField().WithName("Vector").WithDataType(entity.FieldTypeFloatVector).WithDim(128)) + + if err := c.CreateCollection(ctx, milvusclient.NewCreateCollectionOption("hello_milvus", schema)); err != nil { + log.Fatal("failed to create collection:", err.Error()) + } + + collections, err := c.ListCollections(ctx, milvusclient.NewListCollectionOption()) + if err != nil { + log.Fatal("failed to list collections,", err.Error()) + } + + for _, collectionName := range collections { + collection, err := c.DescribeCollection(ctx, milvusclient.NewDescribeCollectionOption(collectionName)) + if err != nil { + log.Fatal(err.Error()) + } + log.Println(collection.Name) + for _, field := range collection.Schema.Fields { + log.Println("=== Field: ", field.Name, field.DataType, field.AutoID) + } + } + + c.CreateDatabase(ctx, milvusclient.NewCreateDatabaseOption("test")) + c.UsingDatabase(ctx, milvusclient.NewUsingDatabaseOption("test")) + + schema = entity.NewSchema().WithName("hello_milvus"). + WithField(entity.NewField().WithName("ID").WithDataType(entity.FieldTypeVarChar).WithMaxLength(64).WithIsPrimaryKey(true)). + WithField(entity.NewField().WithName("Vector").WithDataType(entity.FieldTypeFloatVector).WithDim(128)) + + if err := c.CreateCollection(ctx, milvusclient.NewCreateCollectionOption("hello_milvus", schema)); err != nil { + log.Fatal("failed to create collection:", err.Error()) + } + + collections, err = c.ListCollections(ctx, milvusclient.NewListCollectionOption()) + if err != nil { + log.Fatal("failed to list collections,", err.Error()) + } + + for _, collectionName := range collections { + collection, err := c.DescribeCollection(ctx, milvusclient.NewDescribeCollectionOption(collectionName)) + if err != nil { + log.Fatal(err.Error()) + } + log.Println(collection.Name) + for _, field := range collection.Schema.Fields { + log.Println("=== Field: ", field.Name, field.DataType, field.AutoID) + } + } } diff --git a/client/example/playground/main.go b/client/example/playground/main.go index e5984648cf..43ae57915c 100644 --- a/client/example/playground/main.go +++ b/client/example/playground/main.go @@ -18,6 +18,7 @@ const ( helloMilvusCmd = `hello_milvus` partitionsCmd = `partitions` indexCmd = `indexes` + countCmd = `count` milvusAddr = `localhost:19530` nEntities, dim = 3000, 128 @@ -38,9 +39,109 @@ func main() { Partitions() case indexCmd: Indexes() + case countCmd: + Count() } } +func Count() { + ctx := context.Background() + + collectionName := "hello_count_inverted" + + c, err := milvusclient.New(ctx, &milvusclient.ClientConfig{ + Address: "127.0.0.1:19530", + }) + if err != nil { + log.Fatal("failed to connect to milvus, err: ", err.Error()) + } + + schema := entity.NewSchema().WithName(collectionName). + WithField(entity.NewField().WithName("id").WithDataType(entity.FieldTypeInt64).WithIsAutoID(true).WithIsPrimaryKey(true)). + WithField(entity.NewField().WithName("vector").WithDataType(entity.FieldTypeFloatVector).WithDim(128)) + + err = c.CreateCollection(ctx, milvusclient.NewCreateCollectionOption(collectionName, schema)) + if err != nil { + log.Fatal("failed to connect to milvus, err: ", err.Error()) + } + + indexTask, err := c.CreateIndex(ctx, milvusclient.NewCreateIndexOption(collectionName, "id", index.NewGenericIndex("inverted", map[string]string{}))) + if err != nil { + log.Fatal("failed to connect to milvus, err: ", err.Error()) + } + + indexTask.Await(ctx) + + indexTask, err = c.CreateIndex(ctx, milvusclient.NewCreateIndexOption(collectionName, "vector", index.NewHNSWIndex(entity.L2, 16, 32))) + if err != nil { + log.Fatal("failed to connect to milvus, err: ", err.Error()) + } + + indexTask.Await(ctx) + + loadTask, err := c.LoadCollection(ctx, milvusclient.NewLoadCollectionOption(collectionName)) + if err != nil { + log.Fatal("faied to load collection, err: ", err.Error()) + } + loadTask.Await(ctx) + + for i := 0; i < 100; i++ { + // randomData := make([]int64, 0, nEntities) + vectorData := make([][]float32, 0, nEntities) + // generate data + for i := 0; i < nEntities; i++ { + // randomData = append(randomData, rand.Int63n(1000)) + vec := make([]float32, 0, dim) + for j := 0; j < dim; j++ { + vec = append(vec, rand.Float32()) + } + vectorData = append(vectorData, vec) + } + + _, err = c.Insert(ctx, milvusclient.NewColumnBasedInsertOption(collectionName).WithFloatVectorColumn("vector", dim, vectorData)) + if err != nil { + log.Fatal("failed to insert data") + } + + log.Println("start flush collection") + flushTask, err := c.Flush(ctx, milvusclient.NewFlushOption(collectionName)) + if err != nil { + log.Fatal("failed to flush", err.Error()) + } + start := time.Now() + err = flushTask.Await(ctx) + if err != nil { + log.Fatal("failed to flush", err.Error()) + } + log.Println("flush done, elapsed", time.Since(start)) + + result, err := c.Query(ctx, milvusclient.NewQueryOption(collectionName). + WithOutputFields([]string{"count(*)"}). + WithConsistencyLevel(entity.ClStrong)) + if err != nil { + log.Fatal("failed to connect to milvus, err: ", err.Error()) + } + for _, rs := range result.Fields { + log.Println(rs) + } + result, err = c.Query(ctx, milvusclient.NewQueryOption(collectionName). + WithOutputFields([]string{"count(*)"}). + WithFilter("id > 0"). + WithConsistencyLevel(entity.ClStrong)) + if err != nil { + log.Fatal("failed to connect to milvus, err: ", err.Error()) + } + for _, rs := range result.Fields { + log.Println(rs) + } + } + + // err = c.DropCollection(ctx, milvusclient.NewDropCollectionOption(collectionName)) + // if err != nil { + // log.Fatal("=== Failed to drop collection", err.Error()) + // } +} + func HelloMilvus() { ctx := context.Background() @@ -92,7 +193,7 @@ func HelloMilvus() { vectorData = append(vectorData, vec) } - err = c.Insert(ctx, milvusclient.NewColumnBasedInsertOption(collectionName).WithFloatVectorColumn("vector", dim, vectorData)) + _, err = c.Insert(ctx, milvusclient.NewColumnBasedInsertOption(collectionName).WithFloatVectorColumn("vector", dim, vectorData)) if err != nil { log.Fatal("failed to insert data") } @@ -107,22 +208,7 @@ func HelloMilvus() { if err != nil { log.Fatal("failed to flush", err.Error()) } - log.Println("flush done, elasped", time.Since(start)) - - indexTask, err := c.CreateIndex(ctx, milvusclient.NewCreateIndexOption(collectionName, "vector", index.NewHNSWIndex(entity.L2, 16, 100))) - if err != nil { - log.Fatal("failed to create index, err: ", err.Error()) - } - err = indexTask.Await(ctx) - if err != nil { - log.Fatal("failed to wait index construction complete") - } - - loadTask, err := c.LoadCollection(ctx, milvusclient.NewLoadCollectionOption(collectionName)) - if err != nil { - log.Fatal("failed to load collection", err.Error()) - } - loadTask.Await(ctx) + log.Println("flush done, elapsed", time.Since(start)) vec2search := []entity.Vector{ entity.FloatVector(vectorData[len(vectorData)-2]), diff --git a/client/go.mod b/client/go.mod index c0f6882c3d..79dce6b878 100644 --- a/client/go.mod +++ b/client/go.mod @@ -10,6 +10,7 @@ require ( github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 github.com/milvus-io/milvus-proto/go-api/v2 v2.3.4-0.20240430035521-259ae1d10016 github.com/milvus-io/milvus/pkg v0.0.2-0.20240317152703-17b4938985f3 + github.com/quasilyte/go-ruleguard/dsl v0.3.22 github.com/samber/lo v1.27.0 github.com/stretchr/testify v1.8.4 github.com/tidwall/gjson v1.17.1 diff --git a/client/go.sum b/client/go.sum index 1efeee2111..44e4615201 100644 --- a/client/go.sum +++ b/client/go.sum @@ -476,6 +476,8 @@ github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1 github.com/prometheus/procfs v0.9.0 h1:wzCHvIvM5SxWqYvwgVL7yJY8Lz3PKn49KQtpgMYJfhI= github.com/prometheus/procfs v0.9.0/go.mod h1:+pB4zwohETzFnmlpe6yd2lSc+0/46IYZRB/chUwxUZY= github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU= +github.com/quasilyte/go-ruleguard/dsl v0.3.22 h1:wd8zkOhSNr+I+8Qeciml08ivDt1pSXe60+5DqOpCjPE= +github.com/quasilyte/go-ruleguard/dsl v0.3.22/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= diff --git a/client/index.go b/client/index.go index 79dd57ed3e..7932048463 100644 --- a/client/index.go +++ b/client/index.go @@ -21,12 +21,13 @@ import ( "fmt" "time" + "google.golang.org/grpc" + "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus/client/v2/entity" "github.com/milvus-io/milvus/client/v2/index" "github.com/milvus-io/milvus/pkg/util/merr" - "google.golang.org/grpc" ) type CreateIndexTask struct { diff --git a/client/index_test.go b/client/index_test.go index ac9f5e4069..920457f9a2 100644 --- a/client/index_test.go +++ b/client/index_test.go @@ -22,14 +22,15 @@ import ( "testing" "time" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "go.uber.org/atomic" + "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus/client/v2/entity" "github.com/milvus-io/milvus/client/v2/index" "github.com/milvus-io/milvus/pkg/util/merr" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" - "go.uber.org/atomic" ) type IndexSuite struct { diff --git a/client/interceptors.go b/client/interceptors.go index 16396c4aed..6756a74895 100644 --- a/client/interceptors.go +++ b/client/interceptors.go @@ -20,12 +20,12 @@ import ( "context" "time" + grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" - grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" ) diff --git a/client/interceptors_test.go b/client/interceptors_test.go index e3bcb34fce..648575dbd4 100644 --- a/client/interceptors_test.go +++ b/client/interceptors_test.go @@ -28,9 +28,11 @@ import ( "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" ) -var mockInvokerError error -var mockInvokerReply interface{} -var mockInvokeTimes = 0 +var ( + mockInvokerError error + mockInvokerReply interface{} + mockInvokeTimes = 0 +) var mockInvoker grpc.UnaryInvoker = func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, opts ...grpc.CallOption) error { mockInvokeTimes++ diff --git a/client/maintenance_test.go b/client/maintenance_test.go index 333146f8ca..0efcd449df 100644 --- a/client/maintenance_test.go +++ b/client/maintenance_test.go @@ -22,13 +22,14 @@ import ( "testing" "time" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "go.uber.org/atomic" + "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" "github.com/milvus-io/milvus/pkg/util/merr" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" - "go.uber.org/atomic" ) type MaintenanceSuite struct { diff --git a/client/partition.go b/client/partition.go index 93036b2300..1848368717 100644 --- a/client/partition.go +++ b/client/partition.go @@ -19,9 +19,10 @@ package client import ( "context" + "google.golang.org/grpc" + "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus/pkg/util/merr" - "google.golang.org/grpc" ) // CreatePartition is the API for creating a partition for a collection. diff --git a/client/partition_test.go b/client/partition_test.go index 2c6c4e2ed8..7bd7cd7436 100644 --- a/client/partition_test.go +++ b/client/partition_test.go @@ -21,11 +21,12 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus/pkg/util/merr" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" ) type PartitionSuite struct { diff --git a/client/read.go b/client/read.go index 3aeaff769d..1907ed8e07 100644 --- a/client/read.go +++ b/client/read.go @@ -19,9 +19,9 @@ package client import ( "context" + "github.com/cockroachdb/errors" "google.golang.org/grpc" - "github.com/cockroachdb/errors" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" "github.com/milvus-io/milvus/client/v2/column" diff --git a/client/read_options.go b/client/read_options.go index a1f563bfc0..2bdaf78a55 100644 --- a/client/read_options.go +++ b/client/read_options.go @@ -21,6 +21,7 @@ import ( "strconv" "github.com/golang/protobuf/proto" + "github.com/milvus-io/milvus-proto/go-api/v2/commonpb" "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus/client/v2/entity" diff --git a/client/read_test.go b/client/read_test.go index 6606226d1b..0e815a0563 100644 --- a/client/read_test.go +++ b/client/read_test.go @@ -6,13 +6,14 @@ import ( "math/rand" "testing" + "github.com/samber/lo" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" "github.com/milvus-io/milvus/client/v2/entity" "github.com/milvus-io/milvus/pkg/util/merr" - "github.com/samber/lo" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" ) type ReadSuite struct { diff --git a/client/ruleguard/rules.go b/client/ruleguard/rules.go new file mode 100644 index 0000000000..5bc3422c9b --- /dev/null +++ b/client/ruleguard/rules.go @@ -0,0 +1,409 @@ +// Licensed to the LF AI & Data foundation under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gorules + +import ( + "github.com/quasilyte/go-ruleguard/dsl" +) + +// This is a collection of rules for ruleguard: https://github.com/quasilyte/go-ruleguard + +// Remove extra conversions: mdempsky/unconvert +func unconvert(m dsl.Matcher) { + m.Match("int($x)").Where(m["x"].Type.Is("int") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + + m.Match("float32($x)").Where(m["x"].Type.Is("float32") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("float64($x)").Where(m["x"].Type.Is("float64") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + + // m.Match("byte($x)").Where(m["x"].Type.Is("byte")).Report("unnecessary conversion").Suggest("$x") + // m.Match("rune($x)").Where(m["x"].Type.Is("rune")).Report("unnecessary conversion").Suggest("$x") + m.Match("bool($x)").Where(m["x"].Type.Is("bool") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + + m.Match("int8($x)").Where(m["x"].Type.Is("int8") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("int16($x)").Where(m["x"].Type.Is("int16") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("int32($x)").Where(m["x"].Type.Is("int32") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("int64($x)").Where(m["x"].Type.Is("int64") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + + m.Match("uint8($x)").Where(m["x"].Type.Is("uint8") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("uint16($x)").Where(m["x"].Type.Is("uint16") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("uint32($x)").Where(m["x"].Type.Is("uint32") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + m.Match("uint64($x)").Where(m["x"].Type.Is("uint64") && !m["x"].Const).Report("unnecessary conversion").Suggest("$x") + + m.Match("time.Duration($x)").Where(m["x"].Type.Is("time.Duration") && !m["x"].Text.Matches("^[0-9]*$")).Report("unnecessary conversion").Suggest("$x") +} + +// Don't use == or != with time.Time +// https://github.com/dominikh/go-tools/issues/47 : Wontfix +func timeeq(m dsl.Matcher) { + m.Match("$t0 == $t1").Where(m["t0"].Type.Is("time.Time")).Report("using == with time.Time") + m.Match("$t0 != $t1").Where(m["t0"].Type.Is("time.Time")).Report("using != with time.Time") + m.Match(`map[$k]$v`).Where(m["k"].Type.Is("time.Time")).Report("map with time.Time keys are easy to misuse") +} + +// err but no an error +func errnoterror(m dsl.Matcher) { + // Would be easier to check for all err identifiers instead, but then how do we get the type from m[] ? + + m.Match( + "if $*_, err := $x; $err != nil { $*_ } else if $_ { $*_ }", + "if $*_, err := $x; $err != nil { $*_ } else { $*_ }", + "if $*_, err := $x; $err != nil { $*_ }", + + "if $*_, err = $x; $err != nil { $*_ } else if $_ { $*_ }", + "if $*_, err = $x; $err != nil { $*_ } else { $*_ }", + "if $*_, err = $x; $err != nil { $*_ }", + + "$*_, err := $x; if $err != nil { $*_ } else if $_ { $*_ }", + "$*_, err := $x; if $err != nil { $*_ } else { $*_ }", + "$*_, err := $x; if $err != nil { $*_ }", + + "$*_, err = $x; if $err != nil { $*_ } else if $_ { $*_ }", + "$*_, err = $x; if $err != nil { $*_ } else { $*_ }", + "$*_, err = $x; if $err != nil { $*_ }", + ). + Where(m["err"].Text == "err" && !m["err"].Type.Is("error") && m["x"].Text != "recover()"). + Report("err variable not error type") +} + +// Identical if and else bodies +func ifbodythenbody(m dsl.Matcher) { + m.Match("if $*_ { $body } else { $body }"). + Report("identical if and else bodies") + + // Lots of false positives. + // m.Match("if $*_ { $body } else if $*_ { $body }"). + // Report("identical if and else bodies") +} + +// Odd inequality: A - B < 0 instead of != +// Too many false positives. +/* +func subtractnoteq(m dsl.Matcher) { + m.Match("$a - $b < 0").Report("consider $a != $b") + m.Match("$a - $b > 0").Report("consider $a != $b") + m.Match("0 < $a - $b").Report("consider $a != $b") + m.Match("0 > $a - $b").Report("consider $a != $b") +} +*/ + +// Self-assignment +func selfassign(m dsl.Matcher) { + m.Match("$x = $x").Report("useless self-assignment") +} + +// Odd nested ifs +func oddnestedif(m dsl.Matcher) { + m.Match("if $x { if $x { $*_ }; $*_ }", + "if $x == $y { if $x != $y {$*_ }; $*_ }", + "if $x != $y { if $x == $y {$*_ }; $*_ }", + "if $x { if !$x { $*_ }; $*_ }", + "if !$x { if $x { $*_ }; $*_ }"). + Report("odd nested ifs") + + m.Match("for $x { if $x { $*_ }; $*_ }", + "for $x == $y { if $x != $y {$*_ }; $*_ }", + "for $x != $y { if $x == $y {$*_ }; $*_ }", + "for $x { if !$x { $*_ }; $*_ }", + "for !$x { if $x { $*_ }; $*_ }"). + Report("odd nested for/ifs") +} + +// odd bitwise expressions +func oddbitwise(m dsl.Matcher) { + m.Match("$x | $x", + "$x | ^$x", + "^$x | $x"). + Report("odd bitwise OR") + + m.Match("$x & $x", + "$x & ^$x", + "^$x & $x"). + Report("odd bitwise AND") + + m.Match("$x &^ $x"). + Report("odd bitwise AND-NOT") +} + +// odd sequence of if tests with return +func ifreturn(m dsl.Matcher) { + m.Match("if $x { return $*_ }; if $x {$*_ }").Report("odd sequence of if test") + m.Match("if $x { return $*_ }; if !$x {$*_ }").Report("odd sequence of if test") + m.Match("if !$x { return $*_ }; if $x {$*_ }").Report("odd sequence of if test") + m.Match("if $x == $y { return $*_ }; if $x != $y {$*_ }").Report("odd sequence of if test") + m.Match("if $x != $y { return $*_ }; if $x == $y {$*_ }").Report("odd sequence of if test") +} + +func oddifsequence(m dsl.Matcher) { + /* + m.Match("if $x { $*_ }; if $x {$*_ }").Report("odd sequence of if test") + + m.Match("if $x == $y { $*_ }; if $y == $x {$*_ }").Report("odd sequence of if tests") + m.Match("if $x != $y { $*_ }; if $y != $x {$*_ }").Report("odd sequence of if tests") + + m.Match("if $x < $y { $*_ }; if $y > $x {$*_ }").Report("odd sequence of if tests") + m.Match("if $x <= $y { $*_ }; if $y >= $x {$*_ }").Report("odd sequence of if tests") + + m.Match("if $x > $y { $*_ }; if $y < $x {$*_ }").Report("odd sequence of if tests") + m.Match("if $x >= $y { $*_ }; if $y <= $x {$*_ }").Report("odd sequence of if tests") + */ +} + +// odd sequence of nested if tests +func nestedifsequence(m dsl.Matcher) { + /* + m.Match("if $x < $y { if $x >= $y {$*_ }; $*_ }").Report("odd sequence of nested if tests") + m.Match("if $x <= $y { if $x > $y {$*_ }; $*_ }").Report("odd sequence of nested if tests") + m.Match("if $x > $y { if $x <= $y {$*_ }; $*_ }").Report("odd sequence of nested if tests") + m.Match("if $x >= $y { if $x < $y {$*_ }; $*_ }").Report("odd sequence of nested if tests") + */ +} + +// odd sequence of assignments +func identicalassignments(m dsl.Matcher) { + m.Match("$x = $y; $y = $x").Report("odd sequence of assignments") +} + +func oddcompoundop(m dsl.Matcher) { + m.Match("$x += $x + $_", + "$x += $x - $_"). + Report("odd += expression") + + m.Match("$x -= $x + $_", + "$x -= $x - $_"). + Report("odd -= expression") +} + +func constswitch(m dsl.Matcher) { + m.Match("switch $x { $*_ }", "switch $*_; $x { $*_ }"). + Where(m["x"].Const && !m["x"].Text.Matches(`^runtime\.`)). + Report("constant switch") +} + +func oddcomparisons(m dsl.Matcher) { + m.Match( + "$x - $y == 0", + "$x - $y != 0", + "$x - $y < 0", + "$x - $y <= 0", + "$x - $y > 0", + "$x - $y >= 0", + "$x ^ $y == 0", + "$x ^ $y != 0", + ).Report("odd comparison") +} + +func oddmathbits(m dsl.Matcher) { + m.Match( + "64 - bits.LeadingZeros64($x)", + "32 - bits.LeadingZeros32($x)", + "16 - bits.LeadingZeros16($x)", + "8 - bits.LeadingZeros8($x)", + ).Report("odd math/bits expression: use bits.Len*() instead?") +} + +// func floateq(m dsl.Matcher) { +// m.Match( +// "$x == $y", +// "$x != $y", +// ). +// Where(m["x"].Type.Is("float32") && !m["x"].Const && !m["y"].Text.Matches("0(.0+)?") && !m.File().Name.Matches("floating_comparision.go")). +// Report("floating point tested for equality") + +// m.Match( +// "$x == $y", +// "$x != $y", +// ). +// Where(m["x"].Type.Is("float64") && !m["x"].Const && !m["y"].Text.Matches("0(.0+)?") && !m.File().Name.Matches("floating_comparision.go")). +// Report("floating point tested for equality") + +// m.Match("switch $x { $*_ }", "switch $*_; $x { $*_ }"). +// Where(m["x"].Type.Is("float32")). +// Report("floating point as switch expression") + +// m.Match("switch $x { $*_ }", "switch $*_; $x { $*_ }"). +// Where(m["x"].Type.Is("float64")). +// Report("floating point as switch expression") + +// } + +func badexponent(m dsl.Matcher) { + m.Match( + "2 ^ $x", + "10 ^ $x", + ). + Report("caret (^) is not exponentiation") +} + +func floatloop(m dsl.Matcher) { + m.Match( + "for $i := $x; $i < $y; $i += $z { $*_ }", + "for $i = $x; $i < $y; $i += $z { $*_ }", + ). + Where(m["i"].Type.Is("float64")). + Report("floating point for loop counter") + + m.Match( + "for $i := $x; $i < $y; $i += $z { $*_ }", + "for $i = $x; $i < $y; $i += $z { $*_ }", + ). + Where(m["i"].Type.Is("float32")). + Report("floating point for loop counter") +} + +func urlredacted(m dsl.Matcher) { + m.Match( + "log.Println($x, $*_)", + "log.Println($*_, $x, $*_)", + "log.Println($*_, $x)", + "log.Printf($*_, $x, $*_)", + "log.Printf($*_, $x)", + + "log.Println($x, $*_)", + "log.Println($*_, $x, $*_)", + "log.Println($*_, $x)", + "log.Printf($*_, $x, $*_)", + "log.Printf($*_, $x)", + ). + Where(m["x"].Type.Is("*url.URL")). + Report("consider $x.Redacted() when outputting URLs") +} + +func sprinterr(m dsl.Matcher) { + m.Match(`fmt.Sprint($err)`, + `fmt.Sprintf("%s", $err)`, + `fmt.Sprintf("%v", $err)`, + ). + Where(m["err"].Type.Is("error")). + Report("maybe call $err.Error() instead of fmt.Sprint()?") +} + +// disable this check, because it can not apply to generic type +//func largeloopcopy(m dsl.Matcher) { +// m.Match( +// `for $_, $v := range $_ { $*_ }`, +// ). +// Where(m["v"].Type.Size > 1024). +// Report(`loop copies large value each iteration`) +//} + +func joinpath(m dsl.Matcher) { + m.Match( + `strings.Join($_, "/")`, + `strings.Join($_, "\\")`, + "strings.Join($_, `\\`)", + ). + Report(`did you mean path.Join() or filepath.Join() ?`) +} + +func readfull(m dsl.Matcher) { + m.Match(`$n, $err := io.ReadFull($_, $slice) + if $err != nil || $n != len($slice) { + $*_ + }`, + `$n, $err := io.ReadFull($_, $slice) + if $n != len($slice) || $err != nil { + $*_ + }`, + `$n, $err = io.ReadFull($_, $slice) + if $err != nil || $n != len($slice) { + $*_ + }`, + `$n, $err = io.ReadFull($_, $slice) + if $n != len($slice) || $err != nil { + $*_ + }`, + `if $n, $err := io.ReadFull($_, $slice); $n != len($slice) || $err != nil { + $*_ + }`, + `if $n, $err := io.ReadFull($_, $slice); $err != nil || $n != len($slice) { + $*_ + }`, + `if $n, $err = io.ReadFull($_, $slice); $n != len($slice) || $err != nil { + $*_ + }`, + `if $n, $err = io.ReadFull($_, $slice); $err != nil || $n != len($slice) { + $*_ + }`, + ).Report("io.ReadFull() returns err == nil iff n == len(slice)") +} + +func nilerr(m dsl.Matcher) { + m.Match( + `if err == nil { return err }`, + `if err == nil { return $*_, err }`, + ). + Report(`return nil error instead of nil value`) +} + +func mailaddress(m dsl.Matcher) { + m.Match( + "fmt.Sprintf(`\"%s\" <%s>`, $NAME, $EMAIL)", + "fmt.Sprintf(`\"%s\"<%s>`, $NAME, $EMAIL)", + "fmt.Sprintf(`%s <%s>`, $NAME, $EMAIL)", + "fmt.Sprintf(`%s<%s>`, $NAME, $EMAIL)", + `fmt.Sprintf("\"%s\"<%s>", $NAME, $EMAIL)`, + `fmt.Sprintf("\"%s\" <%s>", $NAME, $EMAIL)`, + `fmt.Sprintf("%s<%s>", $NAME, $EMAIL)`, + `fmt.Sprintf("%s <%s>", $NAME, $EMAIL)`, + ). + Report("use net/mail Address.String() instead of fmt.Sprintf()"). + Suggest("(&mail.Address{Name:$NAME, Address:$EMAIL}).String()") +} + +func errnetclosed(m dsl.Matcher) { + m.Match( + `strings.Contains($err.Error(), $text)`, + ). + Where(m["text"].Text.Matches("\".*closed network connection.*\"")). + Report(`String matching against error texts is fragile; use net.ErrClosed instead`). + Suggest(`errors.Is($err, net.ErrClosed)`) +} + +func httpheaderadd(m dsl.Matcher) { + m.Match( + `$H.Add($KEY, $VALUE)`, + ). + Where(m["H"].Type.Is("http.Header")). + Report("use http.Header.Set method instead of Add to overwrite all existing header values"). + Suggest(`$H.Set($KEY, $VALUE)`) +} + +func hmacnew(m dsl.Matcher) { + m.Match("hmac.New(func() hash.Hash { return $x }, $_)", + `$f := func() hash.Hash { return $x } + $*_ + hmac.New($f, $_)`, + ).Where(m["x"].Pure). + Report("invalid hash passed to hmac.New()") +} + +func writestring(m dsl.Matcher) { + m.Match(`io.WriteString($w, string($b))`). + Where(m["b"].Type.Is("[]byte")). + Suggest("$w.Write($b)") +} + +func badlock(m dsl.Matcher) { + // Shouldn't give many false positives without type filter + // as Lock+Unlock pairs in combination with defer gives us pretty + // a good chance to guess correctly. If we constrain the type to sync.Mutex + // then it'll be harder to match embedded locks and custom methods + // that may forward the call to the sync.Mutex (or other synchronization primitive). + + m.Match(`$mu.Lock(); defer $mu.RUnlock()`).Report(`maybe $mu.RLock() was intended?`) + m.Match(`$mu.RLock(); defer $mu.Unlock()`).Report(`maybe $mu.Lock() was intended?`) +} diff --git a/client/write_test.go b/client/write_test.go index 3fdb9ece0f..a87957e615 100644 --- a/client/write_test.go +++ b/client/write_test.go @@ -22,13 +22,14 @@ import ( "math/rand" "testing" + "github.com/samber/lo" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + "github.com/milvus-io/milvus-proto/go-api/v2/milvuspb" "github.com/milvus-io/milvus-proto/go-api/v2/schemapb" "github.com/milvus-io/milvus/client/v2/entity" "github.com/milvus-io/milvus/pkg/util/merr" - "github.com/samber/lo" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" ) type WriteSuite struct {