ronantakizawa/codereview-bench huggingface dataset:
Repository: eslint/eslint
File: tests/lib/rules/strict.js (JavaScript)

Code before:
				sourceType: "module",
				parserOptions: { ecmaFeatures: { impliedStrict: true } },
			},
			errors: [
				{
					messageId: "module",
				},
				{
					messageId: "module",
				},
			],
		},

		// "global" mode
		{
			code: "foo();",
			output: null,
			options: ["global"],
			errors: [
				{
					messageId: "global",
				},
			],
		},
		{
			code: "/* license */\nfunction bar() {}\nfunction bar() {}",
			output: null,
			options: ["global"],
			errors: [
				{
					messageId: "global",
					line: 2,
					column: 1,
					endLine: 3,
					endColumn: 18,
				},
			],
		},
		{
			code: "function foo() { 'use strict'; return; }",
			output: null,
			options: ["global"],
			errors: [
				{
					messageId: "global",
				},
				{
					messageId: "global",
				},
			],
		},

Review comment (suggestion):
```suggestion
			code: "/* license */\nfunction bar() {}\nfunction bar() {}\n/* end */",
```

To also verify the behavior of loc.end.

Code after:
				sourceType: "module",
				parserOptions: { ecmaFeatures: { impliedStrict: true } },
			},
			errors: [
				{
					messageId: "module",
				},
				{
					messageId: "module",
				},
			],
		},

		// "global" mode
		{
			code: "foo();",
			output: null,
			options: ["global"],
			errors: [
				{
					messageId: "global",
				},
			],
		},
		{
			code: "/* license */\nfunction foo() {}\nfunction bar() {}\n/* end */",
			output: null,
			options: ["global"],
			errors: [
				{
					messageId: "global",
					line: 2,
					column: 1,
					endLine: 3,
					endColumn: 18,
				},
			],
		},
		{
			code: "function foo() { 'use strict'; return; }",
			output: null,
			options: ["global"],
			errors: [
				{
					messageId: "global",
				},
				{
					messageId: "global",
				},
			],
		},
Repository: eslint/eslint
File: lib/shared/naming.js (JavaScript)

Code before:
	 */
	if (normalizedName.includes("\\")) {
		normalizedName = normalizedName.replace(/\\/gu, "/");
	}

	if (normalizedName.charAt(0) === "@") {
		/**
		 * it's a scoped package
		 * package name is the prefix, or just a username
		 */
		const scopedPackageShortcutRegex = new RegExp(
				`^(@[^/]+)(?:/(?:${prefix})?)?$`,
				"u",
			),
			scopedPackageNameRegex = new RegExp(`^${prefix}(-|$)`, "u");

		if (scopedPackageShortcutRegex.test(normalizedName)) {
			normalizedName = normalizedName.replace(
				scopedPackageShortcutRegex,
				`$1/${prefix}`,
			);
		} else if (!scopedPackageNameRegex.test(normalizedName.split("/")[1])) {
			/**
			 * for scoped packages, insert the prefix after the first / unless
			 * the path is already @scope/eslint or @scope/eslint-xxx-yyy
			 */
			normalizedName = normalizedName.replace(
				/^@([^/]+)\/(.*)$/u,
				`@$1/${prefix}-$2`,
			);
		}
	} else if (!normalizedName.startsWith(`${prefix}-`)) {
		normalizedName = `${prefix}-${normalizedName}`;
	}

	return normalizedName;
}

/**
 * Removes the prefix from a fullname.
 * @param {string} fullname The term which may have the prefix.
 * @param {string} prefix The prefix to remove.
 * @returns {string} The term without prefix.
 */
