fix(inflekt): audit pluralize and singularize against the dictionary in both directions - #114
Merged
Merged
Conversation
…in both directions Both directions now go through a dictionary-proven exception table in front of suffix rules, and both are idempotent: pluralize of a plural and singularize of a singular return the word unchanged.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
PostGraphile v5 inflects with the pluralize package, not inflection: it returns indices and appendices, and treats deer/fish/moose as invariant. Follow it for both, while keeping the English plural for -us (radiuses, not radii).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pluralizewas never audited.singularizehad a dictionary-proven exception table in front of its suffix rules;pluralizewas rawinflectionplus a couple of patches, so it coined non-words (radius -> radius,delta -> delta,chassis -> chasses,bias -> bias) and was not idempotent (pluralize('apis') -> 'apises'— the shape of the bug that broke constructive's GraphQL naming).Both directions are now the same shape — dictionary table, then rules — and both are idempotent, so all four things a caller can do are correct:
pluralizegained the plural half of that:isPluralasks the table first and the rules second, so it holds for words no dictionary knows:isRulesPlural(w)issingularizeByRules(w) !== w && pluralizeByRules(singularizeByRules(w)) === w.The
*ByRulesfunctions are new and are the point of the change: the generator must run against rules only, or the table it emits would be validated against itself and mask every rule defect.pluralizeByRulesalso fixes the three shapesinflectionrefuses to pluralize at all (radius -> radiuses,forum -> forums,delta -> deltas), keeps-ousadjectives alone rather than coininganxiouses, and stops rewriting non-Greek-is(chassis -> chassises, notchasses).scripts/generate-exceptions.tsnow emitsPLURAL_EXCEPTIONSalongsideSINGULAR_EXCEPTIONS(766 singular + 298 plural pairs, idempotent across runs). Where the dictionary attests two plurals it picks by corpus frequency (hero -> heroes, notheros).Two preferences are set against PostGraphile v5's own inflector, which is the
pluralizepackage rather thaninflection:-ex/-ixtakes the attested Latin plural so the family is self-consistent and matches it (indices,appendices,vertices), and its uncountables are honoured (deer,fish,mooseare invariant, though the dictionary lists "deers"/"fishes"). We deliberately diverge from it on-us, where it coins Latin for identifiers (radius -> radii,cactus -> cacti,alumnus -> alumni) and we keep the English plural (radiuses), and onschema, which it pluralizes asschemata.scripts/audit.tsruns the four properties over every singular/plural pair in/usr/share/dict/words(17,546 singulars, 17,653 attested plurals) and is now committed aspnpm audit:Every remaining line was inspected; they are correct answers the dictionary happens to also list a regular variant for, not defects.
__tests__/four-properties.test.tspins all four properties over ~250 hand-picked pairs: identifiers (api,uuid,mutex,schema,datum,api_key,ALL_CAPS), the English shapes that break suffix rules (irregulars,-f/-ves,-sis,-o, Latin), invariants, casing, and a documented block for the three cases no algorithm can resolve (triesis the plural oftry, nottrie;tty -> tties;dns -> dn).Verified downstream: constructive's
graphql-namingsuite (the one theapis -> apisesregression broke) passes 27/27 against this build, andgraphql/codegen+graphql/queryare unchanged; the postgres-dependent suites fail identically with published 0.8.0.Link to Devin session: https://app.devin.ai/sessions/c2c27132839a45219a440fd10cbf13a6
Requested by: @pyramation