1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-10 08:52:39 +01:00
Commit graph

1756 commits

Author SHA1 Message Date
Joshua Spence
e97cdc6c9a Remove "@stable" annotation
Summary: I don't believe that `@stable` is useful anymore?

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D13285
2015-06-15 07:51:43 +10:00
Joshua Spence
3c3438714b Further improvements to test discovery
Summary: I found another issue with T8042... it seems that tests from the root directory (i.e. `src/__tests__` are not being discovered properly). The handling of this case (`$library_path` being the library root directory) can probably be improved, and I am open to suggestions. Depends on D13202.

Test Plan: Added to existing test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13185
2015-06-15 07:23:17 +10:00
Joshua Spence
7d15b85a1b Mark some strings for translation
Summary: Add some more `pht`izations.

Test Plan: Eyeball it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13198
2015-06-09 07:17:39 +10:00
Joshua Spence
c36a825f5c Minor documentation improvements
Summary: Minor tidying of documentation and adding some groups.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13135
2015-06-08 11:31:35 +10:00
Joshua Spence
8c589f1f75 Improve test discovery with PhutilUnitTestEngine
Summary:
Ref T8042. Tests were not being discovered in two different scenarios:

  # Files modified at the same level as the library root directory.
  # "Normal" directories like `src/unit/engine`.

This regression was caused by D12689.

Test Plan: Added unit tests.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8042

Differential Revision: https://secure.phabricator.com/D13126
2015-06-03 21:07:17 +10:00
Joshua Spence
4754a3e156 Linter fixes
Summary: Apply various minor linter fixes.

Test Plan: `arc lint`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13106
2015-06-02 22:13:21 +10:00
epriestley
4e83efb31d Fix a translation ("to ignore these changes...")
Summary: Fixes T8374.

Test Plan:
```
$ arc diff
You have untracked files in this working copy.

  Working copy: /Users/epriestley/dev/core/lib/arcanist/

  Untracked changes in working copy:
  (To ignore this change, add it to ".git/info/exclude".)
    derp

    Ignore this untracked file and continue? [y/N] ^C
```

Reviewers: btrahan, joshuaspence, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T8374

Differential Revision: https://secure.phabricator.com/D13096
2015-06-01 06:50:46 -07:00
Joshua Spence
0b1acf0dc0 Split the ArcanistXHPASTLinter into modular rules
Summary:
The `ArcanistXHPASTLinter` class is becoming quite bloated. This diff separates the class into one-class-per-rule, which makes everything much more modular. One downside to this decoupling is that code reuse between linter rules is much more difficult, although this only affects a very small number of linter rules.

There is still some further work that could be done here, but I defer this until a later diff:

  - Rewrite `ArcanistPhutilXHPASTLinter` using `ArcanistXHPASTLinterRule`.
  - Change the unit tests so that they are truly only testing a single linter rule.
  - Maybe improve the way in which linter configuration options are handled.
  - Make it easier to keep track of the linter rule IDs (see T6859).

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: johnny-bit, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10541
2015-06-01 15:49:16 +10:00
epriestley
cdaa0e32e4 Always return an array from ArcanistWorkflow->loadProjectRepository()
Summary: Fixes T8344. Prior to D12971, this always returned `array()`. It may now sometimes return `null`. Switch the behavior to be more similar to the old behavior.

Test Plan: Inspection.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T8344

Differential Revision: https://secure.phabricator.com/D13061
2015-05-28 16:10:59 -07:00
Joshua Spence
8fe013b0ec Allow PhutilUnitTestEngine to execute tests from a single path
Summary: Fixes T8042. Changes the way that `PhutilUnitTestEngine` discovers unit tests. In particular, if you only modify a single test case then there is no reason to run all other test cases within the same directory (which is the current behavior).

Test Plan: Added some unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8042

Differential Revision: https://secure.phabricator.com/D12689
2015-05-28 18:39:37 +10:00
Joshua Spence
2924ebb922 Send arcanist error output to STDERR
Summary: Currently, arcanist error output is sent to `STDOUT` instead of `STDOUT`. This is annoying because I am running `arc lint --everything --never-apply-patches --output=xml > checkstyle.xml` and the `checkstyle.xml` file is not valid XML.

