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

2246 commits

Author SHA1 Message Date
Joshua Spence
f2f5fb2508 Drop support for parsing non-URL differential IDs from commit message.
Summary: This has been a `TODO` for a few years now... no-one should be using the old-school (non-URL) syntax anymore.

Test Plan: I'm actually not sure if it's okay to drop support for this... please verify.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9595
2014-06-18 01:39:28 +10:00
Joshua Spence
4c99a65567 Allow PHP version to be customized with ArcanistXHPASTLinter
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
2014-06-18 01:30:15 +10:00
Joshua Spence
473c6d89d7 Move the "ragged classtree edge" linter rule to ArcanistPhutilXHPASTLinter.
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
2014-06-18 01:28:40 +10:00
Richard van Velzen
3810cdbbcc Let arc liberate throw on unsupported PHP features
Summary: Ref T4725.

Test Plan: add some files that have unsupported constructs all over the place and run `arc liberate`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4725

Differential Revision: https://secure.phabricator.com/D9585
2014-06-17 05:13:05 -07:00
Richard van Velzen
3201239e85 Correctly recognize "no bookmarks" for Mercurial
Summary: Fixes T5086.

Test Plan: remove all bookmarks and run `arc feature`

Reviewers: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5086

Differential Revision: https://secure.phabricator.com/D9588
2014-06-17 04:51:55 -07:00
Richard van Velzen
8d7ee9ee38 Let phutil_rebuild_map **really** be quiet
Summary: Depends on D9586. The progress bar is written to stderr which results in bad stuff happening when you try run `lint` (`PhutilLibraryLinter` calls it)

Test Plan: run `arc lint`, see no more errors

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9587
2014-06-16 15:49:08 -07:00
Tal Shiri
5a02012706 Allow specifying runtime configuration with --set-config key=value
Summary:
This is useful for wrapper scripts that want to customize arcanist's behavior without affecting the global configuration.
This can be implemented with arcanist_configuration entry in .arcconfig, however it is currently limited to
per-project settings, and this feature makes writing wrapper scripts a little easier.

Test Plan: arc diff --set-config editor=vim (yeah yeah, crappy test case)

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9442
2014-06-16 15:28:33 -07:00
Richard van Velzen
2ccf65353c Let phutil_rebuild_map show a progress bar
Summary: .. Instead of dots that are hard to count.

