1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-05 05:02:43 +01:00
Commit graph

1139 commits

Author SHA1 Message Date
Bob Trahan
67061480f9 Tighten up "arc land"
Summary:
Make sure on failure (restoreBranch()) we call `git submodule update --init --recursive` to handle all those purdy submodules. For the pushing step, wrap the push commands in the try / catch block so everything gets cleaned up nice if there's failure. BONUS - add --recursive to arc patch workflow to so nested submodules work correctly. (Crazy git users)

Fixes T3407, T2945.

Test Plan: I wasn't sure how to simulate a good "push" failure but I think this should work.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T2945, T3407

Differential Revision: https://secure.phabricator.com/D6885
2013-09-05 12:45:59 -07:00
Nick Harper
db3581b8fa Don't error on first run of arc diff
Summary:
When running arc diff in a repository that supports commit ranges, it is
possible that the setting for the default relative commit hasn't been set.
If this is the case, the user will be prompted. This change makes sure that
the prompt happens (and thus the setting is set) before we run the
background lint and unit runs.

Test Plan:
```
rm .git/arc/default-relative-commit
arc diff
```

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T2351

Differential Revision: https://secure.phabricator.com/D6854
2013-08-31 14:57:10 -07:00
Wez Furlong
4a3d829223 Fixup lint testing for changes in D6798
Summary:
We have some linters that trigger based on the path name
in the tree (some rules apply in some dirs and not others).
The changes in D6798 caused all the paths to appear to be outside
the tree, so allow for passing a fake through from those test cases
that are sensitive to this.

We also have a test for the copyright linter, and that needs to read
settings from the .arcconfig file.  The change to faking a working
copy meant that this config option was effectively unset, so add a way
to pass the entire arcconfig through from the tests that need it.

Lastly, the logic to skip deleted files needs to be special cased
when we're faking paths like this: if we've added data for a file
in the testable engine, we should also consider that file as existing.

Test Plan:
`arc unit --everything` here, and passing our tests in
our repo over there.

Reviewers: epriestley, mareksapota

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6841
2013-08-29 09:55:30 -07:00
epriestley
2c5c9815c0 Support PHPCS as a .arclint linter
Summary:
Ref T3186. Ref T2039. Ref T3771. A few effects here:

  # Expose PHPCS as a `.arclint` linter.
  # Turn PHPCS into an ArcanistExternalLinter linter.
  # Add test coverage for PHPCS.
  # Add a `severity.rules` option to `.arclint`. Some linters have very explicit builtin severities ("error", "warning") but their meanings are different from how arc interprets these terms. For example, PHPCS raises "wrong indentation level" as an "error". You can already use the "severity" map to adjust individual rules, but if you want to adjust an entire linter it's currently difficult. This rule map makes it easy. There's substantial precedent for this in other linters, notably all the Python linters.

For `severity.rules`, for example, this will turn all PHPCS "errors" into warnings, and all of its warnings into advice:

      "severity.rules" : {
        "(^PHPCS\\.E\\.)" : "warning",
        "(^PHPCS\\.W\\.)" : "advice"
      }

The user can use `severity` (or more rules) to get additional granularity adjustments if they desire.

Test Plan: 5bb919bc3a

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, ajtrichards

Maniphest Tasks: T2039, T3186, T3771

Differential Revision: https://secure.phabricator.com/D6830
2013-08-29 06:47:27 -07:00
Bob Trahan
96a47759ae Make arc patch slightly better about submodules
Summary:
Ref T3776, Ref T479. Say you have some DN, with a submodule X@Y. Later, X@Z in your working copy / repo. If you run arc patch DN, you'd end up with a dirty working copy claiming that X@Z was wrong and it should be X@Y.

To fix, basically run 'submodule init' and 'submodule update'. This makes it so after "arc patch" if you run "git status" it looks clean.

Gross part though now is if you then "git checkout master" you'll have a dirty checkout the other way. I think this is better though.

Test Plan: made a new repository where I added libphutil @ X, did some work (DX), then made libphutil @ y. When I arc patch'd DX, things looked good!

Reviewers: epriestley

Reviewed By: epriestley

