1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-08 16:02:39 +01:00
Commit graph

2232 commits

Author SHA1 Message Date
Andre Klapper
04e3e250f7 Add missing variable names to PHPDoc @param of methods
Summary:
Add variable names (`$varname` in `@param type $varname explanation`) to PHPDoc method headers, for fun and profit.

Closes T15923

Test Plan:
* Read the method signatures and their corresponding PHPDoc headers at your fireplace
* Still run `./bin/diviner generate` without explosions (though it is very lenient anyway?)

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15923

Differential Revision: https://we.phorge.it/D25799
2024-08-23 18:52:13 +02:00
Andre Klapper
901f060771 Drop question mark suffix from optional PHPDoc @param types
Summary:
The question mark in `@param type? $foo Desc` is a custom notation not consistently applied across the codebase and not necessarily obvious to the reader (because custom and not mentioned in https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md ).
Instead, explicitly state "optional" in the parameter description for clarity.

Closes T15925.

Test Plan: Run PHPStan, see no `PHPDoc @param has invalid value (type? [...]` style output anymore.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15925

Differential Revision: https://we.phorge.it/D25798
2024-08-23 12:50:14 +02:00
jkim
0c9c94748b Fix PHP 8.1 "trim(null)" exception when creating a diff with an empty "Differential Revision:" field
Summary:
This revision fixes a `trim(null)` exception when creating a diff with an empty
field.

Fixes: T15868

Test Plan:
1. Create a commit with an empty "Differential Revision:" field
2. Run `arc diff`

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25703
2024-08-22 15:32:59 +01:00
Andre Klapper
0d5f437970 Fix return value of PhutilTestCase::tryTestCaseMap()
Summary: `PhutilTestCase::tryTestCases()` returns void. Thus `PhutilTestCase::tryTestCaseMap()` should behave the same way as `PhutilTestCase::assertException()`: return void.

Test Plan: Read the code; run static code analysis and don't get "Result of method PhutilTestCase::tryTestCases() (void) is used" anymore.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25791
2024-08-18 09:42:09 +02:00
Andre Klapper
4752567130 Fix $boot_length comparison in PhagePHPAgentBootloader
Summary: `$boot_length = strlen($boot_sequence->toString())` returns an `int` and `strlen()` expects a `string` as a parameter. Thus calling `if (strlen($boot_length) > 8192)` afterwards to get the string length of an integer makes no sense.

Test Plan: Read the code.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25770
2024-08-15 20:24:47 +02:00
Andre Klapper
06028fad3c Add return statements for PhutilChannelChannel::readBytes()/writeBytes()
Summary:
PHPStan complains that `Method PhutilChannelChannel::readBytes() should return string but return statement is missing.` and `Method PhutilChannelChannel::writeBytes() should return int but return statement is missing.`

All other subclasses of `PhutilChannel` implementing these two methods either provide a return value or directly throw an exception.
`PhutilChannelChannel` throws an exception for both methods but hadn't explicitly declared that as a `return` statement.

Test Plan: Run static code analysis; read/grep the code.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25754
2024-08-14 14:44:51 +02:00
Andre Klapper
84210cedc6 Declare missing class properties in ArcanistDownloadWorkflow
Summary:
rARCbd9769ba92df63d0429f74ec7fb8b00a4989f28b added and declared the three private properties `$id`, `$saveAs`, `$show`.
However rARC21e80a635d798c5be2c6e5c272497b3170c1e079 removed their declarations while still initializing these properties in `didParseArguments()`.

Test Plan: Run PHPStan static code analysis; grep the code in this class.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25713
2024-07-10 10:06:35 +02:00
Andre Klapper
76d22d70e4 Correct PHPDoc of dropSymbolCache()
Summary:
`dropSymbolCache()` does not return anything, and its only call in `rebuild-map.php` does not expect a return value either.
Thus replace `@return this` with `@return void`.

Test Plan: Read/grep the code.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25720
2024-07-10 10:06:03 +02:00
Valerio Bozzolan
72e59da804 Fix arc diff in Subversion for non-English languages
Summary:
When using `arc diff`, this crash should not fire anymore on non-English shells:

    Undefined array key "Repository UUID"

The error message comes to this line:

https://we.phorge.it/source/arcanist/browse/master/src/repository/api/ArcanistSubversionAPI.php;f7fcf31c7e23475e345cb3cd4abf0474ba6fd9cf$606

Look at the parser of `svn info` that expects English language:

https://we.phorge.it/source/arcanist/browse/master/src/repository/api/ArcanistSubversionAPI.php;f7fcf31c7e23475e345cb3cd4abf0474ba6fd9cf$326-375

