@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Lint against using %d with PhutilNumber
Summary: Fixes T16454
Test Plan:
Run `bin/i18n validate` and see no errors (and see errors if the changes there are made but not the changes to other files)
Not tested beyond that
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T16454
Differential Revision: https://we.phorge.it/D26703
+116
-57
+2
src/__phutil_library_map__.php
+2
src/__phutil_library_map__.php
···
5423
5423
'PhorgePHPASTViewRunController' => 'applications/phpast/controller/PhorgePHPASTViewRunController.php',
5424
5424
'PhorgePHPASTViewStreamController' => 'applications/phpast/controller/PhorgePHPASTViewStreamController.php',
5425
5425
'PhorgePHPASTViewTreeController' => 'applications/phpast/controller/PhorgePHPASTViewTreeController.php',
5426
+
'PhorgeStringablePlaceholder' => 'infrastructure/internationalization/management/PhorgeStringablePlaceholder.php',
5426
5427
'PhorgeSystemDeprecationWarningListener' => 'applications/system/events/PhorgeSystemDeprecationWarningListener.php',
5427
5428
'PhortuneAccount' => 'applications/phortune/storage/PhortuneAccount.php',
5428
5429
'PhortuneAccountAddManagerController' => 'applications/phortune/controller/account/PhortuneAccountAddManagerController.php',
···
12297
12298
'PhorgePHPASTViewRunController' => 'PhabricatorXHPASTViewController',
12298
12299
'PhorgePHPASTViewStreamController' => 'PhorgePHPASTViewPanelController',
12299
12300
'PhorgePHPASTViewTreeController' => 'PhorgePHPASTViewPanelController',
12301
+
'PhorgeStringablePlaceholder' => 'Phobject',
12300
12302
'PhorgeSystemDeprecationWarningListener' => 'PhabricatorEventListener',
12301
12303
'PhortuneAccount' => array(
12302
12304
'PhortuneDAO',
+1
-1
src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
+1
-1
src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
+1
-1
src/applications/celerity/management/CelerityManagementMapWorkflow.php
+1
-1
src/applications/celerity/management/CelerityManagementMapWorkflow.php
+1
-1
src/applications/conpherence/view/ConpherenceParticipantView.php
+1
-1
src/applications/conpherence/view/ConpherenceParticipantView.php
+3
-3
src/applications/differential/customfield/DifferentialJIRAIssuesField.php
+3
-3
src/applications/differential/customfield/DifferentialJIRAIssuesField.php
···
217
217
$author_phid = $xaction->getAuthorPHID();
218
218
if ($add && $rem) {
219
219
return pht(
220
-
'%s updated JIRA issue(s): added %d %s; removed %d %s.',
220
+
'%s updated JIRA issue(s): added %s: %s; removed %s: %s.',
221
221
$xaction->renderHandleLink($author_phid),
222
222
phutil_count($add),
223
223
implode(', ', $add),
···
225
225
implode(', ', $rem));
226
226
} else if ($add) {
227
227
return pht(
228
-
'%s added %d JIRA issue(s): %s.',
228
+
'%s added %s JIRA issue(s): %s.',
229
229
$xaction->renderHandleLink($author_phid),
230
230
phutil_count($add),
231
231
implode(', ', $add));
232
232
} else if ($rem) {
233
233
return pht(
234
-
'%s removed %d JIRA issue(s): %s.',
234
+
'%s removed %s JIRA issue(s): %s.',
235
235
$xaction->renderHandleLink($author_phid),
236
236
phutil_count($rem),
237
237
implode(', ', $rem));
+1
-1
src/applications/differential/parser/DifferentialChangesetParser.php
+1
-1
src/applications/differential/parser/DifferentialChangesetParser.php
+1
-1
src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php
+1
-1
src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php
···
100
100
if ($loaded_count > $exact_limit) {
101
101
$link_text = pht('View All Files');
102
102
} else {
103
-
$link_text = pht('View All %d Files', new PhutilNumber($loaded_count));
103
+
$link_text = pht('View All %s Files', new PhutilNumber($loaded_count));
104
104
}
105
105
106
106
$ref_list->newTailLink()
+1
-1
src/applications/people/storage/PhabricatorUser.php
+1
-1
src/applications/people/storage/PhabricatorUser.php
+1
-1
src/applications/phortune/controller/account/PhortuneAccountController.php
+1
-1
src/applications/phortune/controller/account/PhortuneAccountController.php
···
146
146
$merchant_list = phutil_implode_html(', ', $merchant_list);
147
147
148
148
$merchant_message = pht(
149
-
'You can view this account because you control %d merchant(s) it '.
149
+
'You can view this account because you control %s merchant(s) it '.
150
150
'has a relationship with: %s.',
151
151
phutil_count($merchants),
152
152
$merchant_list);
+2
-2
src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
+2
-2
src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php
···
115
115
116
116
if ($add && !$rem) {
117
117
return pht(
118
-
'%s updated %s, added %d: %s.',
118
+
'%s updated %s, added %s: %s.',
119
119
$xaction->renderHandleLink($author_phid),
120
120
$this->getFieldName(),
121
121
phutil_count($add),
···
152
152
153
153
if ($add && !$rem) {
154
154
return pht(
155
-
'%s updated %s for %s, added %d: %s.',
155
+
'%s updated %s for %s, added %s: %s.',
156
156
$xaction->renderHandleLink($author_phid),
157
157
$this->getFieldName(),
158
158
$xaction->renderHandleLink($object_phid),
+1
-1
src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php
+1
-1
src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php
+51
-5
src/infrastructure/internationalization/management/PhorgeInternationalizationValidator.php
+51
-5
src/infrastructure/internationalization/management/PhorgeInternationalizationValidator.php
···
16
16
}
17
17
$data = [];
18
18
foreach ($types as $type) {
19
-
$data[] = $type === 'number' ? 3: 'abc';
19
+
if ($type === 'phutilnumber') {
20
+
// Make a class that can be converted into a string
21
+
// (to mimic the conversion pht() will do)
22
+
// but not to a number (the double-conversion loses data)
23
+
// See T16454
24
+
$data[] = new PhorgeStringablePlaceholder();
25
+
} else if ($type === 'number') {
26
+
// no good way to check numbers being converted to strings
27
+
// without parsing the format specifier ourself
28
+
// (remember xsprintf can't work with translated strings since
29
+
// they can use backreferences, format specifiers, etc)
30
+
$data[] = 3;
31
+
} else if ($type === null) {
32
+
// This could either be a string or a number, let PHP type
33
+
// conversions handle it
34
+
$data[] = 'abc';
35
+
} else {
36
+
throw new Exception(pht('Bogus type "%s" for "%s"', $type, $proto));
37
+
}
20
38
}
21
39
try {
22
-
$parsed = vsprintf($transl, $data);
40
+
$parsed = vsprintf($transl, $data);
23
41
} catch (ValueError $ex) {
24
-
// In PHP 8 vsprintf throws a ValueError for bad data;
25
-
// in PHP7 it returns false
26
-
$parsed = false;
42
+
// In PHP 8 vsprintf throws a ValueError for bad data;
43
+
// in PHP7 it returns false
44
+
$parsed = false;
45
+
} catch (RuntimeException $ex) {
46
+
// The types of the args don't match (the RuntimeException comes
47
+
// from PhutilErrorHandler.php throwing what was originally a PHP
48
+
// warning)
49
+
$msg = $ex->getMessage();
50
+
if ($msg === 'Object of class PhorgeStringablePlaceholder '.
51
+
'could not be converted to int') {
52
+
$errors[] = pht(
53
+
'The locale `%s` defines a translation for the key `%s` which '.
54
+
'uses %%d to represent a PhutilNumber. This loses data if the '.
55
+
'number ends up being formatted with thousands specifiers. '.
56
+
'See T16454',
57
+
$locale,
58
+
$proto);
59
+
return $errors;
60
+
}
61
+
// This shouldn't happen, but if something else goes wrong fall
62
+
// through to the generic `failed to interpolate properly` error
63
+
$parsed = false;
27
64
}
28
65
if ($parsed === false) {
29
66
$errors[] = pht(
···
101
138
$spec['types'],
102
139
0));
103
140
}
141
+
// Run it on the proto-English as a translation too
142
+
// since this also does some parameter type checking
143
+
// (some of which may belong better in a linter than here)
144
+
$errors = array_merge($errors, $this->validateTranslation(
145
+
id(new PhutilRawEnglishLocale())->getLocaleCode(),
146
+
$string,
147
+
$string,
148
+
$spec['types'],
149
+
0));
104
150
// Check for missing branches in US english
105
151
if (str_contains($string, '(s)')) {
106
152
if (!isset($keyed_translations[$string]['en_US'])) {
+10
src/infrastructure/internationalization/management/PhorgeStringablePlaceholder.php
+10
src/infrastructure/internationalization/management/PhorgeStringablePlaceholder.php
···
1
+
<?php
2
+
3
+
/**
4
+
* Class that can be converted into a string,
5
+
* to mimic the conversion pht() will do
6
+
* but not a number, since the double-conversion loses data.
7
+
*/
8
+
final class PhorgeStringablePlaceholder extends Phobject {
9
+
public function __toString() { return 'blah blah'; }
10
+
}
+23
-23
src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+23
-23
src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
···
14
14
'These configuration values are related:',
15
15
),
16
16
'%s Answer(s)' => array('%s Answer', '%s Answers'),
17
-
'Show %d Comment(s)' => array('Show %d Comment', 'Show %d Comments'),
17
+
'Show %s Comment(s)' => array('Show %s Comment', 'Show %s Comments'),
18
18
19
19
'%s DIFF LINK(S)' => array('DIFF LINK', 'DIFF LINKS'),
20
20
'You successfully created %d diff(s).' => array(
···
1288
1288
),
1289
1289
1290
1290
1291
-
'%s updated %s, added %d: %s.' =>
1291
+
'%s updated %s, added %s: %s.' =>
1292
1292
'%s updated %s, added: %4$s.',
1293
1293
1294
1294
'%s updated %s, removed %s: %s.' =>
···
1297
1297
'%s updated %s, added %s: %s; removed %s: %s.' =>
1298
1298
'%s updated %s, added: %4$s; removed: %6$s.',
1299
1299
1300
-
'%s updated %s for %s, added %d: %s.' =>
1300
+
'%s updated %s for %s, added %s: %s.' =>
1301
1301
'%s updated %s for %s, added: %5$s.',
1302
1302
1303
1303
'%s updated %s for %s, removed %s: %s.' =>
···
1306
1306
'%s updated %s for %s, added %s: %s; removed %s: %s.' =>
1307
1307
'%s updated %s for %s, added: %5$s; removed; %7$s.',
1308
1308
1309
-
'%s updated JIRA issue(s): added %d %s; removed %d %s.' =>
1310
-
'%s updated JIRA issues: added %3$s; removed: %5$s.',
1309
+
'%s updated JIRA issue(s): added %s: %s; removed %s: %s.' =>
1310
+
'%s updated JIRA issues: added: %3$s; removed: %5$s.',
1311
1311
1312
1312
'Permanently destroyed %s object(s).' => array(
1313
1313
'Permanently destroyed %s object.',
···
1633
1633
'%s updated %s attached file(s), removed %s: %s; modified %s: %s.' =>
1634
1634
'%s updated attached files, removed %4$s; modified: %6$s.',
1635
1635
1636
-
'%s added %d JIRA issue(s): %s.' =>
1636
+
'%s added %s JIRA issue(s): %s.' =>
1637
1637
array(
1638
1638
array(
1639
1639
'%s added a JIRA issue: %3$s.',
1640
1640
'%s added JIRA issues: %3$s.',
1641
1641
),
1642
1642
),
1643
-
'%s removed %d JIRA issue(s): %s.' =>
1643
+
'%s removed %s JIRA issue(s): %s.' =>
1644
1644
array(
1645
1645
array(
1646
1646
'%s removed a JIRA issue: %3$s.',
···
1755
1755
'Reset %s action.',
1756
1756
'Reset %s actions.',
1757
1757
),
1758
-
'Rebuilding %d resource source(s).' => array(
1759
-
'Rebuilding %d resource source.',
1760
-
'Rebuilding %d resource sources.',
1758
+
'Rebuilding %s resource source(s).' => array(
1759
+
'Rebuilding %s resource source.',
1760
+
'Rebuilding %s resource sources.',
1761
1761
),
1762
1762
'Detected %s serious issue(s) with the schemata.' => array(
1763
1763
'Detected a serious issue with the schemata.',
···
1779
1779
'Rebuilding %s changeset for diff ID %d.',
1780
1780
'Rebuilding %s changesets for diff ID %d.',
1781
1781
),
1782
-
'This file has %d collapsed inline comment(s).' => array(
1782
+
'This file has %s collapsed inline comment(s).' => array(
1783
1783
'This file has one collapsed inline comment.',
1784
-
'This file has %d collapsed inline comments.',
1784
+
'This file has %s collapsed inline comments.',
1785
1785
),
1786
1786
'This file took too long to load from the repository '.
1787
1787
'(more than %s second(s)).' => array(
···
1905
1905
),
1906
1906
),
1907
1907
'You can view this account because you control '.
1908
-
'%d merchant(s) it has a relationship with: %s.' =>
1908
+
'%s merchant(s) it has a relationship with: %s.' =>
1909
1909
array(
1910
1910
'You can view this account because you control '.
1911
1911
'a merchant it has a relationship with: %2$s.',
1912
1912
'You can view this account because you control '.
1913
-
'%d merchants it has a relationship with: %s.',
1913
+
'%s merchants it has a relationship with: %s.',
1914
1914
),
1915
1915
'Used on %s active column(s).' =>
1916
1916
array(
···
2021
2021
'Query timed out after %s second!',
2022
2022
'Query timed out after %s seconds!',
2023
2023
),
2024
-
'Failed to write %d byte(s) to file "%s".' =>
2024
+
'Failed to write %s byte(s) to file "%s".' =>
2025
2025
array(
2026
-
'Failed to write %d byte to file "%s".',
2027
-
'Failed to write %d bytes to file "%s".',
2026
+
'Failed to write %s byte to file "%s".',
2027
+
'Failed to write %s bytes to file "%s".',
2028
2028
),
2029
-
'Failed to write %d byte(s) to "%s".' =>
2029
+
'Failed to write %s byte(s) to "%s".' =>
2030
2030
array(
2031
-
'Failed to write %d byte to "%s".',
2032
-
'Failed to write %d bytes to "%s".',
2031
+
'Failed to write %s byte to "%s".',
2032
+
'Failed to write %s bytes to "%s".',
2033
2033
),
2034
2034
'This lock was most recently acquired by '.
2035
2035
'a process (%s) %s second(s) ago.' =>
···
2288
2288
'Done, compacted %s edge transaction.',
2289
2289
'Done, compacted %s edge transactions.',
2290
2290
),
2291
-
'%d line(s)' => array(
2292
-
'%d line',
2293
-
'%d line(s)',
2291
+
'%s line(s)' => array(
2292
+
'%s line',
2293
+
'%s line(s)',
2294
2294
),
2295
2295
'Adjusted **%s** create statements and **%s** use statements.' => array(
2296
2296
array(
+1
-1
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php
+1
-1
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php
+1
-1
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php
+1
-1
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php
+2
-2
src/infrastructure/util/password/PhabricatorPasswordHasher.php
+2
-2
src/infrastructure/util/password/PhabricatorPasswordHasher.php
···
169
169
if ($actual_len > $expect_len) {
170
170
throw new Exception(
171
171
pht(
172
-
"Password hash '%s' produced a hash of length %d, but a ".
173
-
"maximum length of %d was expected.",
172
+
"Password hash '%s' produced a hash of length %s, but a ".
173
+
"maximum length of %s was expected.",
174
174
$name,
175
175
new PhutilNumber($actual_len),
176
176
new PhutilNumber($expect_len)));
+12
-11
support/php-parser/PhorgePHPParserExtractor.php
+12
-11
support/php-parser/PhorgePHPParserExtractor.php
···
3
3
final class PhorgePHPParserExtractor extends PhpParser\NodeVisitorAbstract {
4
4
private static $knownTypes = array(
5
5
'PhabricatorEdgeType' => array(
6
-
'getTransactionAddString' => array(null, 'number', null),
7
-
'getTransactionRemoveString' => array(null, 'number', null),
6
+
'getTransactionAddString' => array(null, 'phutilnumber', null),
7
+
'getTransactionRemoveString' => array(null, 'phutilnumber', null),
8
8
'getTransactionEditString' => array(
9
9
null,
10
-
'number',
11
-
'number',
10
+
'phutilnumber',
11
+
'phutilnumber',
12
12
null,
13
-
'number',
13
+
'phutilnumber',
14
14
null,
15
15
),
16
-
'getFeedAddString' => array(null, null, 'number', null),
17
-
'getFeedRemoveString' => array(null, null, 'number', null),
16
+
'getFeedAddString' => array(null, null, 'phutilnumber', null),
17
+
'getFeedRemoveString' => array(null, null, 'phutilnumber', null),
18
18
'getFeedEditString' => array(
19
19
null,
20
20
null,
21
-
'number',
22
-
'number',
21
+
'phutilnumber',
22
+
'phutilnumber',
23
23
null,
24
-
'number',
24
+
'phutilnumber',
25
25
null,
26
26
),
27
27
),
···
73
73
} else if ($node instanceof PhpParser\Node\Expr\FuncCall) {
74
74
switch ($this->getName($node)) {
75
75
case 'phutil_count':
76
+
return 'phutilnumber';
76
77
case 'count':
77
78
return 'number';
78
79
case 'phutil_person':
···
80
81
}
81
82
} else if ($node instanceof PhpParser\Node\Expr\New_ ) {
82
83
if ($this->getName($node->class) == 'PhutilNumber') {
83
-
return 'number';
84
+
return 'phutilnumber';
84
85
}
85
86
} else if ($node instanceof PhpParser\Node\Expr\Variable) {
86
87
$name = $this->getName($node);