Skip to content

fix(generator): simplify locale resolution filters - #1915

Closed
clairernovotny wants to merge 1 commit into
mainfrom
codex/codeql-locale-yaml-filters
Closed

fix(generator): simplify locale resolution filters#1915
clairernovotny wants to merge 1 commit into
mainfrom
codex/codeql-locale-yaml-filters

Conversation

@clairernovotny

Copy link
Copy Markdown
Member

Summary

  • select the first cached locale directly
  • resolve each feature once, then retain non-null results
  • select the first script subtag directly
  • filter unsupported heading keys before reporting the unchanged diagnostic
  • add a focused unsupported-compass-key case and update the reviewed fingerprint

Closes CodeQL alerts #345, #360, #654, and #764.

Ordering, first-error messages, generated contracts, and incremental dependencies are unchanged.

Validation

  • targeted unsupported-canonical-property test on net10.0 and net11.0 (5 passed each)
  • full source-generator suite on net10.0 and net11.0 (160 passed each)
  • dotnet format Humanizer.slnx --verify-no-changes --verbosity diagnostic
  • pwsh -NoLogo -NoProfile -File tools/docs/build.ps1 -Mode Validate
  • dotnet pack src/Humanizer/Humanizer.csproj -c Release -o <temp>
  • pwsh -NoLogo -NoProfile -File tests/verify-packages.ps1 ...

Coordination

PR #1902 also changes LocaleYamlCatalog.cs, CanonicalLocaleSchemaTests.cs, and the fingerprint document. Its current hunks leave all four alert blocks unchanged, but it must rebase this merged implementation and regenerate the complete fingerprint block.

@coderabbitai

coderabbitai Bot commented Jul 31, 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: 967ab5b0-7f54-4767-98cf-1e15a5b69421

📥 Commits

Reviewing files that changed from the base of the PR and between 6a16ff1 and 820805f.

📒 Files selected for processing (3)
  • src/Humanizer.SourceGenerators/Common/LocaleYamlCatalog.cs
  • tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs
  • website/docs/contributing/locale-yaml-reference.mdx

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved locale configuration validation while preserving existing behavior and diagnostics.
    • Added validation for unsupported properties in the canonical compass configuration.
  • Documentation

    • Updated locale reference metadata to reflect the latest configuration catalog.

Walkthrough

The locale catalog replaces selected explicit loops with LINQ expressions while preserving behavior and diagnostics. Tests cover unsupported properties in surfaces.compass, and the documentation records the updated source fingerprint.

Changes

Locale catalog changes

Layer / File(s) Summary
Catalog resolution and validation refactor
src/Humanizer.SourceGenerators/Common/LocaleYamlCatalog.cs
Locale lookup, feature-map construction, script-subtag extraction, and heading-property filtering now use LINQ expressions.
Diagnostic coverage and catalog metadata
tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs, website/docs/contributing/locale-yaml-reference.mdx
Tests cover unsupported surfaces.compass properties. The recorded catalog fingerprint is updated.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • Humanizr/Humanizer#1780: Refactors locale catalog validation and adds overlapping surfaces.compass diagnostic coverage.
  • Humanizr/Humanizer#1907: Refactors related source-generator validation logic with LINQ and updates fingerprints.

Poem

A rabbit hops through catalog code,
LINQ now shares the load.
Compass errors join the test,
The fingerprint gets its crest.
“Clean paths!” cheers Bunny, pleased.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: simplifying locale resolution filters in the generator.
Description check ✅ Passed The description accurately covers the implementation changes, test coverage, validation steps, and coordination requirements.
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 💡 1
📝 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-locale-yaml-filters

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

Copy link
Copy Markdown
Member Author

Safety close: this stale head overlaps the active localization integration and must be serialized.

Exact head 820805f was built from stale parent a2a8098. It overlaps #1902 in all three changed paths: src/Humanizer.SourceGenerators/Common/LocaleYamlCatalog.cs, tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs, and website/docs/contributing/locale-yaml-reference.mdx. Its fingerprint document also predates the merged #1912 TokenMap fingerprint.

Serialize this correction after #1913 reaches its terminal reviewed merge. Then rebuild from fingerprint-current main, reapply the still-needed #345/#360/#654/#764 corrections, regenerate the complete fingerprint block, and restart fresh exact-head CI and review before any reopen/merge. The branch is intentionally retained; no branch edit or deletion is authorized by this close.

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