Test Plan: remove `.phutil_module_cache` and do `arc liberate`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9584
2014-06-16 15:27:59 -07:00
Joshua Spence
b1ddd0f03b Add an ArcanistJscsLinter.
Summary: Add a linter which wraps around [[https://github.com/mdevils/node-jscs | JSCS]].

Test Plan: Currently, we can't test this linter with out existing infrastructure. Specifically, `jscs` will only lint `*.js` files. I have done a fair bit of manual testing though.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9565
2014-06-17 06:40:36 +10:00
Joshua Spence
419a1247c2 Remove the deprecated ArcanistWorkCopyIdentity::getConfig method.
Summary: This method has been deprecated for a while now (see rARC0d212ccf5a7ac5fc75d2d3036cb3a0bee220fd02). It should be safe to remove.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9466
2014-06-17 06:39:47 +10:00
Ben Alpert
ee3baade6e Add --no-verify to git commit call for arc patch
Summary: I can't find an analogous flag for hg.

Test Plan: I don't have any precommit hooks set up on my machine so I can't test this easily, but I think this seems pretty harmless?

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9523
2014-06-16 11:39:58 -07:00
Joshua Spence
36698b92af Add constants and class methods to php_compat_info.json.
Summary: Allows the `ArcanistXHPASTLinter` to determine whether constants can be used, based on the target PHP version. Also added class methods to the compatibility information, although this isn't used yet (it is small anyway).

Test Plan: Created a test file that contained the `JSON_PRETTY_PRINT` constant. Verified that a linter error was raised.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9571
2014-06-17 01:00:23 +10:00
Joshua Spence
8f8ab969db Update update_compat_info.php to use PHP_CompatInfo version 3
Summary:
Fixes T5377. The current `scripts/update_compat_info.php` script works for PHP CompatInfo version 2, but doesn't work with the newer version 3.

There are a few breaking changes in version 3 that had to be addressed:

- PHP 5.3 is required. Whilst Arcanist is generally compatible with PHP 5.2, I don't think that having this dependency presents any real issues because it is purely a development tool that is rarely updated.
- [[https://getcomposer.org/ | Composer]] is used for packaging, which makes including the library slightly more complicated. Basically, I had to install `PHP_CompatInfo` globally with Composer (`composer global require "bartlett/php-compatinfo"` and then symlink `~/.composer/vendor` into `externals/includes`.

Test Plan: Compared the `resources/php_compat_info.json` file. There are a bunch of functions/classes that //were// in this file but are no longer, but I think that I've covered the most popular extensions.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5377

Differential Revision: https://secure.phabricator.com/D9568
2014-06-17 00:30:16 +10:00
Joshua Spence
497229acd8 Made some additional methods final.
Summary: Just some minor tidying up. I don't think that these methods should ever be overridden in subclasses.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9569
2014-06-17 00:30:04 +10:00
Joshua Spence
523d767646 Add a unit test to check src/__phutil_library_map__.php.
Summary: See D9557. Add a unit test to ensure that `src/__phutil_library_map__.php` is up-to-date.

Test Plan: Ran `arc unit`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9558
2014-06-16 05:42:43 +10:00
Joshua Spence
4371cd81f5 Restore PEP8 errors.
Summary: As per the TODO comment, now that `.arclint` is more mature, we should be able to restore PEP8 errors.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9473
2014-06-16 04:50:59 +10:00
Joshua Spence
45c66a1a7b Fix ArcanistLesscLinter::getVersion function.
Summary:
This `ArcanistLesscLinter::getCacheVersion` should have been renamed to `getVersion` as of D8971, but wasn't updated.

Also fix the regex to properly capture version information.

Test Plan: It's pretty difficult to test this stuff.... I have an idea that I will submit a separate diff for.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9475
2014-06-16 04:50:15 +10:00
Joshua Spence
af8bdb2b94 Update .gitignore.
Summary: See inline comments for reasoning.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9528
2014-06-14 11:44:38 -07:00
epriestley
dfe2bde88d Dirty cache of HEAD commit after an amend/range reload
Summary:
Introducing `--head` caused us to run `git diff base..head` explicitly.

However, we can now hit this workflow:

  - We resolve `HEAD` as commit `aaaa1`.
  - This is cached.
  - We notice dirty working copy changes and prompt the user to amend them to HEAD.
  - The user accepts the amend.
  - We amend, creating commit `bbbb2`.
  - We dirty the commit range and reload the working copy. This //does not// dirty the cache of HEAD.
  - We run `git diff`, but it uses the old cached HEAD: `git diff base..aaaa1`.
  - This works fine (`aaaa1` still exists, it's just not on any branch) but produces the wrong diff (without amended changes).

To resolve this, implement the "dirty the cache when the range reloads" hook.

Also never try to amend if the user provides `--head`.

Test Plan:
Ran `arc diff --only --trace` in a working copy with a new commit and some uncommitted changes.

  - Prior to this change, saw a `git diff base..aaaa1` command and the wrong diff.
  - After this change, saw a `git diff base..bbbb2` command and the correct diff.

Reviewers: chad, csilvers, talshiri

Reviewed By: talshiri

Subscribers: epriestley, spicyj

Differential Revision: https://secure.phabricator.com/D9506
2014-06-12 15:46:15 -07:00
epriestley
9492b4ecba Tweak error and status messages for commit ranges
Summary: Improve/clarify some error messages a bit, hopefully.

Test Plan: Ran `arc which`, `arc diff`, etc., with various explicit, implicit, and `--head` flags. Read error messages, didn't catch anything too awkward.

Reviewers: talshiri

Reviewed By: talshiri

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9489
2014-06-11 16:35:50 -07:00
Tal Shiri
6b192f3178 --range support for git
Summary:
This adds support for passing range of commits for arc diff. This is useful when you want to submit code reviews for past commits without mucking around with the working copy.

This will probably require changes :)

Test Plan: Tested locally, but totally need to add tests for this

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9369
2014-06-11 14:37:01 -07:00
Joshua Spence
08bad11bc8 Remove the ArcanistBaseWorkflow::getUserGUID function.
Summary: This function has been deprecated for a long time (see rARC8150fdf044818c503a588c1b3ef0ddbb93cfa1be). It should be safe to remove it now.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9465
2014-06-11 05:51:06 -07:00
Joshua Spence
b189b594d2 Present case of class and interface names.
Summary: Currently, class and interface names are converted to lowercase when comparing minimum and maximum versions. This is necessary in order to compare with the data from `php_compat_info.json` but causes the lint message to be slightly misleading.

Test Plan:
Linted a test file.

**Before**
```
   Error  (XHP31) Use Of PHP 5.3 Features
    This codebase targets PHP 5.2.3, but `iterator` was not introduced until
    PHP 5.3.0.

               1 <?php
               2
    >>>        3 final class FooBar implements Iterator {}
```

**After**
```
   Error  (XHP31) Use Of PHP 5.3 Features
    This codebase targets PHP 5.2.3, but `Iterator` was not introduced until
    PHP 5.3.0.

               1 <?php
               2
    >>>        3 final class FooBar implements Iterator {}
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9471
2014-06-11 05:39:51 -07:00
Joshua Spence
4c06d1b9e3 [LATER] Remove deprecated mark-committed workflow.
Summary: This workflow has been deprecated for a long time now. At some stage, it should be removed.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9463
2014-06-10 18:29:00 -07:00
Joshua Spence
02e3905cf5 Don't lint symlinks by default.
Summary: Fixes T5300. Currently, if a dead symbolic link is linted, all kinds of errors will be thrown by most linters because they will try to read the (non-existent) file contents. Instead, let's not lint symbolic links by default. In the case that the target of a symbolic link is inside the working copy, then it should be being linted anyway.

Test Plan: Created a symbolic link and verified that it wasn't linted (by any linter other than the `ArcanistFilenameLinter`).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5300

Differential Revision: https://secure.phabricator.com/D9448
2014-06-10 18:20:59 -07:00
Joshua Spence
ff1915ecff Apply various linter fixes.
Summary: Applied various linter fixes. Also make the `.editorconfig` file a bit more specific. Unfortunately, `arc lint --apply-patches` currently modifies some test data that it shouldn't, but this should be fixed after T5105.

Test Plan: Ran `arc unit` to make sure things weren't broken.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin, aurelijus

Differential Revision: https://secure.phabricator.com/D9440
2014-06-10 11:02:42 -07:00
Richard van Velzen
b60eaa6487 Make retrieving parents for mercurial commits work consistently across platforms
Summary:
Currently the template is single quoted, but Windows only supports double quotes. This meant that the output would be like:
  lang=text
  'aabbccddeeffaabbccddeeffaabbccddeeff0123
  '

Which is clearly wrong.

This is displayed like that in Phabricator as well, which is confusing.

Test Plan: ran `arc diff` on a Windows machine and saw the correct behaviour.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9450
2014-06-10 10:20:39 -07:00
Joshua Spence
57f4bbae42 Allow spaces after the . operator if followed by a comment.
Summary: Currently, `'foo'.    // Some comment` is not allowed by the `ArcanistXHPASTLinter::LINT_BINARY_EXPRESSION_SPACING` rule. I believe that in the case of a trailing comment, we //should// allow whitespace after the `.` operator.

Test Plan: Wrote and executed a unit test for this case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9438
2014-06-09 12:25:14 -07:00
Joshua Spence
833ce45155 Lint the ArcanistSpellingDefaultData class.
Summary:
Currently, the `ArcanistSpellingDefaultData` is marked with a `@nolint` annotation. This means that it doesn't get linted by any linters. Really, the intention here was to make sure that this file isn't linted by the `ArcanistSpellingLinter` linter.

Now that the `.arclint` file is more mature, we can easily just exclude this file from being linted //only// by the spelling linter, whilst allowing other linters to run.

Test Plan: Ran `arc lint`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9432
2014-06-09 11:36:00 -07:00
Joshua Spence
86eec5b44c Fix "array to string" conversion error.
Summary: Fixes T5298. This bug was probably introduced in D9248. It looks like I forgot to update some references to `$version`.

Test Plan: Ran `arc lint --everything` in rPHU.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5298

Differential Revision: https://secure.phabricator.com/D9434
2014-06-09 11:32:23 -07:00
Joshua Spence
bec13cfea8 Update PEP8.
Summary: Update `pep8.py` from v1.3.4 to v1.5.7. Because why not?

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9412
2014-06-07 11:31:26 -07:00
epriestley
50caec620a Point github.com/facebook URIs at github.com/phacility (Arcanist Edition)
Summary: See D9328.

Test Plan: `grep`

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9329
2014-05-29 09:15:33 -07:00
Joshua Spence
7b61faa192 Add a version number to ArcanistXHPASTLinter.
Summary: Fixes T5209. It makes sense for the `ArcanistXHPASTLinter` to implement the `getVersion` method in order to be able to enforce version requirements (as in T4954).

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5209

Differential Revision: https://secure.phabricator.com/D9317
2014-05-28 06:33:58 -07:00
epriestley
ff97a77786 Add lint rules for => (fat arrow) and . (string concatenation)
Summary: We have coverage for normal binary operators, but not these unusual cases.

Test Plan: Added and executed unit tests.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9314
2014-05-27 16:16:39 -07:00
Joshua Spence
2881686407 Handle arc --version.
Summary: It seems reasonable to transform `arc --version` into `arc version`. The version command is similar to the help command in that it is a command that users unfamiliar with Arcanist will probably try to run at some stage.

Test Plan: Ran `arc --version`.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9274
2014-05-23 19:54:04 -07:00
Joshua Spence
b7bb6c8348 Add a version workflow to arcanist.
Summary: A `version` workflow would be useful, especially for less technical users. Additionally, whenever I am faced with a new command I reasonably expect `$CMD [--help|help]` and `$CMD [--version|version]` to work.

Test Plan: Ran `arc version`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9265
2014-05-23 19:35:33 -07:00
epriestley
0d0b8abcdd set custom arcrc file earlier
Summary: It appears to have never really work? At least as far as phabricator.uri (empirically).

Test Plan:
removed ~/.arcrc, run call-conduit user.whoami with --arcrc-file and --trace.
set-config, get-config and alias also read and write to the right place now.

Reviewers: avivey

Reviewed By: avivey

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9263
2014-05-23 14:06:29 -07:00
Joshua Spence
17820442da Change double quotes to single quotes.
Summary: Ran `arc lint --apply-patches --everything` over rARC, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.

Test Plan: Eyeballed //most// of the diff.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin, aurelijus

Differential Revision: https://secure.phabricator.com/D9269
2014-05-23 13:53:05 -07:00
epriestley
e34bdf6c74 Fix diffusion.looksoon call to actually work
Summary:
This is all derped up, and we ignore the error so old installs don't break.

Underp it.

Test Plan: Did `arc land`, verified that the call actually went through successfully.

Reviewers: btrahan, davedash

Reviewed By: davedash

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9271
2014-05-23 11:00:07 -07:00
Joshua Spence
042aa2ee38 Add an ArcanistGoLintLinter.
Summary: Create bindings for `golint` as an external linter.

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/D9246
2014-05-23 07:59:57 -07:00
Joshua Spence
85b8b2e5f8 Make arc shell-complete more robust.
Summary: Fixes T5122. Add some basic error handling to the `shell-complete` workflow.

Test Plan: `arc shell-complete` no longer throws errors.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5122

Differential Revision: https://secure.phabricator.com/D9268
2014-05-23 07:59:23 -07:00
epriestley
f64dee022e Fix "arc linters" if no engine is built or configured
Summary: `$engine` will be undefined here. This was just copy/paste derp, combined with me accidentally having an `.arclint` file in a place I didn't expect to have one when I tried to test this.

Test Plan: Ran `arc linters` in a totally bare, non-.arclint-having working copy

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9255
2014-05-22 10:36:31 -07:00
Joshua Spence
e0ee3e8a07 Capture maximum version information in php_compat_info.
Summary: Ref T5141. In order to be able to warn when deprecated functions are used, we need to be aware of from which version the functions were deprecated.

Test Plan: Ran `arc lint` and made sure no unexpected warnings were raised.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5141

Differential Revision: https://secure.phabricator.com/D9248
2014-05-22 09:22:36 -07:00
Joshua Spence
f13aa21b8e Capture a wider range of version information in php_compat_info.json.
Summary: Ref T5141. Currently, `php_compat_info.json` is hardcoded to support PHP 5.2.3. Instead, store as much version information as possible in `php_compat_info.json` and filter accordingly in `ArcanistXHPASTLinter.`

Test Plan: Ran `arc lint` and made sure no additional warnings were raised.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5141

Differential Revision: https://secure.phabricator.com/D9247
2014-05-22 09:14:14 -07:00
Joshua Spence
bca14a368b Fix the wrong argument type being passed to setFlags.
Summary:
Currently, the `ComprehensiveLintEngine` fails to lint `*.py` files with the following exception:

```
> arc lint foo.py
[2014-05-22 01:17:12] ERROR 2: escapeshellarg() expects parameter 1 to be string, array given at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #0 escapeshellarg(Array { 0 => --ignore=E101,E501,W291,W292,W293 })
  #1 array_map(escapeshellarg, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #2 xsprintf_command(Array { unmasked => false }, %s %s, 4, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #3 xsprintf(xsprintf_command, Array { unmasked => false }, Array of size 3 starting with: { 0 => %C %Ls }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #4 PhutilCommandString::renderString(false) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:24]
  #5 PhutilCommandString::getMaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:12]
  #6 PhutilCommandString::__construct(Array of size 3 starting with: { 0 => %C %Ls }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:34]
  #7 csprintf(%C %Ls, Object PhutilCommandString, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/ArcanistExternalLinter.php:400]
  #8 ArcanistExternalLinter::buildFutures(Array { 0 => foo.py }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/ArcanistFutureLinter.php:17]
  #9 ArcanistFutureLinter::willLintPaths(Array { 0 => foo.py }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/engine/ArcanistLintEngine.php:297]
  #10 ArcanistLintEngine::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/src/workflow/ArcanistLintWorkflow.php:336]
  #11 ArcanistLintWorkflow::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/scripts/arcanist.php:322]
[2014-05-22 01:17:12] ERROR 2: escapeshellarg() expects parameter 1 to be string, array given at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #0 escapeshellarg(Array { 0 => --ignore=E101,E501,W291,W292,W293 })
  #1 array_map(escapeshellarg, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #2 xsprintf_command(Array { unmasked => false }, %s %s, 4, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #3 xsprintf(xsprintf_command, Array { unmasked => false }, Array of size 3 starting with: { 0 => %C %Ls }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #4 PhutilCommandString::renderString(false) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:24]
  #5 PhutilCommandString::getMaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:65]
  #6 xsprintf_command(Array { unmasked => false }, %C %C, 1, Object PhutilCommandString, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #7 xsprintf(xsprintf_command, Array { unmasked => false }, Array of size 3 starting with: { 0 => %C %C }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #8 PhutilCommandString::renderString(false) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:24]
  #9 PhutilCommandString::getMaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:12]
  #10 PhutilCommandString::__construct(Array of size 3 starting with: { 0 => %C %C }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:34]
  #11 csprintf(%C %C, Object PhutilCommandString, Object PhutilCommandString)
  #12 call_user_func_array(csprintf, Array of size 3 starting with: { 0 => %C %C }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/exec/ExecFuture.php:72]
  #13 ExecFuture::__construct(%C %C, Object PhutilCommandString, Object PhutilCommandString) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/ArcanistExternalLinter.php:414]
  #14 ArcanistExternalLinter::buildFutures(Array { 0 => foo.py }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/ArcanistFutureLinter.php:17]
  #15 ArcanistFutureLinter::willLintPaths(Array { 0 => foo.py }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/engine/ArcanistLintEngine.php:297]
  #16 ArcanistLintEngine::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/src/workflow/ArcanistLintWorkflow.php:336]
  #17 ArcanistLintWorkflow::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/scripts/arcanist.php:322]
[2014-05-22 01:17:12] ERROR 2: escapeshellarg() expects parameter 1 to be string, array given at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #0 escapeshellarg(Array { 0 => --ignore=E101,E501,W291,W292,W293 })
  #1 array_map(escapeshellarg, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #2 xsprintf_command(Array { unmasked => false }, %s %s, 4, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #3 xsprintf(xsprintf_command, Array { unmasked => false }, Array of size 3 starting with: { 0 => %C %Ls }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #4 PhutilCommandString::renderString(false) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:24]
  #5 PhutilCommandString::getMaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:65]
  #6 xsprintf_command(Array { unmasked => false }, %C %C, 1, Object PhutilCommandString, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #7 xsprintf(xsprintf_command, Array { unmasked => false }, Array of size 3 starting with: { 0 => %C %C }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #8 PhutilCommandString::renderString(false) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:24]
  #9 PhutilCommandString::getMaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:16]
  #10 PhutilCommandString::__toString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/exec/ExecFuture.php:611]
  #11 ExecFuture::isReady() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/FutureIterator.php:322]
  #12 FutureIterator::updateWorkingSet() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/FutureIterator.php:97]
  #13 FutureIterator::addFuture(Object ExecFuture, foo.py) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/ArcanistFutureLinter.php:18]
  #14 ArcanistFutureLinter::willLintPaths(Array { 0 => foo.py }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/engine/ArcanistLintEngine.php:297]
  #15 ArcanistLintEngine::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/src/workflow/ArcanistLintWorkflow.php:336]
  #16 ArcanistLintWorkflow::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/scripts/arcanist.php:322]
[2014-05-22 01:17:12] ERROR 2: escapeshellarg() expects parameter 1 to be string, array given at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #0 escapeshellarg(Array { 0 => --ignore=E101,E501,W291,W292,W293 })
  #1 array_map(escapeshellarg, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:87]
  #2 xsprintf_command(Array { unmasked => true }, %s %s, 4, Array { 0 => Array { 0 => --ignore=E101,E501,W291,W292,W293 } }, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #3 xsprintf(xsprintf_command, Array { unmasked => true }, Array of size 3 starting with: { 0 => %C %Ls }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #4 PhutilCommandString::renderString(true) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:20]
  #5 PhutilCommandString::getUnmaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/csprintf.php:63]
  #6 xsprintf_command(Array { unmasked => true }, %C %C, 1, Object PhutilCommandString, 5) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/xsprintf.php:63]
  #7 xsprintf(xsprintf_command, Array { unmasked => true }, Array of size 3 starting with: { 0 => %C %C }) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:33]
  #8 PhutilCommandString::renderString(true) called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/xsprintf/PhutilCommandString.php:20]
  #9 PhutilCommandString::getUnmaskedString() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/exec/ExecFuture.php:622]
  #10 ExecFuture::isReady() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/FutureIterator.php:322]
  #11 FutureIterator::updateWorkingSet() called at [/home/joshua/dotfiles/modules/phabricator/libphutil/src/future/FutureIterator.php:97]
  #12 FutureIterator::addFuture(Object ExecFuture, foo.py) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/linter/ArcanistFutureLinter.php:18]
  #13 ArcanistFutureLinter::willLintPaths(Array { 0 => foo.py }) called at [/home/joshua/workspace/github.com/facebook/arcanist/src/lint/engine/ArcanistLintEngine.php:297]
  #14 ArcanistLintEngine::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/src/workflow/ArcanistLintWorkflow.php:336]
  #15 ArcanistLintWorkflow::run() called at [/home/joshua/workspace/github.com/facebook/arcanist/scripts/arcanist.php:322]
```

Test Plan: Ran `arc lint foo.py` on an empty file.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9251
2014-05-21 18:23:21 -07:00
epriestley
07895c41c7 Fix warning in arc backout
Summary: Fixes T5145. Neither variable is ever used.

Test Plan: Searched for uses of either variable.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5145

Differential Revision: https://secure.phabricator.com/D9244
2014-05-21 13:07:11 -07:00
epriestley
c999f3e6b5 Fix XHPAST to detect use of undeclared variables in catch
Summary:
Currently, when code has a block like:

  } catch (Exception $ex) {

...we attempt to mark "$ex" as declared. However, we incorrectly mark every variable used anywhere in the block as declared. This means we'll never raise this warning in a `catch` block.

Instead //only// mark the caught exception as declared.

Test Plan: Added a failing unit test and made it pass.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: lpriestley, epriestley

Differential Revision: https://secure.phabricator.com/D9239
2014-05-21 12:23:51 -07:00
Joshua Spence
8274ffa44d Update PHP compatibility information.
Summary: Ref T5141. Generated using PHP v5.5.12 and PHP_CompatInfo v2.26.0.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5141

Differential Revision: https://secure.phabricator.com/D9236
2014-05-21 10:13:58 -07:00
James Rhodes
6b8552291d Handles an issue in windows with large code bases not squash merging correctly.
Summary: Handles an issue in windows with large code bases not squash merging correctly, this only catches the issue it doesn't really do any recovery.

Test Plan: We have been running this change in our work environment, it's a hard bug to replicate but when it has reared it's head this has caught it.

Reviewers: waynea, #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Maniphest Tasks: T4884

Differential Revision: https://secure.phabricator.com/D8729
2014-05-21 10:15:31 +10:00
epriestley
a19503bb59 Don't support severity customization in pyflakes
Summary: Pyflakes doesn't have any message codes, so we can't do anything with severities.

Test Plan: Ran `arc lint` with severities on one of these linters, got an error.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9222
2014-05-20 12:44:55 -07:00