CC: csilvers, Korvin, aran

Maniphest Tasks: T479, T3776

Differential Revision: https://secure.phabricator.com/D6837
2013-08-28 16:47:30 -07:00
epriestley
3ad72195bf When converting a file to a binary, populate the binary's data
Summary:
Currently, we prompt the user to mark non-UTF8 files as binary, but don't actually attach the data to the change when they do. This means we don't upload the data, and can't patch it later.

A simple reproduction case is to build a test file (I used one with bytes from 1..255):

  $ # Don't include \0, since Git treats that specially.
  $ ./echo_every_byte_from_1_to_255_inclusive.erl > example.txt

Then add it:

  $ git add example.txt
  $ git commit -a -m derp
  $ arc diff --only HEAD^

You'll be prompted to convert the file to binary:

  Do you want to mark this file as binary and continue? [Y/n] y

Before this patch, that would be followed by:

  Uploading 0 files...

...which is incorrect; we need to upload the new data. After this patch, this shows:

  Uploading 1 files...

...which is also incorrect, but only grammatically. Diffs created after this patch apply back cleanly with `arc patch` and restore the file properly.

Test Plan: Followed instructions above, restoring a textual binary conversion by using `arc patch`.

Reviewers: zeeg, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6815
2013-08-27 09:34:30 -07:00
Eric Stern
a28d4ff3e4 Add support for 'phpunit_binary' config setting
Summary:
If present, this will override the default phpunit path. Allows easy
integration of Composer-provided installs of phpunit (ex. set phpunit_binary to
vendor/bin/phpunit). If the path provided doesn't resolve to an executable it
will assume the path is relative to the project root.

fix line length issue

Test Plan:
Added phpunit_binary to .arcconfig in a simple project using Composer with
phpunit/phpunit package as part of require-dev (installed to
$ROOT/vendor/bin/phpunit). Phpunit not otherwise installed on the system. Set
unit.engine to PhpunitTestEngine. Confirmed that 'arc unit' used the specified
binary, both at project root and from subdirectories.

Reviewers: epriestley

CC: aurelijus, Korvin, aran

Differential Revision: https://secure.phabricator.com/D6791
2013-08-26 09:59:51 -07:00
epriestley
65c19ff0c0 Automatically answer excuse prompts if stdin is not a TTY
Summary: Fixes T3696. Currently, we abort. If stdin is not a TTY, we should just continue. A script which cares could conceivably run `arc lint` and `arc unit` separately, but it seems unlikely that any script would ever want to fail here.

Test Plan: Ran `echo -n '' | arc diff --create --verbatim` with a lint error and got a revision (D6720).

Reviewers: Firehed, btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3696

Differential Revision: https://secure.phabricator.com/D6721
2013-08-26 05:41:32 -07:00
epriestley
0f30aca626 Ready more linters and linter functions for .arclint
Summary:
Ref T3186. Ref T2039. Continues work on readying linters for `.arclint`.

  - **Ruby**: Make this an ExternalLinter.
  - **Priority**: Currently, linters have an implicit "correct" order (notably, the "NoLint" linter needs to run before other linters). Make this explicit by introducing `getLinterPriority()`.
  - **Binaries**: Currently, linters manually reject binary files. Instead, reject binary files by default (linters can override this if they do want to lint binary files).
  - **Deleted Files**: Currently, linters manually reject deleted files (usually in engines). Instead, reject deleted files by default (linters can override this).
  - **Severity**: Move this `.arclint` config option up to top level.
  - **willLintPaths()**: This method is abstract, but almost all linters provide a trivial implementation. Provide a trivial implementation in the base class.
  - **getLintSeverityMap()/getLintNameMap()**: A bunch of linters have empty implementations; these are redundant. Remove them.
  - **Spelling**: clean up some dead / test-only / unconventional code.
  - **`.arclint`**: Allow the filename, generated, nolint, text, spelling and ruby linters to be configured via `.arclint`.

Test Plan:
458beca3d6

Ran unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: Firehed, aran

Maniphest Tasks: T2039, T3186