The historical behavior of `ExecFuture` is to inherit environment variables. It may have sense, to have the user home, etc.

So, the approach is to just clear the incompatible specific environment variable that alters the output language.

Closes T15855

Test Plan:
- clone a random a Subversion repository
- have your operating system in a non-English language
- (e.g. `export LANGUAGE=en_US:it_IT`)
- run 'arc diff'

No crash anymore. You are able to submit the patch, just like in git.

Reviewers: O1 Blessed Committers, l2dy, avivey

Reviewed By: O1 Blessed Committers, l2dy, avivey

Subscribers: l2dy, aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15855

Differential Revision: https://we.phorge.it/D25691
2024-07-08 10:03:32 +02:00
Andre Klapper
331b255b15 Declare missing class properties
Summary: Code in these classes tries to access undefined properties. Thus define these properties.

Test Plan: Run PHPStan static code analysis; grep the code in each class.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25712
2024-07-03 09:05:34 +02:00
Andre Klapper
21fbc806e4 Fix call to non-existing ArcanistAliasEffect::EFFECT_CONFIGURATION in ArcanistAliasEngine
Summary:
`ArcanistAliasEffect::EFFECT_CONFIGURATION` does not exist.
Probably `ArcanistAliasEffect::EFFECT_MISCONFIGURATION` was meant, given the message it sets.

Test Plan: Carefully read the code.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25652
2024-06-23 10:03:01 +02:00
Pppery
0af89f7d32 Add fallback languages for locale files
Summary: Upstream version of https://gerrit.wikimedia.org/r/c/phabricator/translations/+/1047593

Test Plan:
Set any languages and observe untranslated strings display proper PLURAL rules

With the downstream "translations" extension installed, set the language to traditional Chinese and see Simplified Chinese rather than English translations if they exist, like "Foo added/removed a project"

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25695
2024-06-21 13:10:22 -04:00
Pppery
f7fcf31c7e Remove PhutilPhtTestCase::getDateTranslations()
Summary: See my comment at T15815#17864. I don't think the translation extractor has any consumers other than the downstream translations extension, and I'd prefer to handle this issue there rather than upstream.

Test Plan: Read the code.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25684
2024-06-07 15:10:18 -04:00
Andre Klapper
3cb117684f Fix calls to non-existing getDescription() in ArcanistWorkingCopyPath
Summary: `ArcanistWorkingCopyPath::getDescription()` is undefined. Given its use in the exception message, the path is supposed to be used, so call `ArcanistWorkingCopyPath::getPath()` instead.

Test Plan: Carefully read the code and check for existing methods in the class.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25657
2024-05-19 10:28:16 +02:00
Andre Klapper
5477568274 Correct call to non-existing Filesystem::removePath() in ArcanistDownloadWorkflow.php
Summary:
rARC21e80a635d798c5be2c6e5c272497b3170c1e079 introduced a call to non-existing `Filesystem::removePath()`.
Presumably, `remove()` in `src/filesystem/Filesystem.php` was meant, so use that function instead.

Test Plan: Unknown, apart from reading and grep'ing the code base.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25640
2024-05-18 21:38:57 +02:00
Andre Klapper
7f28d7266f Fix PHP 8.1 "preg_match(null)" exception for missing Content-Type
Summary:
When the `Content-Type` HTTP header is empty or missing, `null` is passed to `preg_match()` which is deprecated behavior since PHP 8.1. Thus only call `preg_match()` when the value is set.

```
ERROR 8192: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated at [$HOME/arcanist/src/future/http/status/HTTPFutureHTTPResponseStatus.php:24]
```

Closes T15821

Test Plan: Visit something using `HTTPFutureHTTPResponseStatus`, like, a profile image. No crashes.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15821

Differential Revision: https://we.phorge.it/D25632
2024-05-10 18:12:59 +02:00
Tim Schumacher
6718b32a64
Improve PHPDoc of id()
Summary: Improve PHPDoc of id()

Test Plan: Check the types that are returned by id

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25576
2024-04-15 16:39:21 +02:00
Andre Klapper
ef73b12b58 Fix "strpos(): Non-string needles will be interpreted as strings" in PhutilSortVector
Summary:
Code checking if the needle string `$value` is somewhere in the haystack `"\0"` makes no sense for a single byte (if it did, then `strcmp` instead of `strpos` should have been used) and the created exception output implies that it's supposed to check that a string does not contain NULL bytes.
Thus switch the order of arguments passed to `strpos()` to be correct.