function getShorthandName(fullname, prefix) {
	if (fullname[0] === "@") {
		let matchResult = new RegExp(`^(@[^/]+)/${prefix}$`, "u").exec(
			fullname,
		);

		if (matchResult) {

Review comment (security):
This was also reported by `jsdoc/escape-inline-tags`. I removed one `*` so that this comment isn't interpreted as JSDoc.

Code after:
	 */
	if (normalizedName.includes("\\")) {
		normalizedName = normalizedName.replace(/\\/gu, "/");
	}

	if (normalizedName.charAt(0) === "@") {
		/**
		 * it's a scoped package
		 * package name is the prefix, or just a username
		 */
		const scopedPackageShortcutRegex = new RegExp(
				`^(@[^/]+)(?:/(?:${prefix})?)?$`,
				"u",
			),
			scopedPackageNameRegex = new RegExp(`^${prefix}(-|$)`, "u");

		if (scopedPackageShortcutRegex.test(normalizedName)) {
			normalizedName = normalizedName.replace(
				scopedPackageShortcutRegex,
				`$1/${prefix}`,
			);
		} else if (!scopedPackageNameRegex.test(normalizedName.split("/")[1])) {
			/*
			 * for scoped packages, insert the prefix after the first / unless
			 * the path is already @scope/eslint or @scope/eslint-xxx-yyy
			 */
			normalizedName = normalizedName.replace(
				/^@([^/]+)\/(.*)$/u,
				`@$1/${prefix}-$2`,
			);
		}
	} else if (!normalizedName.startsWith(`${prefix}-`)) {
		normalizedName = `${prefix}-${normalizedName}`;
	}

	return normalizedName;
}

/**
 * Removes the prefix from a fullname.
 * @param {string} fullname The term which may have the prefix.
 * @param {string} prefix The prefix to remove.
 * @returns {string} The term without prefix.
 */
function getShorthandName(fullname, prefix) {
	if (fullname[0] === "@") {
		let matchResult = new RegExp(`^(@[^/]+)/${prefix}$`, "u").exec(
			fullname,
		);

		if (matchResult) {
Repository: etcd-io/etcd
File: client/pkg/transport/keepalive_listener.go (Go)

Code before:
func NewKeepAliveListener(l net.Listener, scheme string, tlscfg *tls.Config) (net.Listener, error) {
	kal := &keepaliveListener{
		Listener: l,
	}

	if scheme == "https" {
		if tlscfg == nil {
			return nil, fmt.Errorf("cannot listen on TLS for given listener: KeyFile and CertFile are not presented")
		}
		return newTLSKeepaliveListener(kal, tlscfg), nil
	}

	return kal, nil
}

type keepaliveListener struct{ net.Listener }

func (kln *keepaliveListener) Accept() (net.Conn, error) {
	c, err := kln.Listener.Accept()
	if err != nil {
		return nil, err
	}

	kac, err := createKeepaliveConn(c)
	if err != nil {
		return nil, fmt.Errorf("create keep alive connection failed, %w", err)
	}
	// detection time: tcp_keepalive_time + tcp_keepalive_probes + tcp_keepalive_intvl
	// default on linux:  30 + 8 * 30
	// default on osx:    30 + 8 * 75
	if err := kac.SetKeepAlive(true); err != nil {
		return nil, fmt.Errorf("SetKeepAlive failed, %w", err)
	}
	if err := kac.SetKeepAlivePeriod(30 * time.Second); err != nil {
		return nil, fmt.Errorf("SetKeepAlivePeriod failed, %w", err)
	}
	return kac, nil
}

func createKeepaliveConn(c net.Conn) (*keepAliveConn, error) {
	tcpc, ok := c.(*net.TCPConn)
	if !ok {
		return nil, ErrNotTCP
	}
	return &keepAliveConn{tcpc}, nil
}

type keepAliveConn struct {
	*net.TCPConn
}


Review comment (suggestion):
nit, suggest to use `keepalive` (as it's one word)

Code after:
func NewKeepAliveListener(l net.Listener, scheme string, tlscfg *tls.Config) (net.Listener, error) {
	kal := &keepaliveListener{
		Listener: l,
	}

	if scheme == "https" {
		if tlscfg == nil {
			return nil, fmt.Errorf("cannot listen on TLS for given listener: KeyFile and CertFile are not presented")
		}
		return newTLSKeepaliveListener(kal, tlscfg), nil
	}

	return kal, nil
}

type keepaliveListener struct{ net.Listener }

func (kln *keepaliveListener) Accept() (net.Conn, error) {
	c, err := kln.Listener.Accept()
	if err != nil {
		return nil, err
	}

	kac, err := createKeepaliveConn(c)
	if err != nil {
		return nil, fmt.Errorf("create keepalive connection failed, %w", err)
	}
	// detection time: tcp_keepalive_time + tcp_keepalive_probes + tcp_keepalive_intvl
	// default on linux:  30 + 8 * 30
	// default on osx:    30 + 8 * 75
	if err := kac.SetKeepAlive(true); err != nil {
		return nil, fmt.Errorf("SetKeepAlive failed, %w", err)
	}
	if err := kac.SetKeepAlivePeriod(30 * time.Second); err != nil {
		return nil, fmt.Errorf("SetKeepAlivePeriod failed, %w", err)
	}
	return kac, nil
}

func createKeepaliveConn(c net.Conn) (*keepAliveConn, error) {
	tcpc, ok := c.(*net.TCPConn)
	if !ok {
		return nil, ErrNotTCP
	}
	return &keepAliveConn{tcpc}, nil
}

type keepAliveConn struct {
	*net.TCPConn
}
Repository: etcd-io/etcd
File: client/v3/client.go (Go)

Code before:
	Cluster
	KV
	Lease
	Watcher
	Auth
	Maintenance

	conn *grpc.ClientConn

	cfg      Config
	creds    grpccredentials.TransportCredentials
	resolver *resolver.EtcdManualResolver

	epMu      *sync.RWMutex
	endpoints []string

	ctx    context.Context
	cancel context.CancelFunc

	// Username is a user name for authentication.
	Username string
	// Password is a password for authentication.
	Password string
	// Token is a JWT used for authentication instead of a password.
	Token string

	authTokenBundle credentials.PerRPCCredentialsBundle

	callOpts []grpc.CallOption

	lgMu *sync.RWMutex
	lg   *zap.Logger
}

// New creates a new etcdv3 client from a given configuration.
func New(cfg Config) (*Client, error) {
	if len(cfg.Endpoints) == 0 {
		return nil, ErrNoAvailableEndpoints
	}

	return newClient(&cfg)
}

// NewCtxClient creates a client with a context but no underlying grpc
// connection. This is useful for embedded cases that override the
// service interface implementations and do not need connection management.
func NewCtxClient(ctx context.Context, opts ...Option) *Client {
	cctx, cancel := context.WithCancel(ctx)
	c := &Client{ctx: cctx, cancel: cancel, lgMu: new(sync.RWMutex)}
	for _, opt := range opts {
		opt(c)

Review comment (suggestion):
We should add a validation on the config, only the following three cases are allowed,
- Only username & password are specified;
- Only token is specified;
- Neither is specified.

If both username & password and token are specified, then we should raise an error or warning.

Code after:
type Client struct {
	Cluster
	KV
	Lease
	Watcher
	Auth
	Maintenance

	conn *grpc.ClientConn

	cfg      Config
	creds    grpccredentials.TransportCredentials
	resolver *resolver.EtcdManualResolver

	epMu      *sync.RWMutex
	endpoints []string

	ctx    context.Context
	cancel context.CancelFunc

	// Username is a user name for authentication.
	Username string
	// Password is a password for authentication.
	Password string
	// Token is a JWT used for authentication instead of a password.
	Token string

	authTokenBundle credentials.PerRPCCredentialsBundle

	callOpts []grpc.CallOption

	lgMu *sync.RWMutex
	lg   *zap.Logger
}

// New creates a new etcdv3 client from a given configuration.
func New(cfg Config) (*Client, error) {
	if len(cfg.Endpoints) == 0 {
		return nil, ErrNoAvailableEndpoints
	}

	return newClient(&cfg)
}

// NewCtxClient creates a client with a context but no underlying grpc
// connection. This is useful for embedded cases that override the
// service interface implementations and do not need connection management.
func NewCtxClient(ctx context.Context, opts ...Option) *Client {
	cctx, cancel := context.WithCancel(ctx)
	c := &Client{ctx: cctx, cancel: cancel, lgMu: new(sync.RWMutex), epMu: new(sync.RWMutex)}
	for _, opt := range opts {
Repository: etcd-io/etcd
File: client/v3/client.go (Go)

Code before:
	switch r {
	case endpoint.CredsDrop:
		return nil
	case endpoint.CredsOptional:
		return c.creds
	case endpoint.CredsRequire:
		if c.creds != nil {
			return c.creds
		}
		return credentials.NewTransportCredential(nil)
	default:
		panic(fmt.Errorf("unsupported CredsRequirement: %v", r))
	}
}

func newClient(cfg *Config) (*Client, error) {
	if cfg == nil {
		cfg = &Config{}
	}
	var creds grpccredentials.TransportCredentials
	if cfg.TLS != nil {
		creds = credentials.NewTransportCredential(cfg.TLS)
	}

	if cfg.Token != "" && (cfg.Username != "" || cfg.Password != "") {
		return nil, fmt.Errorf("Username/Password and Token configurations are mutually exclusive")
	}

	// use a temporary skeleton client to bootstrap first connection
	baseCtx := context.TODO()
	if cfg.Context != nil {
		baseCtx = cfg.Context
	}

	ctx, cancel := context.WithCancel(baseCtx)
	client := &Client{
		conn:     nil,
		cfg:      *cfg,
		creds:    creds,
		ctx:      ctx,
		cancel:   cancel,
		epMu:     new(sync.RWMutex),
		callOpts: defaultCallOpts,
		lgMu:     new(sync.RWMutex),
	}

	var err error
	if cfg.Logger != nil {
		client.lg = cfg.Logger
	} else if cfg.LogConfig != nil {
		client.lg, err = cfg.LogConfig.Build()

Review comment (suggestion):
```suggestion
		return nil, errors.New("Username/Password and Token configurations are mutually exclusive")
```

Code after:
			return c.creds
		}
		return credentials.NewTransportCredential(nil)
	default:
		panic(fmt.Errorf("unsupported CredsRequirement: %v", r))
	}
}

func newClient(cfg *Config) (*Client, error) {
	if cfg == nil {
		cfg = &Config{}
	}
	var creds grpccredentials.TransportCredentials
	if cfg.TLS != nil {
		creds = credentials.NewTransportCredential(cfg.TLS)
	}

	if cfg.Token != "" && (cfg.Username != "" || cfg.Password != "") {
		return nil, errors.New("Username/Password and Token configurations are mutually exclusive")
	}

	// use a temporary skeleton client to bootstrap first connection
	baseCtx := context.TODO()
	if cfg.Context != nil {
		baseCtx = cfg.Context
	}

	ctx, cancel := context.WithCancel(baseCtx)
	client := &Client{
		conn:     nil,
		cfg:      *cfg,
		creds:    creds,
		ctx:      ctx,
		cancel:   cancel,
		epMu:     new(sync.RWMutex),
		callOpts: defaultCallOpts,
		lgMu:     new(sync.RWMutex),
	}

	var err error
	if cfg.Logger != nil {
		client.lg = cfg.Logger
	} else if cfg.LogConfig != nil {
		client.lg, err = cfg.LogConfig.Build()
	} else {
		client.lg, err = logutil.CreateDefaultZapLogger(etcdClientDebugLevel())
		if client.lg != nil {
			client.lg = client.lg.Named("etcd-client")
		}
	}
	if err != nil {
Repository: etcd-io/etcd
File: tests/framework/e2e/etcdctl.go (Go)

Code before:
			Password:    ctl.authConfig.Password,
		})
		if err != nil {
			return nil, err
		}
		client.Close()
	}

	return ctl, nil
}

func WithAuth(userName, password string) config.ClientOption {
	return func(c any) {
		ctl := c.(*EtcdctlV3)
		ctl.authConfig.Username = userName
		ctl.authConfig.Password = password
	}
}

func WithAuthToken(token string) config.ClientOption {
	return func(c any) {
		switch c := c.(type) {
		case *EtcdctlV3:
			c.authToken = token
		case *clientv3.Config:
			c.Token = token
		}
	}
}

func WithEndpoints(endpoints []string) config.ClientOption {
	return func(c any) {
		ctl := c.(*EtcdctlV3)
		ctl.endpoints = endpoints
	}
}

func (ctl *EtcdctlV3) DowngradeEnable(ctx context.Context, version string) error {
	_, err := SpawnWithExpectLines(ctx, ctl.cmdArgs("downgrade", "enable", version), nil, expect.ExpectedResponse{Value: "Downgrade enable success"})
	return err
}

func (ctl *EtcdctlV3) Get(ctx context.Context, key string, o config.GetOptions) (*clientv3.GetResponse, error) {
	resp := clientv3.GetResponse{}
	var args []string
	if o.Timeout != 0 {
		args = append(args, fmt.Sprintf("--command-timeout=%s", o.Timeout))
	}
	if o.Serializable {
		args = append(args, "--consistency", "s")
	}

Review comment (security):
Please break down this function into two, one for e2e, and the other for integration test. Please refer to `WithAuth`,

https://github.com/etcd-io/etcd/blob/c147425c519652014df09325692a063e07fc69f2/tests/framework/e2e/etcdctl.go#L68-L74

https://github.com/etcd-io/etcd/blob/c147425c519652014df09325692a063e07fc69f2/tests/framework/integration/cluster.go#L1473-L1479

Code after:
			Password:    ctl.authConfig.Password,
			Token:       ctl.authConfig.Token,
		})
		if err != nil {
			return nil, err
		}
		client.Close()
	}

	return ctl, nil
}

func WithAuth(userName, password string) config.ClientOption {
	return func(c any) {
		ctl := c.(*EtcdctlV3)
		ctl.authConfig.Username = userName
		ctl.authConfig.Password = password
	}
}

func WithAuthToken(token string) config.ClientOption {
	return func(c any) {
		ctl := c.(*EtcdctlV3)
		ctl.authConfig.Token = token
	}
}

func WithEndpoints(endpoints []string) config.ClientOption {
	return func(c any) {
		ctl := c.(*EtcdctlV3)
		ctl.endpoints = endpoints
	}
}

func (ctl *EtcdctlV3) DowngradeEnable(ctx context.Context, version string) error {
	_, err := SpawnWithExpectLines(ctx, ctl.cmdArgs("downgrade", "enable", version), nil, expect.ExpectedResponse{Value: "Downgrade enable success"})
	return err
}

func (ctl *EtcdctlV3) DowngradeCancel(ctx context.Context) error {
	_, err := SpawnWithExpectLines(ctx, ctl.cmdArgs("downgrade", "cancel"), nil, expect.ExpectedResponse{Value: "Downgrade cancel success"})
	return err
}

func (ctl *EtcdctlV3) Get(ctx context.Context, key string, o config.GetOptions) (*clientv3.GetResponse, error) {
	resp := clientv3.GetResponse{}
	var args []string
	if o.Timeout != 0 {
		args = append(args, fmt.Sprintf("--command-timeout=%s", o.Timeout))
	}
	if o.Serializable {
Repository: etcd-io/etcd
File: tests/framework/e2e/etcdctl.go (Go)

Code before:

package e2e

import (
	"context"
	"encoding/json"
	"fmt"
	"io"
	"strconv"
	"strings"
	"time"

	"google.golang.org/grpc"

	"go.etcd.io/etcd/api/v3/authpb"
	"go.etcd.io/etcd/api/v3/etcdserverpb"
	clientv3 "go.etcd.io/etcd/client/v3"
	"go.etcd.io/etcd/pkg/v3/expect"
	"go.etcd.io/etcd/tests/v3/framework/config"
)

type EtcdctlV3 struct {
	cfg        ClientConfig
	endpoints  []string
	authConfig clientv3.AuthConfig
	authToken  string
}

func NewEtcdctl(cfg ClientConfig, endpoints []string, opts ...config.ClientOption) (*EtcdctlV3, error) {
	ctl := &EtcdctlV3{
		cfg:       cfg,
		endpoints: endpoints,
	}

	for _, opt := range opts {
		opt(ctl)
	}

	if !ctl.authConfig.Empty() {
		client, err := clientv3.New(clientv3.Config{
			Endpoints:   ctl.endpoints,
			DialTimeout: 5 * time.Second,
			DialOptions: []grpc.DialOption{grpc.WithBlock()},
			Username:    ctl.authConfig.Username,
			Password:    ctl.authConfig.Password,
		})
		if err != nil {
			return nil, err
		}
		client.Close()
	}

Review comment (security):
can you reuse `authConfig.Token`?
```suggestion
```

Code after:

package e2e

import (
	"context"
	"encoding/json"
	"errors"
	"fmt"
	"io"
	"strconv"
	"strings"
	"time"

	"google.golang.org/grpc"

	"go.etcd.io/etcd/api/v3/authpb"
	"go.etcd.io/etcd/api/v3/etcdserverpb"
	clientv3 "go.etcd.io/etcd/client/v3"
	"go.etcd.io/etcd/pkg/v3/expect"
	"go.etcd.io/etcd/tests/v3/framework/config"
)

type EtcdctlV3 struct {
	cfg        ClientConfig
	endpoints  []string
	authConfig clientv3.AuthConfig
}

func NewEtcdctl(cfg ClientConfig, endpoints []string, opts ...config.ClientOption) (*EtcdctlV3, error) {
	ctl := &EtcdctlV3{
		cfg:       cfg,
		endpoints: endpoints,
	}

	for _, opt := range opts {
		opt(ctl)
	}

	if !ctl.authConfig.Empty() {
		client, err := clientv3.New(clientv3.Config{
			Endpoints:   ctl.endpoints,
			DialTimeout: 5 * time.Second,
			DialOptions: []grpc.DialOption{grpc.WithBlock()},
			Username:    ctl.authConfig.Username,
			Password:    ctl.authConfig.Password,
			Token:       ctl.authConfig.Token,
		})
		if err != nil {
			return nil, err
		}
		client.Close()
Repository: etcd-io/etcd
File: tests/common/auth_util.go (Go)

Code before:
		if _, err := c.RoleGrantPermission(context.TODO(), r.role, r.key, r.keyEnd, r.permission); err != nil {
			return fmt.Errorf("RoleGrantPermission failed: %w", err)
		}
	}

	return nil
}

func createUsers(c interfaces.Client, users []authUser) error {
	for _, u := range users {
		// add user
		if _, err := c.UserAdd(context.TODO(), u.user, u.pass, config.UserAddOptions{}); err != nil {
			return fmt.Errorf("UserAdd failed: %w", err)
		}

		// grant role to user
		if _, err := c.UserGrantRole(context.TODO(), u.user, u.role); err != nil {
			return fmt.Errorf("UserGrantRole failed: %w", err)
		}
	}

	return nil
}

func createSignedJWT(keyPath, username string, authRevision uint64) (string, error) {
	signMethod := jwt.GetSigningMethod("RS256")

	keyBytes, err := os.ReadFile(keyPath)
	if err != nil {
		return "", err
	}

	key, err := jwt.ParseRSAPrivateKeyFromPEM(keyBytes)
	if err != nil {
		return "", err
	}

	tk := jwt.NewWithClaims(signMethod,
		jwt.MapClaims{
			"username": username,
			"revision": authRevision,
			"exp":      time.Now().Add(time.Minute).Unix(),
		})

	return tk.SignedString(key)
}

func setupAuth(c interfaces.Client, roles []authRole, users []authUser) error {
	// create roles
	if err := createRoles(c, roles); err != nil {
		return err

Review comment (security):
```suggestion
func createSignedJWT(keyPath, alg, username string, authRevision uint64) (string, error) {
	if alg == "" {
		alg = "RS256"
	}
	signMethod := jwt.GetSigningMethod(alg)
```

Code after:
		if _, err := c.RoleGrantPermission(context.TODO(), r.role, r.key, r.keyEnd, r.permission); err != nil {
			return fmt.Errorf("RoleGrantPermission failed: %w", err)
		}
	}

	return nil
}

func createUsers(c interfaces.Client, users []authUser) error {
	for _, u := range users {
		// add user
		if _, err := c.UserAdd(context.TODO(), u.user, u.pass, config.UserAddOptions{}); err != nil {
			return fmt.Errorf("UserAdd failed: %w", err)
		}

		// grant role to user
		if _, err := c.UserGrantRole(context.TODO(), u.user, u.role); err != nil {
			return fmt.Errorf("UserGrantRole failed: %w", err)
		}
	}

	return nil
}

func createSignedJWT(keyPath, alg, username string, authRevision uint64) (string, error) {
	signMethod := jwt.GetSigningMethod(alg)

	keyBytes, err := os.ReadFile(keyPath)
	if err != nil {
		return "", err
	}

	key, err := jwt.ParseRSAPrivateKeyFromPEM(keyBytes)
	if err != nil {
		return "", err
	}

	tk := jwt.NewWithClaims(signMethod,
		jwt.MapClaims{
			"username": username,
			"revision": authRevision,
			"exp":      time.Now().Add(time.Minute).Unix(),
		})

	return tk.SignedString(key)
}

func setupAuth(c interfaces.Client, roles []authRole, users []authUser) error {
	// create roles
	if err := createRoles(c, roles); err != nil {
		return err