-
-
Notifications
You must be signed in to change notification settings - Fork 5
Fix the findings of a verification pass over #11 #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
6ea8e72
Read a currency's minor unit from its code
pelmered c8c4166
Put the currency on an amount ICU has no symbol for
pelmered b15c963
Read a decimal column back the way it was written
pelmered 4733aa9
Take a currency of whatever shape a request carries
pelmered 11dbcb4
Resolve the empty locale before it becomes a cache key
pelmered 49ca102
Carry the magnitude when significant digits round the mantissa up
pelmered c8ef30e
Say how much precision a parsed amount actually has
pelmered 359417d
Keep the separator classes with the comment that explains them
pelmered 3182127
Name the method the config comment describes
pelmered 2ae6b76
Read a provider's minor unit as defensively as its name
pelmered 481a7ee
Fix config comment
pelmered 6bcc0be
Correct the formatter examples in the upgrade guide
pelmered 54e48bd
Name the rejected value's type instead of echoing it
pelmered 4d0960d
Deduplicate the mechanisms the review branch introduced
pelmered 6e6e046
Fix the four defects the adversarial review found
pelmered a17eac7
Add an agent guide, with CLAUDE.md importing it
pelmered c156299
Read a column in exponent notation, or refuse it
pelmered bba90be
Serialize the currency a row holds, not the one the registry has
pelmered 49379c8
Exclude a currency whichever way its provider spells it
pelmered 630af6f
Cache the currencies money:cache says it cached
pelmered c49b9c4
Store the amount that was written, or refuse it
pelmered 4e65907
Read a currency code through the registry in either cast
pelmered 7465b68
Say which configured currency the provider does not have
pelmered 9d79ece
Take a currency's minor unit from the provider that supplies it
pelmered e3d6e14
Hold the currency list for the process that built it
pelmered 22138dc
Refuse what formatting would deform, and what a scale cannot mean
pelmered 0cdd154
Store an amount an integer holds, or refuse it
pelmered f8be8ab
Cover the lines the suite never reached
pelmered 66b058c
Pin what strict parsing accepts for a long currency code
pelmered c466861
Say what a double carries, and what replaces formatAsDecimal
pelmered 68f8b1e
Read the amount a column holds, or refuse it, on every path
pelmered File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # AGENTS.md | ||
|
|
||
| This file provides guidance to coding agents working in this repository, and is the only copy of it: | ||
| `CLAUDE.md` is a stub that imports this file, so edit this one. | ||
|
|
||
| ## What this is | ||
|
|
||
| `pelmered/larapara` — a standalone Laravel package (no app skeleton) wrapping [Money PHP] with the | ||
| Laravel pieces it leaves out: a localized formatter/parser, Eloquent casts, migration macros, and a | ||
| cached currency registry. `README.md` is the reference documentation and is unusually complete — read | ||
| the relevant section before changing behaviour, and update it when behaviour changes. | ||
|
|
||
| ## Commands | ||
|
|
||
| ```bash | ||
| composer test # Pest via testbench package:test | ||
| composer test -- tests/Unit/MoneyFormatterTest.php # single file (args forward to Pest) | ||
| composer test -- --filter="parses" # single test by name | ||
| composer lint # pint + rector --dry-run + phpstan (level 8) | ||
| composer fix # pint --fix + rector (applies changes) | ||
| composer coverage # clover coverage over src/ | ||
| composer types # Pest type coverage | ||
| ``` | ||
|
|
||
| Tests run on Testbench with sqlite `:memory:`; there is no dev database to protect. | ||
|
|
||
| ## Architecture | ||
|
|
||
| **Minor units are the currency of the codebase.** Every amount crossing an API boundary — what | ||
| `Money::getAmount()` returns, what `parseToMinor()` produces, what `formatFromMinor()` takes, what the | ||
| casts are assigned — is an integer count of a currency's minor units. Scaling happens only where the | ||
| currency's `minorUnit` is in hand, and JPY (0) and BHD (3) are the cases that break naive `* 100` code. | ||
| The database boundary is the exception: under `store.format = decimal`, `MoneyCast::toDecimal()` writes | ||
| major units (`123456` USD as the column value `1234.56`) and `fromDecimal()` reads them back, so a | ||
| migration, a raw query or a backfill against a decimal column is working in major units. | ||
|
|
||
| **An amount is two columns.** `LaraParaServiceProvider::moneyColumns()` (the `money()`/`nullableMoney()`/ | ||
| `smallMoney()`/`unsignedMoney()` Blueprint macros) writes the amount column, a never-nullable currency | ||
| column, and an index over both. `currencyColumnFor()` is the single source of the suffix, used by the | ||
| macros and by both casts, so the two sides cannot disagree. `store.format` (`int` vs `decimal`) changes | ||
| what the macros create *and* how `MoneyCast` converts; `MoneyCast` refuses an amount the configured | ||
| `decimal_scale` would round away rather than letting the database silently lose it. | ||
|
|
||
| **ICU is the authority on formatting, and it is not ours.** `MoneyFormatter` (static, locale passed on | ||
| every call — no configured locale) delegates every symbol, separator and digit to `intl`/CLDR. ICU | ||
| version differs per PHP build, so exact formatted output is *not* stable across platforms: tests must | ||
| derive the volatile characters from `MoneyFormatter::getFormattingRules()` or normalize the space | ||
| characters, never assert a literal like `'1 234,56 kr'`. `tests/Pest.php` provides | ||
| `replaceNonBreakingSpaces()` for this; CI prints `INTL_ICU_VERSION` in every job. | ||
|
|
||
| **Parsing is deliberately asymmetric to formatting.** `parseToMinor()` accepts what `format()` and | ||
| `formatFromMinor()` write, plus two forgiveness rules (a dot read as the locale's decimal separator; a | ||
| grouping separator out of position dropped), refuses a number written in some *other* locale, and turns | ||
| both off under `strict`. The round trip is format→parse and never parse→parse: the input is a localized | ||
| amount in *major* units and the output is *minor* units, so `'100'` in USD parses to `'10000'` and | ||
| feeding the result back in scales it by the minor unit a second time. The accept/refuse boundary is | ||
| specified case-by-case in the README's parsing section — treat those examples as the spec. | ||
|
|
||
| **Currencies flow provider → repository → cache.** A `CurrenciesProvider` (ISO by default, optional | ||
| crypto, or a custom container-resolved class) supplies the list; `CurrencyRepository` applies | ||
| `available_currencies`/`excluded_currencies` and caches the result; `Currency::fromCode()` throws | ||
| `UnsupportedCurrency` for anything outside it. The cache hooks into `php artisan optimize` via the | ||
| `money:cache`/`money:clear` commands. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - Supports PHP 8.2–8.5 and Laravel 11.28 / 12 / 13 — this package **does** keep backwards | ||
| compatibility, overriding the global "current versions only" preference. Code must work across that | ||
| whole matrix (see `CurrencyRepository::FLEXIBLE_CREATED_KEY_PREFIX` for the shape this takes). | ||
| - No UI dependencies. No Filament, Livewire or Blade code belongs here; that lives in | ||
| `pelmered/filament-money-field`, which builds on this package. | ||
| - Types are declared with `php-static-analysis` attributes (`#[Returns]`, `#[Throws]`), not only | ||
| docblocks, and PHPStan runs at level 8 with full type coverage. | ||
| - Behaviour changes go in `UPGRADE.md` with the migration an affected application needs. | ||
|
|
||
| [Money PHP]: https://www.moneyphp.org/en/stable/ |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # CLAUDE.md | ||
|
|
||
| The guide lives in `AGENTS.md`, so there is one copy to maintain for every agent that reads this | ||
| repository. Claude Code follows the import below; read `AGENTS.md` directly if you do not. | ||
|
|
||
| @AGENTS.md |
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.