diff --git a/.github/workflows/base64-docs.yml b/.github/workflows/base64-docs.yml index dcac72d..d51a1c3 100644 --- a/.github/workflows/base64-docs.yml +++ b/.github/workflows/base64-docs.yml @@ -39,8 +39,7 @@ jobs: with: php-version: ${{ matrix.php }} - # The autoloader the check runs under is the one a consumer gets, src/Upload/i18n.php - # included: an unresolved `files` entry reports __() as undefined from inside File.php. + # The autoloader the check runs under is the one a consumer gets - name: Install the library run: composer install --no-dev --no-interaction --no-progress diff --git a/CHANGELOG.md b/CHANGELOG.md index ff13b01..1a8edeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,7 @@ A security release. New protections are on by default and will refuse some uploa * **`Upload\Exception`'s constructor takes the error code and the message's values before `$code` and `$previous`.** The signature is now `__construct(string $message, ?FileInfoInterface $fileInfo = null, string $errorCode = ErrorCode::NONE, array $messageArgs = [], int $code = 0, ?Throwable $previous = null)`. **A validation of your own passing `$code` or `$previous` positionally is passing them to the wrong parameters.** `new Exception($message, $fileInfo)` is unaffected * **`File::recordError()` takes a message id, its values, an error code and a filename** rather than a finished line: `recordError(string $messageId, array $args = [], string $errorCode = ErrorCode::NONE, ?string $filename = null)`. A `File` subclass that records an error of its own passes the message and the filename separately, and the `"filename: message"` composition happens in one place. `recordError('Rejected by the scanner')` still does what it did * **`File::$errors` and `File::$constructorErrors` are gone**, replaced by a `private $errorDetails` holding each error as its parts. A subclass appending to `$errors` directly was the one route around the sanitizing guarantee `getErrors()` carries, and it is what would have left `getErrorDetails()` with holes. **Use `recordError()`**, which is still `protected`, and `getErrors()` to read -* **`File::$errorCodeMessages` is now the method `File::getUploadErrorMessages()`.** A PHP 7.3 constant expression cannot call a function, so an array of literals is all a property could hold and no extractor can see one. Each value is marked with `__()` instead. A subclass overriding the wording overrides the method +* **`File::$errorCodeMessages` is now the method `File::getUploadErrorMessages()`.** A PHP 7.3 constant expression cannot call a function, so an array of literals is all a property could hold and no extractor can see one. Each value is marked with `Translation::__()` instead. A subclass overriding the wording overrides the method * **`'5MB'` now means 5 MiB, not 5 bytes.** `File::humanReadableToBytes()` reads a trailing `B`, where `substr($input, -1)` saw only the `B`. **Every `KB`/`MB`/`GB` bound you pass becomes much larger.** Bounds without the trailing `B` are unchanged * **An unrecognized unit throws** instead of being read as bytes. `new Size('1T')` was a one-byte bound that rejected every upload while reading as a generous one. **Check any bound whose unit isn't `B`, `K`, `M` or `G`** * **`FileInfo::getMd5()` is removed** from `FileInfo` and `FileInfoInterface`. `getHash('md5')` returns the same digest @@ -65,7 +65,7 @@ A security release. New protections are on by default and will refuse some uploa ## New Features * **Error messages can be translated, and no translations ship.** `Translation::setTranslator(callable)` installs a `function (string $text, string $domain): string` that `getErrors()` is looked up through. With none installed every message is the English it has always been, because the English string *is* the message id. Values are interpolated after the lookup, so a `%` in a filename or a size bound is never read as a `printf` conversion. A translator that throws, returns a non-string, returns an empty string, or returns a template whose placeholders do not match is ignored in favour of English, so a broken catalogue cannot turn a rejected upload into a fatal error and a partly-translated one cannot blank a message. Called at render time, so a locale switched mid-request is the one used -* **`GravityPdf\Upload\__()` marks each translatable string** where it is written — gettext's `N_()` idiom under a familiar name, `__(string $text, string $domain = Translation::DOMAIN)`. It returns `$text` and never translates; the lookup happens once, when `getErrors()` is read, which keeps `Exception::getMessage()` in English for your log and the raw msgid in `getErrorDetails()`. `xgettext --keyword=__` finds every call. `$domain` is for your extractor, not the runtime, which always looks up under `Translation::DOMAIN`. Namespaced, so it coexists with WordPress's global `__()` — but a file outside `GravityPdf\Upload` must import it or call it fully qualified, since an unqualified call with neither falls back to the global one +* **`Translation::__()` marks each translatable string** where it is written — gettext's `N_()` idiom under a familiar name, `__(string $text, string $domain = Translation::DOMAIN)`. It returns `$text` and never translates; the lookup happens once, when `getErrors()` is read, which keeps `Exception::getMessage()` in English for your log and the raw msgid in `getErrorDetails()`. `xgettext --keyword=__` finds every call: the keyword matches the trailing identifier, so `Translation::__()` extracts exactly as a bare `__()` does. `$domain` is for your extractor, not the runtime, which always looks up under `Translation::DOMAIN`. A static method rather than a function, so it loads with the class — a bare function needs Composer's `files` autoload entry, which an autoloader that only indexes classes never runs, and this library ships no such entry * **`i18n/upload.pot`**, the catalogue of every string that gets looked up, with translator comments on the ones that take values. A template only — no `.po` or `.mo` files. Rebuilt by `composer i18n:pot` and pinned by a workflow that fails on a diff, so a reworded message cannot orphan a consumer's translation of it. `Storage\FileSystem`'s messages are absent: they name the destination and are written for your log, not for whoever submitted the file * **`File::getErrorDetails(): array`** returns the same failures as their parts: `code`, the untranslated `message_id` and its `args`, the sanitized `filename` or `null`, and the finished `message`. Use it to branch on a failure, or to word one yourself. `getErrors()` is its `message` column * **`ErrorCode`** names every failure this library reports, as string constants that stay stable while wording does not: the `UPLOAD_ERR_*` outcomes, the collection's own, the shipped validations' and storage's. `Exception::getErrorCode()` returns one, alongside `getMessageId()` and `getMessageArgs()` diff --git a/CLAUDE.md b/CLAUDE.md index 89336b3..ddb1db1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,7 +43,7 @@ The `i18n` workflow regenerates the catalogue and fails on a diff, which is what `tools/psr7-readme/` holds a second scratch manifest, for the same reason `tools/phpstan/` does: `docs/psr7.md` documents a PSR-7 bridge as caller code, and `verify.php` runs those snippets — **read out of the Markdown at run time, never copied** — against nyholm/psr7 and guzzlehttp/psr7. Rename a documented class or drop an example and the extraction fails by name. The library still requires nothing but `ext-fileinfo`; the check has its own `composer.json`, its own composer script and its own workflow. `verify.php` is covered by `lint` and `check-syntax` — it is the documented code's only safety net, so it does not get to sit outside the checks everything else passes. -`docs/base64-uploads.md` is the same arrangement for a file posted as base64 in a JSON body, run by `tools/base64-docs/verify.php`. That directory has no manifest: a base64 payload is a string in the request body, so there is no third-party implementation to run the bridge against, and the check loads the library through the root `vendor/autoload.php` — the autoloader a consumer gets, `src/Upload/i18n.php` included. The page declares one snippet each for the bridge and for validating and storing what it returns. +`docs/base64-uploads.md` is the same arrangement for a file posted as base64 in a JSON body, run by `tools/base64-docs/verify.php`. That directory has no manifest: a base64 payload is a string in the request body, so there is no third-party implementation to run the bridge against, and the check loads the library through the root `vendor/autoload.php`, the autoloader a consumer gets. The page declares one snippet each for the bridge and for validating and storing what it returns. The `phpunit` workflow carries a second job, `cross-file-system`, which mounts a tmpfs and points `UPLOAD_TEST_OTHER_FS` at it so `FileSystemTest::testStoresAFileFromAnotherFileSystem()` runs against two real file systems rather than skipping. It runs the **whole suite** with `--fail-on-skipped` rather than filtering to that test: a `--filter` matching nothing exits 0 with "No tests executed!", so it needed a second guard to prove it had run, and renaming the test was enough to trigger exactly that. That test is the suite's only `markTestSkipped()`, which is what makes one guard sufficient — keep it that way, or the job goes quiet. Tests that cannot run in a given configuration are excluded by group instead: `@group mbstring` marks the ones that need the UTF-8 repair, so the `no-mbstring` job drops them with `--exclude-group` rather than adding a second skip, and `@group posix` marks the ones that need `symlink()`, `chmod()` or `umask()` to mean something, or a name NTFS refuses outright, which the `windows` job drops the same way. Set `UPLOAD_TEST_OTHER_FS` to run it locally: on macOS, `hdiutil attach -nomount ram://8192` then `diskutil erasevolume HFS+ UPLOADTMP `. @@ -95,9 +95,13 @@ Four optional callbacks (`beforeValidate`, `afterValidate`, `beforeUpload`, `aft **`render()` rejects a mismatched translation, and only where there are values to fill.** Conversions that do not match the source's would interpolate wrongly — but with no values `interpolate()` hands the template back whole and nothing can go wrong, so checking anyway only rejected good translations. A space is a legal `printf` flag, so `50% of your quota is used` carries one conversion by PHP's own grammar (`vsprintf()` raises on it) while a German translation carrying no `%` counts none. Before the interpolation rather than in it, because that is the last point where the English is still in hand. -**The call sites mark, they do not translate.** `GravityPdf\Upload\__(string $text, string $domain = Translation::DOMAIN)` is gettext's `N_()` idiom under a familiar name and shape: it returns its argument, and the lookup happens once, when `getErrors()` is read. That keeps `getMessage()` English, keeps an untranslated msgid in `getErrorDetails()`, and takes the locale when the message is read rather than when the file was rejected. A `__()` that translates in place was rejected for those three reasons, despite the shape being the familiar one. +**The call sites mark, they do not translate.** `Translation::__(string $text, string $domain = self::DOMAIN)` is gettext's `N_()` idiom under a familiar name and shape: it returns its argument, and the lookup happens once, when `getErrors()` is read. That keeps `getMessage()` English, keeps an untranslated msgid in `getErrorDetails()`, and takes the locale when the message is read rather than when the file was rejected. A `__()` that translates in place was rejected for those three reasons, despite the shape being the familiar one. -**Sharing the name with WordPress's global `__()` is what that familiarity costs.** PHP resolves an unqualified call against the global namespace when the current one has no match, so a file under `GravityPdf\Upload\Validation` that forgets `use function GravityPdf\Upload\__;` reaches WordPress's function instead — silently, translating at the throw. `File` and `Translation` declare `GravityPdf\Upload` and cannot take that path; the four validators can. `I18nTest::testEveryCallerCanReachTheMarker()` reads every file under `src/` for it, and is the only guard. It runs as **one** test over the corpus and asserts it found at least one marked string: per-file, it could only assert about a file that marks something, so renaming the marker made every case vacuous while the suite stayed green. The marker has been renamed twice. Outside WordPress the same mistake is a plain fatal, which is why `testNoGlobalMarkerCanMaskAMissingImport()` pins that `tests/bootstrap.php` never grows a global `__()` stub — do not add one. A fully qualified `\GravityPdf\Upload\__()` is immune to the fallback and extracts identically. +**The marker is a static method because a function is unreachable under a classmap.** It was `GravityPdf\Upload\__()`, declared in `src/Upload/i18n.php` and loaded by Composer's `files` autoload. That entry is Composer's own: an autoloader that indexes classes and nothing else — a classmap over php-scoper output, which is how a WordPress plugin usually consumes this library — never runs it, so `__()` was undefined on every error path while uploads succeeded (issue #42). Do not put a bare function back, and do not add a `files` entry; `I18nTest::testTheMarkerTravelsWithTheClass()` refuses both. + +Moving it onto `Translation` also ends the namespace fallback. An unqualified **function** call resolves against the global namespace when the current one has no match, so a validator forgetting `use function GravityPdf\Upload\__;` reached WordPress's `__()` and translated at the throw. A **class** name has no such fallback: a validator forgetting `use GravityPdf\Upload\Translation;` fatals at the call instead. That is still only reached on an error path, so `I18nTest::testEveryCallerCanReachTheMarker()` reads every file under `src/` for the import rather than waiting for a rejected upload. It runs as **one** test over the corpus and asserts it found at least one marked string: per-file, it could only assert about a file that marks something, so renaming the marker made every case vacuous while the suite stayed green. The marker has been renamed twice. + +Extraction needs no configuration for the move. `xgettext` matches the keyword against the trailing identifier and ignores the class prefix, so `-k__:1` reads `Translation::__()`, `\GravityPdf\Upload\Translation::__()` and an aliased `T::__()` alike. `i18n/upload.pot` regenerating byte-identical across the move is the proof. **The marker's `$domain` is extractor-facing.** It is discarded, and `render()` looks every message up under `Translation::DOMAIN` regardless, so marking a string of your own under your own domain describes it to your catalogue tooling and redirects nothing. It defaults to `Translation::DOMAIN` for the same reason WordPress's `__()` defaults to `'default'`. **The calls in `src` do not pass it** — every string here belongs to that domain and the default already says so, where a literal at twenty call sites is twenty places to drift. The extraction keyword must stay `-k__:1`: `-k__:1,2` reads the second argument as a plural form and extracts **nothing at all**. @@ -105,11 +109,11 @@ Four optional callbacks (`beforeValidate`, `afterValidate`, `beforeUpload`, `aft **`Validation\Size` has a message id per unit, not a unit per value.** The bound is scaled to the largest of B/KB/MB/GB it reaches and the unit sits inside the string, because a value is interpolated after the lookup and a translator never sees it — French writes `Mo`, so `MB` cannot be a value. `getTooLargeMessages()`/`getTooSmallMessages()` hold the eight, as methods for the same reason `getUploadErrorMessages()` is one. `scale()` rounds a maximum down and a minimum up so the size named is always one the file would pass at: 5,000,000 bytes reported as `4.8 MB` would name a size still rejected. Its decimal separator is a `.`, because picking one needs a locale this library does not take — **`scale()` is `protected` and called through `static::` for that reason**, and is the seam for it. An override owns the number and the unit together, which a number-formatter hook on `Translation` could not: the unit is chosen here, and by the time a value is interpolated the message id naming it has already been picked. It only ever comes up for a limit configured in decimal bytes; `'5M'`, `'500K'` and any binary count divide exactly into the 1024-based ladder and render whole. -**`getUploadErrorMessages()` is a method because a PHP 7.3 constant expression cannot call `__()`.** The seven `UPLOAD_ERR_*` strings were a `protected static` array of literals, which no extractor can see; a property could hold nothing else, so they moved to a method for the same reason `FileSystem::getDefaultBlockedExtensions()` is one. A subclass overriding the wording overrides the method, and its strings are its own to extract. +**`getUploadErrorMessages()` is a method because a PHP 7.3 constant expression cannot call `Translation::__()`.** The seven `UPLOAD_ERR_*` strings were a `protected static` array of literals, which no extractor can see; a property could hold nothing else, so they moved to a method for the same reason `FileSystem::getDefaultBlockedExtensions()` is one. A subclass overriding the wording overrides the method, and its strings are its own to extract. -**A translator comment must sit immediately above the `__()` call**, inside the argument list, not above the `throw`. `xgettext` attaches a comment to the line the string is on; move it up two lines to sit above `throw new Exception(` and it is silently dropped from the catalogue. +**A translator comment must sit immediately above the `Translation::__()` call**, inside the argument list, not above the `throw`. `xgettext` attaches a comment to the line the string is on; move it up two lines to sit above `throw new Exception(` and it is silently dropped from the catalogue. -**`src/Upload/i18n.php` is loaded by Composer's `files` autoload**, the only entry in it. One consequence: a tool installing this library from a path repository — `tools/psr7-readme/` — only sees a change to the autoload configuration when the package is re-resolved, so that script runs `composer update` rather than `install`. A stale vendor directory reports `__()` as undefined, from inside `File.php`. +**`composer.json` declares PSR-4 and nothing else.** The tools installing this library from a path repository — `tools/psr7-readme/`, `tools/translator-readme/` — run `composer update` rather than `install`, because a lock records the path package's autoload configuration and `install` replays the one it was written against. **`ErrorCode` is the stable identifier; the message is not.** Wording is translated and edited between releases, so a caller branching on it breaks quietly — `getErrorDetails()` and `Exception::getErrorCode()` carry a code for that instead. A code is not a message id: the id travels with the catalogue and changes when the wording does. A validator of your own that throws without naming one is recorded as `VALIDATION_REJECTED` rather than as nothing, so every entry in `getErrorDetails()` has something to branch on. diff --git a/README.md b/README.md index 008841b..2919b85 100644 --- a/README.md +++ b/README.md @@ -435,8 +435,8 @@ always been. The English string **is** the message id, so there is nothing to ma `callable` with `Translation::setTranslator()` and `getErrors()` is looked up through it. Nothing else is: `Exception::getMessage()` stays English for your log. -[docs/translation/](docs/translation/README.md) covers the hook, the catalogue, the `__()` -marker and what a broken translation cannot do, with a working adapter for Symfony, Laravel, +[docs/translation/](docs/translation/README.md) covers the hook, the catalogue, the +`Translation::__()` marker and what a broken translation cannot do, with a working adapter for Symfony, Laravel, php-gettext and WordPress. ## Reacting to a failure rather than showing it diff --git a/UPGRADE.md b/UPGRADE.md index 5b43f3e..27fb467 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -267,10 +267,6 @@ The English string is the message id, so there is nothing to map, and a lookup t nothing returns what the library would have said anyway. Seed a catalogue from `i18n/upload.pot`; no `.po` or `.mo` files ship. -The leading backslash matters if this file also uses the library's `__()` marker for a -validation of your own. Without it the call reaches the marker, which returns its argument, and -your translator silently does nothing. - In WordPress, merge the catalogue into yours when you extract — `wp i18n make-pot` never looks inside `vendor/`: diff --git a/composer.json b/composer.json index 8193a21..f7d1178 100644 --- a/composer.json +++ b/composer.json @@ -39,17 +39,14 @@ "autoload": { "psr-4": { "GravityPdf\\Upload\\": "src/Upload" - }, - "files": [ - "src/Upload/i18n.php" - ] + } }, "scripts": { "base64-docs": "@php tools/base64-docs/verify.php", "check-syntax": "@php ./vendor/bin/parallel-lint --exclude .git --exclude vendor --exclude tools/phpstan/vendor --exclude tools/psr7-readme/vendor --exclude tools/translator-readme/vendor .", "i18n:pot": "@php tools/i18n/generate-pot.php", - "lint": "@php ./vendor/bin/phpcs --standard=PSR12 ./src/ ./tests/ ./tools/i18n/ ./tools/base64-docs/verify.php ./tools/psr7-readme/verify.php ./tools/translator-readme/verify.php ./tools/translator-readme/marker-import.php ./tools/translator-readme/global-underscore.php", - "lint:fix": "@php ./vendor/bin/phpcbf --standard=PSR12 ./src/ ./tests/ ./tools/i18n/ ./tools/base64-docs/verify.php ./tools/psr7-readme/verify.php ./tools/translator-readme/verify.php ./tools/translator-readme/marker-import.php ./tools/translator-readme/global-underscore.php", + "lint": "@php ./vendor/bin/phpcs --standard=PSR12 ./src/ ./tests/ ./tools/i18n/ ./tools/base64-docs/verify.php ./tools/psr7-readme/verify.php ./tools/translator-readme/verify.php ./tools/translator-readme/global-underscore.php", + "lint:fix": "@php ./vendor/bin/phpcbf --standard=PSR12 ./src/ ./tests/ ./tools/i18n/ ./tools/base64-docs/verify.php ./tools/psr7-readme/verify.php ./tools/translator-readme/verify.php ./tools/translator-readme/global-underscore.php", "phpstan": [ "@composer install --working-dir=tools/phpstan --no-interaction --quiet", "@php tools/phpstan/vendor/bin/phpstan analyse" diff --git a/docs/api-reference.md b/docs/api-reference.md index b30c034..69fa782 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -181,13 +181,16 @@ string, and they reach `getErrorDetails()` intact. | `Translation::interpolate(string $template, array $args = []): string` | The interpolation without the lookup, falling back to the template when the values do not fit. What `Exception` composes its English message with. | | `Translation::DOMAIN` | `'gravitypdf-upload'`. Fixed: WordPress forbids a variable text domain, and no extractor can follow one. | -`GravityPdf\Upload\__(string $text, string $domain = Translation::DOMAIN): string` is the -marker — a function, not a method. It is gettext's `N_()` idiom: it returns `$text`, so an -extractor records the msgid while the lookup happens elsewhere. It is not WordPress's `__()`, -and the two coexist. The domain is discarded, and is optional as it is on WordPress's `__()`; -the calls in `src` omit it. Outside the library's own namespace, import it with -`use function GravityPdf\Upload\__;` or call it fully qualified; an unqualified call with -neither falls back to the global `__()`. +`Translation::__(string $text, string $domain = Translation::DOMAIN): string` is the marker. +It is gettext's `N_()` idiom: it returns `$text`, so an extractor records the msgid while the +lookup happens elsewhere. It is not WordPress's `__()`, and the two coexist. The domain is +discarded, and is optional as it is on WordPress's `__()`; the calls in `src` omit it. + +A static method rather than a function so that it travels with the class. A function needs +Composer's `files` autoload entry, and an autoloader that only indexes classes — a classmap +built over a php-scoper'd tree, which is how a WordPress plugin usually consumes this library +— never runs it, leaving every error path a fatal on a site where uploads had only ever +succeeded. ## ErrorCode diff --git a/docs/extending.md b/docs/extending.md index 224b068..83cffd9 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -12,10 +12,9 @@ The exception message is what `getErrors()` shows the end user: ```php use GravityPdf\Upload\Exception; use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\Translation; use GravityPdf\Upload\ValidationInterface; -use function GravityPdf\Upload\__; - class MaxDimensions implements ValidationInterface { private $maxWidth; @@ -33,7 +32,7 @@ class MaxDimensions implements ValidationInterface if ($size['width'] > $this->maxWidth || $size['height'] > $this->maxHeight) { throw new Exception( - __('Image must be no larger than %1$sx%2$s pixels', 'my-plugin'), + Translation::__('Image must be no larger than %1$sx%2$s pixels', 'my-plugin'), $fileInfo, 'max_dimensions', [$this->maxWidth, $this->maxHeight] @@ -55,19 +54,15 @@ chose shows up in `getErrorDetails()` for a caller branching on it. Pass a finis no values if you would rather; `getErrorDetails()` then reports the code `ErrorCode::VALIDATION_REJECTED` for it, so every entry has one. -`__()` here is `GravityPdf\Upload\__()`: it marks the string for an extractor and hands it -straight back. It is **not** WordPress's `__()` and it never translates. Using it is -optional — leave the literal bare, or use your own marker if your rule's wording lives in -your own catalogue. Its second argument names a catalogue for whatever reads these calls; -the marker discards it, and this library looks the message up under `Translation::DOMAIN` -regardless. - -If you do use the marker, import it in **every** file that calls it with -`use function GravityPdf\Upload\__;`, or call it fully qualified. PHP resolves an unqualified -function call against the global namespace when the current one has no match, so in a WordPress -plugin a missing import reaches WordPress's `__()` instead: no error, but the string is -translated at the throw rather than at the render, and `Exception::getMessage()` stops being -the English you search your log for. `xgettext` extracts either form. +`Translation::__()` marks the string for an extractor and hands it straight back. It is +**not** WordPress's `__()` and it never translates. Using it is optional — leave the literal +bare, or use your own marker if your rule's wording lives in your own catalogue. Its second +argument names a catalogue for whatever reads these calls; the marker discards it, and this +library looks the message up under `Translation::DOMAIN` regardless. + +`xgettext -k__:1` extracts it exactly as it extracts a bare `__()`: the keyword matches the +trailing identifier and ignores the class prefix, so an aliased `T::__()` and a fully +qualified `\GravityPdf\Upload\Translation::__()` are read the same way. Your message goes through `Filename::sanitizeForDisplay()` first: bidi controls are deleted, runs of control characters collapse to a single space, the line is cut to diff --git a/docs/translation/README.md b/docs/translation/README.md index 5f2ce68..3d99644 100644 --- a/docs/translation/README.md +++ b/docs/translation/README.md @@ -23,12 +23,11 @@ In the source each string is marked, not translated: ```php use GravityPdf\Upload\ErrorCode; use GravityPdf\Upload\Exception; - -use function GravityPdf\Upload\__; +use GravityPdf\Upload\Translation; throw new Exception( /* translators: %1$s: the largest accepted size, in megabytes */ - __('File size is too large. Must be no more than %1$s MB'), + Translation::__('File size is too large. Must be no more than %1$s MB'), $fileInfo, ErrorCode::SIZE_TOO_LARGE, [$amount] diff --git a/docs/translation/laravel.md b/docs/translation/laravel.md index f55d6c3..a83c76e 100644 --- a/docs/translation/laravel.md +++ b/docs/translation/laravel.md @@ -19,11 +19,6 @@ Translation::setTranslator(static function (string $text): string { `__()` lives in `illuminate/foundation`. Using the Translator standalone, call `$translator->get($text)` instead. -**Note the leading backslash.** If this file also uses this library's `__()` marker for a -validation of your own, it will have imported it, and an unqualified `__($text)` calls the -marker instead. The marker returns its argument, so your translator would silently do nothing -and every message would stay English. `\__()` is always Laravel's. - ## Produce the catalogue `lang/de.json` takes the msgids as keys: diff --git a/src/Upload/File.php b/src/Upload/File.php index 103bca8..63213e4 100644 --- a/src/Upload/File.php +++ b/src/Upload/File.php @@ -164,7 +164,7 @@ public function __construct(string $key, StorageInterface $storage) || array_key_exists('error', $_FILES[$key]) === false ) { $this->recordError( - __('An uploaded file was sent in a format that cannot be read'), + Translation::__('An uploaded file was sent in a format that cannot be read'), [], ErrorCode::MALFORMED_UPLOAD ); @@ -186,7 +186,7 @@ public function __construct(string $key, StorageInterface $storage) the other two keys, which a ragged or mistyped entry leaves short. */ if (is_string($tmpName) === false || is_string($name) === false || is_int($errorCode) === false) { $this->recordError( - __('An uploaded file was sent in a format that cannot be read'), + Translation::__('An uploaded file was sent in a format that cannot be read'), [], ErrorCode::MALFORMED_UPLOAD ); @@ -209,7 +209,7 @@ public function __construct(string $key, StorageInterface $storage) TypeError out of createFromFactory(). The code is checked too because `(int) [0]` is 1, which reported the file as exceeding `upload_max_filesize`. */ $this->recordError( - __('An uploaded file was sent in a format that cannot be read'), + Translation::__('An uploaded file was sent in a format that cannot be read'), [], ErrorCode::MALFORMED_UPLOAD ); @@ -309,9 +309,9 @@ private static function renderMessage(string $messageId, array $args, ?string $f * A method rather than the property this was before 4.0.0, for the reason * `FileSystem::getDefaultBlockedExtensions()` is one: a PHP 7.3 constant expression * cannot call a function, so an array of literals is all a property could hold and an - * extractor cannot see one. `__()` marks each without translating it; the lookup happens - * where the message is rendered. Still `static::`, so a subclass replacing the wording - * keeps working — its strings are its own to extract. + * extractor cannot see one. `Translation::__()` marks each without translating it; the + * lookup happens where the message is rendered. Still `static::`, so a subclass replacing + * the wording keeps working — its strings are its own to extract. * * @return array Keyed by `UPLOAD_ERR_*` */ @@ -319,16 +319,16 @@ protected static function getUploadErrorMessages(): array { return [ UPLOAD_ERR_INI_SIZE => - __('The uploaded file exceeds the upload_max_filesize directive in php.ini'), + Translation::__('The uploaded file exceeds the upload_max_filesize directive in php.ini'), UPLOAD_ERR_FORM_SIZE => - __( + Translation::__( 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form' ), - UPLOAD_ERR_PARTIAL => __('The uploaded file was only partially uploaded'), - UPLOAD_ERR_NO_FILE => __('No file was uploaded'), - UPLOAD_ERR_NO_TMP_DIR => __('The server is missing its temporary upload folder'), - UPLOAD_ERR_CANT_WRITE => __('The server could not write the file to disk'), - UPLOAD_ERR_EXTENSION => __('A PHP extension stopped the file upload'), + UPLOAD_ERR_PARTIAL => Translation::__('The uploaded file was only partially uploaded'), + UPLOAD_ERR_NO_FILE => Translation::__('No file was uploaded'), + UPLOAD_ERR_NO_TMP_DIR => Translation::__('The server is missing its temporary upload folder'), + UPLOAD_ERR_CANT_WRITE => Translation::__('The server could not write the file to disk'), + UPLOAD_ERR_EXTENSION => Translation::__('A PHP extension stopped the file upload'), ]; } @@ -337,7 +337,7 @@ protected static function getUploadErrorMessages(): array */ protected static function uploadFailureMessageId(int $errorCode): string { - return static::getUploadErrorMessages()[$errorCode] ?? __('Unknown error'); + return static::getUploadErrorMessages()[$errorCode] ?? Translation::__('Unknown error'); } /** @@ -542,7 +542,7 @@ public function getErrorDetails(): array * way in since `$errorDetails` became `private` in 4.0.0. * * @param string $messageId The English message, or a message id whose `%1$s` placeholders - * `$args` fills. Marked with `__()` at the call site, not + * `$args` fills. Marked with `Translation::__()` at the call site, not * translated here: `getErrorDetails()` renders it, so the locale * in force when the message is read is the one used * @param array $args Values for the message's placeholders @@ -862,7 +862,7 @@ private function runValidations(): array them. Skipping the after-hook leaks on exactly the files that failed. */ if ($fileInfo->isUploadedFile() === false) { $this->recordError( - __('This file was not received as an upload'), + Translation::__('This file was not received as an upload'), [], ErrorCode::NOT_AN_UPLOADED_FILE, $this->getSanitizedFilename($fileInfo) @@ -907,7 +907,7 @@ private function runValidations(): array shown to end users. Rethrow an Upload\Exception to surface either. */ $sanitizedFilename = $sanitizedFilename ?? $this->getSanitizedFilename($fileInfo); $this->recordError( - __('Validation could not be completed'), + Translation::__('Validation could not be completed'), [], ErrorCode::VALIDATION_INCOMPLETE, $sanitizedFilename diff --git a/src/Upload/Translation.php b/src/Upload/Translation.php index a35b313..6ecde60 100644 --- a/src/Upload/Translation.php +++ b/src/Upload/Translation.php @@ -71,6 +71,34 @@ final class Translation */ private static $translator; + /** + * Mark a string for the catalogue without translating it + * + * gettext's `N_()` under a more familiar name: it returns `$text` unchanged. `File` does + * the lookup later, when it renders `getErrors()`. That is what keeps + * `Exception::getMessage()` in English and the raw msgid in `getErrorDetails()`. + * + * A static method rather than a function so that a classmap autoloader reaches it. The + * `files` autoload entry a function needs is Composer's alone, and an autoloader built + * over a php-scoper'd tree does not run it — leaving every error path in the library a + * fatal on a site where uploads had only ever succeeded. + * + * **Not WordPress's `__()`.** Unqualified, `Translation` resolves in the calling file's + * own namespace and nowhere else, so a missing import is a fatal at the call rather than + * a silent hop to whatever global `__()` is loaded. + * + * `$domain` is discarded — the lookup always uses `self::DOMAIN` — and is there for your + * extractor. `xgettext -k__:1` reads the first argument of `Translation::__()` exactly as + * it reads a bare `__()`; `-k__:1,2` finds nothing. + * + * @param string $text The English string, which is also the gettext msgid + * @param string $domain The catalogue the string belongs to, for tooling that reads these + */ + public static function __(string $text, string $domain = self::DOMAIN): string + { + return $text; + } + /** * Install the callable that looks a message id up * diff --git a/src/Upload/Validation/Extension.php b/src/Upload/Validation/Extension.php index d241861..fc551a5 100644 --- a/src/Upload/Validation/Extension.php +++ b/src/Upload/Validation/Extension.php @@ -37,10 +37,9 @@ use GravityPdf\Upload\ErrorCode; use GravityPdf\Upload\Exception; use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\Translation; use GravityPdf\Upload\ValidationInterface; -use function GravityPdf\Upload\__; - /** * Validate File Extension * @@ -83,7 +82,7 @@ public function validate(FileInfoInterface $fileInfo): void if (!in_array($fileExtension, $this->allowedExtensions, true)) { throw new Exception( /* translators: %1$s: comma-separated list of the accepted file extensions */ - __('Invalid file extension. Must be one of: %1$s'), + Translation::__('Invalid file extension. Must be one of: %1$s'), $fileInfo, ErrorCode::EXTENSION_NOT_ALLOWED, [implode(', ', $this->allowedExtensions)] diff --git a/src/Upload/Validation/FileType.php b/src/Upload/Validation/FileType.php index cfcc4f8..1795912 100644 --- a/src/Upload/Validation/FileType.php +++ b/src/Upload/Validation/FileType.php @@ -37,11 +37,10 @@ use GravityPdf\Upload\ErrorCode; use GravityPdf\Upload\Exception; use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\Translation; use GravityPdf\Upload\ValidationInterface; use InvalidArgumentException; -use function GravityPdf\Upload\__; - /** * Validate File Extension Against Media Type * @@ -164,7 +163,7 @@ public function validate(FileInfoInterface $fileInfo): void if (!isset($this->allowedTypes[$extension])) { throw new Exception( /* translators: %1$s: comma-separated list of the accepted file extensions */ - __('Invalid file extension. Must be one of: %1$s'), + Translation::__('Invalid file extension. Must be one of: %1$s'), $fileInfo, ErrorCode::EXTENSION_NOT_ALLOWED, [implode(', ', array_keys($this->allowedTypes))] @@ -178,7 +177,7 @@ public function validate(FileInfoInterface $fileInfo): void if (!in_array($mimetype, $this->allowedTypes[$extension], true)) { throw new Exception( /* translators: 1: the file's extension, 2: the media types it may hold */ - __('File contents do not match the "%1$s" extension. Must be one of: %2$s'), + Translation::__('File contents do not match the "%1$s" extension. Must be one of: %2$s'), $fileInfo, ErrorCode::FILE_CONTENTS_MISMATCH, [$extension, implode(', ', $this->allowedTypes[$extension])] diff --git a/src/Upload/Validation/Mimetype.php b/src/Upload/Validation/Mimetype.php index cff79b0..67f76fd 100644 --- a/src/Upload/Validation/Mimetype.php +++ b/src/Upload/Validation/Mimetype.php @@ -37,10 +37,9 @@ use GravityPdf\Upload\ErrorCode; use GravityPdf\Upload\Exception; use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\Translation; use GravityPdf\Upload\ValidationInterface; -use function GravityPdf\Upload\__; - /** * Validate Upload Media Type * @@ -94,7 +93,7 @@ public function validate(FileInfoInterface $fileInfo): void if (!in_array($mimetype, $this->mimetypes, true)) { throw new Exception( /* translators: %1$s: comma-separated list of the accepted media types */ - __('Invalid mimetype. Must be one of: %1$s'), + Translation::__('Invalid mimetype. Must be one of: %1$s'), $fileInfo, ErrorCode::MIMETYPE_NOT_ALLOWED, [implode(', ', $this->mimetypes)] diff --git a/src/Upload/Validation/Size.php b/src/Upload/Validation/Size.php index 6818c49..d45d061 100644 --- a/src/Upload/Validation/Size.php +++ b/src/Upload/Validation/Size.php @@ -37,11 +37,10 @@ use GravityPdf\Upload\Exception; use GravityPdf\Upload\File; use GravityPdf\Upload\FileInfoInterface; +use GravityPdf\Upload\Translation; use GravityPdf\Upload\ValidationInterface; use InvalidArgumentException; -use function GravityPdf\Upload\__; - /** * Validate Upload File Size * @@ -141,7 +140,11 @@ public function validate(FileInfoInterface $fileInfo): void would absorb that as the generic "Validation could not be completed"; this reports what actually went wrong with the file. */ if ($fileSize === false) { - throw new Exception(__('File size could not be determined'), $fileInfo, ErrorCode::SIZE_UNKNOWN); + throw new Exception( + Translation::__('File size could not be determined'), + $fileInfo, + ErrorCode::SIZE_UNKNOWN + ); } if ($fileSize < $this->minSize) { @@ -175,7 +178,7 @@ public function validate(FileInfoInterface $fileInfo): void * The unit is part of the message rather than a value interpolated into it, because * values are never translated and `MB` is not universal — French writes `Mo`. * - * A method, not a property, because a PHP 7.3 constant expression cannot call `__()`. + * A method, not a property: a PHP 7.3 constant expression cannot call `Translation::__()`. * Override it to reword; your replacements are then yours to extract. * * @return array Keyed by the unit keys `scale()` returns @@ -185,16 +188,16 @@ protected static function getTooLargeMessages(): array return [ 'GB' => /* translators: %1$s: the largest accepted size, in gigabytes */ - __('File size is too large. Must be no more than %1$s GB'), + Translation::__('File size is too large. Must be no more than %1$s GB'), 'MB' => /* translators: %1$s: the largest accepted size, in megabytes */ - __('File size is too large. Must be no more than %1$s MB'), + Translation::__('File size is too large. Must be no more than %1$s MB'), 'KB' => /* translators: %1$s: the largest accepted size, in kilobytes */ - __('File size is too large. Must be no more than %1$s KB'), + Translation::__('File size is too large. Must be no more than %1$s KB'), 'B' => /* translators: %1$s: the largest accepted size, in bytes */ - __('File size is too large. Must be no more than %1$s bytes'), + Translation::__('File size is too large. Must be no more than %1$s bytes'), ]; } @@ -210,16 +213,16 @@ protected static function getTooSmallMessages(): array return [ 'GB' => /* translators: %1$s: the smallest accepted size, in gigabytes */ - __('File size is too small. Must be at least %1$s GB'), + Translation::__('File size is too small. Must be at least %1$s GB'), 'MB' => /* translators: %1$s: the smallest accepted size, in megabytes */ - __('File size is too small. Must be at least %1$s MB'), + Translation::__('File size is too small. Must be at least %1$s MB'), 'KB' => /* translators: %1$s: the smallest accepted size, in kilobytes */ - __('File size is too small. Must be at least %1$s KB'), + Translation::__('File size is too small. Must be at least %1$s KB'), 'B' => /* translators: %1$s: the smallest accepted size, in bytes */ - __('File size is too small. Must be at least %1$s bytes'), + Translation::__('File size is too small. Must be at least %1$s bytes'), ]; } diff --git a/src/Upload/i18n.php b/src/Upload/i18n.php deleted file mode 100644 index e15b350..0000000 --- a/src/Upload/i18n.php +++ /dev/null @@ -1,61 +0,0 @@ -assertSame('No file was uploaded', __('No file was uploaded')); + $this->assertSame('No file was uploaded', Translation::__('No file was uploaded')); } /** @@ -27,8 +25,14 @@ public function testTheMarkerReturnsItsArgument(): void */ public function testTheDomainChangesNothingAboutTheAnswer(): void { - $this->assertSame('No file was uploaded', __('No file was uploaded', Translation::DOMAIN)); - $this->assertSame('No file was uploaded', __('No file was uploaded', 'someone-elses-plugin')); + $this->assertSame( + 'No file was uploaded', + Translation::__('No file was uploaded', Translation::DOMAIN) + ); + $this->assertSame( + 'No file was uploaded', + Translation::__('No file was uploaded', 'someone-elses-plugin') + ); } /** @@ -41,13 +45,14 @@ public function testTheMarkerNeverTranslates(): void return 'traduit'; }); - $this->assertSame('No file was uploaded', __('No file was uploaded')); + $this->assertSame('No file was uploaded', Translation::__('No file was uploaded')); } /** - * A file that forgets the import calls the global `__()` instead. Under PHPUnit there is - * none, so that is a fatal; under WordPress there is one, and the string is translated far - * too early with nothing to report it. + * A file that forgets `use GravityPdf\Upload\Translation;` fatals at the call — a class + * name never falls back to the global namespace the way a function name does. That is + * still only reached on an error path, so the import is checked here rather than left to + * the first rejected upload. * * One test over the whole corpus, not one per file. Per-file it could only assert about a * file that marks something, so renaming the marker made every case vacuous while the @@ -60,31 +65,43 @@ public function testEveryCallerCanReachTheMarker(): void foreach ($this->sourceFiles() as $file) { $source = (string) file_get_contents($file); - if (preg_match('/(?assertNotSame([], $reachable, 'Nothing under src/ marks a string — has the marker been renamed?'); $this->assertSame( array_fill_keys(array_keys($reachable), true), $reachable, - 'A file calls __() but neither sits in GravityPdf\Upload nor imports it' + 'A file calls Translation::__() but neither sits in GravityPdf\Upload nor imports the class' ); } /** - * That fatal only exists while nothing defines a global `__()`. A WordPress stub in - * `tests/bootstrap.php` would swallow it, leaving the source scan as the only guard. + * The marker is a static method rather than a function so that an autoloader which only + * indexes classes still reaches it. Composer's `files` entry is the only thing that loads + * a bare function, and an autoloader built over a php-scoper'd tree does not run it. */ - public function testNoGlobalMarkerCanMaskAMissingImport(): void + public function testTheMarkerTravelsWithTheClass(): void { + $this->assertFileDoesNotExist(dirname(__DIR__, 2) . '/src/Upload/i18n.php'); $this->assertFalse( - function_exists('__'), - 'A global __() would let a file that forgets the import pass its tests' + function_exists('GravityPdf\\Upload\\__'), + 'A bare marker function would shadow the global __() a WordPress or Laravel ' + . 'translator calls, in any file that imported it' + ); + + $composer = json_decode((string) file_get_contents(dirname(__DIR__, 2) . '/composer.json'), true); + + $this->assertIsArray($composer); + $this->assertArrayNotHasKey( + 'files', + $composer['autoload'], + 'A `files` autoload entry is unreachable from a classmap over a scoped tree' ); } diff --git a/tools/i18n/generate-pot.php b/tools/i18n/generate-pot.php index dad5683..108bd98 100644 --- a/tools/i18n/generate-pot.php +++ b/tools/i18n/generate-pot.php @@ -8,7 +8,7 @@ * * ## What is in the catalogue, and what is deliberately not * - * Whatever is marked with `GravityPdf\Upload\__()`, which is exactly what `File` looks up. + * Whatever is marked with `Translation::__()`, which is exactly what `File` looks up. * That is narrower than "every string in `src/`", and the narrowing is the marker's doing * rather than this script's: * @@ -22,8 +22,9 @@ * * One flag against a marker that does nothing but return its argument, so a PHP-based scanner * would do no better here and `tools/i18n/` needs no manifest of its own. A consumer - * generating their own catalogue from `vendor/` runs the same flag. It matches the fully - * qualified `\GravityPdf\Upload\__()` as well as the imported short form. + * generating their own catalogue from `vendor/` runs the same flag. `xgettext` matches the + * trailing identifier and ignores the class prefix, so `-k__:1` reads `Translation::__()`, + * `\GravityPdf\Upload\Translation::__()` and an aliased `T::__()` alike. * * `-k__:1` reads the first argument and ignores the marker's `$domain`. Do not "complete" it * to `-k__:1,2`: gettext reads a second argument number as the plural form, no call in `src` diff --git a/tools/translator-readme/global-underscore.php b/tools/translator-readme/global-underscore.php index fb366cc..c0bad0a 100644 --- a/tools/translator-readme/global-underscore.php +++ b/tools/translator-readme/global-underscore.php @@ -8,8 +8,7 @@ * key to a translator. With none set it answers with its own name, so `verify.php` can tell * which function a call actually reached. * - * Declared in the global namespace deliberately. The whole point of the README's leading - * backslash is that `\__()` resolves here rather than to this library's marker. + * Declared in the global namespace, which is where the documented adapters reach it from. * * @package Upload */ diff --git a/tools/translator-readme/marker-import.php b/tools/translator-readme/marker-import.php deleted file mode 100644 index a70ecc9..0000000 --- a/tools/translator-readme/marker-import.php +++ /dev/null @@ -1,50 +0,0 @@ -