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

2255 commits

Author SHA1 Message Date
Joshua Spence
cd591318ca Fix visibility of the ArcanistLintEngine::buildLinters() method
Summary: Ref T6822. This method needs to be public so that it can be called from `ArcanistLintersWorkflow`.

Test Plan: Ran `arc linters`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11239
2015-01-07 07:35:20 +11:00
epriestley
2035330780 Don't try to call library functions during arc sanity checking
Summary: These functions won't exist yet. See 7e2df9a5bf (commitcomment-9172274)

Test Plan: Faked a fatal, ran `arc`, got message.

Reviewers: joshuaspence, chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11254
2015-01-06 09:53:11 -08:00
jjoos
439413ae29 Remove the noconfig flag
Summary:
D11196 introduced a regression that made it impossible for coffeelint to read any configuration. I'm reverting the change in that diff.

Created a pull request changing the documentation of coffeelint to make sure I'm interpreting this flag correctly: https://github.com/clutchski/coffeelint/pull/364

Test Plan:
- patch
- npm install -g coffeelint

- create a `test.coffee` file with:
```
# 1234567890
```
- `arc lint test.coffee`
```
 OKAY  No lint warnings.
```
- create a `coffeelint.json` with
```
{
  "max_line_length": {
    "value": 10
  }
}
```
- `arc lint test.coffee`, expected output:
```
>>> Lint for test.coffee:

   Error  (COFFEE)
    Line exceeds maximum allowed length.

    >>>        1 # 1234567890
```
- create a `.arclint` with
```
{
  "linters": {
    "coffeelint": {
      "type": "coffeelint",
      "coffeelint.config": "coffee_lint_config_with_different_name.json"
    }
  }
}
```
- rename `coffeelint.json` to `coffee_lint_config_with_different_name.json`
- `arc lint test.coffee`, expected output:
```
>>> Lint for test.coffee:

   Error  (COFFEE)
    Line exceeds maximum allowed length.

    >>>        1 # 1234567890
```

Reviewers: Korvin, joshuaspence, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11250
2015-01-06 06:59:45 -08:00
Joshua Spence
7e2df9a5bf phtize some strings
Summary: Self-explanatory.

Test Plan: Eye-balled it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10596
2015-01-06 23:15:50 +11:00
Joshua Spence
6eed5c2514 Create a custom exception class for missing linter dependencies
Summary: I feel that we are abusing `ArcanistUsageException`. Throw a more tailed exception instead. Depends on D11197.

