Summary:
`pht()` can be some random function.
Better solution would be to separate this linter to its own class but it would be slower.
Also use `PhutilLintEngine` as `lint.engine`.
Test Plan:
$ arc lint # on file with pht($a)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3480
Summary: Add `ruby -wc` as a linter and raise Error whenever there's a syntax error
Test Plan: Just a few dumb unit tests.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3447
Test Plan:
Wrote "Posible" in the linter, let it change to "Possible".
New unit test.
Reviewers: jack, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3469
Summary: See D3449, comments, unit tests.
Test Plan: Unit tests.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3463
Summary:
I tried to fixed it but I've given up.
See rP958e6cd109f3.
Test Plan:
$ arc unit
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3449
Summary: Use python from env, instead of the distro installed one in /usr/bin
Test Plan:
Ran arc anoid on a system that only had python in /usr/bin, still
works. Ran arc anoid on a system that had a python in /usr/local/bin before
/usr/bin. Still works as well
Reviewers: epriestley, alanh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3451
Summary: This is Arcanist part of D3434.
Test Plan: None.
Reviewers: epriestley, wez
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3450
Git commit messages must have 2 newlines between the subject and body
If used to rewrite a commit message then the commit message will be
incorrectly reformatted.
Summary:
- See D3422.
- Also improve some event configuration/debugging stuff.
Test Plan: Ran `arc list --trace`, set bogus/valid event handlers.
Reviewers: vrana, nh
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3423
Summary:
HPHP now autoloads in `is_subclass_of()`.
I've left the `class_exists()` check as the code is more explicit.
Test Plan:
// under HPHP
function __autoload($c) {
echo "$c\n";
}
is_subclass_of('C', 'stdClass');
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3435
Summary:
This object is highly useful in many client event handlers (particularly for access to the CLI) and allows them to be implemented less intrusively.
This also slightly reduces code duplication.
Test Plan: Ran "arc diff".
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1753
Differential Revision: https://secure.phabricator.com/D3421
Summary:
w00t!
Created new functions getBookmarkName, createBookmark that use mercurial
commands to create a new bookmark and apply the commit message when
running arc patch. Like with git, it checks if the bookmark already
exists. If it does, creates arcpatch-DXXX-1, -2 etc
Updated the mercurial section of run() to include applying the commit
message.
Pretty new to programming in general still, so I wasn't sure if I should
have just modified getBranchName and createBranch to work with Mercurial
(instead of creating new functions). This was easier for me to make sure
I didn't break the git functionality. Let me know I can merge the
functions.
Test Plan:
Tested in my www-hg repository. Probably need to test further (suggestions?)
Ran the following trials with arc patch D539987
1) the bookmark arcpatch-D539987 already existed
2) the bookmark did not exist
3) tried an invalid commit, arc patch R539987
4) --nobookmark flag (bookmark was not created, but the commit applied)
5) --nocommit flag (boomark was created, and the commit was not applied)
6) --nocommit and --nobokmark
Here's the summary of the results:
https://secure.phabricator.com/P493
Reviewers: dschleimer
Reviewed By: dschleimer
CC: aran, epriestley, phleet, bos, csilvers
Differential Revision: https://secure.phabricator.com/D3334
Summary:
Adds an event prior to creation of a new revision so installs can muck around with titles, etc.
I'll also update the docs.
Test Plan: Ran "arc diff --trace" and observed event dispatch. Added `var_dump()` and verified $revision is a reasonable object.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, geoffberger
Differential Revision: https://secure.phabricator.com/D3408
Summary:
We log time of running `arc diff`. It's easy with `--verbatim` or with users just confirming the message built in commit template.
With `--background`, I want to log only waiting time, not message editing time. This event should allow it.
Test Plan:
$ arc diff
Haven't created the listener yet.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3394
Summary:
This is how it looks like now:
> What do you want to name this library? x
> Writing '__phutil_library_init__.php' to 'x/__phutil_library_init__.php'...
> Usage Exception: This library is using libphutil v1, which is no longer supported. Run 'arc liberate --upgrade' to upgrade to v2.
Test Plan:
$ arc liberate # in new dir
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3368
Summary:
We want to have a class of lint problems which are displayed to author and attached to revision but don't require excuse in diff workflow.
Lint advices already serve this purpose but no linters emit them because they need `--advice` flag to be processed.
By always enabling advices, we can switch more linters from warnings to advices and don't stop the diff workflow for them.
This diff also bumps down default severity of TODO rule.
Test Plan: Made lint advice mistake, ran `arc lint`.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, ide, Korvin
Differential Revision: https://secure.phabricator.com/D3364
Summary:
- Add zlib functions to extension functions.
- Provide a better error if the extension is actually missing.
Test Plan: Eyeballed it.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D3321
Summary: See D3296#1.
Test Plan:
New test.
Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3297
Summary:
I'm not sure if we are interested in this kind of linters ("Don't use slow function if there is a fast alternative").
I didn't find a case where `strpos() === 0` could be useful except [[ http://www.php.net/manual/en/mbstring.overload.php | mbstring.func_overload ]] craziness.
Test Plan:
New unit test.
Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3296
Summary:
See discussion on T1630.
extraData provides more scope for extensions to piggy-back
more data on the test results and have that pulled up to the UI.
We're using keys like "facebook:complexity" to store additional
data as part of the test results.
Test Plan:
Nothing in the codebase touches extraData at the moment, so
you'll just have to have faith/prove by inspection.
Reviewers: vrana, nh, tuomaspelkonen, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T1630
Differential Revision: https://secure.phabricator.com/D3276
Summary: XHPAST doesn't currently parse most PHP 5.4 stuff, but it does parse this. Warn about it.
Test Plan:
Unit tests, and:
Error (XHP35) Use Of PHP 5.4 Features
The f()[...] syntax was not introduced until PHP 5.4, but this codebase
targets an earlier version of PHP. You can rewrite this expression using
idx().
365 public function lintPHP54Features($root) {
366
367 if (false) {
>>> 368 id()[0];
^
369 }
370
Reviewers: alanh, vrana
Reviewed By: alanh
CC: aran
Differential Revision: https://secure.phabricator.com/D3291
Summary:
I need to run some jobs only if the tests hasn't been skipped.
I know that this could end up by passing more and more data to the event but this is all I need so far.
Test Plan: Dumped `unitResult` from the listener.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3259
Summary: See T1635 and the giant inline comment.
Test Plan: Ran unit tests on 32-bit and 64-bit machines.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1635
Differential Revision: https://secure.phabricator.com/D3250
Summary: See T1635. I'm going to make an effort to rewrite this in a way that's safe in 32-bit PHP, i.e. without bcmath. Add test coverage to limit the chance I screw it up.
Test Plan: Ran unit tests.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1635
Differential Revision: https://secure.phabricator.com/D3249
Summary: See D3252. Reduces code duplication a little bit. Also remove some dire warnings about impending doom -- this has been in use in the wild for a long time.
Test Plan: Added a file in ISO-8859-1, ran `arc diff --encoding ISO-8859-1` to generate this revision, got an encoding note in output.
Reviewers: davidreuss, vrana, btrahan
Reviewed By: davidreuss
CC: aran
Maniphest Tasks: T452
Differential Revision: https://secure.phabricator.com/D3253
Summary:
In some cases (notably, homebrew) an installer may not control where arcanist/ and libphutil/ live and may not be able to control 'include_path'.
Allow libphutil/ to be symlinked into arcanist/externals/includes/ if all else fails.
Test Plan:
- Moved `libphutil` to `libphutilx`. Ran "arc" and got a failure.
- Symlinked it into externals/includes/, ran `arc`, got success.
- Moved it back to `libphutil`, ran `arc`, success.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, tfmeusburger
Differential Revision: https://secure.phabricator.com/D3243
Summary: I will also commit fixes in other repos.
Test Plan: LinkChecker
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3242
Summary: We always do this on --recon now, see D3213.
Test Plan: Ran `arc diff --background 1` to generate this diff.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3241
Summary:
The only purpose of "Arcanist Overview" is to tell people they shouldn't be here, but we bury the lede.
Make it clear that this is not user documentation.
After T988 we can improve the organization here, but some recent users found this pretty confusing.
Test Plan: Generated and read documentation.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3235
Summary: We emit a confusing error if there's no ".arcconfig" in the local right now.
Test Plan:
$ arc patch D3185
This patch is for the 'phabricator' project, but the working copy
belongs to the 'phabricatox' project. Still try to apply the patch? [Y/n]
$ arc patch D3185
This patch is for the 'phabricator' project, but the working copy does
not have an '.arcconfig' file to identify which project it belongs to.
Still try to apply the patch? [Y/n]
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3231
Summary:
We shove alias parameters onto the front of the arg list so if you make an alias like "qdiff" = "diff x y z" and then run "qdiff a b c", we end up with "diff x y z a b c". However, currently we reverse alias parameters, so you actually get "diff z y x a b c".
This is a problem for `arc alias bdiff -- diff --background 1`, which evaluates to `arc diff 1 --background` and fails.
Test Plan: Created a `bdiff` alias and ran it successfully.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3196
Summary: Depends on D2614.
Test Plan:
Updated a diff with no lint errors.
Updated a diff with lint errors, verified that the previous message is not lost.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3174
Summary:
I usually write commit messages 1-2 minutes.
`arc lint` in our repository usually runs for around 30 seconds, `arc unit` another minute or two.
Even Phabricator unit tests sometimes runs long (because `CREATE DATABASE` and `DROP DATABASE` is slow in our setup for some reason).
Waiting for the results is boring and unnecessary.
This diff presents two different concepts how to run them on background:
# Lint is run with `--output json`, results are parsed and presented back to user. It isn't perfect - there's no context in printed lint errors which is a serious problem.
# Unit tests are run normally and the results are written to a scratch file. It also isn't perfect - colors are lost during the process.
I'll probably choose one approach and use it on both places. Let me know your thoughts about them.
This can be further improved to resolve the futures also after inputting the update message but it can be done in a separate diff.
Test Plan:
- Remove lint engine.
- Remove unit engine.
- Make lint errors.
- Make unit errors.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, beng, tuomaspelkonen, alanh
Differential Revision: https://secure.phabricator.com/D2614
Summary:
According to @epriestley, it's nasty and kind of crazy: D2933#1.
It also stands in my way for D2614.
Test Plan: Rewrote our callsite to event listener and verified that it still works.
Reviewers: tuomaspelkonen, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D3171