```
EXCEPTION: (RuntimeException) strpos(): Non-string needles will be interpreted as strings in the future. Use an explicit chr() call to preserve the current behavior at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> strpos(string, integer) called at [<arcanist>/src/utils/PhutilSortVector.php:33]
```

Closes T15755

Test Plan: Read the surrounding code carefully.

Reviewers: O1 Blessed Committers, valerio.bozzolan, speck

Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15755

Differential Revision: https://we.phorge.it/D25557
2024-03-22 13:34:55 +01:00
Aviv Eyal
f6261dc614 Arc liberate: support traits
Summary:
Looks like this is all that's needed?

Ref T15751

Test Plan:
R12 has some scenarios for testing this.
Also ran `arc liberate --clean` on arc and phorge repos, and the generated map did not change.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15751

Differential Revision: https://we.phorge.it/D25551
2024-03-15 19:58:41 +02:00
sten
7c5e607e97 Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json.
Summary:
This change updates PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use phpunit's junit output rather than json.

--log-json was deprecated in PHPUnit 5.7, and removed in PHPUnit 6.0.0 (2017-02-03).

Fixes T15667

Test Plan:
Download an example PHP repo, with arcconfig set to 'PhpunitTestEngine'. Example:
```
git clone https://github.com/campbsb/example-phorge-php-project.git
```

Install phpunit using composer, and test it
```
cd example-phorge-php-project
php ~/composer.phar update
phpunit
```
In a PHP phorge repo, set the following in .arcconfig:
```
    "unit.engine": "PhpunitTestEngine",
    "phpunit_config": "phpunit.xml",
```
Make a non-breaking change to src/Service.php (eg add a space), and run 'arc unit'

Reviewers: O1 Blessed Committers, avivey, speck

Reviewed By: O1 Blessed Committers, avivey, speck

Subscribers: avivey, Ekubischta, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15667

Differential Revision: https://we.phorge.it/D25472
2024-03-15 16:09:16 +00:00
Andre Klapper
174bf094ef Replace all phurl.io short URIs with target URIs
Summary:
As of February 2024, phurl.io (which is not run by Phorge.it) shows HTTP 503 errors instead of redirecting to the target URIs.

Thus replace any URIs pointing to phurl.io with the corresponding target URIs, based on the mapping listed on https://secure.phabricator.com/phurl/.

Closes T15746

Test Plan: Carefully read the diff.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15746

Differential Revision: https://we.phorge.it/D25542
2024-02-28 14:59:26 +01:00
Christopher Speck
8ef1ead6ac T15064: Several arcanist PHP 8.1 compat issues on Windows
Summary:
Ran into `strlen`/`strpos` issues while using Arcanist on Windows:
- Running `arc version` when not in a project/repository directory blew up (may not be specific to Windows).
- Determining mime type of binary file in repository fails.

Refs T15064

Test Plan: Ran diff including a binary file. Ran `arc version` when not in a project/repository directory.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25534
2024-02-06 06:56:39 -05:00
Andre Klapper
6c7caf3572 Correct manual upload of Differential patch with a leading BOM
Summary:
Strip a leading UTF-8 Byte Order Mark to avoid silently dropping the first file in a manually uploaded patch.

This change only strips the UTF-8 BOM as UTF-8 is acceptable input.
(Probably non-existing) handling of any other BOMs as first bytes in a diff remains unchanged.

Closes T15452

Test Plan:
Go to `/differential/diff/create/` and upload the test case in T15452 via `Raw Diff From File`.
See two files listed on resulting page `/differential/diff/1/` instead of previously only one file.
Optionally, confirm that byte length of `$diff` is three bytes less now (via `strlen($diff)`).

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15452

Differential Revision: https://we.phorge.it/D25514
2024-01-13 22:05:47 +01:00
Valerio Bozzolan
6142fcd526 Fix Subversion "commit" support in PHP 8.1
Summary:
Premising that "arc commit" is a beautiful Workflow dedicated to svn repositories,
I tried it at work, causing the usual PHP 8.1 deprecation warning:

    $ arc diff
    $ arc commit
    ERROR 8192: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated at [arcanist/src/workflow/ArcanistWorkflow.php:1520]
    arcanist(head=master, ref.master=e46025f7a914)
      #0 preg_replace(string, string, NULL) called at [<arcanist>/src/workflow/ArcanistWorkflow.php:1520]
      #1 ArcanistWorkflow::normalizeRevisionID(NULL) called at [<arcanist>/src/workflow/ArcanistCommitWorkflow.php:68]
      #2 ArcanistCommitWorkflow::run() called at [<arcanist>/scripts/arcanist.php:427]
    Usage Exception: Unable to identify the revision in the working copy. Use '--revision <revision_id>' to select a revision.

