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: 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: 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: 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
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: 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: 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: 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
Summary: flake8 is the better maintained combination of pep8 and pyflakes
Test Plan: There's a test!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, jack
Differential Revision: https://secure.phabricator.com/D4082
Summary:
Installations extend this.
Another solution would be to extend `ArcanistLinterTestCase` from `ArcanistArcanistLinterTestCase` and return null in `getLink()` to avoid code duplication but I prefer clean class hierarchy.
Test Plan:
$ arc unit
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3878
Summary: We could also inject the value from the test case config but this is simpler.
Test Plan:
$ arc unit src/lint/linter/ArcanistLicenseLinter.php
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3895
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in unit tests and LICENSE file.
Reviewers: epriestley, btrahan, edward
Reviewed By: epriestley
CC: aran, Korvin, davidrecordon
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3881
Summary: I use it more often without second parameter than with it.
Test Plan:
Lint of:
preg_quote('');
preg_quote('', '/');
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3647
Summary: I don't offer a replacement because `f() ?: 1` converted to `f() ? f() : 1` can cause side effects and whitespace issues.
Test Plan: New test.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3489
Summary:
`pht()` can be some random function.
Better solution would be to separate this linter to its own class but it would be slower.
Also use `PhutilLintEngine` as `lint.engine`.
Test Plan:
$ arc lint # on file with pht($a)
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3480
Summary: Add `ruby -wc` as a linter and raise Error whenever there's a syntax error
Test Plan: Just a few dumb unit tests.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3447
Test Plan:
Wrote "Posible" in the linter, let it change to "Possible".
New unit test.
Reviewers: jack, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3469
Summary: See D3449, comments, unit tests.
Test Plan: Unit tests.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D3463
Summary:
I tried to fixed it but I've given up.
See rP958e6cd109f3.
Test Plan:
$ arc unit
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3449
Summary: See D3296#1.
Test Plan:
New test.
Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3297
Summary:
I'm not sure if we are interested in this kind of linters ("Don't use slow function if there is a fast alternative").
I didn't find a case where `strpos() === 0` could be useful except [[ http://www.php.net/manual/en/mbstring.overload.php | mbstring.func_overload ]] craziness.
Test Plan:
New unit test.
Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3296
Summary: XHPAST doesn't currently parse most PHP 5.4 stuff, but it does parse this. Warn about it.
Test Plan:
Unit tests, and:
Error (XHP35) Use Of PHP 5.4 Features
The f()[...] syntax was not introduced until PHP 5.4, but this codebase
targets an earlier version of PHP. You can rewrite this expression using
idx().
365 public function lintPHP54Features($root) {
366
367 if (false) {
>>> 368 id()[0];
^
369 }
370
Reviewers: alanh, vrana
Reviewed By: alanh
CC: aran
Differential Revision: https://secure.phabricator.com/D3291
Summary:
I use `//~` for marking places which shouldn't be committed.
It also looks ugly.
Maybe it can be just a warning but we provide a patch so error is OK?
Test Plan: New test.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2824
Summary: We will extract them at some point, lint before it's too late.
Test Plan:
New test.
Linted all callsites.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2825
