ronantakizawa/codereview-bench huggingface dataset:
Repository: recharts/recharts
File: test/util/LogUtils.spec.ts (TypeScript)

Code before:
import { vi } from 'vitest';
import { warn } from '../../src/util/LogUtils';

describe('LogUtils', () => {
  test('dont log when condition is true', () => {
    const logSpy = vi.spyOn(console, 'warn').mockImplementation((): void => undefined);
    warn(true, 'test');
    expect(logSpy).not.toHaveBeenCalledWith('test');
  });

  test('warn when condition is false', () => {
    const logSpy = vi.spyOn(console, 'warn').mockImplementation((): void => undefined);
    // log 'format'
    warn(false, 'format');

    expect(logSpy).toHaveBeenCalledWith('format');

    // log empty string
    warn(false, '');
    expect(logSpy).toHaveBeenCalledWith('');

    // log string with a numerical variable
    warn(false, 'format width variable %s', 0);
    expect(logSpy).toHaveBeenCalledWith('format width variable 0');
  });
});


Review comment (suggestion):
since this is internal and we use typescript I don't really see any value in this test but doesn't hurt anything I suppose

Code after:
    expect(logSpy).not.toHaveBeenCalledWith('test');
  });

  test('warn when condition is false', () => {
    const logSpy = vi.spyOn(console, 'warn').mockImplementation((): void => undefined);
    // log 'format'
    warn(false, 'format');

    expect(logSpy).toHaveBeenCalledWith('format');

    // log empty string
    warn(false, '');
    expect(logSpy).toHaveBeenCalledWith('');

    // log string with a numerical variable
    warn(false, 'format width variable %s', 0);
    expect(logSpy).toHaveBeenCalledWith('format width variable 0');
  });

  test('warns when format is undefined', () => {
    const logSpy = vi.spyOn(console, 'warn').mockImplementation((): void => undefined);

    // condition is true, format is undefined
    // @ts-expect-error - testing undefined format
    warn(true, undefined);
    expect(logSpy).toHaveBeenCalledWith('LogUtils requires an error message argument');

    logSpy.mockClear();

    // condition is false, format is undefined
    // @ts-expect-error - testing undefined format
    warn(false, undefined);
    expect(logSpy).toHaveBeenCalledWith('LogUtils requires an error message argument');
    expect(logSpy).toHaveBeenCalledWith(
      'Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings.',
    );
  });
});
Repository: redis/redis-py
File: redis/asyncio/connection.py (Python)

Code before:
from redis.backoff import NoBackoff
from redis.compat import Protocol, TypedDict
from redis.exceptions import (
    AuthenticationError,
    AuthenticationWrongNumberOfArgsError,
    BusyLoadingError,
    ChildDeadlockedError,
    ConnectionError,
    DataError,
    ExecAbortError,
    InvalidResponse,
    ModuleError,
    NoPermissionError,
    NoScriptError,
    ReadOnlyError,
    RedisError,
    ResponseError,
    TimeoutError,
)
from redis.typing import EncodableT, EncodedT
from redis.utils import HIREDIS_AVAILABLE, str_if_bytes

hiredis = None
if HIREDIS_AVAILABLE:
    import hiredis

if sys.version_info[:2] >= (3, 10):
    nullcontext = contextlib.nullcontext()
else:

    class NullContext:
        async def __aenter__(self):
            pass

        async def __aexit__(self, *args):
            pass

    nullcontext = NullContext()

SYM_STAR = b"*"
SYM_DOLLAR = b"$"
SYM_CRLF = b"\r\n"
SYM_LF = b"\n"
SYM_EMPTY = b""

SERVER_CLOSED_CONNECTION_ERROR = "Connection closed by server."


class _Sentinel(enum.Enum):
    sentinel = object()