This bug happens at least when Arcanist does not find any related Revision ID.

It seems there is a method that always normalizes the Revision ID, but sometime that is unknown (null).
And so, NULL ends inside a preg_replace(). It's probably OK to have a normalize method that accept wild
things, including NULL. So, fixed that specific method.

Closes T15693

Test Plan:
This revision was tested in production in my company.

Take a random Subversion repository. Edit a line. Run "arc diff". Then run "arc commit". No warnings.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15693

Differential Revision: https://we.phorge.it/D25498
2023-12-18 11:37:41 +01:00
Andre Klapper
e46025f7a9 Fix PHP 8.1 "urlencode(null)" exception blocking account registration redirect for custom OAuth provider
Summary:
It seems that a `tokenSecret` is not always passed at this stage, and that PHP's `urlencode()` does not accept passing a `null` string since PHP 8.1 (I could not find any upstream note about this but bug reports across the web seem to confirm this).

Thus do not try to `urlencode($this->tokenSecret)` if it is `null`.

```
EXCEPTION: (RuntimeException) urlencode(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(), ava(), phorge(), wmf-ext-misc()
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/error/PhutilErrorHandler.php:261]
  #1 <#2> urlencode(NULL) called at [<arcanist>/src/future/oauth/PhutilOAuth1Future.php:232]
```

Closes T15589

Test Plan:
* As an admin, set up custom "MediaWiki" OAuth provider from from https://gitlab.wikimedia.org/-/ide/project/repos/phabricator/extensions/edit/wmf/stable/-/src/oauth/
* As an admin, apply D25373
* As a user, go to `/auth/login/mediawiki:whatever/`
* Select login button

Redirect now works as expected: The URL redirect to allow access on
http://mediawiki.localhost/index.php?title=Special%3AOAuth%2Fauthorize&oauth_token=1234567890abcdef1234567890abcdef&oauth_consumer_key=1234567890abcdef1234567890abcdef works as expected, instead of showing a raw error page about `urlencode()` not accepting passing `null`. (After allowing authorization there are more issues in Phorge code but they are out of scope for this Arcanist patch.)

Reviewers: O1 Blessed Committers, valerio.bozzolan, speck

Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15589

Differential Revision: https://we.phorge.it/D25374
2023-12-04 19:28:25 -08:00
Bartosz Dziewoński
16a412b108 Fix ArcanistExternalLinter on Windows
Summary:
When using `proc_open()` with `'bypass_shell' => true` on Windows,
file extensions other than .exe will not be resolved. Various linters
therefore don't work, such as `jshint`, which is actually `jshint.cmd`.

The problem was already observed and fixed in some places (e.g.
ArcanistGitAPI trying to run `git`), but not in ArcanistExternalLinter.

Changes:
* Fix `Filesystem::resolveBinary()` to actually only resolve executable
  files on Windows, and not any other files with no extension or with
  an extension listed in %PATHEXT%. Those files can be executed by
  typing their name in the cmd.exe shell, but not directly by low-level
  Windows functions, and we're using `'bypass_shell'` to bypass the
  shell.
* Fix `ArcanistExternalLinter::getBinary()` to call
  `Filesystem::resolveBinary()` on Windows.

Test Plan:
Run `arc lint` on the Phorge repository while on Windows.
Observe no errors related to jshint.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: aklapper, avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15544

Differential Revision: https://we.phorge.it/D25341
2023-12-04 22:44:29 +01:00
Aviv Eyal
25611ba24a PhutilErrorHandler: support multiple error listeners
Summary: Ref T15554. The plan is to add a new listener that will only listen to DEPRECATED events, and do something useful with them.

Test Plan: Test script in P26 shows registering 2 handlers and getting both invoked.

Reviewers: O1 Blessed Committers, Matthew

Reviewed By: O1 Blessed Committers, Matthew

Subscribers: Sten, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15554

Differential Revision: https://we.phorge.it/D25388
2023-11-27 20:32:12 +02:00
sten
5bc53cfe53 Update PhutilCowsay.php to work for small cows
Summary:
Update PhutilCowsay.php to work for small cows.

In doing so, we also simplify the code to just use multiline regexps rather than trying to parse a line at a time.

cowsay(cow='small'){{{What about me?}}}

Test Plan:
Check cowsay works for the built in non-perl cow:
```
cowsay(cow='companion'){{{Built in}}}
```