Test Plan: Forced a linter to throw an exception and ran `arc lint >/dev/null`... saw error output.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D13043
2015-05-28 09:25:44 +10:00
epriestley
3ac80200e2 Push to staging areas when running "arc diff"
Summary: Ref T8238. If a staging area is configured for a repository (see D13019), push a copy of changes to it after creating a diff.

Test Plan: Ran `arc diff` with various options, saw applicable changes get pushed to staging.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: cburroughs, epriestley

Maniphest Tasks: T8238

Differential Revision: https://secure.phabricator.com/D13020
2015-05-27 10:27:03 -07:00
epriestley
a36dc81715 Provide more documentation for Arcanist upload stuff
Summary: Ref T8259. Make some highly-ambiguous methods like `setData()` more clear.

Test Plan: reading

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13038
2015-05-27 10:26:27 -07:00
epriestley
5c7b22e620 Use file.allocate to upload large files from arc diff
Summary: Fixes T8259. Depends on D13016. Use modern logic to support large file uploads.

Test Plan:
  - Diffed with some images, saw them show up in the diff.
  - Diffed with a 12MB binary, saw it upload in chunks.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13017
2015-05-27 10:26:12 -07:00
epriestley
877e7b6388 Move Conduit file upload logic into a separate class
Summary:
Ref T8259. Currently, `arc upload` uses new logic but `arc diff` uses older logic internally. This prevents `arc diff` from uploading files larger than 4MB to newer servers.

Split the upload logic apart so the two upload pathways can share it. Callers now build a list of FileDataRefs and hand them to an Uploader for upload.

