linter, executor: fix the constructor linter and add it to BaseExecutor/BaseExecutorV2

build/linter/constructor/analyzer.go

@@ -46,7 +46,7 @@ var Analyzer = &analysis.Analyzer{
 	Run:      run,
 }
 
-func getConstructorList(t types.Type) []string {
+func getConstructorList(t types.Type, ignoreFields map[string]struct{}) []string {
 	structTyp, ok := t.(*types.Struct)
 	if !ok {
 		var ptr *types.Pointer
@@ -67,7 +67,12 @@ func getConstructorList(t types.Type) []string {
 		if !ok {
 			continue
 		}
-		if named.Obj().Name() == "Constructor" && named.Obj().Pkg().Path() == "github.com/pingcap/tidb/util/linter/constructor" {
+		if ignoreFields != nil {
+			if _, ok := ignoreFields[field.Name()]; ok {
+				continue
+			}
+		}
+		if named.Obj().Name() == "Constructor" && named.Obj().Pkg().Path() == "github.com/pingcap/tidb/pkg/util/linter/constructor" {
 			tags, err := structtag.Parse(structTyp.Tag(i))
 			// skip invalid tags
 			if err != nil {
@@ -82,7 +87,7 @@ func getConstructorList(t types.Type) []string {
 		}
 
 		if fieldStruct, ok := named.Underlying().(*types.Struct); ok {
-			ctors = append(ctors, getConstructorList(fieldStruct)...)
+			ctors = append(ctors, getConstructorList(fieldStruct, nil)...)
 		}
 	}
 	return ctors
@@ -116,7 +121,25 @@ func handleCompositeLit(pass *analysis.Pass, n *ast.CompositeLit, push bool, sta
 		return true
 	}
 
-	ctors := getConstructorList(t)
+	// Just ignore the specified fields. They'll be checked recursively later. In this round, we only need to avoid
+	// the case that the struct is implicitly initiated.
+	ignoreFields := make(map[string]struct{})
+	for i, elt := range n.Elts {
+		switch elt := elt.(type) {
+		case *ast.KeyValueExpr:
+			if ident, ok := elt.Key.(*ast.Ident); ok {
+				ignoreFields[ident.Name] = struct{}{}
+			}
+		default:
+			strctTyp, ok := t.(*types.Struct)
+			if !ok {
+				continue
+			}
+			ignoreFields[strctTyp.Field(i).Name()] = struct{}{}
+		}
+	}
+
+	ctors := getConstructorList(t, ignoreFields)
 	if len(ctors) == 0 {
 		return true
 	}
@@ -134,7 +157,7 @@ func handleCallExpr(pass *analysis.Pass, n *ast.CallExpr, push bool, stack []ast
 	}
 
 	t := pass.TypesInfo.TypeOf(n).Underlying()
-	ctors := getConstructorList(t)
+	ctors := getConstructorList(t, nil)
 	if len(ctors) == 0 {
 		return true
 	}
@@ -148,7 +171,12 @@ func handleValueSpec(pass *analysis.Pass, n *ast.ValueSpec, _ bool, stack []ast.
 		return true
 	}
 
-	ctors := getConstructorList(t.Underlying())
+	// allow declaring a pointer, as it's actually not constructed.
+	if _, ok := t.(*types.Pointer); ok {
+		return true
+	}
+
+	ctors := getConstructorList(t.Underlying(), nil)
 	if len(ctors) == 0 {
 		return true
 	}

build/linter/constructor/analyzer_test.go

@@ -28,6 +28,6 @@ import (
 
 func Test(t *testing.T) {
 	testdata := analysistest.TestData()
-	pkgs := []string{"t", "github.com/pingcap/tidb/util/linter/constructor"}
+	pkgs := []string{"t", "github.com/pingcap/tidb/pkg/util/linter/constructor"}
 	analysistest.Run(t, testdata, constructor.Analyzer, pkgs...)
 }

build/linter/constructor/testdata/src/github.com/pingcap/tidb/util/linter/constructor/BUILD.bazel → build/linter/constructor/testdata/src/github.com/pingcap/tidb/pkg/util/linter/constructor/BUILD.bazel

@@ -3,6 +3,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
 go_library(
     name = "constructor",
     srcs = ["constructorflag.go"],
-    importpath = "github.com/pingcap/tidb/build/linter/constructor/testdata/src/github.com/pingcap/tidb/util/linter/constructor",
+    importpath = "github.com/pingcap/tidb/build/linter/constructor/testdata/src/github.com/pingcap/tidb/pkg/util/linter/constructor",
     visibility = ["//visibility:public"],
 )

build/linter/constructor/testdata/src/t/construct.go

@@ -59,15 +59,25 @@ func otherFunction() {
 	_ = new(StructWithSpecificConstructor) // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
 
 	// var
-	var _ StructWithSpecificConstructor  // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
-	var _ *StructWithSpecificConstructor // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
+	var _ StructWithSpecificConstructor // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
+	var _ *StructWithSpecificConstructor
 
 	type compositeImplicitInitiate1 struct {
 		StructWithSpecificConstructor
 	}
 	var _ compositeImplicitInitiate1    // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
 	_ = new(compositeImplicitInitiate1) // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
 
+	// specified field is also allowed
+	_ = compositeImplicitInitiate1{
+		StructWithSpecificConstructor: *NewStructWithSpecificConstructor(),
+	}
+
+	// specified field with manually constructed struct is not allowed
+	_ = compositeImplicitInitiate1{
+		StructWithSpecificConstructor: StructWithSpecificConstructor{}, // want `struct can only be constructed in constructors NewStructWithSpecificConstructor, AnotherConstructor`
+	}
+
 	// pointer field is allowed
 	type compositeImplicitInitiate2 struct {
 		*StructWithSpecificConstructor

pkg/executor/builder.go

@@ -957,12 +957,11 @@ func (b *executorBuilder) buildInsert(v *plannercore.Insert) exec.Executor {
 	if b.err != nil {
 		return nil
 	}
-	var baseExec exec.BaseExecutor
+	var children []exec.Executor
 	if selectExec != nil {
-		baseExec = exec.NewBaseExecutor(b.ctx, nil, v.ID(), selectExec)
-	} else {
-		baseExec = exec.NewBaseExecutor(b.ctx, nil, v.ID())
+		children = append(children, selectExec)
 	}
+	baseExec := exec.NewBaseExecutor(b.ctx, nil, v.ID(), children...)
 	baseExec.SetInitCap(chunk.ZeroCapacity)
 
 	ivs := &InsertValues{
@@ -1016,17 +1015,16 @@ func (b *executorBuilder) buildImportInto(v *plannercore.ImportInto) exec.Execut
 
 	var (
 		selectExec exec.Executor
-		base       exec.BaseExecutor
+		children   []exec.Executor
 	)
 	if v.SelectPlan != nil {
 		selectExec = b.build(v.SelectPlan)
 		if b.err != nil {
 			return nil
 		}
-		base = exec.NewBaseExecutor(b.ctx, v.Schema(), v.ID(), selectExec)
-	} else {
-		base = exec.NewBaseExecutor(b.ctx, v.Schema(), v.ID())
+		children = append(children, selectExec)
 	}
+	base := exec.NewBaseExecutor(b.ctx, v.Schema(), v.ID(), children...)
 	executor, err := newImportIntoExec(base, selectExec, b.ctx, v, tbl)
 	if err != nil {
 		b.err = err

pkg/executor/detach.go

@@ -107,6 +107,7 @@ func (sCtx selectionExecutorContext) Detach() selectionExecutorContext {
 
 // Detach detaches the current executor from the session context.
 func (e *TableReaderExecutor) Detach() (exec.Executor, bool) {
+	//nolint:constructor
 	newExec := new(TableReaderExecutor)
 	*newExec = *e
 
@@ -117,6 +118,7 @@ func (e *TableReaderExecutor) Detach() (exec.Executor, bool) {
 
 // Detach detaches the current executor from the session context.
 func (e *IndexReaderExecutor) Detach() (exec.Executor, bool) {
+	//nolint:constructor
 	newExec := new(IndexReaderExecutor)
 	*newExec = *e
 
@@ -127,6 +129,7 @@ func (e *IndexReaderExecutor) Detach() (exec.Executor, bool) {
 
 // Detach detaches the current executor from the session context.
 func (e *IndexLookUpExecutor) Detach() (exec.Executor, bool) {
+	//nolint:constructor
 	newExec := new(IndexLookUpExecutor)
 	*newExec = *e
 
@@ -144,6 +147,7 @@ func (e *ProjectionExec) Detach() (exec.Executor, bool) {
 		return nil, false
 	}
 
+	//nolint:constructor
 	newExec := new(ProjectionExec)
 	*newExec = *e
 
@@ -163,6 +167,7 @@ func (e *SelectionExec) Detach() (exec.Executor, bool) {
 		}
 	}
 
+	//nolint:constructor
 	newExec := new(SelectionExec)
 	*newExec = *e
 

pkg/executor/detach_test.go

@@ -27,6 +27,7 @@ type mockSimpleExecutor struct {
 	exec.BaseExecutorV2
 }
 
+//nolint:constructor
 func TestDetachExecutor(t *testing.T) {
 	// call `Detach` on a mock executor will fail
 	_, ok := Detach(&mockSimpleExecutor{})

pkg/executor/internal/exec/BUILD.bazel

@@ -23,6 +23,7 @@ go_library(
         "//pkg/util",
         "//pkg/util/chunk",
         "//pkg/util/execdetails",
+        "//pkg/util/linter/constructor",
         "//pkg/util/topsql",
         "//pkg/util/topsql/state",
         "//pkg/util/tracing",

pkg/executor/internal/exec/executor.go

@@ -31,6 +31,7 @@ import (
 	"github.com/pingcap/tidb/pkg/util"
 	"github.com/pingcap/tidb/pkg/util/chunk"
 	"github.com/pingcap/tidb/pkg/util/execdetails"
+	"github.com/pingcap/tidb/pkg/util/linter/constructor"
 	"github.com/pingcap/tidb/pkg/util/topsql"
 	topsqlstate "github.com/pingcap/tidb/pkg/util/topsql/state"
 	"github.com/pingcap/tidb/pkg/util/tracing"
@@ -276,6 +277,8 @@ func newExecutorKillerHandler(handler signalHandler) executorKillerHandler {
 
 // BaseExecutorV2 is a simplified version of `BaseExecutor`, which doesn't contain a full session context
 type BaseExecutorV2 struct {
+	_ constructor.Constructor `ctor:"NewBaseExecutorV2,BuildNewBaseExecutorV2"`
+
 	executorMeta
 	executorKillerHandler
 	executorStats
@@ -352,6 +355,8 @@ func (e *BaseExecutorV2) BuildNewBaseExecutorV2(stmtRuntimeStatsColl *execdetail
 
 // BaseExecutor holds common information for executors.
 type BaseExecutor struct {
+	_ constructor.Constructor `ctor:"NewBaseExecutor"`
+
 	ctx sessionctx.Context
 
 	BaseExecutorV2

pkg/executor/sortexec/topn_spill_test.go

@@ -376,6 +376,7 @@ func severalChunksInDiskCase(t *testing.T, topnExec *sortexec.TopNExec) {
 }
 
 func TestGenerateTopNResultsWhenSpillOnlyOnce(t *testing.T) {
+	//nolint:constructor
 	topnExec := &sortexec.TopNExec{}
 	topnExec.Limit = &plannercore.PhysicalLimit{}
 

pkg/executor/test/seqtest/prepared_test.go

@@ -246,6 +246,7 @@ func TestPrepared(t *testing.T) {
 		tk.MustQuery("select a from prepare1;").Check(testkit.Rows("7"))
 
 		// Coverage.
+		//nolint:constructor
 		exec := &executor.ExecuteExec{}
 		err = exec.Next(ctx, nil)
 		require.NoError(t, err)

pkg/kv/key_test.go

@@ -396,7 +396,7 @@ func nativeIntMap(size int, handles []Handle) int {
 }
 
 func BenchmarkMemAwareHandleMap(b *testing.B) {
-	var sc stmtctx.StatementContext
+	sc := stmtctx.NewStmtCtx()
 	for _, s := range inputs {
 		handles := make([]Handle, s.input)
 		for i := 0; i < s.input; i++ {
@@ -418,7 +418,7 @@ func BenchmarkMemAwareHandleMap(b *testing.B) {
 }
 
 func BenchmarkNativeHandleMap(b *testing.B) {
-	var sc stmtctx.StatementContext
+	sc := stmtctx.NewStmtCtx()
 	for _, s := range inputs {
 		handles := make([]Handle, s.input)
 		for i := 0; i < s.input; i++ {

pkg/statistics/handle/updatetest/update_test.go

@@ -564,7 +564,7 @@ func TestSplitRange(t *testing.T) {
 			result:  "[8,9)",
 		},
 	}
-	sc := new(stmtctx.StatementContext)
+	sc := stmtctx.NewStmtCtx()
 	sc.SetTimeZone(time.UTC)
 	for _, test := range tests {
 		ranges := make([]*ranger.Range, 0, len(test.points)/2)