expression,*: remove the session context inside ParamMarker (#53534)

close #53533

build/BUILD.bazel

@@ -163,6 +163,7 @@ nogo(
                "//build/linter/noloopclosure",
                "//build/linter/prealloc",
                "//build/linter/predeclared",
+               "//build/linter/printexpression",
                "//build/linter/unconvert",
                "//build/linter/rowserrcheck",
                "//build/linter/toomanytests",

build/linter/printexpression/BUILD.bazel

@@ -0,0 +1,25 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "printexpression",
+    srcs = ["analyzer.go"],
+    importpath = "github.com/pingcap/tidb/build/linter/printexpression",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//build/linter/util",
+        "@org_golang_x_tools//go/analysis",
+        "@org_golang_x_tools//go/analysis/passes/inspect",
+        "@org_golang_x_tools//go/ast/inspector",
+    ],
+)
+
+go_test(
+    name = "printexpression_test",
+    timeout = "short",
+    srcs = ["analyzer_test.go"],
+    flaky = True,
+    deps = [
+        ":printexpression",
+        "@org_golang_x_tools//go/analysis/analysistest",
+    ],
+)

build/linter/printexpression/analyzer.go

@@ -0,0 +1,150 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed 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 printexpression
+
+import (
+	"go/ast"
+	"go/types"
+
+	"github.com/pingcap/tidb/build/linter/util"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+// Analyzer defines the linter for `emptynil` check
+//
+// This linter avoids calling `fmt.Println(expr)` or `fmt.Printf(\"%s\", expr)` directly, because
+// `Expression` doesn't implement `String()` method, so it will print the address or internal state
+// of the expression.
+// It handles the following function call:
+// 1. `fmt.Println(expr)`
+// 2. `fmt.Printf(\"%s\", expr)`
+// 4. `fmt.Sprintf(\"%s\", expr)`
+// 5. `(*Error).GenWithStack/GenWithStackByArgs/FastGen/FastGenByArgs`
+//
+// Every struct which implemented `StringWithCtx` but not implemented `String` cannot be used as an argument.
+var Analyzer = &analysis.Analyzer{
+	Name:     "printexpression",
+	Doc:      `Avoid printing expression directly.`,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	nodeFilter := []ast.Node{
+		(*ast.CallExpr)(nil),
+	}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		expr, ok := n.(*ast.CallExpr)
+		if !ok {
+			return
+		}
+
+		if !funcIsFormat(expr.Fun) {
+			return
+		}
+
+		for _, arg := range expr.Args {
+			if argIsNotAllowed(pass.TypesInfo, arg) {
+				pass.Reportf(arg.Pos(), "avoid printing expression directly. Please use `Expression.StringWithCtx()` to get a string")
+			}
+		}
+	})
+
+	return nil, nil
+}
+
+func funcIsFormat(x ast.Expr) bool {
+	switch x := x.(type) {
+	case *ast.SelectorExpr:
+		switch x.Sel.Name {
+		case "Printf", "Sprintf", "Println":
+			if i, ok := x.X.(*ast.Ident); ok {
+				return i.Name == "fmt"
+			}
+			return false
+		case "GenWithStack", "GenWithStackByArgs", "FastGen", "FastGenByArgs":
+			// TODO: check whether the receiver is an `*Error`
+			return true
+		}
+	}
+	return false
+}
+
+func argIsNotAllowed(typInfo *types.Info, x ast.Expr) bool {
+	typ := typInfo.Types[x].Type
+	if typ == nil {
+		return false
+	}
+
+	typWithMethods, ok := elementType(typ).(methodLookup)
+	if !ok {
+		return false
+	}
+
+	return typIsNotAllowed(typWithMethods)
+}
+
+type methodLookup interface {
+	NumMethods() int
+	Method(i int) *types.Func
+	Underlying() types.Type
+}
+
+func typIsNotAllowed(typ methodLookup) bool {
+	implString := false
+	implStringWithCtx := false
+	for i := 0; i < typ.NumMethods(); i++ {
+		method := typ.Method(i)
+		name := method.Name()
+		if name == "String" {
+			implString = true
+		}
+		if name == "StringWithCtx" {
+			implStringWithCtx = true
+		}
+	}
+
+	if implStringWithCtx && !implString {
+		return true
+	}
+
+	// the `Underlying` of an interface is still the interface, so we need to avoid unlimited recursion here.
+	_, typIsIface := typ.(*types.Interface)
+	if iface, underlyingIsIface := typ.Underlying().(*types.Interface); !typIsIface && underlyingIsIface {
+		return typIsNotAllowed(iface)
+	}
+
+	return false
+}
+
+// elementType returns the element type of a pointer or slice recursively.
+func elementType(typ types.Type) types.Type {
+	switch t := typ.(type) {
+	case *types.Pointer:
+		return elementType(t.Elem())
+	case *types.Slice:
+		return elementType(t.Elem())
+	}
+	return typ
+}
+
+func init() {
+	util.SkipAnalyzerByConfig(Analyzer)
+	util.SkipAnalyzer(Analyzer)
+}