Test Plan:
Ran `arc upload` on:

  - One file.
  - Several files.
  - Small files.
  - Big files.
  - Directories.
  - Unreadable files.
  - Files full of random data.
  - The same file over and over again.
  - The same big file over and over again.
  - Artificially broke `file.allocate` and redid some of the simple cases (large/chunked aren't expected to work in this case).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8259

Differential Revision: https://secure.phabricator.com/D13016
2015-05-27 10:25:53 -07:00
Nevogd
407af00ef6 Fix invalid parameter in phutil_console_wrap()
Summary:
Fixes T8314. Change the phutil_console_wrap() call to only have a single
string parameter.

Test Plan:
Ran `arc diff` added text into the title, summary and test plan fields,
but did not specify any reviewers. When prompted to continue, clicked 'No' and
saw the '(Message saved to commit message.)' string.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T8314

Differential Revision: https://secure.phabricator.com/D13015
2015-05-26 06:07:25 -07:00
Joshua Spence
64d03ff68b Remove arcanist projects from working copy code
Summary: Ref T7604. Remove "arcanist projects" from `ArcanistWorkingCopy` and a few other callsites. Depends on D12999.

Test Plan: Can't really think of how to test this.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12945
2015-05-26 07:08:56 +10:00
Joshua Spence
4c208cec16 Remove "project.name" from `.arcconfig"
Summary: Ref T7604. Remove the `project.name` configuration from `.arcconfig`. Depends on D12999.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D13003
2015-05-25 22:42:31 +10:00
Joshua Spence
92dbc36250 Remove reentrant code
Summary: Ref T7604. Remove the `reenter_if_this_is_arcanist_or_libphutil` function. Some discussion in D12945.

Test Plan: Ran `./bin/arc --trace help` and saw things happen as expected.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12999
2015-05-25 22:41:22 +10:00
Joshua Spence
59ef783f4a Remove call to "arcanist.projectinfo" from ArcanistWorkflow
Summary: Ref T7604. Remove call to the `arcanist.projectinfo` #conduit endpoint from `ArcanistWorkflow`. Depends on D12992.

Test Plan: Ran `arc which` and verified that repository information was present.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12971
2015-05-25 22:04:50 +10:00
Joshua Spence
410331db75 Fix some format strings
Summary: These `pht` calls use `%d` instead of `%s`.

Test Plan: Eyeball it.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12994
2015-05-25 21:24:40 +10:00
Joshua Spence
1f2b51c4a3 Fix sanity checks for patch workflow
Summary:
Ref T7604. Ref T8307. This was broken in D12962 because I only tested `arc patch --arcbundle`. Furthermore, this particular sanity check doesn't actually do anything now (see T8307).

Test Plan:
Ran `arc patch --nobranch D12971` successfully.

Auditors: epriestley
2015-05-25 18:41:28 +10:00
Joshua Spence
be9dd352be Remove call to "arcanist.projectinfo" from arc export
Summary:
Ref T7604. Remove a call to `arcanist.projectinfo` from `arc export`. This Conduit call was only used to detect the repository encoding, which we should be able to do locally anyway.

I was considering removing the `$projectName` from `ArcanistBundle` as well, but I'm not convinved that this is a good idea. Specifically, doing so would make it difficult to issue the "This patch is for the 'X' project, but the working copy belongs to the 'Y' project" error. We could potentially use the repository callsign for this purposes, but this isn't portable across installs.

Test Plan: I'm not quite sure how to test this. I suspect that this functionality isn't widely used anyway.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12962
2015-05-25 18:31:09 +10:00
Joshua Spence
9b7c6786cd Change visibility of some linter methods
Summary: Change the visibility of the `raiseLintAtSomething` methods, mainly to allow linter rules to raise linter messages (see D10541).

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12976
2015-05-24 07:45:01 +10:00
Joshua Spence
6f86866104 phtize a bunch more strings
Summary: I found a few strings that I had missed, using a mostly-broken-but-somewhat-okay custom linter ruler (https://secure.phabricator.com/differential/diff/30988/).

Test Plan: Intense eyeballing.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aurelijus, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12888
2015-05-22 17:09:56 +10:00
Joshua Spence
9edcaadb2a Remove an unused variable
Summary: Ref T7604. Remove an unused variable from `ArcanistDiffWorkflow`.

Test Plan: This diff, I guess.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12972
2015-05-22 17:09:18 +10:00
Joshua Spence
8244b3582a Improve behavior for detecting the failure to parse external linter output
Summary:
Currently, a lot of `ArcanistExternalLinter` subclasses have something like this:

```lang=php
if ($err && !$messages) {
  return false;
}
```

We can avoid this code duplication by moving this check to the `ArcanistExternalLinter` class.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11321
2015-05-22 07:30:17 +10:00
Joshua Spence
26c4f12a21 Allow getXHPASTTreeForPath to return the tree for an arbitrary path
Summary: This allows `ArcanistBaseXHPASTLinter::getXHPASTTreeForPath` to return the parse tree for a file that wasn't explicitly linted. In my particular use case, I am writing a linter rule to determine if it is necessary to import a PHP file with `require_once` (i.e. the file consists only of class/interface definitions which are autoloadable).

Test Plan: Tested externally using a custom linter.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12271
2015-05-21 15:08:39 +10:00
Joshua Spence
e72a43a992 Remove "commit hook mode" workarounds for ArcanistXHPASTLinter
Summary: Ref T7674. `ArcanistXHPASTLinter` has some workarounds for running in "commit hook" mode (which has since been removed). This linter should always have a working copy.

Test Plan: `arc unit`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7674

Differential Revision: https://secure.phabricator.com/D12956
2015-05-21 15:08:39 +10:00
Joshua Spence
af343e4ca6 Remove deprecated linter configuration
Summary:
Removes the deprecated method of configuring linters (via the `.arcconfig` file). There is one main caveat here:

- There is currently no convenient method by which to change the path for an external linter (T5057). This means that there is no direct replacement for the deprecated `lint.ruby.prefix` configuration. The workaround is to symlink these binaries into `arcanist/externals/bin`.

Test Plan: Wait a sufficient amount of time before landing this.

Reviewers: chad, epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: aik099, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10005
2015-05-21 07:01:37 +10:00
Joshua Spence
e3311f0832 Remove "arcanist projects" from arcanist
Summary: Ref T7604. Remove "arcanist projects". Depends on D12894.

Test Plan: This diff (so meta).

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12901
2015-05-20 11:29:41 +10:00
Joshua Spence
097c894d08 Add linter rule for invalid modifiers
Summary: Add some linter rule to detect invalid modifiers for class methods/properties.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12922
2015-05-20 09:45:13 +10:00
Joshua Spence
753705b2c5 Rename ArcanistPhutilTestCase to PhutilTestCase and Remove ArcanistTestCase
Summary: Ref T7977. The `ArcanistTestCase` class is pointless and can be replaced by `ArcanistPhutilTestCase`. Furthermore, it sorta makes sense to just rename `ArcanistPhutilTestCase` to `PhutilTestCase`. Depends on D12664 and D12666.

Test Plan: `arc unit`

Reviewers: avivey, #blessed_reviewers, epriestley

Reviewed By: avivey, #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12665
2015-05-20 09:40:24 +10:00
Joshua Spence
399f040018 Improve getLink method for unit tests
Summary:
Ref T7977. The `PhutilTestCase::getLink` method currently relies on arcanist projects instead of repositories. Instead, make this logic a bit smarter by looking up the base URI from `phabricator.uri` (currently it is hardcoded to `https://secure.phabricator.com`).

Ideally, we would pass `?repositories=$REPOSITORY_PHID` to `DiffusionSymbolController` as well, but I don't know if this is worth pursuing.

Test Plan: This diff.

Reviewers: avivey, #blessed_reviewers, epriestley

Reviewed By: avivey, #blessed_reviewers, epriestley

Subscribers: aurelijus, Korvin, epriestley

Maniphest Tasks: T7977

Differential Revision: https://secure.phabricator.com/D12664
2015-05-20 09:36:22 +10:00
Joshua Spence
1d72cfc944 Provide a getProjectRoot method for ArcanistLinter
Summary: Adds a `getProjectRoot()` method to the `ArcanistLinter` class, simplifying a bunch of code.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12474
2015-05-20 09:16:36 +10:00
Joshua Spence
9444072e21 Improve the robustness of the "alias function" linter rule
Summary: Improve the `ArcanistXHPASTLinter::LINT_ALIAS_FUNCTION` linter rule. Currently this rule does not correctly handle alias functions which are not strictly lowercase.

Test Plan: Added a test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12921
2015-05-20 08:36:36 +10:00
Joshua Spence
80691e0a66 Add a linter rule for modifier ordering
Summary: Fixes T7417. Adds `ArcanistXHPASTLinter::LINT_MODIFIER_ORDERING` for ensuring that method/property modifiers (`public`, `protected`, `private`, `static`, `abstract` and `final`) are consistently ordered. The modifier ordering that is enforced is consistent with [[http://www.php-fig.org/psr/psr-2 | PSR-2]].

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7417

Differential Revision: https://secure.phabricator.com/D12421
2015-05-20 07:26:16 +10:00
Joshua Spence
993166fa49 Further improvements to array comma linter rule
Summary:
Improve the `ArcanistXHPASTLinter::LINT_ARRAY_SEPARATOR` rule to be able to autofix the following code:

```lang=php
array(
  1,
  2,
  3, /* comment */ );
```

Test Plan: Added unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D12308
2015-05-20 07:05:34 +10:00
Joshua Spence
25855c4427 Don't explicitly use an interpreter for pep8
Summary: Depends on D9430. If we are using the system installation of `pep8`, then it probably unnecessary (and possibly wrong) to specify `python2.6` as the default interpreter.

Test Plan: Ran `arc unit`.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: vrusinov, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9474
2015-05-20 07:04:07 +10:00
Joshua Spence
5b2571f5ae Don't bundle PEP8
Summary: Currently, we bundle a specific version of `pep8` with Arcanist. Whilst maybe this made sense historically, as pointed out by @epriestley in D9412#6, there is no longer much point in doing so.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: talshiri, vrusinov, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D9430
2015-05-20 07:03:22 +10:00
Joshua Spence
73feffbe70 Remove the ArcanistConduitLinter
Summary: This linter is a relic from Facebook days. As Harbormaster becomes more useful (T1049) this linter should become obsolete. Instead of modernising this linter to be compatible with modern infrastructure, it is easier to just let it die.

Test Plan: N/A

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10533
2015-05-20 07:02:43 +10:00
Joshua Spence
9862d7c0cb Move the getFunctionCalls method to ArcanistBaseXHPASTLinter
Summary: Move the `getFunctionCalls` method to the `ArcanistBaseXHPASTLinter` so that it can be used by subclasses.

Test Plan: `arc unit`

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12472
2015-05-20 07:00:29 +10:00
epriestley
ddfdfce3f5 Fix CPP Lint severity
Summary: Currently all CPPLint issues are hard-coded to warning level, which prevents customising the severity in .arclint. Change to pick up the configured severity. Note that getLintMessageSeverity will call getDefaultMessageSeverity if nothing is configured for that error category.

Test Plan: Tested manually to confirm configured categories display with the correct severity and that non-configured ones return with the default severity (ERROR).

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9682
2015-05-19 07:58:59 -07:00
Joshua Spence
60a5a24033 Add a linter rule for invalid default parameters
Summary: See http://www.phpwtf.org/default-arguments-and-type-hinting.

Test Plan: Added test case.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12855
2015-05-19 06:59:50 +10:00
Joshua Spence
d0d2cf903c Change parameters for diffusion.getlintmessages call
Summary: Ref T7604. Change the parameter for `diffusion.getlintmessages` from `arcanistProject` to `repositoryPHID`.

Test Plan: Ran `arc --conduit-uri='http://phabricator.local lint --only-new=1`

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T7604

Differential Revision: https://secure.phabricator.com/D12900
2015-05-19 00:10:44 +10:00
Joshua Spence
4bb2eecdaf Add a linter rule for the instanceof operator
Summary: The `instanceof` operator expects the first argument to be an object instance. See http://www.phpwtf.org/instanceof-smart.

Test Plan: Added test case

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12856
2015-05-18 18:02:51 +10:00
Joshua Spence
0c9a037719 Fix arc which output
Summary: Fixes T8231. This was derped in some recent refactoring.

Test Plan: Ran `arc which` and saw expected output.

Reviewers: avivey, epriestley, #blessed_reviewers

Reviewed By: avivey, epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8231

Differential Revision: https://secure.phabricator.com/D12890
2015-05-18 09:40:43 +10:00
Joshua Spence
8c6e1284cc Improve ArcanistXHPASTLinter::LINT_TOSTRING_EXCEPTION
Summary:
The `ArcanistXHPASTLinter::LINT_TOSTRING_EXCEPTION` linter rule was introduced in D12854, but fails for the following:

  - Interfaces which declare the `__toString()` method.
  - Classes which mark the `__toString()` method as `abstract`.

Test Plan: Added test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12892
2015-05-18 09:14:55 +10:00
Joshua Spence
2422202ba4 Add a linter rule to prevent the __lambda_func function from being redeclared
Summary:
See http://phpsadness.com/sad/39. Declaring a function named `__lambda_func` prevents the `create_function` function from working. This is because `create_function` eval-declares the function `__lambda_func`, then modifies the symbol table so that the function is instead named `"\0lambda_".(++$i)`, and returns that name.

NOTE: Personally, I don't think that anyone should use `create_function`. However, despite this, I think it is reasonable that no function is named `__lambda_func` in case some external library relies on `create_function`.

Test Plan: Added test case.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D12870
2015-05-18 08:24:49 +10:00
Joshua Spence
1e3073a4f8 Add a linter rule to prevent exceptions from being thrown in a toString method
Summary: Fixes T8207. It is not possible to throw an exception from within the `__toString()` method. Add a linter rule to prevent this from happening.

Test Plan: Added test cases.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Maniphest Tasks: T8207

Differential Revision: https://secure.phabricator.com/D12854
2015-05-18 08:18:34 +10:00