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

72 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