Differential Revision: https://secure.phabricator.com/D6805
2013-08-26 05:37:10 -07:00
durham
57bc582ad2 Better heuristic for checking for hgsubversion
Summary:
The existing heuristic checks for the existence of .hg/svn, but it
turns out that this directory can exist in a non-svn hg repo if the user or a
script ever runs a 'hg svn' command.

Now we check for a file inside .hg/svn.  The hgsubversion maintainer said this
particular file will always be present in hgsubversion repos.

Test Plan:
arc land --trace
Verified it used 'hg push' and not 'hg push -r ...'. This indicates it
considered the repo to be an hgsubversion repo.

Reviewers: epriestley

Reviewed By: epriestley

CC: dschleimer, sid0, Korvin, aran

Differential Revision: https://secure.phabricator.com/D6807
2013-08-23 16:40:15 -07:00
epriestley
1f3cb63db2 Expose PEP8, Flake8 and CSSLint engines to .arclint
Summary:
Ref T3186. Ref T2039. Allow these linters to be selected with `.arclint`.

Also allow severities to be set.

Also fix some other minor bugs.

Test Plan:
https://github.com/epriestley/arclint-examples
https://github.com/epriestley/arclint-examples/blob/master/.arclint

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T2039, T3186

Differential Revision: https://secure.phabricator.com/D6803
2013-08-23 11:58:07 -07:00
epriestley
6e5be59ad6 Port flake8 to ArcanistExternalLinter
Summary:
Ref T3186. Brings another linter onboard. This one uses the stdin stuff.

The unit test was ostensibly broken so I fixed it, but that might just be some kind of version issue.

Test Plan: Unit tests.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3186

Differential Revision: https://secure.phabricator.com/D6802
2013-08-23 11:52:54 -07:00
epriestley
e23fc30c19 Introduce a rough abstract base class for "linters which run programs and read the results"
Summary:
Ref T3186. We have about 50 linters which run programs and read the results, all of which have ad-hoc one-off custom config that isn't formalized anywhere.

Consolidate all this stuff into `ArcanistExternalLinter`, which is configurable through `.arclint` (although nothing supports this quite yet).

Extend CSSLint and Pep8Lint from `ArcanistExternalLinter`.

Add unit tests for both.

There are still some rough edges here, but it mostly seems to work pretty well.

Test Plan: Ran unit tests, hit some (most?) of the error cases.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran, Firehed

Maniphest Tasks: T3186

Differential Revision: https://secure.phabricator.com/D6800
2013-08-23 11:52:44 -07:00
Chad Little
c882877f50 Rebuild map
Summary: rebuild arc map

Test Plan: run phabricator

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3748

Differential Revision: https://secure.phabricator.com/D6804
2013-08-23 05:29:40 -07:00
epriestley
f18130a6aa Simplify and demuck some of the linter test cases
Summary:
Ref T3186.

  - Every linter builds a WorkingCopyIdentity in the same way, with no specialized data. Don't do that.
  - Linters get passed a goofy hardcoded ".php" path. Don't do that.
  - Linters generally run on an imaginary path, which might not work. Just give them a real path by building a tiny working copy in `/tmp`.
  - Fix a TODO now that we have better typechecking.

Test Plan: `arc unit --everything`, intentionally broke a test to make sure that still works.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3186

Differential Revision: https://secure.phabricator.com/D6798
2013-08-22 16:02:41 -07:00
epriestley
97ad54ed00 Lay groundwork for configuration-driven linters
Summary:
Ref T2039. That task has a bunch of discussion, but basically we do a poor job of serving the midrange of lint configuration right now.

If you have something simple, the default linters work.

If you have something complex, building your own engine lets you do whatever you want.

But many users want something in between, which isn't really well accommodated. The idea is to let you write a `.arclint` file, which looks something like this:

  {
   "linters" : {
      "css" : {
        "type" : "csslint",
        "include" : "(\.css$)",
        "exclude" : "(^externals/)",
        "bin" : "/usr/local/bin/csslint"
      },
      "js" : {
        "type" : "jshint",
        "include" : "(\.js$)",
        "exclude" : "(^externals/)",
        "bin" : "support/bin/jshint",
        "interpreter" : "/usr/local/bin/node"
      }
   }
  }

