Summary: Improve the behavior of `ArcanistTextLinter` in dealing with `LINT_DOS_NEWLINE`. In particular, provide replacement text so that these linter issues can be autofixed.
Test Plan: Added a unit test.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10472
Summary: Explicitly list PHP magic methods (methods beginning with `__`) instead of assuming that //all// methods beginning with `__` are okay in terms of naming conventions. These magic methods were obtained from http://us1.php.net/manual/en/language.oop5.magic.php.
Test Plan:
```
Warning (XHP9) Naming Conventions
Follow naming conventions: methods should be named using lowerCamelCase.
1 <?php
2
3 class Foo {
>>> 4 function __foo() {}
5
6 function _bar() {}
7 }
Warning (XHP9) Naming Conventions
Follow naming conventions: methods should be named using lowerCamelCase.
3 class Foo {
4 function __foo() {}
5
>>> 6 function _bar() {}
7 }
```
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10539
Summary: Self-explanatory, improve test coverage.
Test Plan: Wrote and executed unit tests.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10474
Summary: It seems that `severity.rules` does not override the default linter severities set by the `ArcanistLinter` itself. This seems incorrect and counter-intuitive.
Test Plan: Created a `"severity.rules": { "(.*)": "disabled" }"` configuration and verified that it was being applied.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10475
Summary: D10132 improved this check by eliminating some false postives, but assumes an `n_FUNCTION_CALL` always starts with `n_SYMBOL_NAME`. This is true for `f()`, but not true for `C::m()` or `$v()`.
Test Plan: Added and ran unit tests.
Reviewers: btrahan, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10467
Summary:
Fixes T5299. Currently, linting `src/future/http/HTTPSFuture.php` from rPHU will raise the following error:
```
Error (XHP31) Use Of PHP 5.3 Features
This codebase targets PHP 5.2.3, but `curlfile` was not introduced until
PHP 5.5.0.
532 // use this "@" stuff.
533
534 if (class_exists('CURLFile')) {
>>> 535 $file_value = new CURLFile((string)$tmp, $info['mime'], $info['name']);
536 } else {
537 $file_value = '@'.(string)$tmp;
538 }
```
However, since this class is being used conditionally, it //should// be fine and no linter errors should be being raised.
Test Plan: Added a test case.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5299
Differential Revision: https://secure.phabricator.com/D10132
Summary: Fix linting for spacing around default parameter assignment in function/method declaration so that `function foo($x = null) {}` is preferred in favor of `function foo($x=null) {}`.
Test Plan: Modified existing unit tests.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10429
Summary: Adds an XHPAST linter rule for empty block statements. Basically, if a block statement is empty then it is much neater if the opening (`{`) and closing (`}`) braces are adjacent. Maybe this is just my own personal preference, in which case we could reduce the default severity to `ArcanistLintSeverity::SEVERITY_DISABLED`.
Test Plan: Wrote unit tests. I had to modify a bunch of existing unit tests accordingly.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10434
Summary: Add some missing function parameter typehints to the `ArcanistPhutilXHPASTLinter` class.
Test Plan: Eyeball it.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10430
Summary: `pht`ize some string in `ArcanistPhutilXHPASTLinter` so that they are translatable.
Test Plan: Not 100% sure that I am doing it right...
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10431
Summary: Update the `puppet-lint` linter to support the latest version (v1.0.1). This version allows a custom configuration file to be passed via `--config`, which brings this linter into line with other linters.
Test Plan: Update test cases to pass.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10384
Summary: Adds a linter that checks php files for syntax errors using php -l
Test Plan:
Add a section to your .arclint file similar to:
```
"php": {
"type": "php",
"include": "(\\.php$)"
}
```
Then run arc lint on a php file with a syntax error.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, jacobwalker0814, Korvin
Differential Revision: https://secure.phabricator.com/D10370
Summary: flake8 v2.2.3 (released Aug 25) broke this. Just drop the stdin stuff.
Test Plan: User confirmed in IRC that this resolves the issue.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10358
Summary:
This adds a lint engine for `hlint`, which is the standard and most general Haskell lint tool around these days.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Test Plan: Install `hlint`, and run `arc unit`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Projects: #arcanist
Differential Revision: https://secure.phabricator.com/D10250
Summary:
We currently require an exact number of "~" characters, but this is needless and error-prone.
Instead, allow any number larger than 3.
Test Plan:
- Added meta test-case.
- Ran `arc unit --everything`.
Reviewers: btrahan, thoughtpolice
Reviewed By: thoughtpolice
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10251
Summary: Rename `ArcanistPHPCSLinterTestCase` to `ArcanistPhpcsLinterTestCase` for consistency with the class being tested.
Test Plan: `arc unit`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D10145
Summary: Ref T5640. In D9864, the data this linter pulls out of the map was changed, breaking "use of undeclared function" warnings.
Test Plan:
```
>>> Lint for src/future/FutureProxy.php:
Error (PHL1) Unknown Symbol
Use of unknown function 'qqqqqq'. Common causes are:
- Your libphutil/ is out of date.
This is the most common cause.
Update this copy of libphutil: /INSECURE/devtools/libphutil
- Some other library is out of date.
Update the library this symbol appears in.
- This symbol is misspelled.
Spell the symbol name correctly.
Symbol name spelling is case-sensitive.
- This symbol was added recently.
Run `arc liberate` on the library it was added to.
- This symbol is external. Use `@phutil-external-symbol`.
Use `grep` to find usage examples of this directive.
*** ALTHOUGH USUALLY EASY TO FIX, THIS IS A SERIOUS ERROR.
*** THIS ERROR IS YOUR FAULT. YOU MUST RESOLVE IT.
15 $this->setProxiedFuture($proxied);
16 }
17
>>> 18 qqqqqq();
19 }
20
21 public function setProxiedFuture(Future $proxied) {
```
Reviewers: joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5640
Differential Revision: https://secure.phabricator.com/D9954
Summary: The `ArcanistXHPASTLinter` is no longer //too// specific to Phabricator (Phabricator-specific XHPAST lints generally live in `ArcanistPhutilXHPASTLinter`) and //should// be better suited for general purpose usage.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9892
Summary: Change `==` to `===`. This is a little bit cleaner because we shouldn't need type coercion here.
Test Plan: Ran `arc unit`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9893
Summary: Fixes T5577. If the `ArcanistPhutilLibraryLinter` lints a PHP file that contains a syntax error, it will die horribly. Instead, force it to continue as if nothing was wrong.
Test Plan: Introduced a PHP syntax error. Ran `arc lint` and made sure the output looked reasonable.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5577
Differential Revision: https://secure.phabricator.com/D9865
Summary: Ref T5577. Decrease the priority of the `ArcanistPhutilLibraryLinter` such that it is lower than that of the `ArcanistXHPASTLinter`. There isn't any specific reason that we should do this, although it seems to generally be a reasonable idea.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5577
Differential Revision: https://secure.phabricator.com/D9866
Summary: Ref T5577. Modify `ArcanistPhutilLibraryLinter` to use `PhutilLibraryMapBuilder` instead of using an `ExecFuture` to run `phutil_rebuild_map.php`.
Test Plan: Ran `arc lint`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5577
Differential Revision: https://secure.phabricator.com/D9864
Summary: I'm pretty sure that `@group` annotations are useless now... I believe that they were originally used by Diviner?
Test Plan: Eye-balled it.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin, aurelijus
Differential Revision: https://secure.phabricator.com/D9855
Summary: After D9576, `LINT_PHP_53_FEATURES` and `LINT_PHP_54_FEATURES` are deprecated. They have been replaced by `LINT_PHP_COMPATIBILITY`.
Test Plan: `arc unit`
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9657
Summary: Currently, the `ArcanistSpellingLinter` loads data from `ArcanistSpellingDefaultData`, with no way to configure the linter from an `.arclint` file. Instead we should define a format for a "dictionary" file, of which the `ArvcanistSpellingLinter` can load and of which the paths are easily configured through `.arclint`.
Test Plan:
Updated the test case and ran `arc unit`.
NOTE: I have removed the `LINT_SPELLING_PICKY` and `LINT_SPELLING_IMPORTANT` constants and replaced them with `LINT_SPELLING_FULL` and `LINT_SPELLING_PARTIAL`. This was done because it simplifies the implementation considerably and makes customization of the `ArcanistSpellingLinter` simpler, but also because these constants were not widely used in the existing implementation.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9805
Summary:
https://github.com/mdevils/node-jscs/issues/444 has been resolved in commit [[0de667bac2 | 0de667bac2491ba04d930ff94e990965213ba36b]], which means that `jscs` can be easily run on files with any extension.
NOTE: JSCS version 1.5.7 is required.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9789
Summary: Fixes T5466. An image is an example of a binary which should //not// be executable. Modify the `ArcanistChmodLinter` to disallow certain blacklisted MIME types from being executable.
Test Plan: Created an executable image file and ran `arc lint` over this file.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: richardvanvelzen, epriestley, Korvin
Maniphest Tasks: T5466
Differential Revision: https://secure.phabricator.com/D9723
Summary: If JSHint encounters //too many// errors (by default, more than 50) errors, then it quits prematurely and raises an additional (rather unhelpful) "Too Many Errors" error. We should just ignore this additional error.
Test Plan: Added a test case.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9693
Summary:
The error context (a.k.a. "original text") as provided by `jshint` is not very useful and actually causes `arc lint` to display the lint message incorrectly.
{F169277}
The underlying problem here is that the error context from `jshint` contains the entire line from the input file rather than just the offending source code.
Test Plan: Ran `arc lint -- webroot/rsrc/js/core/behavior-hovercard.js` (in rP) and verified that the output looked reasonable.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9675
Summary: As discussed in D9097, the `ArcanistScalaSBTLinter` isn't //really// a linter. Eventually we should add support for a proper Scala linter, but I think that supporting the `ArcanistScalaSBTLinter` as-is is more effort than it is worth.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9664
Summary: This `LINT_PHP_53_FEATURES` should have been replaced with `LINT_PHP_COMPATIBILITY` in D9576.
Test Plan: Ran `arc unit`.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9655
Summary: See inline comments on D9576. Also replace `json_decode` with `phutil_json_decode`, for better error handling.
Test Plan: Make sure `arc unit` still works.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9656
Summary:
Ref T5297. Utilize `PhutilJSONParser` to create a native JSON linter (native in that it is pure PHP with no external dependencies).
Since [[https://github.com/Seldaek/jsonlint | JsonLint]] is literally a PHP port of [[https://github.com/zaach/jsonlint | jsonlint]], I figured that we should also deprecate `ArcanistJSONLintLinter`. `ArcanistJSONLinter` //should// behave identically to `ArcanistJSONLintLinter`.
Depends on D9634.
Test Plan: Ran the `ArcanistJSONLintLinter` unit tests.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5297
Differential Revision: https://secure.phabricator.com/D9628
Summary: This was broken in D9641. If `$config` is not set, then a `PhutilJSONParserException` will be thrown. The expected behaviour is that it is okay to not explicitly set any configuration.
Test Plan: Ran `arc unit`.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9643
Summary: Depends on D9634. `phutil_json_decode` now throws an exception on invalid JSON.
Test Plan: Ran the unit tests.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9641
Summary: Various tidying up of linting code.
Test Plan: `arc lint` and `arc unit` still pass.
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9625
Summary: Self explanatory. Also fixed the character offset to start at 1 instead of 0 (because the other linters seem to do this).
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9626
Summary: Fixes T5385. Provide a flexible means of setting a minimum PHP version for the `ArcanistXHPASTLinter`, instead of relying on `ArcanistXHPASTLinter::LINT_PHP_53_FEATURES` and `ArcanistXHPASTLinter::LINT_PHP_54_FEATURES`.
Test Plan: Fixed up and ran unit tests.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T5385
Differential Revision: https://secure.phabricator.com/D9576
Summary: This linter rule is very specific to the Phabricator and it is unlikely that it is being used elsewhere. We should move it to `ArcanistPhutilXHPASTLinter`, where other Phabricator-specific linting code lives.
Test Plan: Moved the corresponding test case.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9590
