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

1719 commits

Author SHA1 Message Date
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