...which will provide a bunch of common options around lint severity, interpreter and binary locaitons, included and excluded files, etc.

This implements some basics, and very rough support in the Filename linter.

Test Plan:
Generated a `.arclint` file and saw it apply filename lint correctly. Used `debug` mode and tried invalid regexps.

  {
    "debug" : true,
    "linters" : {
      "filename" : {
        "type" : "filename",
        "exclude" : ["@^externals/@"]
      }
    }
  }

Next steps include:

  - Provide an external linter archetype (T3186) and expose a common set of configuration here ("bin", "interpreter", "flags", "severity").
  - Provide a `.arcunit` file which works similarly (it can probably be simpler).

Reviewers: btrahan, Firehed

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2039

Differential Revision: https://secure.phabricator.com/D6797
2013-08-22 16:02:16 -07:00
Firehed
5eb82c8e7d fix issue where multi-line dataproviders in phpunit were parsed incorrectly when displaying results
Summary: Found an issue where if arc unit runs phpunit and the datasets included a dataprovider which spanned multiple lines, it wasn't filtered out correctly in the result parser, this fixes it

Test Plan: accidentally ran tests against phpunit itself which exhibits this problem. no longer a problem after this fix, nothing else breaks.

Reviewers: epriestley

Reviewed By: epriestley

CC: aurelijus, epriestley, aran

Differential Revision: https://secure.phabricator.com/D6773
2013-08-20 06:58:14 -07:00
durham
8465c4dd53 Fix arc land for mercurial on windows
Summary:
arc for mercurial on windows was broken in several way.
Executing a command via passthru failed because passthru on windows
skips the shell so 'set HGPLAIN=1 & ...' was an invalid command. The
fix was to just not set HGPLAIN for passthru commands on windows.

Also removed hardcoded '' quotes in mercurial commands since windows
doesn't support single quots.

Test Plan: arc land --hold on a windows machine

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6763
2013-08-14 18:00:51 -07:00
Firehed
acb90fd9e7 fixes local svn branch name detection
Summary: Corrects relative vs absolute branch name when using 'arc commit'

