Skip to content

test(CV03): add test for unquoted multibyte identifiers#2246

Draft
benfdking wants to merge 1 commit intomainfrom
fix/cv03-multibyte-identifier-test
Draft

test(CV03): add test for unquoted multibyte identifiers#2246
benfdking wants to merge 1 commit intomainfrom
fix/cv03-multibyte-identifier-test

Conversation

@benfdking
Copy link
Collaborator

Summary

Fixes #2067

Adds a regression test for the issue where unquoted multibyte identifiers (e.g., 日本語) would cause a panic in the CV03 rule:

thread 'main' panicked at crates/lib/src/rules/convention/cv03.rs:85:14:
called `Option::unwrap()` on a `None` value

The underlying issue was fixed in previous refactoring (the code now properly handles identifiers without position markers), but there was no test coverage to prevent regression.

Test plan

  • Added test case test_pass_unquoted_multibyte_identifier with DuckDB dialect
  • Verified the test passes locally

🤖 Generated with Claude Code

Add regression test for issue #2067 where unquoted multibyte identifiers
(e.g., 日本語) would cause a panic in the CV03 rule.

The underlying issue was fixed in previous refactoring but had no test
coverage to prevent regression.

Fixes #2067

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link

Benchmark for 5e61f4d

Click to view benchmark
Test Base PR %
DepthMap::from_parent 53.1±0.71µs 52.0±1.44µs -2.07%
fix_complex_query 12.4±0.33ms 12.3±0.18ms -0.81%
fix_superlong 168.9±11.57ms 168.2±10.41ms -0.41%
parse_complex_query 4.2±0.06µs 4.1±0.04µs -2.38%
parse_expression_recursion 7.2±0.12µs 7.1±0.10µs -1.39%
parse_simple_query 1054.3±13.10ns 1049.5±9.56ns -0.46%

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a regression test for a bug where unquoted multibyte identifiers (e.g., 日本語) caused a panic in the CV03 rule. The underlying issue was previously fixed during refactoring when the code was updated to properly handle identifiers without position markers, but test coverage was missing to prevent future regressions.

Changes:

  • Added test case test_pass_unquoted_multibyte_identifier that verifies CV03 handles unquoted multibyte identifiers without panicking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +36 to +46
test_pass_unquoted_multibyte_identifier:
pass_str: |
SELECT
日本語
FROM foo
configs:
core:
dialect: duckdb
rules:
convention.select_trailing_comma:
select_clause_trailing_comma: forbid
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a companion test case with select_clause_trailing_comma: require mode to ensure the multibyte identifier handling works correctly in both modes. While the current test is sufficient to catch the panic, testing both modes would provide more comprehensive coverage and better match the pattern established by other tests in this file (which test both require and forbid modes).

Copilot uses AI. Check for mistakes.
@benfdking benfdking marked this pull request as draft January 31, 2026 17:23
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 1, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing fix/cv03-multibyte-identifier-test (b4f077c) with main (7e380c6)1

Summary

✅ 6 untouched benchmarks
⏩ 6 skipped benchmarks2

Footnotes

  1. No successful run was found on main (c842e67) during the generation of this report, so 7e380c6 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Unexpected exception with unquoted multibyte identifier

1 participant