Test all the perl cows:
```
cowsay(cow='bunny'){{{Testing bunny}}}
cowsay(cow='cower'){{{Testing cower}}}
cowsay(cow='daemon'){{{Testing daemon}}}
cowsay(cow='default'){{{Testing default}}}
cowsay(cow='dragon-and-cow'){{{Testing dragon-and-cow}}}
cowsay(cow='dragon'){{{Testing dragon}}}
cowsay(cow='elephant'){{{Testing elephant}}}
cowsay(cow='eyes'){{{Testing eyes}}}
cowsay(cow='flaming-sheep'){{{Testing flaming-sheep}}}
cowsay(cow='head-in'){{{Testing head-in}}}
cowsay(cow='kitty'){{{Testing kitty}}}
cowsay(cow='koala'){{{Testing koala}}}
cowsay(cow='meow'){{{Testing meow}}}
cowsay(cow='moofasa'){{{Testing moofasa}}}
cowsay(cow='moose'){{{Testing moose}}}
cowsay(cow='mutilated'){{{Testing mutilated}}}
cowsay(cow='satanic'){{{Testing satanic}}}
cowsay(cow='sheep'){{{Testing sheep}}}
cowsay(cow='skeleton'){{{Testing skeleton}}}
cowsay(cow='small'){{{Testing small}}}
cowsay(cow='squirrel'){{{Testing squirrel}}}
cowsay(cow='stegosaurus'){{{Testing stegosaurus}}}
cowsay(cow='supermilker'){{{Testing supermilker}}}
cowsay(cow='surgery'){{{Testing surgery}}}
cowsay(cow='turkey'){{{Testing turkey}}}
cowsay(cow='turtle'){{{Testing turtle}}}
cowsay(cow='tux'){{{Testing tux}}}
cowsay(cow='www'){{{Testing www}}}
```

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25436
2023-11-14 15:05:33 +00:00
Aviv Eyal
98d16d27cf only update cache file if something changed
Summary:
See Q77. When installing in a read-only location, updating the file is both redundant (nothing changed) and fails.

Make sure to only save the updated file if anything changed.

Test Plan: Run `arc lint` somewhere, make `.phutil_module_cache` and `src/` read-only, run `arc lint` again - it should avoid crashing.

Reviewers: fgaz, O1 Blessed Committers, valerio.bozzolan

Reviewed By: fgaz, O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25446
2023-10-12 19:21:56 +03:00
sten
ba42b63704 Update ArcanistComposerLinter.php to check content-hash instead of hash
Summary:
The 'hash' key in composer.json files was removed by composer version 1.3.0 in December 2016, in favour of the 'content-hash' key.

Update the code to validate the content-hash instead.

Fixes T15647

Test Plan:
* Update a composer.json file, without running 'composer update'
* Run 'arc lint' and confirm it warns you that the composer.lock file is out of date
* Run 'composer update'
* Run 'arc lint' and confirm it returns OKAY

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15647

Differential Revision: https://we.phorge.it/D25442
2023-09-28 09:12:29 +01:00
sten
35e127da57 Fix rendering of cowsay sheep.cow
Summary:
In the templates of the external cowsay library, most replaceable tokens are specified as $var_name.
However, the sheep.cow and flaming-sheep.cow use the ${eyes} syntax instead. This is not recognised by PhutilCowsay.php resulting in incorrect rendering of the template.

This change updates PhutilCowsay.php to handle ${var_name} tokens as well as $var_name

cowsay(cow='sheep'){{{My eyes, my eyes!}}}

Test Plan:
In a Remarkup comment or document, add
```
cowsay(cow='sheep'){{{How do my eyes look now?}}}
```

When testing in differential, you don't even need to submit the comment.

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25435
2023-09-12 15:57:15 +01:00
Aviv Eyal
d343be5926 Error handling: send Deprecation messages as explicit Event
Summary:
Ref T15554. When a deprecation warning is captured here, mark it as such and send using the same channel as error messages.
Error Handlers will generally ignore it now, so they'll need to be updated, e.g. D25386

Test Plan:
Hitting a `strlen(null)` before This Change:

- Web:
    - PhutilAggregateException - white boxes with red border.
- Daemons:
    - trace in daemon log, task fails. Daemon sleeps for 5 seconds.
- Arcanist and Scripts in phorge/bin/ and phorge/scripts:
    - execution blows up with error trace.
- SSH server-side scripts (ssh-auth and ssh-exec):
    - trace in configured log, execution fails
