diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9b714ada4e..0eddadd389 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -5423,6 +5423,7 @@ phutil_register_library_map(array( 'PhorgePHPASTViewRunController' => 'applications/phpast/controller/PhorgePHPASTViewRunController.php', 'PhorgePHPASTViewStreamController' => 'applications/phpast/controller/PhorgePHPASTViewStreamController.php', 'PhorgePHPASTViewTreeController' => 'applications/phpast/controller/PhorgePHPASTViewTreeController.php', + 'PhorgeStringablePlaceholder' => 'infrastructure/internationalization/management/PhorgeStringablePlaceholder.php', 'PhorgeSystemDeprecationWarningListener' => 'applications/system/events/PhorgeSystemDeprecationWarningListener.php', 'PhortuneAccount' => 'applications/phortune/storage/PhortuneAccount.php', 'PhortuneAccountAddManagerController' => 'applications/phortune/controller/account/PhortuneAccountAddManagerController.php', @@ -12297,6 +12298,7 @@ phutil_register_library_map(array( 'PhorgePHPASTViewRunController' => 'PhabricatorXHPASTViewController', 'PhorgePHPASTViewStreamController' => 'PhorgePHPASTViewPanelController', 'PhorgePHPASTViewTreeController' => 'PhorgePHPASTViewPanelController', + 'PhorgeStringablePlaceholder' => 'Phobject', 'PhorgeSystemDeprecationWarningListener' => 'PhabricatorEventListener', 'PhortuneAccount' => array( 'PhortuneDAO', diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index a279b941fd..c120b01d20 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -309,7 +309,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { throw new Exception( pht( 'Reached TOTP challenge validation with an unexpected number of '. - 'unexpired challenges (%d), expected exactly one.', + 'unexpired challenges (%s), expected exactly one.', phutil_count($challenges))); } diff --git a/src/applications/celerity/management/CelerityManagementMapWorkflow.php b/src/applications/celerity/management/CelerityManagementMapWorkflow.php index 22fdebd686..7aa8a78791 100644 --- a/src/applications/celerity/management/CelerityManagementMapWorkflow.php +++ b/src/applications/celerity/management/CelerityManagementMapWorkflow.php @@ -17,7 +17,7 @@ final class CelerityManagementMapWorkflow $this->log( pht( - 'Rebuilding %d resource source(s).', + 'Rebuilding %s resource source(s).', phutil_count($resources_map))); foreach ($resources_map as $name => $resources) { diff --git a/src/applications/conpherence/view/ConpherenceParticipantView.php b/src/applications/conpherence/view/ConpherenceParticipantView.php index 912a3b8b21..43b86ff855 100644 --- a/src/applications/conpherence/view/ConpherenceParticipantView.php +++ b/src/applications/conpherence/view/ConpherenceParticipantView.php @@ -90,7 +90,7 @@ final class ConpherenceParticipantView extends AphrontView { ->addSigil('conpherence-widget-adder'); $header = id(new PHUIHeaderView()) - ->setHeader(pht('Participants (%d)', $count)) + ->setHeader(pht('Participants (%s)', $count)) ->addClass('widgets-header') ->addActionItem($new_icon); diff --git a/src/applications/differential/customfield/DifferentialJIRAIssuesField.php b/src/applications/differential/customfield/DifferentialJIRAIssuesField.php index 8bd0b052dd..fd9b69f42e 100644 --- a/src/applications/differential/customfield/DifferentialJIRAIssuesField.php +++ b/src/applications/differential/customfield/DifferentialJIRAIssuesField.php @@ -217,7 +217,7 @@ final class DifferentialJIRAIssuesField $author_phid = $xaction->getAuthorPHID(); if ($add && $rem) { return pht( - '%s updated JIRA issue(s): added %d %s; removed %d %s.', + '%s updated JIRA issue(s): added %s: %s; removed %s: %s.', $xaction->renderHandleLink($author_phid), phutil_count($add), implode(', ', $add), @@ -225,13 +225,13 @@ final class DifferentialJIRAIssuesField implode(', ', $rem)); } else if ($add) { return pht( - '%s added %d JIRA issue(s): %s.', + '%s added %s JIRA issue(s): %s.', $xaction->renderHandleLink($author_phid), phutil_count($add), implode(', ', $add)); } else if ($rem) { return pht( - '%s removed %d JIRA issue(s): %s.', + '%s removed %s JIRA issue(s): %s.', $xaction->renderHandleLink($author_phid), phutil_count($rem), implode(', ', $rem)); diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 1dd654c391..89a17f73d4 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -960,7 +960,7 @@ final class DifferentialChangesetParser extends Phobject { $shield_text, ' ', pht( - 'This file has %d collapsed inline comment(s).', + 'This file has %s collapsed inline comment(s).', new PhutilNumber($collapsed_count)), ); } diff --git a/src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php b/src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php index 9a3a707f99..eb69909233 100644 --- a/src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php +++ b/src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php @@ -100,7 +100,7 @@ final class PhabricatorFilesCurtainExtension if ($loaded_count > $exact_limit) { $link_text = pht('View All Files'); } else { - $link_text = pht('View All %d Files', new PhutilNumber($loaded_count)); + $link_text = pht('View All %s Files', new PhutilNumber($loaded_count)); } $ref_list->newTailLink() diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index a69ec13292..008e9d5c13 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -564,7 +564,7 @@ final class PhabricatorUser public static function describeValidRealName() { return pht( - 'Real Name must have no more than %d characters.', + 'Real Name must have no more than %s characters.', new PhutilNumber(self::MAXIMUM_REALNAME_LENGTH)); } diff --git a/src/applications/phortune/controller/account/PhortuneAccountController.php b/src/applications/phortune/controller/account/PhortuneAccountController.php index d3d60d2789..70d47783b8 100644 --- a/src/applications/phortune/controller/account/PhortuneAccountController.php +++ b/src/applications/phortune/controller/account/PhortuneAccountController.php @@ -146,7 +146,7 @@ abstract class PhortuneAccountController $merchant_list = phutil_implode_html(', ', $merchant_list); $merchant_message = pht( - 'You can view this account because you control %d merchant(s) it '. + 'You can view this account because you control %s merchant(s) it '. 'has a relationship with: %s.', phutil_count($merchants), $merchant_list); diff --git a/src/applications/ponder/view/PonderFooterView.php b/src/applications/ponder/view/PonderFooterView.php index 6d814d2b71..df59087f43 100644 --- a/src/applications/ponder/view/PonderFooterView.php +++ b/src/applications/ponder/view/PonderFooterView.php @@ -38,7 +38,7 @@ final class PonderFooterView extends AphrontTagView { if ($this->count == 0) { $text = pht('Add a Comment'); } else { - $text = pht('Show %d Comment(s)', new PhutilNumber($this->count)); + $text = pht('Show %s Comment(s)', new PhutilNumber($this->count)); } $actions = array(); diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php index 153facd89f..4cd6cf7e0b 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php @@ -115,7 +115,7 @@ abstract class PhabricatorStandardCustomFieldPHIDs if ($add && !$rem) { return pht( - '%s updated %s, added %d: %s.', + '%s updated %s, added %s: %s.', $xaction->renderHandleLink($author_phid), $this->getFieldName(), phutil_count($add), @@ -152,7 +152,7 @@ abstract class PhabricatorStandardCustomFieldPHIDs if ($add && !$rem) { return pht( - '%s updated %s for %s, added %d: %s.', + '%s updated %s for %s, added %s: %s.', $xaction->renderHandleLink($author_phid), $this->getFieldName(), $xaction->renderHandleLink($object_phid), diff --git a/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php b/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php index c131819935..0332cb6385 100644 --- a/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php +++ b/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php @@ -140,7 +140,7 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView { return null; } - return pht('%d line(s)', new PhutilNumber($line_count)); + return pht('%s line(s)', new PhutilNumber($line_count)); } public function renderCoverage() { diff --git a/src/infrastructure/internationalization/management/PhorgeInternationalizationValidator.php b/src/infrastructure/internationalization/management/PhorgeInternationalizationValidator.php index fbae35680f..056a2a1a74 100644 --- a/src/infrastructure/internationalization/management/PhorgeInternationalizationValidator.php +++ b/src/infrastructure/internationalization/management/PhorgeInternationalizationValidator.php @@ -16,14 +16,51 @@ final class PhorgeInternationalizationValidator extends Phobject { } $data = []; foreach ($types as $type) { - $data[] = $type === 'number' ? 3: 'abc'; + if ($type === 'phutilnumber') { + // Make a class that can be converted into a string + // (to mimic the conversion pht() will do) + // but not to a number (the double-conversion loses data) + // See T16454 + $data[] = new PhorgeStringablePlaceholder(); + } else if ($type === 'number') { + // no good way to check numbers being converted to strings + // without parsing the format specifier ourself + // (remember xsprintf can't work with translated strings since + // they can use backreferences, format specifiers, etc) + $data[] = 3; + } else if ($type === null) { + // This could either be a string or a number, let PHP type + // conversions handle it + $data[] = 'abc'; + } else { + throw new Exception(pht('Bogus type "%s" for "%s"', $type, $proto)); + } } try { - $parsed = vsprintf($transl, $data); + $parsed = vsprintf($transl, $data); } catch (ValueError $ex) { - // In PHP 8 vsprintf throws a ValueError for bad data; - // in PHP7 it returns false - $parsed = false; + // In PHP 8 vsprintf throws a ValueError for bad data; + // in PHP7 it returns false + $parsed = false; + } catch (RuntimeException $ex) { + // The types of the args don't match (the RuntimeException comes + // from PhutilErrorHandler.php throwing what was originally a PHP + // warning) + $msg = $ex->getMessage(); + if ($msg === 'Object of class PhorgeStringablePlaceholder '. + 'could not be converted to int') { + $errors[] = pht( + 'The locale `%s` defines a translation for the key `%s` which '. + 'uses %%d to represent a PhutilNumber. This loses data if the '. + 'number ends up being formatted with thousands specifiers. '. + 'See T16454', + $locale, + $proto); + return $errors; + } + // This shouldn't happen, but if something else goes wrong fall + // through to the generic `failed to interpolate properly` error + $parsed = false; } if ($parsed === false) { $errors[] = pht( @@ -101,6 +138,15 @@ final class PhorgeInternationalizationValidator extends Phobject { $spec['types'], 0)); } + // Run it on the proto-English as a translation too + // since this also does some parameter type checking + // (some of which may belong better in a linter than here) + $errors = array_merge($errors, $this->validateTranslation( + id(new PhutilRawEnglishLocale())->getLocaleCode(), + $string, + $string, + $spec['types'], + 0)); // Check for missing branches in US english if (str_contains($string, '(s)')) { if (!isset($keyed_translations[$string]['en_US'])) { diff --git a/src/infrastructure/internationalization/management/PhorgeStringablePlaceholder.php b/src/infrastructure/internationalization/management/PhorgeStringablePlaceholder.php new file mode 100644 index 0000000000..cd8bcb3417 --- /dev/null +++ b/src/infrastructure/internationalization/management/PhorgeStringablePlaceholder.php @@ -0,0 +1,10 @@ + array('%s Answer', '%s Answers'), - 'Show %d Comment(s)' => array('Show %d Comment', 'Show %d Comments'), + 'Show %s Comment(s)' => array('Show %s Comment', 'Show %s Comments'), '%s DIFF LINK(S)' => array('DIFF LINK', 'DIFF LINKS'), 'You successfully created %d diff(s).' => array( @@ -1288,7 +1288,7 @@ final class PhabricatorUSEnglishTranslation ), - '%s updated %s, added %d: %s.' => + '%s updated %s, added %s: %s.' => '%s updated %s, added: %4$s.', '%s updated %s, removed %s: %s.' => @@ -1297,7 +1297,7 @@ final class PhabricatorUSEnglishTranslation '%s updated %s, added %s: %s; removed %s: %s.' => '%s updated %s, added: %4$s; removed: %6$s.', - '%s updated %s for %s, added %d: %s.' => + '%s updated %s for %s, added %s: %s.' => '%s updated %s for %s, added: %5$s.', '%s updated %s for %s, removed %s: %s.' => @@ -1306,8 +1306,8 @@ final class PhabricatorUSEnglishTranslation '%s updated %s for %s, added %s: %s; removed %s: %s.' => '%s updated %s for %s, added: %5$s; removed; %7$s.', - '%s updated JIRA issue(s): added %d %s; removed %d %s.' => - '%s updated JIRA issues: added %3$s; removed: %5$s.', + '%s updated JIRA issue(s): added %s: %s; removed %s: %s.' => + '%s updated JIRA issues: added: %3$s; removed: %5$s.', 'Permanently destroyed %s object(s).' => array( 'Permanently destroyed %s object.', @@ -1633,14 +1633,14 @@ final class PhabricatorUSEnglishTranslation '%s updated %s attached file(s), removed %s: %s; modified %s: %s.' => '%s updated attached files, removed %4$s; modified: %6$s.', - '%s added %d JIRA issue(s): %s.' => + '%s added %s JIRA issue(s): %s.' => array( array( '%s added a JIRA issue: %3$s.', '%s added JIRA issues: %3$s.', ), ), - '%s removed %d JIRA issue(s): %s.' => + '%s removed %s JIRA issue(s): %s.' => array( array( '%s removed a JIRA issue: %3$s.', @@ -1755,9 +1755,9 @@ final class PhabricatorUSEnglishTranslation 'Reset %s action.', 'Reset %s actions.', ), - 'Rebuilding %d resource source(s).' => array( - 'Rebuilding %d resource source.', - 'Rebuilding %d resource sources.', + 'Rebuilding %s resource source(s).' => array( + 'Rebuilding %s resource source.', + 'Rebuilding %s resource sources.', ), 'Detected %s serious issue(s) with the schemata.' => array( 'Detected a serious issue with the schemata.', @@ -1779,9 +1779,9 @@ final class PhabricatorUSEnglishTranslation 'Rebuilding %s changeset for diff ID %d.', 'Rebuilding %s changesets for diff ID %d.', ), - 'This file has %d collapsed inline comment(s).' => array( + 'This file has %s collapsed inline comment(s).' => array( 'This file has one collapsed inline comment.', - 'This file has %d collapsed inline comments.', + 'This file has %s collapsed inline comments.', ), 'This file took too long to load from the repository '. '(more than %s second(s)).' => array( @@ -1905,12 +1905,12 @@ final class PhabricatorUSEnglishTranslation ), ), 'You can view this account because you control '. - '%d merchant(s) it has a relationship with: %s.' => + '%s merchant(s) it has a relationship with: %s.' => array( 'You can view this account because you control '. 'a merchant it has a relationship with: %2$s.', 'You can view this account because you control '. - '%d merchants it has a relationship with: %s.', + '%s merchants it has a relationship with: %s.', ), 'Used on %s active column(s).' => array( @@ -2021,15 +2021,15 @@ final class PhabricatorUSEnglishTranslation 'Query timed out after %s second!', 'Query timed out after %s seconds!', ), - 'Failed to write %d byte(s) to file "%s".' => + 'Failed to write %s byte(s) to file "%s".' => array( - 'Failed to write %d byte to file "%s".', - 'Failed to write %d bytes to file "%s".', + 'Failed to write %s byte to file "%s".', + 'Failed to write %s bytes to file "%s".', ), - 'Failed to write %d byte(s) to "%s".' => + 'Failed to write %s byte(s) to "%s".' => array( - 'Failed to write %d byte to "%s".', - 'Failed to write %d bytes to "%s".', + 'Failed to write %s byte to "%s".', + 'Failed to write %s bytes to "%s".', ), 'This lock was most recently acquired by '. 'a process (%s) %s second(s) ago.' => @@ -2288,9 +2288,9 @@ final class PhabricatorUSEnglishTranslation 'Done, compacted %s edge transaction.', 'Done, compacted %s edge transactions.', ), - '%d line(s)' => array( - '%d line', - '%d line(s)', + '%s line(s)' => array( + '%s line', + '%s line(s)', ), 'Adjusted **%s** create statements and **%s** use statements.' => array( array( diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php index 8cc5b1ae63..707fc2ad97 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php @@ -431,7 +431,7 @@ final class PhabricatorStorageManagementDumpWorkflow if ($ok !== strlen($data)) { throw new Exception( pht( - 'Failed to write %d byte(s) to file "%s".', + 'Failed to write %s byte(s) to file "%s".', new PhutilNumber(strlen($data)), $output_file)); } diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php index 8a0c7817b7..fcd757c3f5 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php @@ -173,7 +173,7 @@ final class PhabricatorStorageManagementRenamespaceWorkflow if ($bytes !== strlen($data)) { throw new Exception( pht( - 'Failed to write %d byte(s) to "%s".', + 'Failed to write %s byte(s) to "%s".', new PhutilNumber(strlen($data)), $output_name)); } diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasher.php b/src/infrastructure/util/password/PhabricatorPasswordHasher.php index d44454f1e4..7b7e7c1c81 100644 --- a/src/infrastructure/util/password/PhabricatorPasswordHasher.php +++ b/src/infrastructure/util/password/PhabricatorPasswordHasher.php @@ -169,8 +169,8 @@ abstract class PhabricatorPasswordHasher extends Phobject { if ($actual_len > $expect_len) { throw new Exception( pht( - "Password hash '%s' produced a hash of length %d, but a ". - "maximum length of %d was expected.", + "Password hash '%s' produced a hash of length %s, but a ". + "maximum length of %s was expected.", $name, new PhutilNumber($actual_len), new PhutilNumber($expect_len))); diff --git a/support/php-parser/PhorgePHPParserExtractor.php b/support/php-parser/PhorgePHPParserExtractor.php index 42852f8126..17ee452456 100644 --- a/support/php-parser/PhorgePHPParserExtractor.php +++ b/support/php-parser/PhorgePHPParserExtractor.php @@ -3,25 +3,25 @@ final class PhorgePHPParserExtractor extends PhpParser\NodeVisitorAbstract { private static $knownTypes = array( 'PhabricatorEdgeType' => array( - 'getTransactionAddString' => array(null, 'number', null), - 'getTransactionRemoveString' => array(null, 'number', null), + 'getTransactionAddString' => array(null, 'phutilnumber', null), + 'getTransactionRemoveString' => array(null, 'phutilnumber', null), 'getTransactionEditString' => array( null, - 'number', - 'number', + 'phutilnumber', + 'phutilnumber', null, - 'number', + 'phutilnumber', null, ), - 'getFeedAddString' => array(null, null, 'number', null), - 'getFeedRemoveString' => array(null, null, 'number', null), + 'getFeedAddString' => array(null, null, 'phutilnumber', null), + 'getFeedRemoveString' => array(null, null, 'phutilnumber', null), 'getFeedEditString' => array( null, null, - 'number', - 'number', + 'phutilnumber', + 'phutilnumber', null, - 'number', + 'phutilnumber', null, ), ), @@ -73,6 +73,7 @@ final class PhorgePHPParserExtractor extends PhpParser\NodeVisitorAbstract { } else if ($node instanceof PhpParser\Node\Expr\FuncCall) { switch ($this->getName($node)) { case 'phutil_count': + return 'phutilnumber'; case 'count': return 'number'; case 'phutil_person': @@ -80,7 +81,7 @@ final class PhorgePHPParserExtractor extends PhpParser\NodeVisitorAbstract { } } else if ($node instanceof PhpParser\Node\Expr\New_ ) { if ($this->getName($node->class) == 'PhutilNumber') { - return 'number'; + return 'phutilnumber'; } } else if ($node instanceof PhpParser\Node\Expr\Variable) { $name = $this->getName($node);