Summary:
I want to run lint on background and I'm interested only in side effect of caching (and maybe exit status).
This is better than discarding stdout later because we don't do unnecessary work and error conditions are still printed.
Test Plan:
$ arc lint --output none # with error
$ echo $?
$ arc lint --output none # with no lintable paths
$ arc lint --output none # witout errors
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5106
Summary:
The perf fix actually catches some real problems.
I didn't find anything in libphutil, Arcanist and Phabricator though.
Also bump version.
Also allow configuring the hook.
Test Plan:
Added a test, saw it fail with the old code.
Repeat for hook.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5097
Summary: At least on my sample file.
Test Plan: Saw time 0.073 s instead of 12.606 s.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5086
Summary:
The message is too defensive.
Test Plan: Tested in the fork.
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin, s.o.butler
Differential Revision: https://secure.phabricator.com/D5043
Summary: This is a little bit tricky - if both XHPAST and PhutilXHPAST linters lint the same path then they get the same future wrapped in two different Future iterators.
Test Plan:
$ arc unit
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5015
Summary: Nobody needs it because `raiseLintAtLine()` returns the message.
Test Plan:
$ arc unit
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4870
Summary:
Make all the broad-spectrum text linters use the new binary check.
I didn't touch `ComprehensiveLintEngine` because it's a nest of vipers and no one has complained; see T2039.
Test Plan: Ran `arc lint` on text files and binaries in a libphutil project (arcanist).
Reviewers: lisp
Reviewed By: lisp
CC: aran
Differential Revision: https://secure.phabricator.com/D5040
Summary:
Add linting capability for detecting files which contain
syntax introduced by unresolved merge conflicts. The detection is
file-type-agnostic (the only requirement is that the file is a text
file).
Test Plan:
Tested in three ways.
The first way is to add all three forms of syntax to a file to
indicate a merge conflict. HPHP will pick this up as a syntax error
before this linter reaches it.
The second way is to add the syntax in a comment. In that case, this
linter will show three warnings. For example:
$ arc lint ArcanistMergeConflictLinter.php
>>> Lint for arcanist/src/lint/linter/ArcanistMergeConflictLinter.php:
Warning (MERGECONFLICT1) Unresolved merge conflict
This syntax indicates there is still an unresolved merge conflict.
20
21 foreach ($lines as $lineno => $line) {
22 /*
>>> 23 >>>>>>>
24
25 =======
Warning (MERGECONFLICT1) Unresolved merge conflict
This syntax indicates there is still an unresolved merge conflict.
22 /*
23 >>>>>>>
24
>>> 25 =======
26
27 <<<<<<<
Warning (MERGECONFLICT1) Unresolved merge conflict
This syntax indicates there is still an unresolved merge conflict.
24
25 =======
26
>>> 27 <<<<<<<
28
29 */
The last test was to test on various different file types, including
JavaScript, PHP, an animated GIF, a PNG, and a Bash file to make sure
the file type detection worked. Each of the aforementioned tests
passed.
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, epriestley, Korvin
Maniphest Tasks: T2547
Differential Revision: https://secure.phabricator.com/D4966
Summary: This is technically documented, but not currently enforced and we aren't consistent about it in the codebase.
Test Plan: See D5002.
Reviewers: chad, vrana
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5003
Summary: D4963 for other linters.
Test Plan: Saw time 0.001 instead of 0.113 in spellcheck linter.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4965
Summary:
We always generate all messages and then filter them out based on minimum severity.
It's lots of useless work, especially in commit hook mode where we are interested only in errors.
Test Plan:
$ arc lint --cache 0 --severity error ArcanistXHPASTLinter.php
0.406 s before, 0.074 after
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4963
Summary: We only caught half of this.
Test Plan: Unit test.
Reviewers: edward
Reviewed By: edward
CC: aran
Maniphest Tasks: T1261
Differential Revision: https://secure.phabricator.com/D4920
Summary:
Restores linters only, without unit tests or entry in ComprehensiveLinter. Marks them deprecated.
If use at Facebook isn't widespread I'd prefer to simply delete them.
Test Plan: none
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2274
Differential Revision: https://secure.phabricator.com/D4906
Summary: Remove all references to ArcanistLicenseLinter and ArcanistApacheLicenseLinter.
Test Plan: Rerun the linter and ensure nothing is broken.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4901
Summary:
My original idea was to return safe HTML from this function.
But we are down to 20 occurrences in Phabricator and you shouldn't need this function in safe HTML world at all.
Test Plan:
$ arc lint src/applications/audit/controller/PhabricatorAuditListController.php
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4890
Summary: Raise deprecation warnings for these methods. I won't commit this until the phutil_tag branch merges.
Test Plan: Unit tests.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4770
Summary: It's not trivial to find them inside 700+ lines long functions.
Test Plan:
Linted `reused-iterators.lint-test` renamed to `_.php`, saw other locations.
Repeated for `reused-local.lint-test`.
Repeated for `duplicate-key-in-array.lint-test`.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4871
Summary: This is fairly confusing. Make the error message suggest the common remedy (update libphutil).
Test Plan: Eyeballed it.
Reviewers: Afaque_Hussain, btrahan, vrana
Reviewed By: Afaque_Hussain
CC: aran
Differential Revision: https://secure.phabricator.com/D4834
Summary:
We want to use it for `yield` and `invariant_violation()` which throws.
Having node instead of token would be better but this would be enough.
Test Plan: Implemented a hook in FB repo and added a test case there.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4821
Summary:
This disallows code like this:
$cmd = 'ls';
execx($cmd);
But I guess it's not that big deal?
Test Plan: Linted whole Arcanist and Phabricator codebases, most parts looks fixable.
Reviewers: epriestley
CC: nh, aran, Korvin
Differential Revision: https://secure.phabricator.com/D4794
Test Plan: Didn't see a fatal in new test case.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4783
Summary: This can be useful by itself, we want to use it in FB linter.
Test Plan: This diff.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4744
Summary: Also provides an example how to build custom linter using XHPAST.
Test Plan: Added debug output to `willLintPaths()`, verified that each path is parsed only once.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4718
Summary: The binary may not be built, in which case this raises a warning.
Test Plan: Will make @zeeg test.
Reviewers: zeeg, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4569
Summary: Also include binary hash in the version.
Test Plan: New unit test.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4535
Summary: This should be done for all external and configurable linters.
Test Plan: Linted file with lint problems, changed options, relinted.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4475
Summary:
Some errors (duplicate declaration, invalid number of arguments) have more related places.
We need to notify user if he changes any related place.
This could be currently achieved by triggering errors instead of warnings or by including both files in the range (impossible if the locations are in different files) or by issuing multiple errors.
All options are too aggressive.
Test Plan: Issued error on unmodified line with other location on modified line.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4392
Summary: This fix lets you run arc lint from any directory in the repository
Test Plan: Ran arc lint from any directory
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4356
Summary:
Lints cpp code using the cppcheck static linter. This linter needs to
be downloaded and built from http://cppcheck.sourceforge.net/
Test Plan: Used it on a few files.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4353
Summary: Adds arc lint support for cpp files with Google's cpplint.py lint checking.
Test Plan: ran it on some cpp files. Added unit tests
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4344
