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

1690 commits

Author SHA1 Message Date
epriestley
a36dc81715 Provide more documentation for Arcanist upload stuff
Summary: Ref T8259. Make some highly-ambiguous methods like `setData()` more clear.

Test Plan: reading

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13038
2015-05-27 10:26:27 -07:00
epriestley
5c7b22e620 Use file.allocate to upload large files from arc diff
Summary: Fixes T8259. Depends on D13016. Use modern logic to support large file uploads.

Test Plan:
  - Diffed with some images, saw them show up in the diff.
  - Diffed with a 12MB binary, saw it upload in chunks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13017
2015-05-27 10:26:12 -07:00
epriestley
877e7b6388 Move Conduit file upload logic into a separate class
Summary:
Ref T8259. Currently, `arc upload` uses new logic but `arc diff` uses older logic internally. This prevents `arc diff` from uploading files larger than 4MB to newer servers.

Split the upload logic apart so the two upload pathways can share it. Callers now build a list of FileDataRefs and hand them to an Uploader for upload.

Test Plan:
Ran `arc upload` on:

  - One file.
  - Several files.
  - Small files.
  - Big files.
  - Directories.
  - Unreadable files.
  - Files full of random data.
  - The same file over and over again.
  - The same big file over and over again.
  - Artificially broke `file.allocate` and redid some of the simple cases (large/chunked aren't expected to work in this case).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13016
2015-05-27 10:25:53 -07:00
Nevogd
407af00ef6 Fix invalid parameter in phutil_console_wrap()
Summary:
Fixes T8314. Change the phutil_console_wrap() call to only have a single
string parameter.

Test Plan:
Ran `arc diff` added text into the title, summary and test plan fields,
but did not specify any reviewers. When prompted to continue, clicked 'No' and
saw the '(Message saved to commit message.)' string.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T8314

Differential Revision: https://secure.phabricator.com/D13015
2015-05-26 06:07:25 -07:00
Joshua Spence
64d03ff68b Remove arcanist projects from working copy code
Summary: Ref T7604. Remove "arcanist projects" from `ArcanistWorkingCopy` and a few other callsites. Depends on D12999.

Test Plan: Can't really think of how to test this.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12945
2015-05-26 07:08:56 +10:00
Joshua Spence
59ef783f4a Remove call to "arcanist.projectinfo" from ArcanistWorkflow
Summary: Ref T7604. Remove call to the `arcanist.projectinfo` #conduit endpoint from `ArcanistWorkflow`. Depends on D12992.

Test Plan: Ran `arc which` and verified that repository information was present.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12971
2015-05-25 22:04:50 +10:00
Joshua Spence
410331db75 Fix some format strings
Summary: These `pht` calls use `%d` instead of `%s`.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12994
2015-05-25 21:24:40 +10:00
Joshua Spence
1f2b51c4a3 Fix sanity checks for patch workflow
Summary:
Ref T7604. Ref T8307. This was broken in D12962 because I only tested `arc patch --arcbundle`. Furthermore, this particular sanity check doesn't actually do anything now (see T8307).

Test Plan:
Ran `arc patch --nobranch D12971` successfully.

Auditors: epriestley
2015-05-25 18:41:28 +10:00
Joshua Spence
be9dd352be Remove call to "arcanist.projectinfo" from arc export
Summary:
Ref T7604. Remove a call to `arcanist.projectinfo` from `arc export`. This Conduit call was only used to detect the repository encoding, which we should be able to do locally anyway.

I was considering removing the `$projectName` from `ArcanistBundle` as well, but I'm not convinved that this is a good idea. Specifically, doing so would make it difficult to issue the "This patch is for the 'X' project, but the working copy belongs to the 'Y' project" error. We could potentially use the repository callsign for this purposes, but this isn't portable across installs.

Test Plan: I'm not quite sure how to test this. I suspect that this functionality isn't widely used anyway.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12962
2015-05-25 18:31:09 +10:00
Joshua Spence
9b7c6786cd Change visibility of some linter methods
Summary: Change the visibility of the `raiseLintAtSomething` methods, mainly to allow linter rules to raise linter messages (see D10541).

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12976
2015-05-24 07:45:01 +10:00
Joshua Spence
6f86866104 phtize a bunch more strings
Summary: I found a few strings that I had missed, using a mostly-broken-but-somewhat-okay custom linter ruler (https://secure.phabricator.com/differential/diff/30988/).

Test Plan: Intense eyeballing.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12888
2015-05-22 17:09:56 +10:00
Joshua Spence
9edcaadb2a Remove an unused variable
Summary: Ref T7604. Remove an unused variable from `ArcanistDiffWorkflow`.

Test Plan: This diff, I guess.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12972
2015-05-22 17:09:18 +10:00
Joshua Spence
8244b3582a Improve behavior for detecting the failure to parse external linter output
Summary:
Currently, a lot of `ArcanistExternalLinter` subclasses have something like this:

```lang=php
if ($err && !$messages) {
  return false;
}
```

We can avoid this code duplication by moving this check to the `ArcanistExternalLinter` class.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11321
2015-05-22 07:30:17 +10:00
Joshua Spence
26c4f12a21 Allow getXHPASTTreeForPath to return the tree for an arbitrary path
Summary: This allows `ArcanistBaseXHPASTLinter::getXHPASTTreeForPath` to return the parse tree for a file that wasn't explicitly linted. In my particular use case, I am writing a linter rule to determine if it is necessary to import a PHP file with `require_once` (i.e. the file consists only of class/interface definitions which are autoloadable).

Test Plan: Tested externally using a custom linter.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12271
2015-05-21 15:08:39 +10:00
Joshua Spence
e72a43a992 Remove "commit hook mode" workarounds for ArcanistXHPASTLinter
Summary: Ref T7674. `ArcanistXHPASTLinter` has some workarounds for running in "commit hook" mode (which has since been removed). This linter should always have a working copy.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12956
2015-05-21 15:08:39 +10:00
Joshua Spence
af343e4ca6 Remove deprecated linter configuration
Summary:
Removes the deprecated method of configuring linters (via the `.arcconfig` file). There is one main caveat here:

- There is currently no convenient method by which to change the path for an external linter (T5057). This means that there is no direct replacement for the deprecated `lint.ruby.prefix` configuration. The workaround is to symlink these binaries into `arcanist/externals/bin`.

Test Plan: Wait a sufficient amount of time before landing this.

Reviewers: chad, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aik099, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10005
2015-05-21 07:01:37 +10:00
Joshua Spence
e3311f0832 Remove "arcanist projects" from arcanist
Summary: Ref T7604. Remove "arcanist projects". Depends on D12894.

Test Plan: This diff (so meta).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12901
2015-05-20 11:29:41 +10:00
Joshua Spence
097c894d08 Add linter rule for invalid modifiers
Summary: Add some linter rule to detect invalid modifiers for class methods/properties.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12922
2015-05-20 09:45:13 +10:00
Joshua Spence
753705b2c5 Rename ArcanistPhutilTestCase to PhutilTestCase and Remove ArcanistTestCase
Summary: Ref T7977. The `ArcanistTestCase` class is pointless and can be replaced by `ArcanistPhutilTestCase`. Furthermore, it sorta makes sense to just rename `ArcanistPhutilTestCase` to `PhutilTestCase`. Depends on D12664 and D12666.

Test Plan: `arc unit`

Reviewers: avivey, #blessed_reviewers, epriestley

Reviewed By: avivey, #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12665
2015-05-20 09:40:24 +10:00
Joshua Spence
399f040018 Improve getLink method for unit tests
Summary:
Ref T7977. The `PhutilTestCase::getLink` method currently relies on arcanist projects instead of repositories. Instead, make this logic a bit smarter by looking up the base URI from `phabricator.uri` (currently it is hardcoded to `https://secure.phabricator.com`).

Ideally, we would pass `?repositories=$REPOSITORY_PHID` to `DiffusionSymbolController` as well, but I don't know if this is worth pursuing.

Test Plan: This diff.

Reviewers: avivey, #blessed_reviewers, epriestley

Reviewed By: avivey, #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12664
2015-05-20 09:36:22 +10:00
Joshua Spence
1d72cfc944 Provide a getProjectRoot method for ArcanistLinter
Summary: Adds a `getProjectRoot()` method to the `ArcanistLinter` class, simplifying a bunch of code.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12474
2015-05-20 09:16:36 +10:00
Joshua Spence
9444072e21 Improve the robustness of the "alias function" linter rule
Summary: Improve the `ArcanistXHPASTLinter::LINT_ALIAS_FUNCTION` linter rule. Currently this rule does not correctly handle alias functions which are not strictly lowercase.

Test Plan: Added a test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12921
2015-05-20 08:36:36 +10:00
Joshua Spence
80691e0a66 Add a linter rule for modifier ordering
Summary: Fixes T7417. Adds `ArcanistXHPASTLinter::LINT_MODIFIER_ORDERING` for ensuring that method/property modifiers (`public`, `protected`, `private`, `static`, `abstract` and `final`) are consistently ordered. The modifier ordering that is enforced is consistent with [[http://www.php-fig.org/psr/psr-2 | PSR-2]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7417

Differential Revision: https://secure.phabricator.com/D12421
2015-05-20 07:26:16 +10:00
Joshua Spence
993166fa49 Further improvements to array comma linter rule
Summary:
Improve the `ArcanistXHPASTLinter::LINT_ARRAY_SEPARATOR` rule to be able to autofix the following code:

```lang=php
array(
  1,
  2,
  3, /* comment */ );
```

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12308
2015-05-20 07:05:34 +10:00
Joshua Spence
25855c4427 Don't explicitly use an interpreter for pep8
Summary: Depends on D9430. If we are using the system installation of `pep8`, then it probably unnecessary (and possibly wrong) to specify `python2.6` as the default interpreter.

Test Plan: Ran `arc unit`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: vrusinov, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9474
2015-05-20 07:04:07 +10:00
Joshua Spence
5b2571f5ae Don't bundle PEP8
Summary: Currently, we bundle a specific version of `pep8` with Arcanist. Whilst maybe this made sense historically, as pointed out by @epriestley in D9412#6, there is no longer much point in doing so.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: talshiri, vrusinov, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D9430
2015-05-20 07:03:22 +10:00
Joshua Spence
73feffbe70 Remove the ArcanistConduitLinter
Summary: This linter is a relic from Facebook days. As Harbormaster becomes more useful (T1049) this linter should become obsolete. Instead of modernising this linter to be compatible with modern infrastructure, it is easier to just let it die.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10533
2015-05-20 07:02:43 +10:00
Joshua Spence
9862d7c0cb Move the getFunctionCalls method to ArcanistBaseXHPASTLinter
Summary: Move the `getFunctionCalls` method to the `ArcanistBaseXHPASTLinter` so that it can be used by subclasses.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12472
2015-05-20 07:00:29 +10:00
epriestley
ddfdfce3f5 Fix CPP Lint severity
Summary: Currently all CPPLint issues are hard-coded to warning level, which prevents customising the severity in .arclint. Change to pick up the configured severity. Note that getLintMessageSeverity will call getDefaultMessageSeverity if nothing is configured for that error category.

Test Plan: Tested manually to confirm configured categories display with the correct severity and that non-configured ones return with the default severity (ERROR).

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9682
2015-05-19 07:58:59 -07:00
Joshua Spence
60a5a24033 Add a linter rule for invalid default parameters
Summary: See http://www.phpwtf.org/default-arguments-and-type-hinting.

Test Plan: Added test case.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12855
2015-05-19 06:59:50 +10:00
Joshua Spence
d0d2cf903c Change parameters for diffusion.getlintmessages call
Summary: Ref T7604. Change the parameter for `diffusion.getlintmessages` from `arcanistProject` to `repositoryPHID`.

Test Plan: Ran `arc --conduit-uri='http://phabricator.local lint --only-new=1`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12900
2015-05-19 00:10:44 +10:00
Joshua Spence
4bb2eecdaf Add a linter rule for the instanceof operator
Summary: The `instanceof` operator expects the first argument to be an object instance. See http://www.phpwtf.org/instanceof-smart.

Test Plan: Added test case

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12856
2015-05-18 18:02:51 +10:00
Joshua Spence
0c9a037719 Fix arc which output
Summary: Fixes T8231. This was derped in some recent refactoring.

Test Plan: Ran `arc which` and saw expected output.

Reviewers: avivey, epriestley, #blessed_reviewers

Reviewed By: avivey, epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8231

Differential Revision: https://secure.phabricator.com/D12890
2015-05-18 09:40:43 +10:00
Joshua Spence
8c6e1284cc Improve ArcanistXHPASTLinter::LINT_TOSTRING_EXCEPTION
Summary:
The `ArcanistXHPASTLinter::LINT_TOSTRING_EXCEPTION` linter rule was introduced in D12854, but fails for the following:

  - Interfaces which declare the `__toString()` method.
  - Classes which mark the `__toString()` method as `abstract`.

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12892
2015-05-18 09:14:55 +10:00
Joshua Spence
2422202ba4 Add a linter rule to prevent the __lambda_func function from being redeclared
Summary:
See http://phpsadness.com/sad/39. Declaring a function named `__lambda_func` prevents the `create_function` function from working. This is because `create_function` eval-declares the function `__lambda_func`, then modifies the symbol table so that the function is instead named `"\0lambda_".(++$i)`, and returns that name.

NOTE: Personally, I don't think that anyone should use `create_function`. However, despite this, I think it is reasonable that no function is named `__lambda_func` in case some external library relies on `create_function`.

Test Plan: Added test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12870
2015-05-18 08:24:49 +10:00
Joshua Spence
1e3073a4f8 Add a linter rule to prevent exceptions from being thrown in a toString method
Summary: Fixes T8207. It is not possible to throw an exception from within the `__toString()` method. Add a linter rule to prevent this from happening.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8207

Differential Revision: https://secure.phabricator.com/D12854
2015-05-18 08:18:34 +10:00
Joshua Spence
2e66d03c68 Add a linter rule for spacing after a cast
Summary: Ref T7409. Add a linter rule to ensure that a cast is not followed by a space. This is largely based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/Formatting/NoSpaceAfterCastSniff.php | Generic_Sniffs_Formatting_NoSpaceAfterCastSniff]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12321
2015-05-18 08:07:43 +10:00
Joshua Spence
e1a057b4d9 Add a linter rule for alias functions
Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_ALIAS_FUNCTION` for linting the use of alias funtions. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/PHP/ForbiddenFunctionsSniff.php | Generic_Sniffs_PHP_ForbiddenFunctionsSniff]]. See [[http://php.net/manual/en/aliases.php | list of function aliases]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12422
2015-05-18 07:55:27 +10:00
Joshua Spence
e3a556af9b Improve error message from ArcanistXHPASTLinter::LINT_FORMATTED_STRING
Summary: Currently the error message from `ArcanistXHPASTLinter::LINT_FORMATTED_STRING` is slightly wrong, mentioning `xsprintf` instead of the actual `sprintf`-style function which was used. This diff changes the error message to be more correct.

Test Plan: Linted a file containing `sprintf('%s')` and saw a reasonable lint message.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12871
2015-05-18 07:48:03 +10:00
Joshua Spence
1df248cc2d Improve the "invalid parent scope" linter rule
Summary: This linter rule was introduced in D12420, but fails to handle a specific edge case.

Test Plan: Added test case.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12853
2015-05-18 07:47:19 +10:00
Joshua Spence
f9cefb7e2d Remove deprecated "base" classes
Summary: Ref T5655. After D9983, `ArcanistBaseWorkflow` and `ArcanistBaseUnitTestEngine` are deprecated.

Test Plan: Wait a sufficient amount of time before landing this.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D10004
2015-05-16 10:16:48 +10:00
Joshua Spence
e1a051a033 Improve linter rule for useless overriding methods
Summary: Ref T7409. This linter rule was added in D12419, but fails in some specific cases. Improve the handling of `n_DECLARATION_PARAMETER_LIST`.

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12828
2015-05-15 07:10:28 +10:00
Joshua Spence
4864435a50 Minor linter fixes
Summary: Minor linter fixes.

Test Plan: `arc lint`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12836
2015-05-15 07:09:30 +10:00
epriestley
c6fa42b9c5 Improve translation of an "arc land" string
Summary: This lost formatting in a pht() conversion; give the bold back and make it properly translatable.

Test Plan: Will land.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12843
2015-05-14 10:58:22 -07:00
Andrew Finnemore
383633e63f Fix missing comma in sprintf() in ArcanistWorkflow.
Summary: Refs D12607. Fixes T8195. Replace period in the sprintf() arguments with a comma.

Test Plan: Ran `arc diff` in the patch applied, did not get the sprintf() error

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T8195

Differential Revision: https://secure.phabricator.com/D12837
2015-05-14 05:59:44 -07:00
Joshua Spence
f00d4219dd Add a linter rule for incorrect use of parent scope
Summary:
The following code is invalid:

```
final class MyClass {
  public function __construct() {
    parent::__construct(null);
  }
}

$x = new MyClass();
```

Running the above code will produce a fatal error:

```
PHP Fatal error:  Cannot access parent:: when current class scope has no parent
```

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12420
2015-05-14 18:05:14 +10:00
Joshua Spence
a6a26bb3a3 Modernize ArcanistPylintLinter
Summary:
Ref T2039. Convert the `ArcanistPylintLinter` to an `ArcanistExternalLinter` and make it compatible with `.arclint`. In doing so, it was necessary to drop support of older versions of `pylint` by setting the minimum version to be 1.0.0. I think that dropping support for older versions is reasonable because version 1.0.0 was released ~18 months ago. In the case than an incompatible version is detected, an `ArcanistMissingLinterException` is thrown.

One caveat here is that support for `lint.pylint.codes.(error|warning|advice)` is dropped. Any installs that are relying on this configuration will need to migrate to using the `.arclint` file for specifying linter severity. We could potentially continue to support this deprecated configuration, but I'm not sure if this is worthwhile.

Test Plan: Wrote and executed unit tests with `arc unit`. I ran the unit tests on all `pylint` releases after (and including) 1.0.0. Specifically, the tests were run on v1.0.0, v1.1.0, v1.2.0, v1.3.0 and v1.4.0.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T2039

Differential Revision: https://secure.phabricator.com/D9109
2015-05-14 17:58:55 +10:00
Joshua Spence
3ae1fed4f9 Use __CLASS__ instead of hardcoding class names
Summary: Use `__CLASS__` instead of hardcoding class names. Depends on D12804.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12805
2015-05-14 07:21:27 +10:00
Joshua Spence
6295134bc7 Add a linter rule for useless overriding methods
Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_USELESS_OVERRIDING_METHOD` for detecting method which only call their parent method. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/CodeAnalysis/UselessOverridingMethodSniff.php | Generic_Sniffs_CodeAnalysis_UselessOverridingMethodSniff]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12419
2015-05-13 21:07:47 +10:00
Joshua Spence
d2b38cdf94 pht all the things
Summary: `pht`ize almost all strings in rARC.

Test Plan: ¯\_(ツ)_/¯

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12607
2015-05-13 21:00:53 +10:00
Joshua Spence
9090efcb40 Add a linter rule to prevent hardcoded class names
Summary: Add a linter rule to advise against the use of hardcoded class names. Hardcoded class names make the code harder to refactor and it is generally preferable to use the `__CLASS__` magic constant instead.

Test Plan: This works, but needs some polish.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12605
2015-05-13 06:51:15 +10:00
epriestley
73aca1f9f0 Allow Arcanist to parse bare revision IDs from "Differential Revisions:" field
Summary: Ref T8087. Prepares for eventually making these optional after T6030. See also T7443.

Test Plan:
  - See the next change for the server-side part of this.
  - With both patches applied, rigged the server to return `D123`.
  - Created a revision, saw bare `D123`.
  - Updated it with bare `D123`, things worked properly.
  - Created this revision with full URIs.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: cburroughs, epriestley

Maniphest Tasks: T8087

Differential Revision: https://secure.phabricator.com/D12748
2015-05-07 11:10:08 -07:00
epriestley
6c5d12d839 Make --conduit-token work without requiring .arcrc
Summary:
Ref T5955. This logic is a little cleaner than the previous version.

Don't require `~/.arcrc` to exist if the caller provides `--conduit-token`.

Test Plan:
  - Made calls with `--conduit-token` and no `~/.arcrc`.
  - Made calls with `--conduit-token` and a normal `~/.arcrc`.
  - Made calls with normal `arc`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: mbishopim3, epriestley

Maniphest Tasks: T5955

Differential Revision: https://secure.phabricator.com/D12750
2015-05-07 11:09:58 -07:00
Sascha Egerer
3e36b3fc43 Fix broken ArcanistPhpunitTestResultParser
Summary:
The ArcanistPhpunitTestResultParser has been changed
to use phutil_json_decode instead of json_decode in
rARCa4d33ef117aa8702181154b60ce1ce52bcfc119b
That has broken the functionality as json_decode has returned
an object before but phutil_json_decode does return an array.
The code has now been adopted to work with the array result
instead of an object.

Test Plan: Run a phpunit test case

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, epriestley

Differential Revision: https://secure.phabricator.com/D12751
2015-05-07 08:41:28 -07:00
Aviv Eyal
6f7bedaceb Fix linter config check
Summary: Fixes T6311 and T5124 by returning all configured linters from `buildLinter()`, and making `ArcanistExternalLinter::checkBinaryConfiguration()` not crash if there's no executable to run.

Test Plan: `arc linters` in rP shows "Configured" and "ERROR" as appropriate; Adding a broken linter to `.arclint` in rARC doesn't invoke it's not actually needed, and prints error if it is.

Reviewers: joshuaspence, epriestley, #blessed_reviewers

Reviewed By: joshuaspence, epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6311

Differential Revision: https://secure.phabricator.com/D10773
2015-05-05 14:03:01 -07:00
epriestley
111b9b035a Add a --conduit-token parameter to arc
Summary: Ref T5955. This makes it easier to write scripts which call Conduit via `arc call-conduit`.

Test Plan: Used `arc --conduit-token ... call-conduit user.whoami` to make calls as various users.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: mbishopim3, epriestley

Maniphest Tasks: T5955

Differential Revision: https://secure.phabricator.com/D12717
2015-05-05 14:02:54 -07:00
Joshua Spence
8919a9c5b5 Remove hook functionality
Summary: Fixes T7674. Remove remaining commit hook functionality.

Test Plan: Unit tests still pass?

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12698
2015-05-05 21:10:47 +10:00
Aviv Eyal
c00899ad60 Modernize Script-and-Regex linter for ConfigurationDriven
Summary: Support and prefer configuration from .arclint over Configuration Manager

Test Plan: arc lint with several combinations of values in .arcconfig and .arclint.

Reviewers: joshuaspence, epriestley, #blessed_reviewers

Reviewed By: joshuaspence, epriestley, #blessed_reviewers

Subscribers: vrusinov, jpoehls, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10704
2015-05-05 04:10:19 -07:00
Joshua Spence
0846c6aff5 Remove commit linter
Summary: Ref T7674. This linter doesn't make sense without commit hooks.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12697
2015-05-05 20:56:45 +10:00
Joshua Spence
4330b27c07 Remove support for passing data via STDIN
Summary: Ref T7674. Remove support for passing data to linters via `STDIN`. This functionality exists primarily for pre-receive workflows (which don't have a working copy to act on), but these workflows are going away soon.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12696
2015-05-05 20:55:49 +10:00
Joshua Spence
0a711315f8 Fix a failing XML unit test
Summary: Ref T7215. This test case is consistently failing locally (with a `LIBXML_VERSION` of `20902`).

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7215

Differential Revision: https://secure.phabricator.com/D12706
2015-05-05 18:57:22 +10:00
Joshua Spence
2b6568a4b9 Remove pre-commit hooks
Summary: Ref T7674. The `arc git-hook-pre-receive` and `arc svn-hook-pre-commit` workflows are being removed.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12690
2015-05-05 07:22:26 +10:00
Joshua Spence
f477682726 Fix a typo
Summary: This variable is misnamed.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12695
2015-05-05 07:14:40 +10:00
Joshua Spence
a4d33ef117 Use phutil_json_decode instead of json_decode
Summary: Generally, `phutil_json_decode` should be preferred over `json_decode`.

Test Plan: Eyeballed.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12678
2015-05-04 22:36:14 +10:00
Joshua Spence
977baacc32 Remove unused ArcanistUncommittedChangesException class
Summary: Remove the `ArcanistUncommittedChangesException` class which is unused after D11843.

Test Plan: `grep`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12515
2015-05-03 10:07:15 +10:00
Joshua Spence
eb5621c998 Move the getSuperGlobalNames method to ArcanistBaseXHPASTLinter
Summary: Move the `getSuperGlobalNames` method to `ArcanistBaseXHPASTLinter` so that it can be used within subclasses.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: virendra20888, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12473
2015-05-03 10:06:49 +10:00
Joshua Spence
46ce8a5a35 Memoize paths
Summary: Ref T7892. Memoize paths returned from `ARcanistLinter::getPaths()` to improve runtime performance.

Test Plan: Wiped ~0.5 seconds off the total time to lint rARC.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7892

Differential Revision: https://secure.phabricator.com/D12520
2015-04-27 23:20:31 +10:00
Joshua Spence
805ae12408 Remove vqueryfx from linters
Summary: These methods are gone. Depends on D12566.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12567
2015-04-27 21:19:38 +10:00
Joshua Spence
9ddf37b9ee Improve performance of LINT_UNNECESSARY_SEMICOLON
Summary: Ref T7892. Improve the performance of `ArcanistXHPASTLinter::LINT_UNNECESSARY_SEMICOLON`.

Test Plan: Wiped 6 seconds off the total time to lint rARC.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7892

Differential Revision: https://secure.phabricator.com/D12519
2015-04-23 20:18:54 +10:00
Joshua Spence
d6f4bded25 Memoize PHP compatibility info
Summary: Ref T7892. Avoid reading the PHP compatibility information for every file being linted.

Test Plan:
```name=Before
real    1m24.327s
user    1m19.571s
sys 0m5.239s
```

```lang=After
real    1m12.029s
user    1m5.756s
sys 0m5.502s
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7892

Differential Revision: https://secure.phabricator.com/D12518
2015-04-23 20:14:46 +10:00
epriestley
f4aadb9604 Split large path lists into blocks when linting
Summary:
Fixes T5097. When linting a large list of paths (e.g., with `--everything`), do this internally:

  $chunks = array_chunk($paths, 32);
  foreach ($chunks as $chunk) {
    $this->lintChunk($chunk);
  }

This keeps the advantages of parallelism and artifact sharing for future-based linters, without having memory usage grow in an unbounded way.

These callbacks changed:

  - `willLintPath()`: Useless, no meaningful implementations. Internalized the required side effect and broke the hook.
  - `didRunLinters()`: Now useless, with no meaningful implementations. Broke the hook.
  - `didLintPaths()`: New hook which executes opposite `willLintPaths()`.
  - `lintPath()`: Linters no longer need to implement this method.

XHPAST now has an explicit way to release shared futures.

These minor changes also happened:

  - Formalized the "linter ID", which is a semi-durable identifier for the cache.
  - Removed linter -> exception explicit mapping, which was unused. We now just collect exceptions.
  - We do the `canRun()` checks first (and separately) now.
  - Share more service call profiling code.
  - Fix an issue where the test harness would use the path on disk, even if configuration set a different path.

Test Plan:
  - Ran `arc lint --everything` in `arcanist/`.
    - With no chunking, saw **unstable** memory usage with a peak at 941 MB.
    - With chunk size 32, saw **stable** memory usage with a peak at 269 MB.
    - With chunk size 8, saw **stable** memory usage with a peak at 180 MB.
  - Ran with `--trace` and saw profiling information.
  - Created this diff.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T5097

Differential Revision: https://secure.phabricator.com/D12501
2015-04-22 05:15:57 -07:00
Joshua Spence
ab4eac6f31 Add a linter rule for lowercase function names
Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_LOWERCASE_FUNCTIONS` to ensure that builtin PHP functions are lowercased. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/PHP/LowercasePHPFunctionsSniff.php | Squiz_Sniffs_PHP_LowercasePHPFunctionsSniff]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12377
2015-04-22 22:06:49 +10:00
ArtemTitoulenko
a9ebe1cb4f allow either (<time> seconds) or (<time>s) format for test result time
Summary: The default Go Test Parser stopped showing the test function name at the end of the package after some Go upgrade. We fixed it locally and wanted to share it upstream.

Test Plan: We ran it on our test suite using go1.4.2 darwin/amd64. We have version-dependent code which cannot be compiled with earlier versions of Go, unfortunately.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: avivey, Korvin, epriestley

Maniphest Tasks: T7845

Differential Revision: https://secure.phabricator.com/D12436
2015-04-21 12:02:32 -07:00
Joshua Spence
c56e4a56dc Add a linter rule for default parameters
Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_DEFAULT_PARAMETERS` for ensuring function/method parameters with a default value are declared after those without a default value. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/PEAR/Sniffs/Functions/ValidDefaultValueSniff.php | PEAR_Sniffs_Functions_ValidDefaultValueSniff]].

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12418
2015-04-20 19:23:23 +10:00
Joshua Spence
eb036465dc Minor tidying of ArcanistXHPASTLinter
Summary: Modernizing a couple of linter rules.

Test Plan: Unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12393
2015-04-14 06:30:59 +10:00
Joshua Spence
58d8656696 phtize a bunch of strings
Summary: `pht`ize a bunch of strings in `ArcanistXHPASTLinter`.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12392
2015-04-14 06:30:13 +10:00
Joshua Spence
51b5c70010 Minor improvement to self member reference linter rule
Summary: Ref T7409. If `::` is surrounded by whitespace tokens and the whitespace token contains a newline character, allow it.

Test Plan: Added test case.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12391
2015-04-14 06:29:58 +10:00
Joshua Spence
e233b158f2 Add a linter rule for declaring nested functions
Summary: Ref T7409. Adds `ArcanistXHPASTLinter::LINT_INNER_FUNCTION` to advise against declaring PHP functions within an outer function. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php | Squiz_Sniffs_PHP_InnerFunctionsSniff]].

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12389
2015-04-14 06:29:44 +10:00
Joshua Spence
7bba30f66c Various linter fixes
Summary: Apply various linter fixes.

Test Plan: Unit tests + eyeballing.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12388
2015-04-14 06:29:07 +10:00
Joshua Spence
c011f948d3 Improvements to "self member reference" linter rule
Summary: Ref T7409. A few fixes and improvements to `ArcanistXHPASTLinter::LINT_SELF_MEMBER_REFERENCE`.

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12385
2015-04-14 06:27:17 +10:00
Joshua Spence
b11757fa23 Add a linter rule for logical operators
Summary:
Ref T7409. Adds `ArcanistXHPASTLinter::LINT_LOGICAL_OPERATORS` for advising:

  - `&&` in favor of `and`.
  - `||` in favor of `or`.

Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Operators/ValidLogicalOperatorsSniff.php | Squiz_Sniffs_Operators_ValidLogicalOperatorsSniff]].

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12376
2015-04-13 07:35:38 +10:00
Joshua Spence
fc70dfe268 Add a linter rule for self member references
Summary:
Ref T7409. Adds a linter rule for the following:

  - `self::` is used instead of `Self::`.
  - `self::` is used for local static member reference.
  - `self::` is used instead of `self ::`.

Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Classes/SelfMemberReferenceSniff.php | Squiz_Sniffs_Classes_SelfMemberReferenceSniff]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12370
2015-04-13 07:32:05 +10:00
Joshua Spence
6525a309c2 Further improvements to keyword casing linter rule
Summary: Ref T7409. Improve the `ArcanistXHPASTLinter::LINT_KEYWORD_CASING` linter rule to check that magic constants are uppercase.

Test Plan: Added unit test.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12368
2015-04-13 07:27:20 +10:00
Joshua Spence
515e98f1d2 Improve LINT_KEYWORD_CASING linter rule
Summary: Ref T7409. Improve `ArcanistXHPASTLinter::LINT_KEYWORD_CASING` to include additional token types. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/PHP/LowerCaseKeywordSniff.php | Generic_Sniffs_PHP_LowerCaseKeywordSniff]].

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12367
2015-04-13 07:25:54 +10:00
Joshua Spence
f86f4f24e6 Add a linter rule for uppercase constant definitions
Summary: Ref T7409. Adds a linter rule to ensure that constant are defined as being uppercase. Based on [[https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/NamingConventions/UpperCaseConstantNameSniff.php | Generic_Sniffs_NamingConventions_UpperCaseConstantNameSniff]].

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12366
2015-04-13 07:23:02 +10:00
Joshua Spence
bf1b30ae26 Trim trailing whitespace from diff hints
Summary: Although these don't do any harm, they show up in my editor which is configured to highlight trailing whitespace.

Test Plan: Submitted this diff... saw no trailing whitespace.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12369
2015-04-13 07:21:07 +10:00
Joshua Spence
e101496508 Fix a failing unit test
Summary: D11640 incorrectly changed the line endings in this file.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12322
2015-04-12 18:46:55 +10:00
Joshua Spence
f2a3fdf5e3 Fix text lint issues
Summary: Ref T5105. This is a proof-of-concept for D11458.

Test Plan: `arc lint --everything`

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5105

Differential Revision: https://secure.phabricator.com/D11640
2015-04-07 18:09:27 +10:00
Joshua Spence
3a5ffdc2fd Improve array comma rule
Summary: Improve `ArcanistXHPASTLinter::LINT_ARRAY_SEPARATOR` in handling multi-line arrays, see D12280 and D12281 for example.  Depends on D12295.

Test Plan: Updated unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12296
2015-04-07 07:51:52 +10:00
Joshua Spence
4b2d8e3f69 Don't pass data to jshint via STDIN
Summary: Fixes T7464. JSHint is unable to exclude files from linting (via the `jshintignore` file) if the data is piped through `STDIN`. An alternative would be to pass `$options[] = '--filename='.$this->getActivePath()`, but `$this->getActivePath()` is not set when the command arguments are constructed.

Test Plan: Created a file and linted it with `ArcanistJSHintLinter`. Was able to exclude the file from linting with a `jshintignore` file.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7464

Differential Revision: https://secure.phabricator.com/D12298
2015-04-07 07:26:10 +10:00
Joshua Spence
637832d06f Fix parsing of PHP compatibility info
Summary: The format of this file has changed. Depends on D12278.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12279
2015-04-07 07:25:58 +10:00
Joshua Spence
c5df885d7b Add a linter rule to detect unnecessary semicolons
Summary:
Ref T7409. Adds a rule to detect unnecessary semicolons. The most common scenario I've seen in the wild is the use of semicolons after a class definition:

```lang=php
class MyClass {
  // ...
};
```

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12139
2015-04-07 07:24:20 +10:00
Joshua Spence
92713cf922 Remove deprecated ComprehensiveLintEngine class
Summary: Ref T5655. This class was deprecated in D11673.

Test Plan: Wait a few months.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11740
2015-04-07 07:22:59 +10:00
cburroughs
41ddd34aeb Added RuboCop linter
Test Plan:
Add the following JSON to your `.arclint`:

```lang=json
"rubocop": {
  "type": "rubocop",
  "include": "(\\.rb$)"
}
```

Then, add a ruby file with errors, for instance:

```lang=ruby
def hello()
  puts 'world'
end
```

Run `arc lint`. It should come up with something along the line of: "Omit the parentheses in defs when the method doesn't accept any arguments."

Reviewers: joshuaspence, remon, #blessed_reviewers, epriestley

Reviewed By: joshuaspence, #blessed_reviewers, epriestley

Subscribers: reu, calfzhou, jjooss, cburroughs, chad, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10738
2015-04-06 09:11:15 -07:00
epriestley
8bbcbdd3b6 Prompt users to ignore or abort on untracked files
Summary:
Fixes T7521. This separates things into two prompts and doesn't try to automatically add untracked files.

This also fixes T7512, or at least I can't reproduce it after this change.

Test Plan:
  - Ran `arc diff` in a `git` directory with untracked files.
  - Ran `arc diff` in a `svn` directory with untracked files.
  - Ran `arc diff` in a `hg` directory with untracked files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7512, T7521

Differential Revision: https://secure.phabricator.com/D12258
2015-04-02 13:40:14 -07:00
Joshua Spence
e79032fec2 Add a linter rule for unnecessary use of the final modifier
Summary: Ref T7409. This was based on rhttps://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Generic/Sniffs/CodeAnalysis/UnnecessaryFinalModifierSniff.php.

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7409

Differential Revision: https://secure.phabricator.com/D12135
2015-03-24 07:30:10 +11:00
Joshua Spence
8530cfcec1 Fix a few edge cases for call-time pass-by-reference linting
Summary: Fix a few false positives.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12134
2015-03-23 23:38:59 +11:00
epriestley
1a2829d281 Fix chunk upload fallback behavior
Summary:
Ref T7594. Currently, if a chunk upload fails, we incorrectly swallow the failure and fall back to single-file upload, which will often fail by hitting size limits. This also silences the original error.

Instead, do chunk uploads outside the block so that any exceptions escape, and we don't try to fall back to single-file upload.

Mostly just trying to get more info about what's going wrong on @joshuaspence's install.

Test Plan: Faked an exception in chunk upload, ran `arc upload` on a big file, saw the exception displayed on the console.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, joshuaspence

Maniphest Tasks: T7594

Differential Revision: https://secure.phabricator.com/D12111
2015-03-18 19:06:27 -07:00
epriestley
b961869eda Explicitly draw progress bar when resuming file uploads in arc upload
Summary: Ref T7149. Make sure we sit at "Resuming: 60%" or whatever while uploading the first chunk.

Test Plan: Ran `arc upload` on a large file, cancelled it, resumed it, got sensible progress bar.

Reviewers: chad, btrahan

Reviewed By: chad, btrahan

Subscribers: epriestley

Maniphest Tasks: T7149

Differential Revision: https://secure.phabricator.com/D12082
2015-03-15 11:31:56 -07:00
epriestley
856cbed527 Remove "force chunking" code from Arcanist
Summary: Ref T7149. This was just for testing and is no longer required.

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7149

Differential Revision: https://secure.phabricator.com/D12077
2015-03-15 11:31:41 -07:00
epriestley
a01d3c3b1a Make "arc upload" chunk-aware
Summary: Ref T7149. This makes the client try to use the new `file.allocate` API before falling back to the old stuff.

Test Plan: Used `arc upload` to upload files. With chunking forced, uploaded chunked files.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T7149

Differential Revision: https://secure.phabricator.com/D12061
2015-03-13 11:30:50 -07:00
Yomi
f51f55a63c Add a space in the translation when a error message is printed.
Summary: Change 'themto' to 'them to'.

Test Plan: Check message printed...

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12057
2015-03-12 15:23:37 -07:00
nevogd
3d871b4214 Add untracked files to commit using prompt
Summary:
Refs D11990. When using `arc diff` with untracked files in the working
copy, add the untracked file(s) to the commit (as they weren't stashed or
ignored). Add the untracked paths to the list of changes in the editor
template, indicating that the files were added to the commit.

This doesn't add a separate prompt to add untracked files as per the
behaviour prior to D11843.

Test Plan:
Ran `arc diff` with only untracked files, answered yes to the 'create
new commit' prompt. Saw the commit-message with the updated changes
including untracked files. Completed the arc template, and got commit
containing uncommitted, unstaged and untracked files.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11995
2015-03-06 04:41:00 -08:00
epriestley
c36b4ceb18 Check for untracked files in order to fire untracked file prompt
Summary: Fixes T7465. I think I just missed this when untangling the old logic.

Test Plan: Ran `arc diff` with //only// untrakced files, saw warning.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T7465

Differential Revision: https://secure.phabricator.com/D11990
2015-03-05 14:44:59 -08:00
epriestley
4e98454840 Fix a newline issue in "arc" workflows
Summary:
We end up with one too few newline here in some workflows, like `arc land` with unstaged changes.

Root issue here is that `phutil_console_prompt|confirm` lead with too much whitespace but that's a harder fix.

Test Plan: Saw reasonable whitespace.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11927
2015-03-02 08:34:46 -08:00
epriestley
d8182cf55d Make "arc which --show-base" work as expected
Summary: This is supposed to just print out the base revision, but actually prints out the repository section first.

Test Plan: Ran `arc which`, `arc which --show-base`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11888
2015-02-25 13:09:00 -08:00
epriestley
dd59423d56 Use $EDITOR to prompt users when creating a new commit out of dirty working copy changes
Summary:
Fixes T7344.
Currently, we use `phutil_console_prompt()`, which isn't a very good editor. Use the real $EDITOR instead.

100% of the logic here was also a gigantic mess. Clean it up.

Test Plan: Will update in a second with console output from this run.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7344

Differential Revision: https://secure.phabricator.com/D11843
2015-02-21 02:37:58 -08:00
epriestley
8f8fe44b05 Update arcanist to work with more modular translations
Summary:
Ref T7152. Ref T1139.

  - Tweak API.
  - Move translations out of __init__ file.

Test Plan:
  - Ran `arc`.
  - Added a goofy translation and made sure it was working.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7152, T1139

Differential Revision: https://secure.phabricator.com/D11746
2015-02-11 13:02:11 -08:00
Joshua Spence
3f132f4a4e Fix xsprintf callback
Summary: After all that, I forgot to change this back.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11735
2015-02-11 18:29:12 +11:00
cburroughs
6dadb6b48c losen pep8 version check
Summary:
pep8 has used both 2 (`1.2`) and 3 (`1.2.1`) digit versions.  Losen
the version check to allow for both.

NOTE: This is the same regex as flake8.

Test Plan: `arc unit` with a 2 and 3 digit pep8 version on `$PATH`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: joshuaspence, epriestley

Differential Revision: https://secure.phabricator.com/D11728
2015-02-10 06:17:25 -08:00
Joshua Spence
17ec3859cd Fix pht method calls
Summary: Ref T7046. This is mainly a proof-of-concept for D11661.

Test Plan: `arc lint`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7046

Differential Revision: https://secure.phabricator.com/D11683
2015-02-10 18:48:38 +11:00
Joshua Spence
d8ba7e6f71 Add a linter rule to detect mismatched parameters for formatted strings
Summary: Fixes T7046. Adds a linter rule to detect mismatched parameters for formatted strings. Originally I had considered putting this rule in `ArcanistPhutilXHPASTLinter`, but I later decided to move it to `ArcanistXHPASTLinter` as I think that it is general enough to be more widely useful.

Test Plan: This seems to work but needs some polish.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7046

Differential Revision: https://secure.phabricator.com/D11661
2015-02-10 18:43:03 +11:00
Joshua Spence
b32cce79b2 Rename ComprehensiveLintEngine class for consistency
Summary: Ref T5655.

Test Plan: Ran `arc lint` with `lint.engine` set to `ComprehensiveLintEngine` and saw a deprecation notice.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11673
2015-02-10 06:57:43 +11:00
Joshua Spence
3498d6adfc Rename ArcanistCompilerLikeLintRenderer
Summary: Ref T5655. "Compiler-like" seems a bit odd to me.

Test Plan: `arc lint` + `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11670
2015-02-10 06:57:10 +11:00
Joshua Spence
f13b7d73d0 Remove deprecated support for specifying flags as a string
Summary: It should be safe to remove this now.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11714
2015-02-10 06:55:49 +11:00
Joshua Spence
bf7b32fe2c Rename ArcanistXHPASTLintTestSwitchHook class
Summary: Ref T5655.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11712
2015-02-10 06:55:03 +11:00
Joshua Spence
7c9b8237a0 Change a method name
Summary: Ref T5655.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11713
2015-02-10 06:53:53 +11:00
Joshua Spence
59640f7eae phtize some strings in ArcanistPhutilTestCaseTestCase
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11671
2015-02-05 07:22:00 +11:00
Joshua Spence
992d939e3a Remove the ArcanistArcanistLinterTestCase
Summary: I don't think that this provides too much value. I think that we should rework this to be inferred from the `.arcconfig` file perhaps?

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11662
2015-02-05 07:21:00 +11:00
Bob Trahan
5c20df1818 Arcanist / Conduit - stop using deprecated diffusion.getcommits
Summary: Fixes T7113. This one was a bit trickier than others as the API output changed a bit. In particular, there is no "errors" emitted so much as the result set just doesn't include the answer if things are garbage. Ergo, check the "identifier map" to either check for diff existence or to lookup the phid to grab the actual diff data from the "data" part of the result.

Test Plan: called `arc backout D11665` and got some working output...!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7113

Differential Revision: https://secure.phabricator.com/D11667
2015-02-03 14:33:06 -08:00
Bob Trahan
d98c104991 Arcanist / Conduit - stop using deprecated differential.getdiff API
Summary: Fixes T7112. Nothing too difficult here.

Test Plan:
meta - submitting this with the new arcanist code
used conduit API to verify the difference between getdiff (just the latest diff) and querydiffs (all diffs that match, with the latest diff first in the set)

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7112

Differential Revision: https://secure.phabricator.com/D11665
2015-02-03 14:18:10 -08:00
epriestley
a08383aa1b Make "no working copy" a more explicit "arc" VCS
Summary:
Some commands (like `get-config`) do not require a working copy at all. Recent changes prevented these commands from running outside a VCS working copy.

Let `null` mean "no working copy".

See also <https://github.com/phacility/phabricator/issues/800>.

Test Plan:
  - Ran `arc get-config` from outside a working copy.
  - Ran `arc list` from outside a working copy, got an error.
  - Ran `arc commit` in a Git working copy, got an error.

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11643
2015-02-03 12:32:46 -08:00
Joshua Spence
77eb24b13a Only allow arc branch to be used under git
Summary: Explicitly declare that the `arc branch` command is only supported under `git`.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11552
2015-02-04 06:58:53 +11:00
Joshua Spence
0fd04109e9 Indicate relevant VCS for arc feature
Summary: This workflow only works on git + mercurial.

Test Plan: I don't even subversion.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11626
2015-02-04 06:54:08 +11:00
Joshua Spence
6b31c0b98a List supported VCS for arc backout
Summary: This workflow does not work on subversion.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11625
2015-02-04 06:53:58 +11:00
Joshua Spence
7eb86b2371 Mark git-hook-pre-receive as git only
Summary: It doesn't make any sense to run this command on another VCS.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11624
2015-02-04 06:53:47 +11:00
Joshua Spence
6bca7f0146 Mark arc svn-hook-pre-commit for subversion only
Summary: It doesn't make any sense to run this command on any other VCS.

Test Plan: Ran `arc svn-hook-pre-commit` and hit the usage exception.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11623
2015-02-04 06:53:41 +11:00
Joshua Spence
24ab7b7f38 Allow ArcanistPhutilLibraryLinter to run with an out-of-date XHPAST binary
Summary: There is no need to check if the XHPAST binary is available here because this linter does not actually parse PHP, it only considers the library map.

Test Plan: Removed the XHPAST binary and ran `arc lint` on a bunch of files.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11528
2015-02-03 08:01:11 +11:00
Joshua Spence
70f9b5d3ae Use PhutilXHPASTBinary methods
Summary: Use `PhutilXHPASTBinary` methods instead of `xhpast_parse` functions. Depends on D11517.

Test Plan: N/A, this is a direct swap.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11613
2015-02-03 06:58:33 +11:00
Joshua Spence
0ef326f3a5 Use XHPAST version instead of checksum as cache version
Summary: Rely on the output of `xhpast --version` when determing the lint cache key.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11611
2015-02-03 06:56:35 +11:00
Joshua Spence
0584d4a06f phtize strings in ArcanistSettings
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11610
2015-02-03 06:54:46 +11:00
Joshua Spence
e4be031778 Explicitly check for supported VCS
Summary: Instead of having an `ArcanistWorkflow` subclass explicitly throw an exception when run in an unsupported VCS, consolidate this code and move it to `arcanist.php`. In doing so, we lose some specificity in some of the error messages, but this otherwise feels cleaner. We could consider adding a `getUnsupportedRevisionControlSystemMessage()` method to provide a more tailored error message. Depends on D11604.

Test Plan:
Ran `arc bookmark` in a `git` working copy:

```
Usage Exception: `arc bookmark` is only supported under hg.
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11550
2015-02-03 06:54:22 +11:00
Joshua Spence
52277fc06f Modernize ArcanistPhpcsLinter
Summary: Allow `--severity` to be specified using `.arclint`.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11511
2015-02-03 06:43:55 +11:00
Joshua Spence
623df14ae5 Change "any" to explicitly list revision control systems
Summary: Using `array('any')` to represent `array('git', 'hg', 'svn')` is a bit magical and leads to a lot of special-casing.

Test Plan: Verified that tab completion (ala `ArcanistShellCompleteWorkflow`) still worked.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11604
2015-02-02 18:19:29 +11:00
Joshua Spence
8cad12034f Minor tidying of ArcanistPasteWorkflow
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11566
2015-02-01 21:57:49 +11:00
Joshua Spence
8208cd3231 Minor tidying of ArcanistRevertWorkflow
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11557
2015-02-01 21:56:28 +11:00
Joshua Spence
589ee20a76 Minor tidying of ArcanistUploadWorkflow
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11556
2015-02-01 21:54:53 +11:00
Joshua Spence
3e63402fef Minor tidying of ArcanistPhrequentWorkflow classes
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11555
2015-02-01 21:49:53 +11:00
Joshua Spence
62e15dcc15 Minor tidying of ArcanistCallConduitWorkflow
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11567
2015-02-01 11:05:27 +11:00
Joshua Spence
fcd882815a Minor tidying of ArcanistUpgradeWorkflow
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11554
2015-02-01 11:01:37 +11:00
Joshua Spence
c76c5d893e Minor tidying of ArcanistVersionWorkflow
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11553
2015-01-30 07:15:05 +11:00
Joshua Spence
e9ffde30ba Fix ArcanistJscsLinter unit tests
Summary: These tests are failing with the latest version of `jscs` (v1.10.0).

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11520
2015-01-28 06:49:02 +11:00
Joshua Spence
edc60cefc4 Fix failing unit tests for ArcanistLesscLinter
Summary: A bunch of unit tests are failing with the latest version of `lessc` (v2.3.0). I decided to delete a bunch of test cases for this linter as we have far too many at the moment.

Test Plan: `arc unit`

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11521
2015-01-28 06:48:50 +11:00
Joshua Spence
d0f84e8492 phtize some strings
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11503
2015-01-27 06:58:31 +11:00
Joshua Spence
c450fa6c06 phtize a bunch of strings
Summary: Self-explanatory.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11501
2015-01-27 06:58:19 +11:00
Joshua Spence
1a3afa4429 Add a linter rule to detect call-time pass-by-reference
Summary: Call-time [[http://php.net/manual/en/language.references.pass.php | pass-by-reference]] is an awful "feature" of PHP that is deprecated and removed in PHP 5.4 (see http://us1.php.net/manual/en/migration54.incompatible.php). Add a linter rule to `ArcanistXHPASTLinter` to detect the use of this feature and raise an error.

Test Plan: Added a test case for `arc unit`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10537
2015-01-23 07:56:58 +11:00
Joshua Spence
0b22838cca Allow arc unit --everything to work without a base commit
Summary: Fixes T2461. Similarly to `arc lint --everything`, `arc unit --everything` should work without a base commit.

Test Plan: Removed the `.git/arc/default-relative-commit` file and ran `arc unit --everything`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T2461

Differential Revision: https://secure.phabricator.com/D11459
2015-01-23 07:21:53 +11:00
Joshua Spence
937861ce58 Improve handling of options for arc workflows
Summary: Allow `--severity=warning` to mean the same as `--severity warning`. Longer term, we should convert this code to use `PhutilArgumentParser`, although it doesn't seem that `PhutilArgumentParser` support British spelling ;)

Test Plan: Ran `arc lint --severity=warning`. Also `var_dump`ed `$args` to make sure it looked reasonable.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11464
2015-01-23 07:07:37 +11:00
epriestley
1cc8f3f377 Don't use newInstanceWithoutConstructor in Arcanist unit test
Summary: This requires PHP 5.4. Use `newv()` instead. I don't think the "without constructor" part is meaningful (or desirable). Simplify slightly.

Test Plan: `arc unit --everything`

Reviewers: hach-que, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11450
2015-01-20 15:39:21 -08:00
Joshua Spence
8173ea3eea Fix visibility of the ArcanistPhpcsLinter::getMandatoryFlags method
Summary: Ref T6822.

Test Plan: All the cool linters are doing it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11414
2015-01-16 07:40:06 +11:00