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
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
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
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
Summary: With a special guest appearance from `ArcanistGeneratedLinter`.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11345
Summary: Fixes T5639. Allows the detection of undefined magic symbols, such as the use of `__DIR__` in a codebase that targets < PHP 5.3.0.
Test Plan: Added a test case.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Maniphest Tasks: T5639
Differential Revision: https://secure.phabricator.com/D11386
Summary: Fixes T6830. Don't require `n_STATEMENT` nested under `n_DECLARE` to be written using braces.
Test Plan: Added a test case.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6830
Differential Revision: https://secure.phabricator.com/D11350
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
