Skip to content

Commit a3fe4e8

Browse files
committed
Generic/JSHint: add missing unit tests + allow running without Rhino
[Sniff completeness series PR] Notes: * The sniff, up to now, required `rhino` to run, but the `jshint` tool can also be run directly when installed via `npm`. I've now adjusted the sniff to allow for that, which makes providing the `rhino` path optional. * It may be worth it to check if `jshint` is available on Travis by default or do a `npm i jshint` in the Travis `before_script` section to install it, so the sniff will actually be tested properly.
1 parent 5ea1be7 commit a3fe4e8

File tree

4 files changed

+91
-9
lines changed

4 files changed

+91
-9
lines changed

package.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,10 @@ http://pear.php.net/dtd/package-2.0.xsd">
437437
<file baseinstalldir="PHP/CodeSniffer" name="InlineControlStructureUnitTest.js.fixed" role="test" />
438438
<file baseinstalldir="PHP/CodeSniffer" name="InlineControlStructureUnitTest.php" role="test" />
439439
</dir>
440+
<dir name="Debug">
441+
<file baseinstalldir="PHP/CodeSniffer" name="JSHintUnitTest.js" role="test" />
442+
<file baseinstalldir="PHP/CodeSniffer" name="JSHintUnitTest.php" role="test" />
443+
</dir>
440444
<dir name="Files">
441445
<file baseinstalldir="PHP/CodeSniffer" name="ByteOrderMarkUnitTest.inc" role="test" />
442446
<file baseinstalldir="PHP/CodeSniffer" name="ByteOrderMarkUnitTest.php" role="test" />

src/Standards/Generic/Sniffs/Debug/JSHintSniff.php

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,28 +51,36 @@ public function process(File $phpcsFile, $stackPtr)
5151
{
5252
$rhinoPath = Config::getExecutablePath('rhino');
5353
$jshintPath = Config::getExecutablePath('jshint');
54-
if ($rhinoPath === null || $jshintPath === null) {
54+
if ($rhinoPath === null && $jshintPath === null) {
5555
return;
5656
}
5757

58-
$fileName = $phpcsFile->getFilename();
59-
60-
$rhinoPath = escapeshellcmd($rhinoPath);
58+
$fileName = $phpcsFile->getFilename();
6159
$jshintPath = escapeshellcmd($jshintPath);
6260

63-
$cmd = "$rhinoPath \"$jshintPath\" ".escapeshellarg($fileName);
64-
exec($cmd, $output, $retval);
61+
if ($rhinoPath !== null) {
62+
$rhinoPath = escapeshellcmd($rhinoPath);
63+
$cmd = "$rhinoPath \"$jshintPath\" ".escapeshellarg($fileName);
64+
exec($cmd, $output, $retval);
65+
66+
$regex = '`^(?P<error>.+)\(.+:(?P<line>[0-9]+).*:[0-9]+\)$`';
67+
} else {
68+
$cmd = "$jshintPath ".escapeshellarg($fileName);
69+
exec($cmd, $output, $retval);
70+
71+
$regex = '`^(.+?): line (?P<line>[0-9]+), col [0-9]+, (?P<error>.+)$`';
72+
}
6573

6674
if (is_array($output) === true) {
6775
foreach ($output as $finding) {
6876
$matches = [];
69-
$numMatches = preg_match('/^(.+)\(.+:([0-9]+).*:[0-9]+\)$/', $finding, $matches);
77+
$numMatches = preg_match($regex, $finding, $matches);
7078
if ($numMatches === 0) {
7179
continue;
7280
}
7381

74-
$line = (int) $matches[2];
75-
$message = 'jshint says: '.trim($matches[1]);
82+
$line = (int) $matches['line'];
83+
$message = 'jshint says: '.trim($matches['error']);
7684
$phpcsFile->addWarningOnLine($message, $line, 'ExternalTool');
7785
}
7886
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
/* jshint undef: true, unused: true */
2+
3+
var foo = bar;
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
/**
3+
* Unit test class for the JSHint sniff.
4+
*
5+
* @author Juliette Reinders Folmer <phpcs_nospam@adviesenzo.nl>
6+
* @copyright 2019 Juliette Reinders Folmer. All rights reserved.
7+
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
8+
*/
9+
10+
namespace PHP_CodeSniffer\Standards\Generic\Tests\Debug;
11+
12+
use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
13+
use PHP_CodeSniffer\Config;
14+
15+
class JSHintUnitTest extends AbstractSniffUnitTest
16+
{
17+
18+
19+
/**
20+
* Should this test be skipped for some reason.
21+
*
22+
* @return void
23+
*/
24+
protected function shouldSkipTest()
25+
{
26+
$rhinoPath = Config::getExecutablePath('rhino');
27+
$jshintPath = Config::getExecutablePath('jshint');
28+
if ($rhinoPath === null && $jshintPath === null) {
29+
return true;
30+
}
31+
32+
return false;
33+
34+
}//end shouldSkipTest()
35+
36+
37+
/**
38+
* Returns the lines where errors should occur.
39+
*
40+
* The key of the array should represent the line number and the value
41+
* should represent the number of errors that should occur on that line.
42+
*
43+
* @return array<int, int>
44+
*/
45+
public function getErrorList()
46+
{
47+
return [];
48+
49+
}//end getErrorList()
50+
51+
52+
/**
53+
* Returns the lines where warnings should occur.
54+
*
55+
* The key of the array should represent the line number and the value
56+
* should represent the number of warnings that should occur on that line.
57+
*
58+
* @return array<int, int>
59+
*/
60+
public function getWarningList()
61+
{
62+
return [3 => 2];
63+
64+
}//end getWarningList()
65+
66+
67+
}//end class

0 commit comments

Comments
 (0)