Skip to content

refactor(words): simplify token membership checks - #1926

Merged
clairernovotny merged 1 commit into
mainfrom
codex/codeql-word-membership
Aug 1, 2026
Merged

refactor(words): simplify token membership checks#1926
clairernovotny merged 1 commit into
mainfrom
codex/codeql-word-membership

Conversation

@clairernovotny

Copy link
Copy Markdown
Member

Summary

Four parser membership predicates now use the runtime's allocation-free array lookup instead of hand-written filtering loops. This clears the routed CodeQL findings while preserving ordinal string equality and public behavior; independent A/B validation found identical outputs and allocations on net8, net10, and net11 with zero public API delta.

Related: alert 312, alert 313, alert 321, alert 322

Validation

  • CoverageGapTests: 195/195 on net8.0, net10.0, and net11.0
  • Full Humanizer.Tests: 61,018/61,018 on net8.0, net10.0, and net11.0
  • net48 Release build: 0 warnings, 0 errors
  • dotnet format Humanizer.slnx --verify-no-changes --verbosity diagnostic
  • Release pack and tests/verify-packages.ps1
  • Complete docs validation and native-AOT runtime suite
  • Independent public-path A/B: identical outputs and allocations across da, so, fil, and fr; zero API delta

Checklist

  • Implementation is clean
  • Code adheres to the existing coding standards
  • No Code Analysis warnings
  • There is proper unit test coverage
  • No copied third-party code
  • Comments remain limited to existing API documentation
  • XML documentation remains accurate
  • Based on live main
  • Routed CodeQL alerts are linked above
  • Readme change is not needed for this behavior-preserving internal refactor
  • Applicable validation from AGENTS.md passed

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 33aa4f8c-bac8-4ea9-bb3d-40e878694a0f

📥 Commits

Reviewing files that changed from the base of the PR and between 7a8a8f1 and 91bc6ad.

📒 Files selected for processing (4)
  • src/Humanizer/Localisation/WordsToNumber/InvertedTensWordsToNumberConverter.cs
  • src/Humanizer/Localisation/WordsToNumber/LinkingAffixWordsToNumberConverter.cs
  • src/Humanizer/Localisation/WordsToNumber/VigesimalCompoundWordsToNumberConverter.cs
  • tests/Humanizer.Tests/CoverageGapTests.cs

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved word-to-number parsing for compound expressions, including phrases such as “one and two.”
    • Confirmed ignored conjunctions are handled correctly and produce the expected numeric result.
  • Refactor

    • Simplified internal token checks while preserving existing parsing behavior.

Walkthrough

The change replaces manual token membership loops with Array.IndexOf expressions in three word-to-number converters. A coverage test verifies that the vigesimal converter parses “one and two” as 3.

Changes

Words-to-number converter refactor

Layer / File(s) Summary
Replace token membership loops
src/Humanizer/Localisation/WordsToNumber/*Converter.cs
ShouldIgnore and IsVigesimalFollower use Array.IndexOf expressions instead of explicit loops.
Validate ignored conjunction parsing
tests/Humanizer.Tests/CoverageGapTests.cs
Coverage data confirms that “one and two” produces 3 with VigesimalCompoundWordsToNumberConverter.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Poem

A rabbit checks each token fast,
With Array.IndexOf at last.
“One and two” becomes three,
Clean loops hop away with glee.
The numbers parse reliably.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the refactoring of token membership checks.
Description check ✅ Passed The description directly explains the membership-check refactor, related CodeQL alerts, preserved behavior, and validation results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/codeql-word-membership

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@clairernovotny
clairernovotny merged commit cec5a34 into main Aug 1, 2026
17 checks passed
@clairernovotny
clairernovotny deleted the codex/codeql-word-membership branch August 1, 2026 03:40
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.

1 participant