Test Plan: `arc lint`, I guess.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: avivey, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11205
2015-01-06 22:51:17 +11:00
Joshua Spence
0b51f4d7c9 phtize some strings
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/D11206
2015-01-06 22:43:52 +11:00
Joshua Spence
d477df00eb Add a linter rule to detect the use of blacklisted functions
Summary:
As mentioned in the [[https://secure.phabricator.com/book/phabcontrib/article/php_coding_standards/ | Phabricator PHP coding standards]], the `eval` function should be avoided. There is some good discussion on [[http://stackoverflow.com/questions/951373/when-is-eval-evil-in-php | StackOverflow]] as well.

Having said that, instead of hardcoding `eval()`, I have generalised this enough to allow a set of "blacklisted" functions to be defined with `xhpast.blacklisted.function` in the `.arclint` file.

Test Plan: Added a test case.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10686
2015-01-05 10:51:33 +11:00
Joshua Spence
eb3129408b Various improvements for ArcanistClosureLinter
Summary: WIP

Test Plan: WIP

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11207
2015-01-05 07:43:55 +11:00
Joshua Spence
9e6f876a68 Export namespace in ArcanistUnitTestResult dictionary
Summary: D7952 added namespaces to `ArcanistUnitTestResult` but these are not exported and thus don't get sent from client to server.

Test Plan: Uploaded a diff and inspected the contents of the `phabricator_differential.differential_diffproperty` table.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11208
2015-01-05 06:48:32 +11:00
Joshua Spence
4fe5243d81 Use PhutilMethodNotImplementedException in ArcanistExternalLinter
Summary: Self-explanatory.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11204
2015-01-05 06:47:12 +11:00
Joshua Spence
44f81f4351 Rename PHPUnitTestEngineTestCase for consistency
Summary: Ref T5655. `PHPUnitTestEngineTestCase` is the test case for `PhpunitTestEngine`.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11203
2015-01-05 06:46:24 +11:00
Joshua Spence
1c0fd5ce5d Move ArcanistTestResultParser subclasses
Summary: Move `ArcanistTestResultParser` subclasses from `src/unit/engine` to `src/unit/parser`. Depends on D11201.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11202
2015-01-05 06:46:14 +11:00
Joshua Spence
63c9c6c4ff Rename ArcanistTestResultParser subclasses for consistency
Summary: Ref T5655.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11201
2015-01-05 06:45:31 +11:00
Joshua Spence
9fdf53452c Rename UnitTestableArcanistLintEngine for consistency
Summary: Ref T5655.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T5655

Differential Revision: https://secure.phabricator.com/D11200
2015-01-05 06:44:49 +11:00
Joshua Spence
23f9a3ae66 Minor tidying of ArcanistCoffeeLintLinter
Summary: Self-explanatory.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11199
2015-01-05 06:44:33 +11:00
Joshua Spence
9c07016ee4 Don't pass --quiet to coffeelint
Summary: This flag doesn't do anything when combined with `--reporter=checkstyle`.

Test Plan:
```
> coffeelint *.coffee
  ✗ fail.coffee
     ✗ #1: Class names should be camel cased. class name: boaConstrictor.
  ✓ pass.coffee

✗ Lint! » 1 error and 0 warnings in 2 files

> coffeelint --quiet *.coffee
  ✗ fail.coffee
     ✗ #1: Class names should be camel cased. class name: boaConstrictor.

✗ Lint! » 1 error and 0 warnings in 2 files

> coffeelint --reporter=checkstyle *.coffee
<?xml version="1.0" encoding="utf-8"?>
<checkstyle version="4.3">
<file name="fail.coffee">
<error line="1"
    severity="error"
    message="Class names should be camel cased; context: class name: boaConstrictor"
    source="coffeelint"/>
</file>
</checkstyle>

> coffeelint --reporter=checkstyle --quiet *.coffee
<?xml version="1.0" encoding="utf-8"?>
<checkstyle version="4.3">
<file name="fail.coffee">
<error line="1"
    severity="error"
    message="Class names should be camel cased; context: class name: boaConstrictor"
    source="coffeelint"/>
</file>
</checkstyle>
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11198
2015-01-05 06:43:59 +11:00
Joshua Spence
da02add6c8 Create an ArcanistExternalLinterTestCase class
Summary: Creates a new base class for unit testing `ArcanistExternalLinter` subclasses. Specifically, add a test case for verifying that we are correctly parsing the output of `$external_linter --version`.

Test Plan: `arc unit`

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11197
2015-01-05 06:41:59 +11:00
Joshua Spence
f6b3f3c46a Ignore environment variables for coffeelint
Summary: According to `coffeelint --help`, `--noconfig` ignores the environment variable `COFFEELINT_CONFIG`. This means that `arc lint` will behave more consistently across developer workstations by forcing configuration to be specified with `coffeelint.config` instead of with environment variables.

Test Plan: This is mostly theoretical.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11196
2015-01-05 06:40:37 +11:00
Joshua Spence
784033dc1a Update flags for coffeelint
Summary: According to `coffeelint --help` (with an up-to-date version of `coffeelint`, version 1.8.1) , `--nocolor` is deprecated in favor of `--color=never`. According to the [[http://www.coffeelint.org/#changelog | changelog]], `--nocolor` was deprecated in v1.6.0.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11195
2015-01-05 06:39:50 +11:00
Joshua Spence
f5db41917b Reduce boilerplate code in ArcanistLinterTestCase subclasses
Summary: This is a bit magical and is maybe a terrible idea, but it seems okayish.

Test Plan: `arc unit`

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11172
2015-01-04 18:40:57 +11:00
Joshua Spence
a10002adec Raise a linter error when removed symbols are used
Summary: Ref T5141. Utilize the `'max'` key from the `php_compat_info.json` compatibility map to lint for the use of classes/functions/symbols which have been removed from the target PHP version.

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

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5141

Differential Revision: https://secure.phabricator.com/D10538
2015-01-04 18:38:07 +11:00
Joshua Spence
b31e9c0cfe Add a linter rule to detect duplicate case statements
Summary: Fixes T6843. Adds a linter rule to `ArcanistXHPASTLinter` to detect duplicate `case` statements within a `switch` statement.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6843

Differential Revision: https://secure.phabricator.com/D11171
2015-01-04 18:08:37 +11:00
Joshua Spence
4e3df80584 Move LINT_NO_COMMIT from ArcanistTextLinter to a new linter
Summary: I don't feel that this linter rule belongs in the `ArcanistTextLinter`. In fact, this linter rule is quite similar to the rules provided by `ArcanistGeneratedLinter` and `ArcanistNoLintLinter` and these classes could possibly be consolidated. I have moved this linter rule to a standalone `ArcanistCommitLinter` class (which could possibly do additional lints in the future).

Test Plan: Moved existing test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10473
2015-01-04 16:20:32 +11:00
Joshua Spence
821ebcdd07 phtize some strings
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/D11175
2015-01-03 23:53:07 +11:00
Joshua Spence
a9cfc17688 Specify the config property of lint-test files as a map
Summary: Self-explanatory. This is implied anyway as we iterate over it with a `foreach` loop.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11174
2015-01-03 23:52:56 +11:00
Joshua Spence
fba62c0d9d Remove arcconfig settings from ArcanistLinterTestCase
Summary: This doesn't make too much sense anymore as all modern linters are configured via the `.arclint` file rather than `.arcconfig`.

Test Plan: `grep`ped to make sure this option isn't used.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11173
2015-01-03 23:52:46 +11:00
Joshua Spence
201d195f29 Fix variable name
Summary: Fix a variable name to be consistent with naming conventions.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11140
2015-01-03 10:30:15 +11:00
Joshua Spence
d6af220921 Remove some unused classes
Summary: Self-explanatory.

Test Plan: `grep`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11151
2015-01-03 09:06:16 +11:00
Joshua Spence
0352db802e Suppress stdin message if input is piped
Summary: Currently we output "Waiting for JSON parameters on stdin...", even if `stdin` is piped (for example, from `echo`).

Test Plan:
```lang=bash
> echo '{}' | arc call-conduit conduit.ping
{"error":null,"errorMessage":null,"response":"ip-10-161-81-110"}

> arc call-conduit conduit.ping
Waiting for JSON parameters on stdin...
{}
^D
{"error":null,"errorMessage":null,"response":"ip-10-161-81-110"}
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11122
2015-01-02 11:36:24 +11:00
Joshua Spence
f86a70f411 Remove some old code handling the absence of the file.uploadhash Conduit method
Summary: The `file.uploadhash` method was added a long time ago (in D4899). It should be safe to assume that this method exists on most installs.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11118
2015-01-02 10:10:10 +11:00
Joshua Spence
f8be9d7737 Remove the inlines workflow
Summary: Ref T5112. The `arc inlines` workflow no longer works because the `differential.getrevisioncomments` API method has been deprecated (see T2222). The command is basically useless right now.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5112

Differential Revision: https://secure.phabricator.com/D9464
2014-12-31 10:45:41 +11:00
epriestley
a3c3d23dd7 Fix two linter issues
Summary:
  - The "exclude static variable access" branch in XHPAST incorrectly excluded all variables in a function or method which used any static access (we were selecting the wrong root node for exclusion).
  - The "PHPLinter" regexp did not work with 5.4.30. Make it more permissive in what it parses.

Test Plan:
  - Added a failing unit test for the variable case and made it pass.
  - PHPLinter tests now pass under 5.4.30

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D11087
2014-12-30 10:02:43 -08:00
Joshua Spence
721bdf424b Use new FutureIterator instead of Futures
Summary: Ref T6829. Deprecate the `Futures()` function.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T6829

Differential Revision: https://secure.phabricator.com/D11079
2014-12-30 23:16:25 +11:00
Joshua Spence
841556134f Fix puppet-lint unit tests for the v1.1.0
Summary: This unit test is failing locally for me, using v1.1.0 of `puppet-lint`.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11057
2014-12-30 02:46:57 -08:00
epriestley
2372863517 Add some more spelling corrections to Spelling linter
Summary: Fixes T6773. See also the shameful `PhabricatorDestructableInterface` debacle.

Test Plan: go go dictionary power

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T6773

Differential Revision: https://secure.phabricator.com/D11033
2014-12-22 16:08:02 -08:00
epriestley
5118987757 Support simpler, token-based Conduit authentication in Arcanist
Summary:
Ref T5955. If the server supports token-based authentication, prefer it over certificate-based authentication.

Also fixes T3117.

Test Plan:
  - Used `arc install-certificate` to install credentials from both token-based and certificate-based hosts.
  - Used `arc list` with a token-based host.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3117, T2878, T5955

Differential Revision: https://secure.phabricator.com/D10988
2014-12-15 11:12:38 -08:00
Joshua Spence
565a96e0ee Minor linter fixes
Summary: Self explanatory/

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10943
2014-12-08 23:48:55 +11:00
Joshua Spence
4ca3463df6 Extend LINT_BRACE_FORMATTING to support more scenarios
Summary: Extend the `ArcanistXHPASTLinter::LINT_BRACE_FORMATTING` rule to support a bunch of extra cases.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10572
2014-12-08 23:31:21 +11:00
Joshua Spence
ee2070fadb Extend LINT_BRACE_FORMATTING to warn on missing braces
Summary: Extend the `ArcanistXHPASTLinter::LINT_BRACE_FORMATTING` rule to raise a warning when `n_STATEMENT` is used without having `n_STATEMENT_LIST` as a parent. Essentially, this means that `if ($x) { do_y(); }` is preferred over `if ($x) do_y();`.

Test Plan: Added some test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10571
2014-12-08 23:30:33 +11:00
epriestley
b46d4ed4ad Update lint engine documentation
Summary: Fixes T6627. This class documentation is out of date and somewhat misleading in the modern environment.

Test Plan: Read it.

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T6627

Differential Revision: https://secure.phabricator.com/D10899
2014-11-24 15:01:19 -08:00
David Cramer
6a36584ae1 Report tests results whenever possible
Summary: This adjusts the behavior of PytestTestEngine so that it correctly reports test failures natively, rather than throwing a command error.

Test Plan: Yeah right

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10866
2014-11-17 15:25:48 -08:00
Chad Little
52947cfd92 Remove gist link that (never?) doesn't work
Summary: Removing 404 link, fixes https://github.com/phacility/arcanist/issues/180

Test Plan: n/a

Reviewers: epriestley, avivey

Reviewed By: epriestley, avivey

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10689
2014-10-13 10:48:18 -07:00
Joshua Spence
8547a25928 Fix LINT_KEYWORD_CASING for class keyword
Summary: D10653 didn't quite cut it.

Test Plan: Tested on a real file.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10656
2014-10-08 09:29:35 +11:00
Joshua Spence
f44a6e00a7 Extend ArcanistXHPASTLinter::LINT_KEYWORD_CASING
Summary: Extend `ArcanistXHPASTLinter::LINT_KEYWORD_CASING` to include the `class` keyword.

Test Plan: I can't really added a test case for this without getting "XHP19 Class-Filename Mismatch".

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10653
2014-10-08 08:54:45 +11:00
Joshua Spence
a0cb484db4 Mark some methods in ArcanistLinter as final
Summary: Minor change, self-explanatory... this seems to make sense.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10476
2014-10-08 00:07:39 +11:00
Joshua Spence
e54725ec89 Add a LINT_CONSTRUCTOR_PARENTHESES rule to ArcanistXHPASTLinter
Summary: Adds an `ArcanistXHPASTLinter::LINT_CONSTRUCTOR_PARENTHESES` rule which ensures that (possibly empty) parentheses are used when calling a constructor.

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10597
2014-10-07 23:59:03 +11:00
Joshua Spence
b1112e73c4 Minor linter fixes
Summary: Apply some linter autofixes.

Test Plan: `arc lint` and `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10568
2014-09-27 10:21:23 +10:00
Joshua Spence
8ed1459ecd Add a linter rule to detect variable arguments for break and continue
Summary: In PHP 5.4, the `break` and `continue` statements no longer accept variable arguments (e.g., `break 1 + foo() * $bar;`). Static arguments still work, such as break 2;. As a side effect of this change `break 0;` and `continue 0;` are no longer allowed.

Test Plan: Added some test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10567
2014-09-26 08:40:30 +10:00
Joshua Spence
247a4fabb8 Fix LINT_ARRAY_SEPARATOR for heredocs
Summary: It is not correct to add a trailing comma if the `n_ARRAY_VALUE` node is a heredoc string.

Test Plan: Added a test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10569
2014-09-26 08:35:40 +10:00
Joshua Spence
cb20078250 Fix LINT_ARRAY_SEPARATOR for nested arrays
Summary: The `LINT_ARRAY_SEPARATOR` rule was added to `ArcanistXHPASTLinter` in D10535. Unfortunately, it does not correctly handle nested arrays correctly. This patch fixes this. Depends on D10554.

Test Plan: Added a test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10558
2014-09-26 07:14:55 +10:00