- SSH client-side scripts (ssh-connect):
    - execution blows up with error trace.

After this change:

- Web:
    - Before `D25386`: Nothing on screen, errors show in log.
    - With `D25386`: logs + dark console.
- Daemons:
    - trace in daemon log, task completes successfully.
- Arcanist and Scripts in phorge/bin/ and phorge/scripts/ :
    - Error trace printed to STDERR, execution continues.
- SSH server-side scripts (ssh-auth and ssh-exec):
    - trace in configured log, execution continues.
- SSH client-side scripts (ssh-connect):
    - Error trace printed to STDERR, execution continues.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15554

Differential Revision: https://we.phorge.it/D25387
2023-08-31 08:14:16 -07:00
sten
8b907d7716 Fix PHP 8.1 arc patch strlen(null) binary file error
Summary:
Fix issue in arcanist whereby when doing an arc patch involving adding or removing a binary file, it falls over with strlen(null) errors.

Fixes T15617

Test Plan: arc patch Dxxxx

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15617

Differential Revision: https://we.phorge.it/D25409
2023-08-18 14:57:54 +01:00
bob
df6c315ace Fix a PHP 8.1/8.2 deprecated use of strlen deprecated call with a NULL argument
Summary:
This strlen call triggering an exception if an user tried to call the patch command without an authentication token
Indeed, strlen() was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1, we use phutil_nonempty_string() as a replacement.

Fix T15599

Test Plan:
Remove your arcanist authentication token file (if you have one) and try to call the patch command in a repository.
You should get an error message suggesting you to call the install-certificate command instead of an exception.

Reviewers: O1 Blessed Committers, Matthew

Reviewed By: O1 Blessed Committers, Matthew

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15599

Differential Revision: https://we.phorge.it/D25383
2023-08-14 10:58:01 +02:00
Aviv Eyal
6832afc300 Fix jshint tests
Summary:
The linter's behavior was changed in https://github.com/jshint/jshint/issues/3444: "warnings" are no longer counted for `maxerr`.

Updating the test to match...

Test Plan: `arc unit src/lint/linter/__tests__/ArcanistJSHintLinterTestCase.php` with a recent-ish (2.13.6) jshint.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25355
2023-08-12 08:43:46 -07:00
Aviv Eyal
726b148afc Rebrand: Add "path" entries to PlatformSymbols
Summary: Ref T15006. Required for D25343

Test Plan: See D25343.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15006

Differential Revision: https://we.phorge.it/D25349
2023-08-12 08:41:53 -07:00
Andre Klapper
788098096e Fix PHP 8.0 ValueError calling mb_convert_encoding() with an invalid encoding
Summary:
Per https://www.php.net/manual/en/function.mb-convert-encoding.php, as of PHP 8.0.0, a ValueError is thrown if the value of `to_encoding` or `from_encoding` is an invalid encoding but a ValueError is not suppressed by the stfu operator ("@").

Origin of the function:

https://secure.phabricator.com/rPHU72ad8fd0f05b0d84f7d8efd7db62ad0b3ba4431f

Premising that Arcanist elevates warnings to exception, now we just try-catch.

Closes T15423

Test Plan:
On `/diffusion/edit/1/page/encoding/`,
* enter a valid encoding, such as "7bit", successfully changed encoding
* enter a valid encoding with random capitalization, such as "7biT", successfully changed encoding
* enter a valid alias encoding, such as "ISO-10646-UCS-2", successfully changed encoding
* enter a valid alias encoding with random capitalization, such as "isO-10646-uCS-2", successfully changed encoding
* enter an invalid encoding, such as "whatever", get error message "Repository encoding "whatever" is not valid: String conversion from encoding 'UTF-8' to encoding 'whatever' failed: mb_convert_encoding(): Argument #2 ($to_encoding) must be a valid encoding, "whatever" given"
In any case, no exception is shown anymore.

Reviewers: O1 Blessed Committers, valerio.bozzolan, speck

Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck

Subscribers: 0, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15423

Differential Revision: https://we.phorge.it/D25249
2023-07-29 12:20:40 +03:00
Aviv Eyal
6c6f47bf9c (arc) Unify type-checking for setHref() type methods
Summary:
Introduce `PhutilURI::checkHrefType` to unify type-check of some PHUI objects (See D25357).

Ref T15316

Test Plan:
Run this script:

