feat(bit): add BYTE/BIT option support for BITPOS command

[gongna-au]

Summary

This PR adds support for the BYTE/BIT unit option to the BITPOS command, aligning it with the Redis 7.0 specification.

The previous implementation only checked for BIT at a fixed argument position (args[5]) and ignored BYTE entirely. This PR refactors the parser to correctly handle the full Redis 7.0 syntax:

BITPOS key bit [start [end [BYTE | BIT]]]

The BYTE|BIT unit is strictly nested: it can only appear after both start and end are provided, matching Redis behavior exactly.

Changes

1. Command Parser Refactor (src/commands/cmd_bit.cc)

• Refactored CommandBitPos::Parse to cleanly separate argument parsing by count.
• args[4] (the 5th argument) is always parsed as the end integer — never as a unit keyword. This matches Redis, which rejects BITPOS key 1 0 BIT with a "not an integer" error.
• args[5] (the 6th argument) is checked for BIT/BYTE keyword, with errInvalidSyntax on anything else.
• Added explicit BYTE recognition (previously only BIT was partially handled).
• Moved bit argument validation to the top for clarity.
• Replaced verbose ParseInt + manual error check with GET_OR_RET macro for consistency.

2. Storage Layer — No Changes Needed

• The CHECK(stop_given) assertion in Bitmap::BitPos is preserved as a defensive guard. Since the parser guarantees stop_given=true whenever is_bit_index=true (unit keyword requires end to be present), this assertion is correct and protects against future regressions.

Compatibility

Command Form	Redis 7.0	Kvrocks (after PR)
BITPOS key bit	OK	OK
BITPOS key bit start	OK	OK
BITPOS key bit start end	OK	OK
BITPOS key bit start end BYTE	OK (since 7.0)	OK (new)
BITPOS key bit start end BIT	OK (since 7.0)	OK (new)
BITPOS key bit start BIT	ERR not integer	ERR not integer

Verification

• Argument parsing logic verified against Redis 7.0 syntax definition: BITPOS key bit [start [end [BYTE | BIT]]].
• is_bit_index is only set when args.size() == 6, guaranteeing stop_given_ == true, so the CHECK(stop_given) assertion in the storage layer is always satisfied.
• BITCOUNT already has correct BYTE/BIT support and is not affected by this PR.

[jihuayu]

Hi @gongna-au. Glad to see your PR. If your code was generated with AI assistance, please refer to the guidelines: https://kvrocks.apache.org/community/contributing#guidelines-for-ai-assisted-contributions. This will help us conduct a more focused and effective review.

I used GPT-5.4 to review your PR, and it identified some issues. Below are the test cases that would fail.

t.Run("BITPOS rejects BIT unit without end offset", func(t *testing.T) {
	require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())

	err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, "BIT").Err()
	require.ErrorContains(t, err, "value is not an integer or out of range")
})

t.Run("BITPOS rejects BYTE unit without end offset", func(t *testing.T) {
	require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())

	err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, "BYTE").Err()
	require.ErrorContains(t, err, "value is not an integer or out of range")
})

t.Run("BITPOS rejects non-integer bit argument with integer error", func(t *testing.T) {
	require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())

	err := rdb.Do(ctx, "BITPOS", "mykey", "x").Err()
	require.ErrorContains(t, err, "value is not an integer or out of range")
})

t.Run("BITPOS rejects non-integer bit argument even with BIT unit form", func(t *testing.T) {
	require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())

	err := rdb.Do(ctx, "BITPOS", "mykey", "x", 0, 0, "BIT").Err()
	require.ErrorContains(t, err, "value is not an integer or out of range")
})

If you’re also using AI tools to assist with coding, you can have them review your code once before submitting the PR.

[jihuayu]

LGTM. Thank you!

It would be great if you could chat with the reviewers a bit more.

[gongna-au]

@jihuayu Thank you! I've added extensive tests covering the BYTE/BIT option:

• BYTE/BIT equivalence and case-insensitivity
• Negative index handling for both modes
• start > end normalization returning -1
• Not-found semantics (bit=0 with/without explicit end, all-ones string)
• Parse error paths (invalid bit value, non-integer offsets, wrong type key)
• BIT-level single-bit precision checks

Happy to discuss any design concerns

[gongna-au]

@jihuayu thank you for the reminder!

AI usage disclosure: I used AI tools to assist with the following parts of this PR:

• Test scaffolding: AI helped generate the initial structure of the Go integration tests, which I then reviewed, adjusted, and verified against the actual command implementation.
• Code formatting: AI assisted with applying clang-format fixes.

The core implementation in cmd_bit.cc (the Parse logic for BYTE/BIT option handling and strict syntax validation) was written with my understanding of the Redis 7.0 BITPOS specification. I can explain the design decisions:

• BYTE/BIT is only accepted as the 6th argument (requiring both start and end), matching Redis 7.0 behavior
• Invalid option values and excess arguments are rejected with errInvalidSyntax
• The bit argument validation (0 or 1 only) is moved to the top of Parse for early rejection

Happy to answer any specific questions about the implementation or test coverage!

[jihuayu]

Well done. Thank you for the explanation.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
71.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud