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

1633 commits

Author SHA1 Message Date
Joshua Spence
f58642a8ab Add unit tests for ArcanistFilenameLinter
Summary: Self-explanatory.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11341
2015-01-13 06:47:19 +11:00
Joshua Spence
c86c1ff6f2 Minor changes to ArcanistChmodLinter
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/D11342
2015-01-13 06:37:25 +11:00
Joshua Spence
25f4563fc8 Minor tidying of ArcanistConduitLinter
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/D11344
2015-01-13 06:36:19 +11:00
Joshua Spence
3152db6cce Disable the customization of linter severities for ArcanistCommitLinter
Summary: The expectation is that this linter only raises errors.

Test Plan: This is mostly theoretical.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11331
2015-01-12 07:24:53 +11:00
Joshua Spence
b780ef0868 Use the raw formatter for coffeelint
Summary: Using `--reporter=raw` exposes raw JSON output rather than a limited XML output. This allows us to pull more context from the `coffeelint` output.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11320
2015-01-12 07:23:42 +11:00
Joshua Spence
23bc9f294c Change the default value for ArcanistExternalLinter::shouldExpectCommandErrors
Summary: It is more common for linters to exit with a non-zero status than it is for linters to return with a zero exit status, Really this function serves very little purposes, it simply determines whether or not to throw an exception if a non-zero status is returned by the external linter.

Test Plan: `arc unit`

Reviewers: chad, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11322
2015-01-12 07:21:33 +11:00
Joshua Spence
564dab0989 Fix a lint renderering issue
Summary: Currently there is an issue when renderering linter messages with `ArcanistConsoleLintRenderer` if `$message->getChar()` returns `0` (i.e. the value is unset).

Test Plan:
**Before**
{F265586}

**After**
{F265587}

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11319
2015-01-12 06:49:14 +11:00
Joshua Spence
c2a9d88711 Don't explicitly pass --color=never to coffeelint
Summary: This flag doesn't do anything in combination with `--reporter=checkstyle`.

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

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

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11318
2015-01-12 06:48:24 +11:00
Joshua Spence
5455fe4430 Don't explicitly implement ArcanistPuppetLintLinter::supportsReadDataFromStdin
Summary: `false` is the default return value for this method.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11314
2015-01-12 06:48:01 +11:00
Joshua Spence
e4149e43cd Improve parsing of csslint output
Summary: `csslint` returns an `evidence` field which contains the line of the "original text" which produced the linter message (see 5dd84b259b/src/core/Reporter.js (L64)). We can use this data instead of trying to achieve the same result using `$this->getData($path)`.

Test Plan: {F265449}

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11311
2015-01-12 06:47:51 +11:00
Joshua Spence
7b383632dd Minor tidying of ArcanistCSSLintLinter
Summary: Minor tidying.

Test Plan: Eyeball it.

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11307
2015-01-12 06:47:16 +11:00
Joshua Spence
1b3d600098 Don't pass --quiet to csslint
Summary: Similar to D11198. This flag doesn't do anything when combined with `--format=lint-xml`.

Test Plan:
```lang=bash
> csslint *.css
csslint *.css

csslint: There are 1 problems in /home/joshua/workspace/github.com/phacility/arcanist/fail.css.

fail.css
1: error at line 1, col 1
Unexpected token '~' at line 1, col 1.
~

csslint: No errors in /home/joshua/workspace/github.com/phacility/arcanist/pass.css.

> csslint --quiet *.css
csslint --quiet *.css

csslint: There are 1 problems in /home/joshua/workspace/github.com/phacility/arcanist/fail.css.

fail.css
1: error at line 1, col 1
Unexpected token '~' at line 1, col 1.
~

> csslint --format=lint-xml *.css
<?xml version="1.0" encoding="utf-8"?><lint>
<file name="/home/joshua/workspace/github.com/phacility/arcanist/fail.css"><issue line="1" char="1" severity="error" reason="Unexpected token '~' at line 1, col 1." evidence="~"/></file>
</lint>

> csslint --format=lint-xml --quiet *.css
<?xml version="1.0" encoding="utf-8"?><lint>
<file name="/home/joshua/workspace/github.com/phacility/arcanist/fail.css"><issue line="1" char="1" severity="error" reason="Unexpected token '~' at line 1, col 1." evidence="~"/></file>
</lint>
```

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11308
2015-01-12 06:46:34 +11:00
Joshua Spence
1c3278e3fb Move linter exception classes to src/lint/linter/exception
Summary: The `src/lint/linter` directory is a bit cluttered at the moment.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11309
2015-01-12 06:46:23 +11:00
Joshua Spence
14e5fbbd6b Fix parsing of puppet-lint --version
Summary: This seems to be off, at least for `puppet-lint` versions greater than 1.0.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11312
2015-01-12 06:44:08 +11:00
Joshua Spence
9c0568e12d Delete a problematic test case
Summary: This test case is currently failing with various different versions of `puppet-lint`. I was considering fixing this test case, but I feel that we have more than enough test cases for `ArcanistPuppetLintLinter` anyway. Since this is an `ArcanistExternalLinter`, we only really need to test that the bindings are working as expected (rather than the linter functionality).

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11313
2015-01-12 06:43:58 +11:00
Joshua Spence
50a5d54e71 Minor tidying of the ArcanistPuppetLintLinter class
Summary: Self-explanatory.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11317
2015-01-12 06:43:07 +11:00
Joshua Spence
ce7e8f28f5 Pass --error-level=all to puppet-lint
Summary: Without explicitly passing `--error-level=all` to `puppet-lint`, the configuration file for `puppet-lint` could contain `--error-level=error`. This would limit the ability of `arc lint` to detect linter issues and therefore we should override this flag from the command line explicitly.

