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

122 commits

Author SHA1 Message Date
epriestley
777c119a0e Search for more test locations in PHPUnitTestEngine
Summary:
See https://github.com/facebook/arcanist/pull/53

  - Work correctly for directories with `%` in their name; this breaks under sprintf().
  - Search for `src/` -> `tests/` style directories.
  - Add coverage for search paths.

Test Plan:
Ran unit tests.

I don't have a working test case for PHPUnit tests, can one of you guys apply this and verify I didn't break your setups?

Reviewers: quard, aurelijus

Reviewed By: aurelijus

CC: aran

Differential Revision: https://secure.phabricator.com/D3558
2012-09-29 18:03:43 -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
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
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
Alan Huang
877e6f743b Add an arc flag workflow
Summary:
Allow querying and modifying flags from arcanist. Currently
supports only printing and deleting flags for Differential revisions,
but it should be straightforward to add more capabilities (given Conduit
support).

Test Plan:
Run arc flag, passing it various revisions. Flags are
modified appropriately.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1556

Differential Revision: https://secure.phabricator.com/D3133
2012-08-03 12:00:54 -07:00
epriestley
768a125b58 Enrich arc configuration and add stronger typing
Summary:
See <https://github.com/facebook/arcanist/issues/45>

Currently, when the user types `arc set-config x false`, we set it as the string "false", which is usually not desirable. We have some steps toward typed config already, but expand on what we have and move as much stuff as possible into it, including all the config settings that aren't currently documented (there are still some lint-specific and project-specific settings not present here, but this is most of it).

Also make the `phutil_libraries` key a legacy name for `load`, and `immutable_history` a legacy name for `history.immutable`. Generally the goal here is to make config simpler and bring it more in-line with Git/Mercurial, which use dotted hierarchies.

I'll add some documentation here but I think most of the changes should be fairly straightforward.

Test Plan:
  - `arc set-config history.immutable on` (And similar -- sets to boolean true.)
  - `arc set-config history.immutable off` (And similar -- sets to boolean false.)
  - `arc set-config history.immutable derp` (And similar -- raises exception.)
  - `arc set-config history.immutable ''` (And similar -- removes setting value.)
  - `arc set-config --show`
  - `arc get-config`
  - `arc get-config base`

Reviewers: dschleimer, bos, btrahan, vrana

Reviewed By: dschleimer

CC: aran

Maniphest Tasks: T1546

Differential Revision: https://secure.phabricator.com/D3045
2012-07-25 18:37:09 -07:00
Alan Huang
316122c4e0 Create a mysterious new workflow
Summary:
Phabricator, as we all know, is marketed as a fun adventure game. However, while it is occasionally fun and often an adventure, it's so far been sorely deficient in the game aspect. This patch aims to rectify that oversight. (Presence of the first two qualities is not guaranteed.)

Note: In case there's any doubt, this is not a serious suggestion. I was bored.

Test Plan: Seriously?

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3026
2012-07-22 14:39:53 -07:00
Alan Huang
1f65a115cc Create an arc todo workflow for quickly creating a task.
Summary:
Run `arc todo o rly?` to create a Maniphest task
titled 'o rly?'. Pass --cc to add CCs besides yourself.

Test Plan: Ran `arc todo o rly?`. Behaved as expected.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T749, T1325

Differential Revision: https://secure.phabricator.com/D2952
2012-07-10 17:32:36 -07:00
epriestley
a235a041e5 Remove PhutilModuleRequirements
Summary: Yeah, we can nuke this.

Test Plan: Grepped for callsites.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2871
2012-06-26 18:22:36 -07:00
epriestley
67956306cb Remove all libphutil v1 support
Summary:
Delete all code related to v1 libraries in arcanist.

When users liberate a v1 library, prompt them to upgrade.

