1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 14:52:40 +01:00
Commit graph

727 commits

Author SHA1 Message Date
vrana
c8687a0c79 Lint functions not available on Windows on PHP 5.2
Summary: Also use absolute paths.

Test Plan: Linted Arcanist, libphutil, Phabricator, found no false positives and one real error in [[ https://secure.phabricator.com/diffusion/PHU/browse/master/src/channel/PhutilSocketChannel.php;42d8e8447c8b5d6a$92 | PhutilSocketChannel ]].

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3504
2012-09-17 13:51:06 -07:00
vrana
94f684e29e Declare ArcanistBaseWorkflow::run()
Summary: We call it from `arcanist.php`.

Test Plan: This diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3496
2012-09-17 13:24:37 -07:00
vrana
589bccb716 Handle arc diff --background 1 --
Summary: We currently insert background parameters to end which causes ignoring them if there is '--'.

Test Plan:
  $ arc diff --background 1 -- HEAD^

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3497
2012-09-17 13:20:31 -07:00
vrana
3bbf9ccc3d Fix typo in comment 2012-09-14 17:52:24 -07:00
vrana
ac7b9e42d6 Publicize formatting unit test result
Summary: I want to use it from outside.

Test Plan:
  $ arc unit src/lint/linter/__tests__/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3493
2012-09-13 22:36:46 -07:00
vrana
fefa9e2d72 Use array_mergev() in Phutil test engine
Test Plan: This diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3491
2012-09-13 22:35:41 -07:00
Evan Priestley
f5bb4177c3 Merge pull request #52 from killme2008/master
Fixed git branch name contains color
2012-09-13 16:35:16 -07:00
dennis zhuang
d65b953252 Get branch name with --no-color option 2012-09-13 15:12:19 -07:00
vrana
7ee0f3e3b3 Lint short ternary as PHP 5.3 feature
Summary: I don't offer a replacement because `f() ?: 1` converted to `f() ? f() : 1` can cause side effects and whitespace issues.

Test Plan: New test.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3489
2012-09-13 11:24:32 -07:00
vrana
a5e2f81928 Skip linter tests with usage problems
Summary: D3480#comment-1

Test Plan: This diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3481
2012-09-12 10:56:00 -07:00
vrana
261fd592b9 Add missing space in string 2012-09-12 10:42:39 -07:00
vrana
64f35f0b83 Tweak severity of pht() linter
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
2012-09-12 10:29:37 -07:00
vrana
7c0d99aac9 Fix typo in comment 2012-09-12 02:11:22 -07:00
Leah Xue
03e5d651b5 Make ruby -wc a linter in Arcanist
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
2012-09-11 10:42:50 -07:00
vrana
9dd1a87066 Make spell check linter patching
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
2012-09-10 16:21:58 -07:00
epriestley
918eff8ff9 Fix false negatives in "break;" lint check
Summary: See D3449, comments, unit tests.

Test Plan: Unit tests.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3463
2012-09-07 17:46:35 -07:00
vrana
339cabedcc Add a test case for false negative switch lint rule
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
2012-09-07 15:55:34 -07:00
vrana
cdb161a19a Allow Phutil tests setting link
Summary: Also delete some copy pasta.

Test Plan: Next diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3455
2012-09-07 15:42:50 -07:00
vrana
af31ee4ed0 Link Arcanist test cases
Summary: See D3455.

Test Plan: This diff (after rebase).

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3460
2012-09-07 15:31:14 -07:00
Peter Ruibal
e3601cfe30 Use /usr/bin/env python
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
2012-09-06 19:13:57 -07:00
vrana
c50b18142d Support linking unit tests
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
2012-09-06 18:48:59 -07:00
Evan Priestley
842ca57b4a Merge pull request #51 from bpedman/master
Fix getCommitMessage to properly separate subject and body with 2 newlines
2012-09-06 09:40:22 -07:00
Brandon Pedersen
4038e7be0a Fix commit message format returned by getCommitMessage
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.
2012-09-06 08:56:44 -06:00
epriestley
bc96d6036e Implement the arcanist top-level log handling in terms of $console->writeLog()
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
2012-09-05 11:45:54 -07:00
vrana
a309e5e1eb Use punctuation in spelling linter
Summary: Also move dependency one directory down.

Test Plan:
  $ arc lint # on file with "teh"

Reviewers: jack, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3436
2012-09-05 10:14:29 -07:00
vrana
466910c700 Delete obsolete comment
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
2012-09-04 23:13:43 -07:00
epriestley
a802a90123 Always include 'workflow' on Arcanist events
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
2012-09-04 12:08:04 -07:00
katherine
b0cfe9d94d [hg - arc patch] make mercurial bookmark with arc patch
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
2012-08-31 17:05:46 -07:00
epriestley
d79664a30d Add a willCreateRevision event
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
2012-08-30 12:28:35 -07:00
Nick Harper
4a786f48de Use paste.query for arc paste
Summary: paste.info is deprecated; switch to using paste.query

Test Plan:
  arc paste P123
  arc paste P123 --json

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3402
2012-08-29 11:04:19 -07:00
vrana
600e052e72 Dispatch event after building diff message
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
2012-08-27 18:34:31 -07:00
epriestley
d83768a949 Minor, fix obvious typo in SVN patch workflow (@mroch). 2012-08-23 17:52:47 -07:00
vrana
7ba210f89a Create v2 libraries
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
2012-08-22 15:43:54 -07:00
vrana
129339019f Always use lint advices
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
2012-08-22 11:49:59 -07:00
epriestley
29ba92c0c2 Improve management of zlib dependency
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
2012-08-20 17:26:42 -07:00
vrana
1779abef6f Link docs from message
Test Plan:
  phutil_console_confirm("See http://www.phabricator.com/docs/phabricator/article/Differential_User_Guide_Large_Changes.html for information");

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3317
2012-08-16 14:29:37 -07:00
vrana
13b64da47a Use console in unit inside diff 2012-08-15 15:54:34 -07:00
vrana
30bc4ca6e9 Don't ask for confirmation with unsound unit tests
Test Plan:
  $ arc diff # with unsound tests
  $ arc diff --ignore-unsound-tests # with unsound tests

Reviewers: epriestley, aran

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1515

Differential Revision: https://secure.phabricator.com/D2985
2012-08-15 15:49:50 -07:00
vrana
014521362f Fix lint warning 2012-08-15 14:27:04 -07:00
vrana
cae7631dff Warn about strstr() misuse
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
2012-08-15 13:44:41 -07:00
vrana
34efe49e12 Warn before using strpos($a, $b) === 0
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
2012-08-15 13:20:25 -07:00
Wez Furlong
390bb280e1 Add extraData field to test results
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
2012-08-15 10:34:29 -07:00
epriestley
a0d4430271 Detect use of f()[0] in lint
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
2012-08-15 04:36:50 -07:00
vrana
ab602e3a52 Pass lint and unit results to diff created event
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
2012-08-13 14:57:18 -07:00
epriestley
061a9f8cbd Implement base85 in a 32-bit safe way, without bcmath
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
2012-08-12 19:21:33 -07:00
epriestley
08b29ad23f Add test coverage to the base85 implementation
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
2012-08-12 19:20:46 -07:00
epriestley
be3ce781bb Switch arcanist to phutil_utf8_convert()
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
2012-08-12 08:50:01 -07:00
epriestley
c0c78c2ff4 Raise exception instead of returning magical number from 'arc anoid'
Summary: how do I python

Test Plan: help

Reviewers: alanh

Reviewed By: alanh

CC: aran

Differential Revision: https://secure.phabricator.com/D3245
2012-08-10 15:29:27 -07:00
epriestley
6b0af26b94 Fix serious problems with 'arc anoid'
Summary: https://github.com/facebook/arcanist/issues/48

Test Plan: hmm

Reviewers: alanh, tuomaspelkonen

Reviewed By: tuomaspelkonen

CC: aran

Differential Revision: https://secure.phabricator.com/D3244
2012-08-10 15:16:02 -07:00
epriestley
a31d88ee82 Allow arcanist to search for libphutil in externals/includes/
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
2012-08-10 14:58:19 -07:00