```
#!/usr/bin/env php
<?php

require_once './arcanist/scripts/__init_script__.php';

$valid_inputs = array(
  'foo',
  null,
  new PhutilURI('http://0@domain.com/'),
);

$invalid_inputs = array(
  9,
  new Filesystem(),
  array(),
);

echo "Valid inputs: \n";
foreach ($valid_inputs as $v) {
  echo("Checking ".gettype($v)." \n");
  PhutilURI::checkHrefType($v);
}

echo "\nError inputs: \n";
foreach ($invalid_inputs as $v) {
  echo("Checking ".gettype($v)." \n");
  PhutilURI::checkHrefType($v);
}

echo "\n";
```

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15316

Differential Revision: https://we.phorge.it/D25356
2023-07-24 12:33:47 -07:00
Aviv Eyal
8a2cb75d63 Fix tab complete in php 8
Summary: Ref T15064

Test Plan: run `arc shell-complete`; `arc shell-complete arc shell`; Should suggest completions, not crash.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: mainframe98, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25350
2023-07-21 09:45:55 -07:00
Valerio Bozzolan
b996b4799b phutil_nonempty_scalar(): don't throw when receiving a boolean scalar
Summary:
Note that booleans are scalars. Full stop.

| is_scalar($v)    | Result |
|------------------|--------|
| `"foo"`          | true   |
| `""`             | true   |
| `null`           | true   |
| `0`              | true   |
| `0.5`            | true   |
| `true`           | true   |
| `false`          | true   |
| `new stdclass()` | false  |
| `array()`        | false  |

Note that phutil_nonempty_scalar() was designed just to tell
whenever a scalar is "empty" or not. So it must not explode
when receiving a valid scalar.

So the question is not whenever a boolean is a scalar or not,
but whenever is empty or not. But also this is a clear fact:

| `$v`    | `is_scalar($v)` | `!is_empty($v)` | `if(strlen($v))`|
|---------|-----------------|-----------------|-----------------|
| `true`  | `true`          | `true`          | `true`          |
| `false` | `true`          | `false`         | `false`         |

In short, now the function does not explode anymore with bool
values. Instead, it says whenever are empty or not.

In bold the exact changes:

| Value             |`phutil_nonempty_scalar($v)`|
|-------------------|----------------------------|
| `"foo"`           | true                       |
| `""`              | false                      |
| `null`            | false                      |
| `0`               | true                       |
| `0.5`             | true                       |
|`obj` with tostring| true                       |
|`obj` withno tostr.| Exception                  |
| `array()`         | Exception                  |
| `true`            | ~~Exception~~ **true**     |
| `false`           | ~~Exception~~ **false**    |

Closes T15239

Test Plan:
- check if it makes sense to you
- check the few usages

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15239

Differential Revision: https://we.phorge.it/D25117
2023-07-18 12:41:43 +02:00
Aviv Eyal
6e4947b55f Improve arc-browse for php 8.1
Summary: Ref T15064.

Test Plan: ran some `arc browse` commands in php 8.1, got no exceptions.

Reviewers: O1 Blessed Committers, valerio.bozzolan, Sten

Reviewed By: O1 Blessed Committers, valerio.bozzolan, Sten

Subscribers: Sten, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25339
2023-07-07 09:16:59 -07:00
Aviv Eyal
152ba1f02a Add explicit tests for phutil_string_cast
Summary: Test (and document) what phutil_string_cast actually does to some things.

Test Plan: `arc unit`

Reviewers: O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D25326
2023-07-03 13:43:07 -07:00
Aviv Eyal
8426ebc053 Fix arc browse for php 8.x
Summary: Ref T15064

Test Plan: Run `arc browse <filename>`, get a browser and no exception.

Reviewers: O1 Blessed Committers, valerio.bozzolan, chris

Reviewed By: O1 Blessed Committers, chris

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15064

Differential Revision: https://we.phorge.it/D25309
2023-06-26 12:18:07 -07:00
Steve Campbell
677e117b97 Fix strlen() being passed a null in ArcanistUnitConsoleRenderer.php
Summary:
Fix the following error in ArcanistUnitConsoleRenderer.php under PHP 8.1:

```
strlen(): Passing null to parameter #1 ($string) of type string is deprecated
```

Stack trace:

```
EXCEPTION: (RuntimeException) strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [<arcanist>/src/error/PhutilErrorHandler.php:261]
arcanist(head=master, ref.master=97e163187418, custom=4)
  #0 PhutilErrorHandler::handleError(integer, string, string, integer) called at [<arcanist>/src/unit/renderer/ArcanistUnitConsoleRenderer.php:15]
  #1 ArcanistUnitConsoleRenderer::renderUnitResult(ArcanistUnitTestResult) called at [<arcanist>/src/workflow/ArcanistUnitWorkflow.php:190]
  #2 ArcanistUnitWorkflow::run() called at [<arcanist>/scripts/arcanist.php:427]
```