Test Plan: Reverted phabricator/ to a couple of months ago and liberated it. Got prompted to upgrade. Upgraded.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2861
2012-06-26 12:40:42 -07:00
epriestley
69246b282d Simplify "arc branch" and make it work in immutable history repositories
Summary:
  - Move "arc branch"-specific code to the branch workflow.
  - Instead of doing "git rev-parse", just do "git branch --verbose --abbrev=40".
  - Use revision owners to identify ownership, not working copy identity. Particularly with the advent of "Commandeer", you might not own commits you made.
  - Do a batch lookup for commits by hash (depends on D2859, but doesn't break without it).
  - Use PhutilConsole for console stuff.
  - Removed color from "arc list" for the moment.
  - The "--by-status" flag has a slightly different output format now.

Test Plan: Ran "arc branch" in various circumstances, verified it identifies branches in immutable history repositories.

Reviewers: btrahan, vrana, jungejason, nh, slawekbiel

Reviewed By: slawekbiel

CC: aran

Maniphest Tasks: T693

Differential Revision: https://secure.phabricator.com/D2860
2012-06-26 10:50:43 -07:00
vrana
1817f929c2 Introduce arc inlines
Summary: This displays all inline comments attached to a revision in a format consumable by editors.

Test Plan: Ran it, opened the file on the line.

Reviewers: epriestley

Reviewed By: epriestley

CC: vii, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2845
2012-06-23 16:01:38 -07:00
epriestley
f2220e74fc Add a DSL for selecting base commits
Summary:
New optional mode. If you set 'base' in local, project or global config or pass '--base' to 'arc diff' or 'arc which', it switches to DSL mode.

In DSL mode, lists of rules from args, local, project and global config are resolved, in that order. Rules can manipulate the rule machine or resolve into actual commits. Provides support for some 'arc' rules (mostly machine manipulation) and 'git' rules (symbolic ref and merge-base).

Test Plan:
Ran unit tests. Also:

```$ arc which --show-base --base 'arc:prompt'

    Against which commit? HEAD
HEAD
$ arc which --show-base --base 'git:HEAD'
HEAD
$ arc which --show-base --base 'git:fake'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc which --show-base --base 'git:origin/master'
origin/master
$ arc which --show-base --base 'git:upstream'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc which --show-base --base 'literal:derp'
derp
$ arc which --show-base --base 'arc:halt'
Usage Exception: None of the rules in your 'base' configuration matched a valid commit. Adjust rules or specify which commit you want to use explicitly.
$ arc set-config --local base git:origin/master
Set key 'base' = 'git:origin/master' in local config.
$ arc which --show-base
origin/master
$ arc which --show-base --base 'git:HEAD^'
HEAD^
$ arc which --show-base --base 'arc:yield, git:HEAD^'
origin/master
$ arc which --show-base --base 'arc:global, git:HEAD^'
HEAD^
$ arc which --show-base --base 'arc:global, git:merge-base(origin/master)'
3f4f8992fba8d1f142974da36a82bae900e247c0```

Reviewers: dschleimer, vrana

Reviewed By: dschleimer

CC: aran

Maniphest Tasks: T1233

Differential Revision: https://secure.phabricator.com/D2748
2012-06-15 14:01:28 -07:00
vrana
08b53c3803 Pass for phutil test that was expected to fail and skip
Summary:
I just hope that we will not create `ArcanistPhutilTestCaseTestCaseTestCase`.

Also fix a copy/paste error (@edward's this time).

Test Plan: `arc unit`

Reviewers: epriestley

Reviewed By: epriestley

CC: edward, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2692
2012-06-08 14:29:51 -07:00
epriestley
f9d55d809f Rough cut of hgdaemon
Summary:
This need a bunch of work, but is a sort of minimum viable product for the hgdaemon.

The two ProtocolChannels implement the client and server halves of the protocol.

The Server implements the server logic, the Client implements the client logic.

Test Plan: Launched a server and client on the same repo, got hg output in ~2-3ms instead of 100ms+.

Reviewers: csilvers, btrahan, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2665
2012-06-07 18:23:57 -07:00
vrana
1d9ffce1ec Define interface for lint renderers
Summary:
It makes me nervous.

Also move them to the dir.

Test Plan:
`arc lint`
`arc lint --output json`

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2644
2012-06-01 17:35:38 -07:00
vrana
0b45ec30be Move files in Arcanist one level up
Summary:
- `kill_init.php`
- Manually change library map.
- Manually rename `/data/` test dirs.
- [src/lint/linter] `git mv base/ArcanistLinterTestCase.php __tests__/`
- `arc liberate`

Test Plan: Browse around to make sure I like it better, especially `repository/api`, and `workflow`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2637
2012-06-01 11:56:00 -07:00
epriestley
9cd098ca01 Add "ArcanistSingleLintEngine" and "ArcanistScriptAndRegexLinter"
Summary:
Request from @csilvers. Allow installs to get most linter features with regexes, configuration and external scripts if they are hesitant to write phutil libraries.

  - Add "ArcanistSingleLintEngine", which implements the smallest possible engine behavior (run one linter on every path).
  - Add "ArcanistScriptAndRegexLinter", which uses a script and a regex to parse lint output from other scripts.

Depends on D2618.

Test Plan:
Basics:

  $ arc set-config lint.engine ArcanistSingleLintEngine
  Set key 'lint.engine' = 'ArcanistSingleLintEngine'.
  $ arc set-config lint.engine.single.linter ArcanistScriptAndRegexLinter
  Set key 'lint.engine.single.linter' = 'ArcanistScriptAndRegexLinter'.
  $ arc set-config linter.scriptandregex.script 'echo derp #'
  Set key 'linter.scriptandregex.script' = 'echo derp #'.
  $ arc set-config linter.scriptandregex.regex '/^(?P<message>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<message>.*)$/m'.
  $ arc lint
  >>> Lint for .arcconfig:

     Error  (S&RX) Lint
      derp

                 1 {
                 2   "project_id" : "arcanist",
                 3   "conduit_uri" : "https://secure.phabricator.com/",

Throw:

  $ arc set-config linter.scriptandregex.regex '/^(?P<throw>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<throw>.*)$/m' (was '/^(?P<message>.*)$/m').
  $ arc lint
  Usage Exception: ArcanistScriptAndRegexLinter: configuration captured a 'throw' named capturing group, 'derp'. Script output:
  derp

Ignore:

  $ arc set-config linter.scriptandregex.regex '/^(?P<ignore>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<ignore>.*)$/m' (was '/^(?P<throw>.*)$/m').
  $ arc lint
   OKAY  No lint warnings.

Severity:

  $ arc set-config linter.scriptandregex.regex '/^(?P<warning>.)(?P<message>.*)$/m'
  Set key 'linter.scriptandregex.regex' = '/^(?P<warning>.)(?P<message>.*)$/m' (was '/^(?P<ignore>.*)$/m').
  $ arc lint
  >>> Lint for src/lint/engine/single/ArcanistSingleLintEngine.php:

     Warning  (S&RX) Lint
      erp

                 1 <?php
                 2
                 3 /*

Reviewers: csilvers, btrahan, vrana

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2619
2012-05-31 12:09:01 -07:00
epriestley
71afde1988 Upgrade arcanist to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet because they aren't necessary for libraries to function.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2589
2012-05-30 14:22:59 -07:00
epriestley
009e6c4dbf Add "ArcanistPhutilLibraryLinter" to replace "ArcanistPhutilModuleLinter"
Summary:
Adds a linter for v2 libraries which raises the relevant errors.

NOTE: Not hooked up anywhere yet, so this diff has no effect.

Test Plan:
Switched the ModuleLinter to LibraryLinter and ran it with a junk block to trigger errors:

  >>> Lint for src/lint/linter/phutillibrary/ArcanistPhutilLibraryLinter.php:

     Error  (PHL3) One Class Per File
      File 'lint/linter/phutillibrary/ArcanistPhutilLibraryLinter.php' mixes
      function (id) and class/interface (ArcanistPhutilLibraryLinter)
      definitions in the same file. A file which declares a class or an
      interface MUST declare nothing else.

               190 }
               191
               192 if (false) {
               193   function id() { }
               194   new XYZ();
               195 }

     Error  (PHL2) Duplicate Symbol
      Definition of function 'id' in
      'lint/linter/phutillibrary/ArcanistPhutilLibraryLinter.php' in library
      'arcanist' duplicates prior definition in 'utils/utils.php' in library
      'phutil'.

               190 }
               191
               192 if (false) {
               193   function id() { }
               194   new XYZ();
               195 }

     Error  (PHL1) Unknown Symbol
      Use of unknown class 'XYZ'. This symbol is not defined in any loaded
      libphutil library.

               191
               192 if (false) {
               193   function id() { }
               194   new XYZ();
               195 }

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2597
2012-05-30 07:25:09 -07:00
Aurelijus
1c6f66dab3 PHPCS Linter
Summary:
Simple wrapper for PHP_CodeSniffer.
You need to have PHP_CodeSniffer and it's dependencies installed.

Test Plan: - Try running it with your custom lint engine

Reviewers: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2560
2012-05-24 10:55:46 +02:00
Aurelijus
4f739014ae PHPUnit test engine
Summary:
PHPUnit wrapper for arc.
The idea here is simple - find the test case which is related to the updated
class file. Generate tmp files for json & clover reports, run phpunit
with provided arguments.
It supports phpunit configuration file setting in `.arcconfig`: `phpunit_config`.
Path should be relative to project root.

Test Plan:
- Set `unit_engine` to `PhpunitTestEngine`
- Try running tests with & without `phpunit_config` option.

Reviewers: epriestley, davidreuss

Reviewed By: epriestley

CC: aran, Koolvin, jungejason

Differential Revision: https://secure.phabricator.com/D2472
2012-05-22 07:12:03 -07:00
epriestley
19aa759f39 "arc upgrade", to automatically upgrade arc (client changes)
Summary:
  - Try to limit the pain of //future// version bumps by making arc self-updating.
  - When the server needs a newer version, prompt the user to update.
  - (We need them to reissue their command because we may already have loaded classes which have changed in the update.)
  - Make the message sound exciting!

Test Plan: Artifically bumped server forward, ran "arc list", got to upgrade!

Reviewers: Makinde, nh, jungejason, btrahan

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D2435
2012-05-09 10:01:31 -07:00
epriestley
9f3a0963cb Add "arc get-config" and "arc set-config" for managing ~/.arcrc values
Summary:
The major thing I want to do here is allow you to set a default Phabricator URI, so we can make "arc paste", and "arc upload", "arc download" work anywhere.

We can also relax the .arcconfig requirements (request from @csilvers).

Test Plan:
Get/set some values?

iiam

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2400
2012-05-07 06:07:23 -07:00
Julius Seporaitis
cb051d8568 Wrapper for 'nose' unittest and code coverage tools.
Summary:
Wrapper for Python 'nose' (http://readthedocs.org/docs/nose/en/latest/)
testing tool.

Test Plan:
Install latest 'nose' v1.1.3. Currently it is available through
Github only (``pep install git+https://github.com/nose-devs/nose.git``).

Create a Python project with following structure:

  /package_name/module_name.py
  /tests/package_name/test_module_name.py

Write some tests

Run ``arc unit``

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin, zeeg

Differential Revision: https://secure.phabricator.com/D2322
2012-04-27 12:12:20 -07:00
Edward Speyer
946a9e44a3 Allow tests to be skipped
Summary:
Allow tests to be skipped by calling assertSkipped().  It's not really
an assertion of anything tangible; more like "assert that we can't
really assert anything right now".

Test Plan: Added a new test to the PhutilUnitTestEngineTestCase.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2312
2012-04-24 21:45:22 -07:00
epriestley
2831d075c0 Remove only trailing "#" and empty lines as comments from CLI editors
Summary: Don't strip numbered lists in comment bodies, etc.

Test Plan: Unit tests, meta-testing this diff.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1124

Differential Revision: https://secure.phabricator.com/D2262
2012-04-18 06:08:41 -07:00
epriestley
da4b6b5799 Rename "arc mark-committed" to "arc close-revision"
Summary:
  - Replace SVN-specific language with VCS-agnostic language.
  - Add new "arc close-revision", works exactly like "arc mark-committed" but with agnostic language.
  - Use status constants, not status strings.
  - Mark "arc mark-committed" deprecated.
  - Remove deprecated "arc merge".

Test Plan: Ran "arc mark-committed", "arc close-revision".

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, Makinde

Maniphest Tasks: T909

Differential Revision: https://secure.phabricator.com/D2244
2012-04-17 13:51:10 -07:00
Koolvin
e7d9ff18d3 Add arc close command
Summary:
```arc close T1088 --status wontfix --message "I'm not going to fix this."```
T1088 is the test task to screw with, so feel free.

Test Plan: ```arc close T1088 --status [ resolved | wontfix | invalid | duplicate | spite | open ] -m "Message"```

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2162
2012-04-08 20:57:38 -07:00
Koolvin
b553af2763 Add arc tasks command
Summary:
Added `arc tasks`:

%%%arc tasks
arc tasks --view-all    // View Open and Closed Tasks
arc tasks --by-status   // Group By Status
arc tasks --by-priority // Group By Priority%%%

Test Plan: Connect to conduit and run arc tasks >> make sure you have tasks =p

Reviewers: epriestley, indiefan

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T749

Differential Revision: https://secure.phabricator.com/D1943
2012-04-07 17:40:25 -07:00
epriestley
31a54d9b4a When a mercurial working copy has uncommitted changes, prompt to include them in "arc diff"
Summary:
See task. Allow mercurial users to diff with uncommitted changes.

  - By default, commit range is merge-base of `hg outgoing` to `.` (dirstate).
  - You can get JUST dirstate with `arc diff tip` or similar.
  - This ended up being a giant mess various other changes to deal with empty `hg outgoing` and empty dirstate.

Test Plan: Diffed with uncommitted changes, got sensible prompts and results.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T998

Differential Revision: https://secure.phabricator.com/D1954
2012-03-19 19:17:10 -07:00
epriestley
f57199268f Remove chooseRevision() and DifferentialRevisionRef
Summary: `arc which` and related workflows have supplanted these now-unused mechanisms.

Test Plan: Grepped for chooseRevision() and DifferentialRevisionRef.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1841
2012-03-09 08:57:24 -08:00
epriestley
b058efbb51 Add an "arc alias" command
Summary:
Allow users to easily define aliased arc commands. This is easier than making
them muck around with shell stuff, and lets me do stuff like "arc alias adiff
diff -- --auto" so I can test --auto more easily.

There are some limitations here (for example, you can't put --trace in an alias
because it gets parsed too early) but I think it's a reasonable starting point.

Test Plan: Set, listed, removed and used aliases.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T896

Differential Revision: https://secure.phabricator.com/D1653
2012-02-21 13:16:52 -08:00
epriestley
0781554a22 Improve Arcanist symbol name linter
Summary:
  - Move name helper functions to ArcanistXHPASTLintNamingHook to make it easier
to write custom linters.
  - Add test coverage for name functions.
  - Add 'variable' and 'global' naming convention tests.
  - Expand test cases.
  - Improve lint message error when an unexpected message is raised during a
test.
  - Remove a defunct XHP lint message.

Test Plan:
  - Ran unit tests.
  - Ran "arc lint --lintall" on arcanist/.

Reviewers: btrahan, nh, jungejason

Reviewed By: btrahan

CC: johnduhart, aran, epriestley, arudolph

Differential Revision: https://secure.phabricator.com/D1506
2012-01-28 11:17:45 -08:00
epriestley
c46dbbf61c Add "arc land" as a first-class workflow
Summary:
This is a fancy version of "land.sh" that uses "git merge --squash" and
"arc which" to cover more cases.

Test Plan:
Ran "arc land" against various repository states (no such branch, not
accepted, valid, etc). Things seemed OK. There are basically an infinite number
of states here so it's hard to test exhaustively.

Reviewers: cpiro, btrahan, jungejason, davidreuss

Reviewed By: davidreuss

CC: zeeg, aran, epriestley, davidreuss

Maniphest Tasks: T787, T723

Differential Revision: https://secure.phabricator.com/D1488
2012-01-25 15:10:59 -08:00
epriestley
3ee01bacac Add 'arc which', and
ArcanistRepositoryAPI->loadWorkingCopyDifferentialRevisions()

Summary:
  - See T787.
  - @cpiro has an immediate use case for this, which is ##arc amend --revision
`arc which --id` --show master## for "git merge --autosquash" or similar.
  - For T614, we need this to choose "--create" vs "--update".
  - Other workflows should also use this to improve how often we automatically
get things right, particularly in Mercurial and SVN.

Test Plan:
Ran "arc which" in SVN, Git and HG working copies with various flags;
results seemed reasonable.

Reviewers: btrahan, cpiro, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T787

Differential Revision: https://secure.phabricator.com/D1478
2012-01-24 09:47:53 -08:00
adonohue
2c4eb00a12 Add ArcanistConduitLinter, a linter that delegates through Conduit
Summary:
Julien built a really cool static analysis database of our codebase. One
capability is that it can suggest typehints that are not in the code. The
analysis to do this is very expensive, so it can't reasonably be run locally.
But it can remain indexed on a server.

The idea here is to provide a familiar interface to it through arc lint, via a
generic Conduit service call.

In our lint engine, this will probably be gated on --advice for performance.
This will introduce a slight awkwardness in that running with --advice can add
new non-advice lint if the server chooses, but this isn't likely to cause a
practical problem.

Test Plan:
Construct a fake Conduit lint endpoint, attach this linter to it, and see bogus
lint
appear with --advice.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1462
2012-01-20 11:17:12 -08:00
Bob Trahan
16e78669d3 Rebuild phutil library map for arc
Summary: D1439 added a new class which had the wrong name in the map from D1439.
 Whoopsiepoos!

Test Plan: unit tests now pass

Reviewers: epriestley, zeeg

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1441
2012-01-17 15:49:20 -08:00
David Cramer
5cd2eec6f1 Implement Comprehensive Lint Engine
Summary:
This adds a new lint engine, ComprehensiveLintEngine, which
includes sane defaults for some generic languages.

Ideally this would include *all* available language linters,
but that can be enhanced at a later point. Right now it's mostly
the base linter with additional JavaScript and Python linters.

Test Plan:
Adjust the lint_engine to be "ComprehensiveLinterEngine". You'll
also need jshint, pyflakes, and pep8 to all be available on PATH.

Run arc lint against files which contain .php, .py, and .js.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1439
2012-01-17 12:46:06 -08:00
Jack Lindamood
c5dfa34f10 Add spell check linter
Summary:
Inspired by http://news.ycombinator.com/item?id=3464671 and a lot of
diffs I've seen @ FB, I've added a spell checking linter.  To reduce
false positives, it's only a blacklist.  Still, it catches a large
number of 'issues'.

Test Plan:
Unit tests.  Ran on FB's codebase.  No false positives
noticed but a lot of cases caught.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, jack

Differential Revision: https://secure.phabricator.com/D1409
2012-01-16 05:46:26 -08:00
Anton Kovalyov
e5bda05200 Adds ArcanistJSHintLinter to check JS files for errors and potential problems.
This patch adds ArcanistJSHintLinter class and three new .arcconfig options:

 * lint.jshint.prefix - directory where JSHint binary resides
 * lint.jshint.bin - JSHint binary name (if different from 'jshint')
 * lint.jshint.config - a JSON file with JSHint project-wide options

By default, this linter assumes that JSHint is installed on user's system
as an NPM package.

Test Plan:

(1)

Run `npm install jshint -g` to install JSHint and add
ArcanistJSHintLinter to your Lint Engine (I didn't see PEP8 or
PyFlakes in the PhutilLintEngine so I decided to not to put
JSHint in there). After that you can do `arc lint my.js`.

(2)

Create a new file, config.json, and add `{ "white": true }` in it.
Add `"lint.jshint.config": "/path/to/config.json"` to your .arcconfig and
run `arc lint my.js`. After that, unless your name is Douglas Crockford,
you'll see tons of JSHint warnings about minor PEP8-like things (spacing, etc.)
2012-01-13 19:17:34 -08:00
Bob Trahan
b61e4eacf1 Arc - add a sanity check to arc patch workflows to make sure the vcs base
revision is the correct base revision relative to the patch.

Summary: What the title says.   If not correct, warn the user.   This check
honors the --force flag to skip all these checks.   This change also includes
moving some Differential constants into Arc so they can be used for both
projects.   There is a corresponding phabricator diff (incoming) to address this
part of the change.

Test Plan:
For a project with actual diffs, a git repository tracked by phabricator, *AND*
development in master branch only, do some...
- git reset --hard HEAD^1
- arc patch DX, where X is what got us to HEAD in the first place
- verify successful patch
...then...
- git reset --hard HEAD^^
- arc patch DX, where X is what got us to HEAD in the first place
- verify warning
- verify Y versus N continues versus stops appropriately
Note if development were done outside the master branch this warning message
will fire early / often as git commit hashes are based on the commit *and* the
rest of the source code the commit is made against.  This is (unfortunately) the
"typical" case so this warning is pretty active at the moment.   T201 will
eventually land and when parsing a given commit update the corresponding diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, btrahan, epriestley

Differential Revision: https://secure.phabricator.com/D1328
2012-01-10 11:48:05 -08:00
Kunal Bhalla
62e527482b [arc svn-hook-pre-commit] Access working copy
Summary:
Creates a new hook API that can be used to interface with
SVN/Git/Mercurial in the context of a commit hook. Currently only adds a
function to read the modified file data in a Subversion commit hook.

An object of this API is created in the SvnHookPreCommitWorkflow and
passed on the Lint Engine which then uses it to access current file
data, of the way the APIs seem to be structured); linters use the
getData function which is essentially a wrapper around the engine's
call, with another layer of caching.

Task ID: #770556

Blame Rev:

Test Plan:
- Create a local svn repository and add a minimal hook to run the local
  version of arc to test commits

(http://phabricator.com/docs/arcanist/article/Installing_Arcanist_SVN_Hooks.html)
- Create a temporary repository that can trigger any of the linters
  available, and test against a temporary linter by committing against
  the test repository: the linter should be able to access all required
  files by using loadData/getData in the LintEngine and Linter.

Revert Plan:

Tags: lint, svn-hook-pre-commit

Reviewers: jungejason, asukhachev, epriestley, aran

Reviewed By: epriestley

CC: aran, jungejason, epriestley, kunalb, asukhachev

Differential Revision: https://secure.phabricator.com/D1256
2011-12-29 14:28:50 -08:00
Marek Sapota
53c64f64b0 Add events framework to Arcanist
Test Plan:
Added a test event listener, added an event dispatch in diff workflow, run the
workflow, the listener was called.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, jungejason, epriestley

Differential Revision: 1102
2011-11-16 16:38:22 -08:00
Emil Hesslow
5be54ec283 Use linear distance instead of square distance in levenshtein
Summary:
We used square distance that optimized for the wrong thing. Making the same
markers spread out instead of being together

Also added a very little cost for switching type. That will make diff types
stick together a bit more

Task ID: #623

Blame Rev:

Test Plan:
Ran my new unit test. And tested a few diff in phabricator

Revert Plan:

Tags:

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1112
2011-11-15 13:48:26 -08:00
epriestley
8c70ee3877 Fix ArcanistBundle to generate disjoint, minimal hunks
Summary: 'patch' chokes on hunks with too much trailing stuff. 'git apply'
chokes on overlapping hunks. Make them both happy. Write some test cases so I
stop breaking this stuff.

Test Plan:
  - Applied a previously-failing patch via SVN.
  - Applied a previously-failing-then-succeeding patch via Git.
  - Ran unit tests.

Reviewers: jungejason, btrahan, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1099
2011-11-09 18:06:14 -08:00
epriestley
cbbd798e48 Split mercurial parsing and API
Summary: Move code to actually parse "hg" output into a separate class with some
tests, so I can reuse it in the import scripts. We should probably do this for
Git/SVN at some point, too.

Test Plan: Ran unit tests, used this class in Phabricator importers, grepped for
calls to removed private methods.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 942
2011-09-16 11:07:48 -07:00
epriestley
44959afd4b Add an "arc merge" workflow
Summary:
This should support conservative rewrite policies in git fairly well, under an
assumed workflow of:

  - Develop in local branches, never rewrite history.
  - Commit with "-m" or by typing a brief, non-template commit message
describing the checkpoint.
  - Provide rich information in the web console (reviewers, etc.)
  - Finalize with "git checkout master && arc merge branch && git push" or some
flavor thereof.

This supports Mercurial somewhat. The major problem is that "hg merge" fails if
the local is a fastforward of the remote, at which point there's nowhere we can
throw the commit message. Oh well. Just push it and we'll do our best to link
them up based on local commit info.

I am increasingly forming an opinion that Mercurial is "saftey-scissors git".
But also maybe I have no clue what I'm doing. I just don't understand why anyone
would think it's a good idea to have a trunk consisting of ~50% known-broken
revisions, random checkpoint parts, whitespace changes, typo fixes, etc. If you
use git with branching you can avoid this by making a trunk out of merges or
with rebase/amend, but there seems to be no way to have "one commit = one idea"
in any real sense in Mercurial.

Test Plan: Execute "arc merge" in git and mercurial.

Reviewers: fratrik, Makinde, aran, jungejason, tuomaspelkonen

Reviewed By: Makinde

CC: aran, epriestley, Makinde

Differential Revision: 860
2011-09-15 07:42:45 -07:00
epriestley
0c11b5c70c Allow XHPAST lint name rules to be overridden through configuration
Summary:
See T326. Allow lint rules to be selectively overridden, e.g. for Conduit
methods.

Since FB has a long history of suggesting crazy patches for this stuff I think
we're safer just adding a hook class than trying to do some kind of regexp
magic.

Test Plan: Wrote a hook for Phabricator and linted some Conduit files without
issues. Ran unit tests.

Reviewers: nh, jungejason, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, nh

Differential Revision: 874
2011-08-31 11:54:24 -07:00
epriestley
268de6428c Basic Mercurial support for Arcanist
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.

This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.

Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
2011-08-09 18:22:58 -07:00
epriestley
0e4f7774fb Add an "arc paste" workflow
Summary: Read and write in the same workflow! Dogs and cats living together!
Test Plan:   - Performed a bunch of paste reads and writes and they looked ok?
Reviewed By: aran
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 748
2011-07-29 20:11:41 -07:00
epriestley
bd9769ba92 Add 'arc upload' and 'arc download'
Summary: Mechanisms for interacting with Files via Arcanist.
Test Plan:
  - Ran 'arc upload x', 'arc upload x y z'
  - Ran 'arc download' with --as and --show.

Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock, epriestley
Differential Revision: 742
2011-07-29 13:38:24 -07:00
epriestley
8234dd8088 Provide better documentation for lint engines, linters, and the PyLint linter
Summary:
Document the relationship between lint engines and linters. Provide an example
linter. Improve the documentation of PyLintLinter, which has a bunch of
configuration stuff which you had to dig into the code to get.

Test Plan:
Ran "arc lint --engine ExampleLintEngine --lintall derp.py" on a file with a
Python syntax error in it. Read documentation.

Reviewed By: j3kuntz
Reviewers: j3kuntz, andrewjcg
CC: aran, j3kuntz
Differential Revision: 557
2011-06-29 21:53:48 -07:00
slawekbiel
5171ec161a arc branch
Summary:
Appending differential status, sorting, filtering and coloring git
branches.

I think it turned out rather nicely. On my repository with 70 branches
it takes 1.6s, not terrible, though 1.2s is in the conduit call - seems
like there is potential for optimization.

I didn't end up changing 'arc list', as their semmantics are slightly
different, but I'm open to ideas of consolidating them

Test Plan:
- Tested on both facebook www and arcanist repositories.
- Validated that view-all flag works
- Validated that the ordering is correct
- Validated that the statuses match the differential status.

Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley, slawekbiel
Revert Plan:
sure

Other Notes:

Differential Revision: 497
2011-06-23 12:12:48 -07:00
epriestley
d73179ea99 "arc install-certificate", client side changes
Summary:
Provide an "install-certificate" workflow to simplify ~/.arcrc edits. See also
D460.

Test Plan:
Installed certificates via "arc install-certificate".

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 465
2011-06-20 05:48:40 -07:00
Andrew Gallagher
d762311a9d arc lint: add support for PyLint
Summary:
Provides a lint class as a wrapper around the external project PyLint.
This exposes some arc config variables to control the behavior:

lint.pylint.prefix - non-standard installation location of pylint
lint.pylint.logilab_astng.prefix - non-standard installation location
  of logilab-astng, a dependency of pylint
lint.pylint.logilab_common.prefix - non-standard installation location
  of logilab-common, a dependency of pylint
lint.pylint.codes.{error,warning,advice} - regexes matching against
  PyLint message codes which should trigger arc errors/warnings/advice
lint.pylint.options - options to pass PyLint

Test Plan:
used to lint python code

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley
Differential Revision: 343
2011-05-25 14:14:51 -07:00
Andrew Gallagher
f8c3a5d555 arc lint: add support for PyFlakes
Summary:
Provide a simple linter wrapper around pyflakes.  This relies on finding
pyflakes via:
- lint.pyflakes.path - arcconfig setting of absolute path to pyflakes
- lint.pyflakes.prefix - arcconfig setting of the prefix that pyflakes
  was installed under
- users path

Test Plan:
linted python code with PyFlakes warnings

Reviewed By: epriestley
Reviewers: jungejason, epriestley
Commenters: jungejason
CC: aran, epriestley, andrewjcg, jungejason
Differential Revision: 310
2011-05-19 16:48:05 -07:00
epriestley
3a559ddd13 'arc liberate', convenience wrapper for various libphutil operations
Summary:
The story for creating and maintaining libphutil libraries and modules
is pretty terrible right now: you need to know a bunch of secret scripts and
dark magic. Provide 'arc liberate' which endeavors to always do the right thing
and put a library in the correct state.

Test Plan:
Ran liberate on libphutil, arcanist, phabricator; created new
libphutil libraries, added classes to them, liberated everything, introduced
errors etc and liberated that stuff, nothing was obviously broken in a terrible
way..?

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 269
2011-05-17 09:53:19 -07:00
epriestley
1fdead8c20 Provide 'arc call-conduit' to facilitate script integration with Phabricator
Summary:
Provide a formal mechanism for making conduit calls from other scripts.

Test Plan:
Called 'conduit.ping'.

Reviewed By: jungejason
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 239
2011-05-06 11:56:21 -07:00
jungejason
64ecf63680 init invest 2011-05-03 23:33:21 -07:00
epriestley
628de7d7a1 Make all the working-copy-paths errors extremely explicit. 2011-03-12 18:23:52 -08:00
adonohue
cc8c7d4997 Respect @nolint
Summary:
Respect @nolint annotations

Test Plan:
New "test case"

Differential Revision: 41
Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
2011-02-26 20:54:06 -08:00
epriestley
5099b005cf Some documentation.
Summary:

Test Plan:

Reviewers:

CC:
2011-02-19 11:36:08 -08:00
epriestley
ba6091f945 Add @nocommit and tests to Text linter.
Summary:

Test Plan:

Reviewers:

CC:
2011-02-16 17:26:51 -08:00
epriestley
b3b2da4608 Fix greediness in Apache License linter.
Summary: The multi-line comment regexp was potentially too greedy. See
"greedy.lint-test".
	- Made it less greedy.
	- Added test coverage.
	- Fixed an issue with the Apache license getting applied with too much
	  whitespace against C files.

Test Plan: Ran unit tests.

Reviewers: aran

CC:

Differential Revision: 36
2011-02-13 16:03:05 -08:00
adonohue
f22d74f88d Refactor Apache license linter to parameterize the license itself.
Summary:
Refactor Apache license linter to parameterize the license itself.

Test Plan:
meta

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Differential Revision: 33
Reviewed By: epriestley
Reviewers: epriestley
2011-02-11 16:36:09 -08:00
epriestley
7f4e6692cd Add ArcanistDiffUtils.
Summary:

Test Plan:

Reviewers:

CC:
2011-01-24 16:48:20 -08:00
epriestley
4818892841 Basic 'shell-complete' workflow.
Summary:
Adds data-driven shell completion help to arcanist.

Test Plan:
ran various commands in git and svn working copies,
output seemed reasonable

Differential Revision: 201754
Reviewed By: adonohue
Reviewers: mroch, adonohue
Commenters: crackerjack
CC: epriestley, adonohue, achao
Revert Plan:
OK
2011-01-17 20:18:27 -08:00
epriestley
c8b303bc90 Add SVN pre-commit hook support.
Summary: Basic support for an SVN pre-commit hook. Still very skeletal.

Test Plan:

Reviewers:

CC:
2011-01-12 03:02:05 -08:00
epriestley
7d36705215 Ship unresolved unit test information to Differential.
Summary:

Test Plan:

Reviewers:

CC:
2011-01-11 22:37:06 -08:00
epriestley
12f1ba1d77 Remove XHPAST from arcanist.
Summary: see corresponding commit in libphutil

Test Plan: linted and analyzed both libraries, got more sensible build behavior

Reviewers:

CC:
2011-01-09 22:13:30 -08:00
epriestley
2e73916fa2 Initial commit. 2011-01-09 15:22:25 -08:00