Test Plan: This is mostly theoretical.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11316
2015-01-12 06:41:46 +11:00
Joshua Spence
fa37d3135f Minor improvements to ArcanistJSONLinter
Summary: Self-explanatory.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11324
2015-01-12 06:40:40 +11:00
Joshua Spence
4e38fac47d Make properties in the PhutilUnitTestEngineTestCase class private
Summary: There's no need for these properties to be (implicitly) public.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11276
2015-01-08 22:43:44 +11:00
Joshua Spence
5492f0fad6 Minor improvements to ArcanistPhpLinter
Summary: Various minor improvements to `ArcanistPhpLinter`.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11265
2015-01-08 19:40:37 +11:00
Joshua Spence
a232f7d5b8 Improvements to LINT_IMPLICIT_VISIBILITY
Summary: See D10687#104589.

Test Plan: Wrote unit tests.

Reviewers: richardvanvelzen, epriestley, #blessed_reviewers

Reviewed By: richardvanvelzen, epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11267
2015-01-08 07:30:31 +11:00
Joshua Spence
2ad2c04774 Simplify version parsing for ArcanistPhpLinter
Summary: We can use `php --run "echo phpversion();"` to return a nice, compact representation of the PHP version.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11264
2015-01-07 23:55:34 +11:00
Joshua Spence
ff3d3792ba Minor tidying of the ArcanistXMLLinter class
Summary: Self-explanatory

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11263
2015-01-07 23:55:08 +11:00
Joshua Spence
a28bf87633 Fix unit parser tests
Summary: This directory was missed, but should have been moved in D11202.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11262
2015-01-07 23:54:49 +11:00
Joshua Spence
4f5203375e Add a linter rule to detect implict method visibility
Summary: I'm not sure whether this is in any coding standards, but it seems to be an unspoken rule that methods should have a visiblity modifier (`public`, `protected` or `private`) explicitly specified. According to the [[http://php.net/manual/en/language.oop5.visibility.php#language.oop5.visiblity-methods | PHP documentation]], methods declared without any explicit visibility keyword are defined as public.

Test Plan: Added a test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D10687
2015-01-07 07:42:41 +11:00
Joshua Spence
fbefe61fb9 Use PhutilLibraryTestCase
Summary: Depends on D11231.

Test Plan: `arc unit`

Reviewers: btrahan, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11232
2015-01-07 07:37:59 +11:00
Joshua Spence
828fb41ea7 Fix visibility of various ArcanistLinter methods
Summary: Ref T6822.

Test Plan: Visual inspection. These methods are only called from within the `ArcanistExternalLinter` and `ArcanistLinter` subclasses.

Reviewers: chad, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11237
2015-01-07 07:36:07 +11:00
Joshua Spence
c70009d90f Fix visibility of various ArcanistWorkflow methods
Summary: Ref T6822.

Test Plan: Visual inspection. These methods are only called from within the `ArcanistShellCompleteWorkflow` class.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T6822

Differential Revision: https://secure.phabricator.com/D11238
2015-01-07 07:36:00 +11:00
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