Ref T15187

Test Plan: arc unit

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15187

Differential Revision: https://we.phorge.it/D25300
2023-06-19 14:55:49 +01:00
Andre Klapper
97e1631874 Fix PHP 8.1 "strlen(null)" exception which blocks creating a project with an empty Description field
Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_stringlike()` as a replacement for string-alike variables.

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_stringlike() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Closes first half of T15331

Test Plan: Applied these two changes (one in Arcanist, one in Phorge). Project with empty Description field was created and `/project/view/projectid/` rendered in web browser.

Reviewers: O1 Blessed Committers, valerio.bozzolan, speck

Reviewed By: O1 Blessed Committers, valerio.bozzolan, speck

Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15331

Differential Revision: https://we.phorge.it/D25176
2023-06-10 16:35:18 +02:00
Valerio Bozzolan
444bb60d43 Fix PHP 8.1 "strlen(null)" exception from ArcanistRefView which blocks "arc look remotes"
Summary:
This change fixes the command `arc look remotes` for PHP 8.1.

Without this change, the null value bubbles up to PhutilUTF8StringTruncator, reaching a strlen().

This control probably does not need to be done at this low level inside PhutilUTF8StringTruncator,
but it is right to be at this high level from the caller in ArcanistRefView.

Closes T15368

Test Plan:
- run "arc look remotes"
- still works in "old PHP" like 7.4
- start to work in recent PHP 8.1+

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15368

Differential Revision: https://we.phorge.it/D25206
2023-06-01 18:20:24 +02:00
Andre Klapper
18554ea76c Correct spelling mistake in PhutilLock
Summary: Closes T15268

Test Plan: Only changes to spelling.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15268

Differential Revision: https://we.phorge.it/D25253
2023-05-29 10:29:05 +02:00
jkim
0e32dbc1ac Correct a PHP8 compatibility issue when running "arc diff" with no active branch
Summary:
When there is no active branch name, arc diff currently fails under PHP8 when we try to strlen(null).

This change is also credited to Evan from upstream Phabricator that applied the same change:

https://secure.phabricator.com/rARCc39ab20eb3717a15aed2467842bd77d9addce96a

Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.

Closes T15412

Test Plan: Under PHP 8.1: ran git checkout <hash of head>, then arc diff to generate this revision.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15412

Differential Revision: https://we.phorge.it/D25237
2023-05-24 20:06:26 +01:00
Andre Klapper
e4fd31ec02 Fix PHP 8.1 exception in Conduit: Make "array_fuse(array $list)" accept null as parameter
Summary:
`array_fuse` in Arcanist is a wrapper for calling `array_combine($list, $list)`.
The latter doesn't accept passing `null` in PHP 8.2.
Going to `/conduit/method/project.create/`, entering a `name` but nothing as `members` (so we pass `null`), and calling this method, an exception is thrown.

Thus make `array_fuse` accept null and return an empty list in such cases.

Closes T15393

Test Plan: Applied this change; afterwards "Method Call Result" page at `/api/project.create`  correctly displayed in the web browser.

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15393

Differential Revision: https://we.phorge.it/D25228
2023-05-21 00:07:13 +02:00
Valerio Bozzolan
c14785c379 Fix PHP 8.1 "strpos(null)" exception from PhutilCommandString which blocks arc patch
Summary:
For some reason it may happen that a specific command line argument receives a null argument
from PHP, instead of just an empty string.

Nowadays, this null value probably breaks almost whatever GNU/Linux or FreeBSD or Microsoft Windows
etc. implementations since everyone expect to receive a string.

This used to work in the past since functions like strpos() or strlen() accepted null, but not
anymore. This generate a deprecation warning since PHP 8.1, that is elevated as exception from
Phabricator/Phorge and breaking features.

Without getting into implementation logics (which doesn't make sense to fix all of them) the
calling function should just be kind. So we normalize nonsense null values to an empty string.

Note: this was the expected behavior prior to PHP 8.1.

Now we do that normalization explicitly, in this early point.

After this fix, also T15368 should probably be fixed.

Closes T15367

Test Plan:
- run "arc patch <something valid>"
- to you it must continue to work
- (to @ton it starts working right now)

Reviewers: O1 Blessed Committers, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: speck, tobiaswiese, Matthew, Cigaryno, ton

Maniphest Tasks: T15367

Differential Revision: https://we.phorge.it/D25205
2023-05-09 08:29:00 +02:00