Review comment (suggestion):
nullcontext is available as of 3.7, but the [async context manager](https://docs.python.org/3/glossary.html#term-asynchronous-context-manager) is available since 3.10

Code after:
from redis.exceptions import (
    AuthenticationError,
    AuthenticationWrongNumberOfArgsError,
    BusyLoadingError,
    ChildDeadlockedError,
    ConnectionError,
    DataError,
    ExecAbortError,
    InvalidResponse,
    ModuleError,
    NoPermissionError,
    NoScriptError,
    ReadOnlyError,
    RedisError,
    ResponseError,
    TimeoutError,
)
from redis.typing import EncodableT, EncodedT
from redis.utils import HIREDIS_AVAILABLE, str_if_bytes

hiredis = None
if HIREDIS_AVAILABLE:
    import hiredis

SYM_STAR = b"*"
SYM_DOLLAR = b"$"
SYM_CRLF = b"\r\n"
SYM_LF = b"\n"
SYM_EMPTY = b""

SERVER_CLOSED_CONNECTION_ERROR = "Connection closed by server."


class _Sentinel(enum.Enum):
    sentinel = object()


SENTINEL = _Sentinel.sentinel
MODULE_LOAD_ERROR = "Error loading the extension. Please check the server logs."
NO_SUCH_MODULE_ERROR = "Error unloading module: no such module with that name"
MODULE_UNLOAD_NOT_POSSIBLE_ERROR = "Error unloading module: operation not possible."
MODULE_EXPORTS_DATA_TYPES_ERROR = (
    "Error unloading module: the module "
    "exports one or more module-side data "
    "types, can't unload"
)
# user send an AUTH cmd to a server without authorization configured
NO_AUTH_SET_ERROR = {
    # Redis >= 6.0
    "AUTH <password> called without any password "
    "configured for the default user. Are you sure "
Repository: redis/redis-py
File: tests/test_asyncio/test_cwe_404.py (Python)

Code before:
import asyncio
import sys

import pytest

from redis.asyncio import Redis
from redis.asyncio.cluster import RedisCluster


async def pipe(
    reader: asyncio.StreamReader, writer: asyncio.StreamWriter, delay: float, name=""
):
    while data := await reader.read(1000):
        await asyncio.sleep(delay)
        writer.write(data)
        await writer.drain()


class DelayProxy:
    def __init__(self, addr, redis_addr, delay: float):
        self.addr = addr
        self.redis_addr = redis_addr
        self.delay = delay

    async def start(self):
        self.server = await asyncio.start_server(self.handle, *self.addr)
        self.ROUTINE = asyncio.create_task(self.server.serve_forever())

    async def handle(self, reader, writer):
        # establish connection to redis
        redis_reader, redis_writer = await asyncio.open_connection(*self.redis_addr)
        pipe1 = asyncio.create_task(pipe(reader, redis_writer, self.delay, "to redis:"))
        pipe2 = asyncio.create_task(
            pipe(redis_reader, writer, self.delay, "from redis:")
        )
        await asyncio.gather(pipe1, pipe2)

    async def stop(self):

Review comment (suggestion):
The walrus operator was only introduced in Python 3.8 (see [PEP 572](https://peps.python.org/pep-0572/)).

Since this library supports Python 3.7 as well, this line should be changed to:

```suggestion
    while True:
        data = await reader.read(1000)
        if not data:
            break
```

Code after:
import asyncio
import sys

import pytest

from redis.asyncio import Redis
from redis.asyncio.cluster import RedisCluster


async def pipe(
    reader: asyncio.StreamReader, writer: asyncio.StreamWriter, delay: float, name=""
):
    while True:
        data = await reader.read(1000)
        if not data:
            break
        await asyncio.sleep(delay)
        writer.write(data)
        await writer.drain()


class DelayProxy:
    def __init__(self, addr, redis_addr, delay: float):
        self.addr = addr
        self.redis_addr = redis_addr
        self.delay = delay

    async def start(self):
        self.server = await asyncio.start_server(self.handle, *self.addr)
        self.ROUTINE = asyncio.create_task(self.server.serve_forever())

    async def handle(self, reader, writer):
        # establish connection to redis
        redis_reader, redis_writer = await asyncio.open_connection(*self.redis_addr)
        pipe1 = asyncio.create_task(pipe(reader, redis_writer, self.delay, "to redis:"))
        pipe2 = asyncio.create_task(
            pipe(redis_reader, writer, self.delay, "from redis:")
        )
Repository: redis/redis-py
File: tests/test_asyncio/test_commands.py (Python)

Code before:
"""
Tests async overrides of commands from their mixins
"""
import asyncio
import binascii
import datetime
import re
import sys
from string import ascii_letters

import pytest
import pytest_asyncio

import redis
from redis import exceptions
from redis.client import EMPTY_RESPONSE, NEVER_DECODE, parse_info
from tests.conftest import (
    skip_if_server_version_gte,
    skip_if_server_version_lt,
    skip_unless_arch_bits,
)

if sys.version_info.major >= 3 and sys.version_info.minor >= 11:
    from asyncio import timeout as async_timeout
else:
    from async_timeout import timeout as async_timeout

REDIS_6_VERSION = "5.9.0"


@pytest_asyncio.fixture()
async def r_teardown(r: redis.Redis):
    """
    A special fixture which removes the provided names from the database after use
    """
    usernames = []

    def factory(username):
        usernames.append(username)
        return r

    yield factory
    for username in usernames:
        await r.acl_deluser(username)


@pytest_asyncio.fixture()
async def slowlog(r: redis.Redis):

Review comment (bug):
This is broken as it'll only allow 3.11+, 4.11+, 5.11+, etc.

Always compare against a tuple like this:

```suggestion
if sys.version_info >= (3, 11):
```

Code after:
"""
Tests async overrides of commands from their mixins
"""
import asyncio
import binascii
import datetime
import re
import sys
from string import ascii_letters

import pytest
import pytest_asyncio

import redis
from redis import exceptions
from redis.client import EMPTY_RESPONSE, NEVER_DECODE, parse_info
from tests.conftest import (
    skip_if_server_version_gte,
    skip_if_server_version_lt,
    skip_unless_arch_bits,
)

if sys.version_info >= (3, 11, 3):
    from asyncio import timeout as async_timeout
else:
    from async_timeout import timeout as async_timeout

REDIS_6_VERSION = "5.9.0"


@pytest_asyncio.fixture()
async def r_teardown(r: redis.Redis):
    """
    A special fixture which removes the provided names from the database after use
    """
    usernames = []

    def factory(username):
        usernames.append(username)
        return r

    yield factory
    for username in usernames:
        await r.acl_deluser(username)


@pytest_asyncio.fixture()
async def slowlog(r: redis.Redis):
Repository: redis/redis-py
File: redis/utils.py (Python)

Code before:
    warnings.warn(msg, category=DeprecationWarning, stacklevel=stacklevel)


def deprecated_function(reason="", version="", name=None):
    """
    Decorator to mark a function as deprecated.
    """

    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            warn_deprecated(name or func.__name__, reason, version, stacklevel=3)
            return func(*args, **kwargs)

        return wrapper

    return decorator


logger = logging.getLogger("push_response")
logger.setLevel(logging.INFO)
handler = logging.StreamHandler()
handler.setLevel(logging.INFO)
logger.addHandler(handler)

INFO_LOGGER = logger


Review comment (suggestion):
INFO_LOGGER Should instead be _info_logger, so that we don't get caught in the same issue as the parsers rename

Code after:
    warnings.warn(msg, category=DeprecationWarning, stacklevel=stacklevel)


def deprecated_function(reason="", version="", name=None):
    """
    Decorator to mark a function as deprecated.
    """

    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            warn_deprecated(name or func.__name__, reason, version, stacklevel=3)
            return func(*args, **kwargs)

        return wrapper

    return decorator


def _set_info_logger():
    """
    Set up a logger that log info logs to stdout.
    (This is used by the default push response handler)
    """
    if "push_response" not in logging.root.manager.loggerDict.keys():
        logger = logging.getLogger("push_response")
        logger.setLevel(logging.INFO)
        handler = logging.StreamHandler()
        handler.setLevel(logging.INFO)
        logger.addHandler(handler)
Repository: redis/redis-py
File: redis/utils.py (Python)

Code before:

    msg = f"Call to deprecated {name}."
    if reason:
        msg += f" ({reason})"
    if version:
        msg += f" -- Deprecated since version {version}."
    warnings.warn(msg, category=DeprecationWarning, stacklevel=stacklevel)


def deprecated_function(reason="", version="", name=None):
    """
    Decorator to mark a function as deprecated.
    """

    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            warn_deprecated(name or func.__name__, reason, version, stacklevel=3)
            return func(*args, **kwargs)

        return wrapper

    return decorator


logger = logging.getLogger("push_response")
logger.setLevel(logging.INFO)
handler = logging.StreamHandler()
handler.setLevel(logging.INFO)
logger.addHandler(handler)

INFO_LOGGER = logger


Review comment (suggestion):
instead, can this be a function? So the first time you'd call it, meaning the logger is triggered. Something like

def setup_logger(logevel=logging.INFO, handler=[logging.StreamHandler()]:
# do things here

This way, a user can also use our log handler and instantly override, just override their own handler if they want.

We need to only export the **name** of the logger. Then after the first time getLogger is called by python.. it's generally available

Code after:

    msg = f"Call to deprecated {name}."
    if reason:
        msg += f" ({reason})"
    if version:
        msg += f" -- Deprecated since version {version}."
    warnings.warn(msg, category=DeprecationWarning, stacklevel=stacklevel)


def deprecated_function(reason="", version="", name=None):
    """
    Decorator to mark a function as deprecated.
    """

    def decorator(func):
        @wraps(func)
        def wrapper(*args, **kwargs):
            warn_deprecated(name or func.__name__, reason, version, stacklevel=3)
            return func(*args, **kwargs)

        return wrapper

    return decorator


def _set_info_logger():
    """
    Set up a logger that log info logs to stdout.
    (This is used by the default push response handler)
    """
    if "push_response" not in logging.root.manager.loggerDict.keys():
        logger = logging.getLogger("push_response")
        logger.setLevel(logging.INFO)
        handler = logging.StreamHandler()
        handler.setLevel(logging.INFO)
        logger.addHandler(handler)
Repository: redis/redis-py
File: doctests/dt_hash.py (Python)

Code before:
# EXAMPLE: hash_tutorial
# HIDE_START
import redis

r = redis.Redis(decode_responses=True)
# HIDE_END
# STEP_START set_get_all
res1 = r.hset(
    "bike:1",
    mapping={
        "model": "Deimos",
        "brand": "Ergonom",
        "type": "Enduro bikes",
        "price": 4972,
    },
)
print(res1)  # 4

res2 = r.hget("bike:1", "model")
print(res2)  # 'Deimos'

res3 = r.hget("bike:1", "price")
print(res3)  # '4972'

res4 = r.hgetall("bike:1")
print(
    res4
)  #  {'model': 'Deimos', 'brand': 'Ergonom', 'type': 'Enduro bikes', 'price': '4972'}

# STEP_END

# REMOVE_START
assert res1 == 4
assert res2 == "Deimos"
assert res3 == "4972"
assert res4 == {
    "model": "Deimos",
    "brand": "Ergonom",
    "type": "Enduro bikes",
    "price": "4972",
}
# REMOVE_END

# STEP_START hmget
res5 = r.hmget("bike:1", ["model", "price"])
print(res5)  # ['Deimos', '4972']
# STEP_END

# REMOVE_START
assert res5 == ["Deimos", "4972"]
# REMOVE_END

Review comment (suggestion):
I recommend a slightly different format for showing expected replies https://github.com/redis/redis-py/blob/emb-examples/doctests/search_quickstart.py#L242
It allows us to avoid such weird formatting for short print statements.

Code after:
# EXAMPLE: hash_tutorial
# HIDE_START
import redis

r = redis.Redis(decode_responses=True)
# HIDE_END
# STEP_START set_get_all
res1 = r.hset(
    "bike:1",
    mapping={
        "model": "Deimos",
        "brand": "Ergonom",
        "type": "Enduro bikes",
        "price": 4972,
    },
)
print(res1)
# >>> 4

res2 = r.hget("bike:1", "model")
print(res2)
# >>> 'Deimos'

res3 = r.hget("bike:1", "price")
print(res3)
# >>> '4972'

res4 = r.hgetall("bike:1")
print(res4)
# >>> {'model': 'Deimos', 'brand': 'Ergonom', 'type': 'Enduro bikes', 'price': '4972'}

# STEP_END

# REMOVE_START
assert res1 == 4
assert res2 == "Deimos"
assert res3 == "4972"
assert res4 == {
    "model": "Deimos",
    "brand": "Ergonom",
    "type": "Enduro bikes",
    "price": "4972",
}
# REMOVE_END

# STEP_START hmget
res5 = r.hmget("bike:1", ["model", "price"])
print(res5)
# >>> ['Deimos', '4972']
# STEP_END
Repository: redis/redis-py
File: redis/_parsers/helpers.py (Python)

Code before:
    if isinstance(response, list):
        data = []
        for log in response:
            log_data = pairs_to_dict(log, True, True)
            client_info = log_data.get("client-info", "")
            log_data["client-info"] = parse_client_info(client_info)

            # float() is lossy comparing to the "double" in C
            log_data["age-seconds"] = float(log_data["age-seconds"])
            data.append(log_data)
    else:
        data = bool_ok(response)
    return data


def parse_client_info(value):
    """
    Parsing client-info in ACL Log in following format.
    "key1=value1 key2=value2 key3=value3"
    """
    value = str_if_bytes(value)
    if value[-1] == "\n":
        value = value[:-1]
    client_info = {}
    infos = value.split(" ")
    for info in infos:
        key, value = info.split("=")
        client_info[key] = value

    # Those fields are defined as int in networking.c
    for int_key in {
        "id",
        "age",
        "idle",
        "db",
        "sub",
        "psub",
        "multi",
        "qbuf",
        "qbuf-free",
        "obl",
        "argv-mem",
        "oll",
        "omem",
        "tot-mem",
    }:
        client_info[int_key] = int(client_info[int_key])
    return client_info


def parse_set_result(response, **options):

Review comment (suggestion):
```suggestion
    for info in str_if_bytes(value).strip().split():
```

Code after:
    if isinstance(response, list):
        data = []
        for log in response:
            log_data = pairs_to_dict(log, True, True)
            client_info = log_data.get("client-info", "")
            log_data["client-info"] = parse_client_info(client_info)

            # float() is lossy comparing to the "double" in C
            log_data["age-seconds"] = float(log_data["age-seconds"])
            data.append(log_data)
    else:
        data = bool_ok(response)
    return data


def parse_client_info(value):
    """
    Parsing client-info in ACL Log in following format.
    "key1=value1 key2=value2 key3=value3"
    """
    client_info = {}
    for info in str_if_bytes(value).strip().split():
        key, value = info.split("=")
        client_info[key] = value

    # Those fields are defined as int in networking.c
    for int_key in {
        "id",
        "age",
        "idle",
        "db",
        "sub",
        "psub",
        "multi",
        "qbuf",
        "qbuf-free",
        "obl",
        "argv-mem",
        "oll",
        "omem",
        "tot-mem",
    }:
        client_info[int_key] = int(client_info[int_key])
    return client_info


def parse_set_result(response, **options):
    """
    Handle SET result since GET argument is available since Redis 6.2.
    Parsing SET result into:
    - BOOL
Repository: redis/redis-py
File: redis/asyncio/cluster.py (Python)

Code before:
    ClusterError,
    ConnectionError,
    DataError,
    MaxConnectionsError,
    MovedError,
    RedisClusterException,
    RedisError,
    ResponseError,
    SlotNotCoveredError,
    TimeoutError,
    TryAgainError,
)
from redis.typing import AnyKeyT, EncodableT, KeyT
from redis.utils import (
    SSL_AVAILABLE,
    deprecated_function,
    get_lib_version,
    safe_str,
    str_if_bytes
)

if SSL_AVAILABLE:
    import ssl
    from ssl import TLSVersion
else:
    ssl = None
    TLSVersion = None

TargetNodesT = TypeVar(
    "TargetNodesT", str, "ClusterNode", List["ClusterNode"], Dict[Any, "ClusterNode"]
)


class RedisCluster(AbstractRedis, AbstractRedisCluster, AsyncRedisClusterCommands):
    """
    Create a new RedisCluster client.

    Pass one of parameters:

      - `host` & `port`
      - `startup_nodes`

    | Use ``await`` :meth:`initialize` to find cluster nodes & create connections.
    | Use ``await`` :meth:`close` to disconnect connections & close client.

    Many commands support the target_nodes kwarg. It can be one of the
    :attr:`NODE_FLAGS`:

      - :attr:`PRIMARIES`
      - :attr:`REPLICAS`
      - :attr:`ALL_NODES`

Review comment (suggestion):
In this case, I don't think you need to import and add any initialization of the ssl. 
In the end, you are using just the TLSVersion, and you can handle just this import and default init in case ssl is not available. The same applies for client and cluster files.

Code after:
    ClusterError,
    ConnectionError,
    DataError,
    MaxConnectionsError,
    MovedError,
    RedisClusterException,
    RedisError,
    ResponseError,
    SlotNotCoveredError,
    TimeoutError,
    TryAgainError,
)
from redis.typing import AnyKeyT, EncodableT, KeyT
from redis.utils import (
    SSL_AVAILABLE,
    deprecated_function,
    get_lib_version,
    safe_str,
    str_if_bytes,
)

if SSL_AVAILABLE:
    from ssl import TLSVersion
else:
    TLSVersion = None

TargetNodesT = TypeVar(
    "TargetNodesT", str, "ClusterNode", List["ClusterNode"], Dict[Any, "ClusterNode"]
)


class RedisCluster(AbstractRedis, AbstractRedisCluster, AsyncRedisClusterCommands):
    """
    Create a new RedisCluster client.

    Pass one of parameters:

      - `host` & `port`
      - `startup_nodes`

    | Use ``await`` :meth:`initialize` to find cluster nodes & create connections.
    | Use ``await`` :meth:`close` to disconnect connections & close client.

    Many commands support the target_nodes kwarg. It can be one of the
    :attr:`NODE_FLAGS`:

      - :attr:`PRIMARIES`
      - :attr:`REPLICAS`
      - :attr:`ALL_NODES`
      - :attr:`RANDOM`
      - :attr:`DEFAULT_NODE`
Repository: redis/redis-py
File: redis/_cache.py (Python)

Code before:
    "ZMSCORE",
    "ZRANGE",
    "ZRANGEBYLEX",
    "ZRANGEBYSCORE",
    "ZRANK",
    "ZREVRANGE",
    "ZREVRANGEBYLEX",
    "ZREVRANGEBYSCORE",
    "ZREVRANK",
    "ZSCORE",
    "ZUNION",
]

_RESPONSE = "response"
_KEYS = "keys"
_CTIME = "ctime"
_ACCESS_COUNT = "access_count"


class EvictionPolicy(Enum):
    LRU = "lru"
    LFU = "lfu"
    RANDOM = "random"


class AbstractCache:
    def set(self, command: str, response: ResponseT, keys_in_command: List[KeyT]):
        raise NotImplementedError

    def get(self, command: str) -> ResponseT:
        raise NotImplementedError

    def delete_command(self, command: str):
        raise NotImplementedError

    def delete_many(self, commands):
        raise NotImplementedError

    def flush(self):
        raise NotImplementedError

    def invalidate_key(self, key: KeyT):
        raise NotImplementedError


class _LocalCache(AbstractCache):
    """
    A caching mechanism for storing redis commands and their responses.

    Args:
        max_size (int): The maximum number of commands to be stored in the cache.

Review comment (refactor):
It looks like you need an Interface instead of Abstract class with NotImplemented methods. Otherwise, your methods should be abstract instead of NotImplemented

Code after:
    "ZLEXCOUNT",
    "ZMSCORE",
    "ZRANGE",
    "ZRANGEBYLEX",
    "ZRANGEBYSCORE",
    "ZRANK",
    "ZREVRANGE",
    "ZREVRANGEBYLEX",
    "ZREVRANGEBYSCORE",
    "ZREVRANK",
    "ZSCORE",
    "ZUNION",
]

_RESPONSE = "response"
_KEYS = "keys"
_CTIME = "ctime"
_ACCESS_COUNT = "access_count"


class EvictionPolicy(Enum):
    LRU = "lru"
    LFU = "lfu"
    RANDOM = "random"


class AbstractCache(ABC):
    """
    An abstract base class for client caching implementations.
    If you want to implement your own cache you must support these methods.
    """

    @abstractmethod
    def set(self, command: str, response: ResponseT, keys_in_command: List[KeyT]):
        pass

    @abstractmethod
    def get(self, command: str) -> ResponseT:
        pass

    @abstractmethod
    def delete_command(self, command: str):
        pass

    @abstractmethod
    def delete_many(self, commands):
        pass

    @abstractmethod
    def flush(self):
        pass