Skip to content

fix(generator): filter invalid canonical entries - #1917

Merged
clairernovotny merged 2 commits into
mainfrom
codex/codeql-canonical-validation
Aug 1, 2026
Merged

fix(generator): filter invalid canonical entries#1917
clairernovotny merged 2 commits into
mainfrom
codex/codeql-canonical-validation

Conversation

@clairernovotny

@clairernovotny clairernovotny commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

  • filter unsupported canonical schema keys before reporting the first validation error
  • use short-circuit scalar validation for month sequences
  • filter formatter properties into the grammar projection through one reusable fixed key list
  • preserve nested grammar values while excluding formatter-only keys
  • update the complete reviewed generator-source fingerprint on current main

Closes CodeQL alerts #374, #386, #387, #416, #419, #420, #440, #441, and #766. Supersedes safety-closed PRs #1913 and #1914.

Source order, first-invalid diagnostics, grammar property order/values, generated contracts, and incremental dependencies are unchanged.

Validation

  • canonical schema tests on net10.0 and net11.0 (58 passed each)
  • full source-generator suite on net10.0 and net11.0 (159 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

Head is based on exact main 6a16ff1aa1f15f1cd5fa5f0d6e1331604a54a16d and includes accepted #1911/#1912 fingerprints. Live PR #1902 has no production overlap with CanonicalLocaleAuthoring.cs; shared test/docs hunks are distinct, and #1902 must regenerate its complete fingerprint block after rebasing.

@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: 74059311-c5a2-4c83-b0d5-caf5a0d19fa1

📥 Commits

Reviewing files that changed from the base of the PR and between e34a405 and 8f620dd.

📒 Files selected for processing (1)
  • tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved locale grammar validation while preserving existing validation behavior and error messages.
    • Ensured inherited grammar values are correctly resolved, including plural rules and time-unit genders.
    • Removed an obsolete grammar setting from resolved locale data.
  • Documentation

    • Updated the locale YAML reference to reflect the latest authoring rules.

Walkthrough

The source generator centralizes formatter grammar property names and simplifies locale validation loops. Calendar item checks reject non-scalar values. Schema tests verify inherited grammar values and obsolete-key removal. Documentation records the updated source fingerprint.

Changes

Canonical locale grammar

Layer / File(s) Summary
Grammar extraction and validation
src/Humanizer.SourceGenerators/Common/CanonicalLocaleAuthoring.cs
Formatter grammar extraction uses shared property names. Locale validation uses filtered key enumeration. Calendar and genitive month checks use Any for scalar validation.
Resolved locale coverage
tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs, website/docs/contributing/locale-yaml-reference.mdx
Tests verify inherited grammar values and exclude the obsolete engine key. The documentation fingerprint is updated.

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

Possibly related PRs

Suggested labels: bug

Poem

A rabbit checks each grammar key,
Filters paths with care.
Inherited values now appear,
While engine is not there.
The locale stays precise.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: filtering invalid canonical entries in the generator.
Description check ✅ Passed The description directly explains the generator validation, grammar projection, fingerprint, and validation changes.
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-canonical-validation

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 force-pushed the codex/codeql-canonical-validation branch from dd677a8 to e34a405 Compare July 31, 2026 23:52
@clairernovotny

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@tests/Humanizer.SourceGenerators.Tests/SourceGenerators/CanonicalLocaleSchemaTests.cs`:
- Around line 302-305: Update the assertions around baseLocale.Grammar to assign
it once to a non-null local using the established assertion pattern, then use
that local for GetScalar, Values, and the timeUnitGenders lookup. Avoid repeated
nullable property accesses while preserving the existing assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 347aac5f-200a-4382-9d45-5094afc2ec7f

📥 Commits

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

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

@clairernovotny
clairernovotny marked this pull request as ready for review August 1, 2026 00:22
@clairernovotny
clairernovotny merged commit e997755 into main Aug 1, 2026
17 checks passed
@clairernovotny
clairernovotny deleted the codex/codeql-canonical-validation branch August 1, 2026 00:42
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