build/linter/printexpression/analyzer_test.go

@@ -0,0 +1,33 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed 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.
+
+//go:build !intest
+
+package printexpression_test
+
+import (
+	"testing"
+
+	"github.com/pingcap/tidb/build/linter/printexpression"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+// TODO: investigate the CI environment and check how to run this test in CI.
+// The CI environment doesn't have `go` executable in $PATH.
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	pkgs := []string{"t", "github.com/pingcap/tidb/pkg/expression"}
+	analysistest.Run(t, testdata, printexpression.Analyzer, pkgs...)
+}

build/linter/printexpression/testdata/src/github.com/pingcap/tidb/pkg/expression/BUILD.bazel

@@ -0,0 +1,8 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "expression",
+    srcs = ["expression.go"],
+    importpath = "github.com/pingcap/tidb/build/linter/printexpression/testdata/src/github.com/pingcap/tidb/pkg/expression",
+    visibility = ["//visibility:public"],
+)

build/linter/printexpression/testdata/src/github.com/pingcap/tidb/pkg/expression/expression.go

@@ -0,0 +1,53 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed 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 expression
+
+import (
+	"fmt"
+)
+
+// Expression is a test struct.
+type Expression interface {
+	// StringWithCtx method only is not allowed to be printed directly.
+	StringWithCtx()
+}
+
+// Constant is a test struct.
+type Constant struct{}
+
+// StringWithCtx implements the Expression interface.
+func (c *Constant) StringWithCtx() {}
+
+// Column is a test struct
+type Column struct{}
+
+// String implements ExpressionWithString interface.
+func (c *Column) String() {}
+
+// StringWithCtx implements the Expression interface.
+func (c *Column) StringWithCtx() {}
+
+func testFunc() {
+	var expr Expression
+	var con *Constant
+	var conList []*Constant
+
+	var col *Column
+
+	fmt.Println(expr)         // want `avoid printing expression directly.*`
+	fmt.Println(con)          // want `avoid printing expression directly.*`
+	fmt.Printf("%v", conList) // want `avoid printing expression directly.*`
+	fmt.Println(col)
+}

build/linter/printexpression/testdata/src/t/BUILD.bazel

@@ -0,0 +1,9 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "t",
+    srcs = ["test_file.go"],
+    importpath = "github.com/pingcap/tidb/build/linter/printexpression/testdata/src/t",
+    visibility = ["//visibility:public"],
+    deps = ["//pkg/expression"],
+)

build/linter/printexpression/testdata/src/t/test_file.go

@@ -0,0 +1,52 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed 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 t
+
+import (
+	"fmt"
+
+	"github.com/pingcap/tidb/pkg/expression"
+)
+
+// EmbeddedConstant is a test struct
+type EmbeddedConstant struct {
+	expression.Constant
+}
+
+// String implements ExpressionWithString interface.
+func (e *EmbeddedConstant) String() {}
+
+// ExpressionWithString is a test struct
+type ExpressionWithString interface {
+	expression.Expression
+	String()
+}
+
+func testFunc() {
+	var expr expression.Expression
+	var con *expression.Constant
+	var conList []*expression.Constant
+
+	var exprWithString ExpressionWithString
+	var col *expression.Column
+	var embedded *EmbeddedConstant
+
+	fmt.Println(expr)         // want `avoid printing expression directly.*`
+	fmt.Println(con)          // want `avoid printing expression directly.*`
+	fmt.Printf("%v", conList) // want `avoid printing expression directly.*`
+	fmt.Println(exprWithString)
+	fmt.Println(col)
+	fmt.Println(embedded)
+}

build/nogo_config.json

@@ -69,6 +69,14 @@
       ".*_generated\\.go$": "ignore generated code"
     }
   },
+  "printexpression": {
+    "exclude_files": {
+      "pkg/parser/parser.go": "parser/parser.go code",
+      "external/": "no need to vet third party code",
+      ".*_generated\\.go$": "ignore generated code",
+      "build/linter/printexpression/testdata/": "ignore test code"
+    }
+  },
   "printf": {
     "exclude_files": {
       "pkg/parser/parser.go": "parser/parser.go code",