Test Plan: 'arc commit' on a release branch gave an error before making the change (change was generated from 'branches/yyy' but working copy root is 'https://xxx/branches/yyy', now it does not.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6743
2013-08-13 14:16:14 -07:00
Eric Stern
c11b9e1af9 Fix 'arc lint --everything' when svn has uncommitted changes
Summary: My recent change adding --everything to arc lint could sometimes cause a "diff is empty" error, this patch fixes it.

Test Plan: Ran "arc lint --everything" before and after patch. No longer errors out. Only appeared to originally happen when there were uncommited changes in an svn repo.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6732
2013-08-12 13:47:43 -07:00
Mehdi Mulani
857b754c7e Use underscores instead of hyphens as bookmark suffixes.
Summary:
When trying to find an unused bookmark we append hyphened suffixes to the end of the bookmark name. In Mercurial, these are treated the same as the bookmark name without the suffix, here's the output from hg log -r:

```
[mehdi] HGPLAIN=1 hg log -r "arcpatch-1"
abort: unknown revision 'arcpatch'!
[mehdi] HGPLAIN=1 hg log -r "arcpatch_1"
abort: unknown revision 'arcpatch_1'!
```

Test Plan: None.

Reviewers: epriestley, dschleimer

Reviewed By: dschleimer

CC: aran, Korvin, DurhamGoode, dschleimer

Differential Revision: https://secure.phabricator.com/D6698
2013-08-07 14:56:03 -07:00
Eric Stern
dccc8571d7 Improve arc commit with SVN branches
Summary:
If you are trying to commit someone else's diff, arc commit gives warnings about path mismatch. This changes the path comparison to be based on the repo url rather than the local working directory. E.g. if both the author and committer are working in branches/release/2013_08_07 despite being checked out in ~/dev/2013_08_07 (system user being different, of course) it no longer warns that the WC path is different

Original behavior:

	eric@Eric-MBP ~/dev/2013_07_31: arc commit --revision 21
		You are not the author of 'D21: WeMerge Automatic Request'. Commit this
		revision anyway? [y/N] y

		Revision 'D21: WeMerge Automatic Request' was generated from '', but
		current working copy root is '/Users/eric/dev/2013_07_31/'. Commit this
		revision anyway? [y/N] y

	Committing 'D21: WeMerge Automatic Request'...
	Adding         test
	Transmitting file data .
	Committed revision 52676.
	Closing revision D21 'WeMerge Automatic Request'...
	Exception
	ERR-CONDUIT-CORE: You can not mark a revision you don't own as closed.
	(Run with --trace for a full exception trace.)

New behavior:

	eric@Eric-MBP ~/dev/2013_07_31: arc commit --revision 24

		You are not the author of 'D24: WeMerge Automatic Request'. Commit this
		revision anyway? [y/N] y

	Committing 'D24: WeMerge Automatic Request'...
	Adding         test
	Transmitting file data .
	Committed revision 52679.
	Closing revision D24 'WeMerge Automatic Request'...
	Exception
	ERR-CONDUIT-CORE: You can not mark a revision you don't own as closed.
	(Run with --trace for a full exception trace.)

Test Plan: 'arc diff' changes with one user. 'arc patch Dxx' on a different working copy by a different user to review and test changes. accept review. 'arc commit --revision xx' as reviewer to land the patch. complaint goes away.

Reviewers: epriestley, ghostwriter78

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D6665
2013-08-04 09:01:22 -07:00
Eric Stern
5e3b436099 Pass global CA bundle to HTTPSFuture, T3668
Summary: Update and clarify precedence of CA bundles

Test Plan:
tested with "arc call-conduit user.whoami" from project root
Same from other subdirectory in project
Repeated tests with both https.cabundle and https.cacert settings, both in
.arcconfig and ~/.arcrc

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3668

Differential Revision: https://secure.phabricator.com/D6647
2013-08-04 08:21:35 -07:00
Lajos Veres
a680c55670 Add CSSLint linter to Arcanist
See: https://github.com/facebook/arcanist/pull/93

Reviewed by: epriestley
2013-08-02 05:14:00 -07:00
Aviv Eyal
d54f0f69d4 arc unit --output none
Summary:
Support for no output from arc unit (For scripts, etc).
Also include --output param, analogous to arc lint --output.

Test Plan: run all 6 variants + `--output bad-value`

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6642
2013-08-01 12:07:21 -07:00
durham
5b69749812 Remove hard coded single quotes in arc feature
Summary:
Single quotes aren't valid in the windows cmd prompt, so arc feature
didn't work in mercurial when it got to this line.

I have no idea why %C was used before.  Nothing in that string should be
broken by the escaping.

Test Plan:
Ran arc feature --trace on my mac. Verified the command was escaped correctly
and the correct feature results were printed.

I don't have a windows machine to try it on, but the builtin escaping should
now account for windows machines.

Reviewers: epriestley

Reviewed By: epriestley

CC: sid0, aran, Korvin

Differential Revision: https://secure.phabricator.com/D6637
2013-08-01 10:14:38 -07:00
Jakub Vrana
19181fb3e8 Remove warning about deprecated phutil_render_tag()
Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6567
2013-07-28 11:00:00 -07:00
Eric Stern
12f2175da1 Add --everything support to 'arc lint'
Summary:
When adding arcanist support to a new project or adding a new linter,
it's helpful to be able to run new linters against the entire codebase. This
patch adds support for this with an '--everything' option, similar to 'arc unit
--everything'

Test Plan:
Run 'arc lint --everything' and check out the code. Optionally dump
the paths to test in the current lint engine's buildLinters() function to
demonstrate that it's receiving all files in the project rather than just the
changed and/or specified ones

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D6592
2013-07-27 18:29:20 -07:00
Eric Stern
b56634ad27 Pass correct filename into PHPUnitTestEngine coverage
Summary: The test result parser in PhpunitTestEngine was receiving $test_path from the previous loop instead of $path from the current one. The variable isn't actually used in the PhpunitResultParser object (it exists for strict compatibility with the parent class) so it didn't cause any problems, but who knows if that could change in the future

Test Plan: Review diff. No changes to the output of running 'arc unit' when using the Phpunit engine, as expected

Reviewers: epriestley

CC: aran, epriestley, aurelijus, chad

Differential Revision: https://secure.phabricator.com/D6587
2013-07-26 19:10:17 -07:00
epriestley
490984936b Revert "Merge pull request #93 from vlajos/csslint"
This reverts commit 2852a965e3, reversing
changes made to 46bb3dbc36.

See: https://github.com/facebook/arcanist/pull/93
2013-07-22 06:52:08 -07:00
Jeff Ferland
2852a965e3 Merge pull request #93 from vlajos/csslint
CSS lint support for arcanist using csslint.
2013-07-22 06:44:22 -07:00
Aviv Eyal
46bb3dbc36 arc browse: Support line number and reduce conduit call
Summary:
support common "filename:line number" format.
Also, remove redundent conduit call.

Test Plan: arc browse with and without :33 suffix.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6451
2013-07-14 19:17:02 -07:00
Jakub Vrana
ad9cb418c4 Display line number with assertion for failed test cases
Summary: We use custom `assert*` functions here and there. Remove them from backtrace.

Test Plan: Ran `XHPASTTreeTestCase`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6411
2013-07-10 08:42:49 -07:00
Jakub Vrana
f5ceceea87 Lint undeclared variables in strings
Summary: Depends on D6405.

Test Plan:
New unit test.
Linted libphutil, Arcanist, Phabricator.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6406
2013-07-10 08:26:11 -07:00
Lajos Veres
2e30c97ade comment typo fix 2013-07-09 14:11:21 +01:00
Lajos Veres
2b07f22e08 update __phutil_library_map__ 2013-07-09 12:48:07 +01:00
Lajos Veres
395c15e630 add css lint 2013-07-09 12:46:59 +01:00
epriestley
fbcd686e18 Update spelling replacement rule for 'algorithmical'
Summary:
I suspect that when people use 'algorithmical', they mean it as an adjective and not an adverb.

'algorithmic' = adjective
'algorithmically' = adverb

Test Plan: Add the word 'algorithmical' to a file. Run `arc lint` on the file. See suggestion to correct it to 'algorithmic'.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6373
2013-07-08 09:46:24 -07:00
epriestley
910df64d31 Stop using deprecated Conduit methods in arc
Summary:
  - Replace `maniphest.find` with `maniphest.query`. These calls are nearly identical, it was just a rename for consistency.
  - Replace `differential.find` with `differential.query`.

Test Plan:
  - Ran `arc tasks`.
  - Ran `arc close-revision` on valid, nonexistent, and existent-but-invalid revisions.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6336
2013-07-02 05:47:52 -07:00
Miorel-Lucian Palii
7130004015 Don't instantiate abstract classes in ArcanistConfiguration::buildAllWorkflows
Summary:
I wanted to write a couple of workflows that shared some code in an abstract superclass, but ##arc## would fail trying to instantiate the abstract class.

As it turns out, we can modernize the ##buildAllWorkflows## function a bit, and it will only load concrete objects.

Test Plan:
1. Define an abstract subclass of ##ArcanistBaseWorkflow##.
2. ##arc liberate## the source directory that contains it.
3. Try any ##arc## operation without this diff, and see it fail.
4. Patch this diff and see that ##arc## operations work now.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6324
2013-06-28 18:24:30 -07:00
Gareth Evans
a746ad8757 Update some output based on flag being set
Summary: Makes sense now

Test Plan: Get someone else to test it

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T3342

Differential Revision: https://secure.phabricator.com/D6307
2013-06-25 15:26:14 -07:00
Gareth Evans
4f3dee80e5 Allow passing library name with flag
Summary: Enables the process to pass the user input for library name. Fixes T3342

Test Plan: Little help... `arc liberate` doesn't run on windows. Though I saw the flag :)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3342

Differential Revision: https://secure.phabricator.com/D6306
2013-06-25 15:20:30 -07:00
Gareth Evans
1d5ee03fdb Add Delete Flag to Paths ignore mask
Summary:
When `arc diff` runs unit tests it uses all of the affected files as the base array of paths. These files may have been deleted. If the deleted file fits the test criteria `unit` will try and run the test and in some cases fail.

An example of this fataling is with PHPUnit;

> Fatal error: Uncaught exception 'Exception' with message 'JSON report file is em
pty, it probably means that phpunit failed to run tests. Try running arc unit wi
th --trace option and then run generated phpunit command yourself, you might get
 the answer.' in C:\Websites\facebook\arcanist\src\unit\engine\PhpunitResultPars
er.php on line 156

Test Plan: Re-ran the tests that were causing issues with `arc unit --rev HEAD^ --trace`

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, aurelijus

Differential Revision: https://secure.phabricator.com/D6246
2013-06-20 07:35:30 -07:00
durham
b713eb51c9 Don't prepopulate update message in hg amend diff workflow
Summary:
Previously, updating a commit via arc diff in mercurial would
prepopulate the update message with part of the commit message.  In an
amend workflow this doesn't make sense, so I disabled it. Git already
does this, so now mercurial matches git in this scenario.

We had users complain that new users would often submit diffs with the
default update message, and it wasn't useful since they were using a
amend flow.

Test Plan:
arc diff on a commit that already had a diff
Verified the editor did not have a update message
arc diff on a stack of commits where the bottom one had a diff
Verified the editor provided a default update message

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6215
2013-06-17 11:40:58 -07:00
epriestley
c38bc4376c When a file upload fails during "arc diff", show why
Summary: We currently swallow the exception message, but this isn't useful. Fixes T3354.

Test Plan:
  $ arc diff HEAD^ --conduit-uri=http://local.aphront.com:8080/ --only
  Uploading 1 files...
  Failed to upload new binary 'large.png'.

  [HTTP/500] Internal Server Error
  As received by the server, this request had a nonzero content length but no POST data.

  Normally, this indicates that it exceeds the 'post_max_size' setting in the PHP configuration on the server. Increase the 'post_max_size' setting or reduce the size of the request.

  Request size according to 'Content-Length' was '2093052', 'post_max_size' is set to '100K'.

      Continue? [Y/n]

Reviewers: jamesr, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3354

Differential Revision: https://secure.phabricator.com/D6186
2013-06-12 05:50:08 -07:00
epriestley
2e61b1a3df Fix a catch class name
Summary: Now that we examine these in lint, this came up. There is no `ConduitException`; the class is `ConduitClientException`.

Test Plan: Lint.

Reviewers: vrana, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6158
2013-06-11 06:26:42 -07:00
Omer Strulovich
9a46ba7ea1 Support for lint messages which are always shown
Summary: If one wishes to implement a linter which finds unused resources or variables the current scheme does not work as the lint engine filters all changes from lines which were not introduced in a diff. To solve that, I've added an "always show" configuration for a lint message allowing creation of such linters.

Test Plan:
1. Created a custom linter for finding unused Android resources in a project
2. Ran arc lint with linter added and received warnings as expected

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6119
2013-06-09 08:38:30 -07:00
Jakub Vrana
3de4984256 Find needed classes in catch, instanceof and newv()
Test Plan:
  try {} catch (X $ex) {}
  $a instanceof X;
  newv('X');

Linted Phabricator.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6143
2013-06-06 07:25:08 -07:00
Jakub Vrana
f3e32d8366 Find classes used in call_user_func()
Summary:
Also fix false positive for call_user_func('C::f').
Also fix false positive for call_user_func("$f").
Also centralize handling of magic classes.

Test Plan:
  call_user_func("$f");
  call_user_func('C::f');
  call_user_func(array('C', 'f'));
  call_user_func(array(1 => 'f', 0 => 'C'));

Linted Phabricator.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6137
2013-06-05 19:19:06 -07:00
Jakub Vrana
5336f4bcf0 Verify classes used in typehints
Summary: Also support `SomeInterface::CONSTANT`.

Test Plan:
  interface I {
    const A = 1;
  }
  I::A;

  function f(stdClass $a, array $b, Iterator $c) {
  }

Linted Phabricator.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6135
2013-06-05 13:53:43 -07:00