Skip to content

Commit 01de622

Browse files
gsherwoodgsherwood
committed
Fixed bug #2478 : FunctionCommentThrowTag.WrongNumber when exception is thrown once but built conditionally
1 parent f5586fd commit 01de622

File tree

3 files changed

+115
-82
lines changed

3 files changed

+115
-82
lines changed

‎package.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
2626
</stability>
2727
<license uri="https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt">BSD 3-Clause License</license>
2828
<notes>
29+
- Fixed bug #2478 : FunctionCommentThrowTag.WrongNumber when exception is thrown once but built conditionally
2930
- Fixed bug #2479 : Generic.WhiteSpace.ScopeIndent error when using array destructuring with exact indent checking
3031
</notes>
3132
<contents>

‎src/Standards/Squiz/Sniffs/Commenting/FunctionCommentThrowTagSniff.php

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function process(File $phpcsFile, $stackPtr)
5858

5959
$stackPtrEnd = $tokens[$stackPtr]['scope_closer'];
6060

61-
// Find all the exception type token within the current scope.
61+
// Find all the exception type tokens within the current scope.
6262
$thrownExceptions = [];
6363
$currPos = $stackPtr;
6464
$foundThrows = false;
@@ -78,11 +78,11 @@ public function process(File $phpcsFile, $stackPtr)
7878

7979
/*
8080
If we can't find a NEW, we are probably throwing
81-
a variable.
81+
a variable or calling a method.
8282
83-
If we're throwing the same variable as the exception container
84-
from the nearest 'catch' block, we take that exception, as it is
85-
likely to be a re-throw.
83+
If we're throwing a variable, and it's the same variable as the
84+
exception container from the nearest 'catch' block, we take that exception
85+
as it is likely to be a re-throw.
8686
8787
If we can't find a matching catch block, or the variable name
8888
is different, it's probably a different variable, so we ignore it,
@@ -91,18 +91,25 @@ public function process(File $phpcsFile, $stackPtr)
9191
*/
9292

9393
$nextToken = $phpcsFile->findNext(T_WHITESPACE, ($currPos + 1), null, true);
94-
if ($tokens[$nextToken]['code'] === T_NEW) {
95-
$currException = $phpcsFile->findNext(
96-
[
97-
T_NS_SEPARATOR,
98-
T_STRING,
99-
],
100-
$currPos,
101-
$stackPtrEnd,
102-
false,
103-
null,
104-
true
105-
);
94+
if ($tokens[$nextToken]['code'] === T_NEW
95+
|| $tokens[$nextToken]['code'] === T_NS_SEPARATOR
96+
|| $tokens[$nextToken]['code'] === T_STRING
97+
) {
98+
if ($tokens[$nextToken]['code'] === T_NEW) {
99+
$currException = $phpcsFile->findNext(
100+
[
101+
T_NS_SEPARATOR,
102+
T_STRING,
103+
],
104+
$currPos,
105+
$stackPtrEnd,
106+
false,
107+
null,
108+
true
109+
);
110+
} else {
111+
$currException = $nextToken;
112+
}
106113

107114
if ($currException !== false) {
108115
$endException = $phpcsFile->findNext(

‎src/Standards/Squiz/Tests/Commenting/FunctionCommentThrowTagUnitTest.inc

Lines changed: 90 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -412,75 +412,100 @@ abstract class SomeClass {
412412
}
413413

414414
class SomeClass {
415-
/**
416-
* Validates something.
417-
*
418-
* @param string $method The set method parameter.
419-
*
420-
* @return string The validated method.
421-
*
422-
* @throws Prefix_Invalid_Argument_Exception The invalid argument exception.
423-
* @throws InvalidArgumentException The invalid argument exception.
424-
*/
425-
protected function validate_something( $something ) {
426-
if ( ! Prefix_Validator::is_string( $something ) ) {
427-
throw Prefix_Invalid_Argument_Exception::invalid_string_parameter( $something, 'something' );
428-
}
429-
430-
if ( ! in_array( $something, $this->valid_http_something, true ) ) {
431-
throw new InvalidArgumentException( sprintf( '%s is not a valid HTTP something', $something ) );
432-
}
433-
434-
return $something;
435-
}
415+
/**
416+
* Validates something.
417+
*
418+
* @param string $method The set method parameter.
419+
*
420+
* @return string The validated method.
421+
*
422+
* @throws Prefix_Invalid_Argument_Exception The invalid argument exception.
423+
* @throws InvalidArgumentException The invalid argument exception.
424+
*/
425+
protected function validate_something( $something ) {
426+
if ( ! Prefix_Validator::is_string( $something ) ) {
427+
throw Prefix_Invalid_Argument_Exception::invalid_string_parameter( $something, 'something' );
428+
}
429+
430+
if ( ! in_array( $something, $this->valid_http_something, true ) ) {
431+
throw new InvalidArgumentException( sprintf( '%s is not a valid HTTP something', $something ) );
432+
}
433+
434+
return $something;
435+
}
436+
437+
/**
438+
* Comment
439+
*
440+
* @throws Exception1 Comment.
441+
* @throws Exception2 Comment.
442+
* @throws Exception3 Comment.
443+
*/
444+
public function foo() {
445+
switch ($foo) {
446+
case 1:
447+
throw Exception1::a();
448+
case 2:
449+
throw Exception1::b();
450+
case 3:
451+
throw Exception1::c();
452+
case 4:
453+
throw Exception2::a();
454+
case 5:
455+
throw Exception2::b();
456+
default:
457+
throw new Exception3;
458+
459+
}
460+
}
436461
}
437462

438463
namespace Test\Admin {
439-
class NameSpacedClass {
440-
/**
441-
* @throws \ExceptionFromGlobalNamespace
442-
*/
443-
public function ExceptionInGlobalNamespace() {
444-
throw new \ExceptionFromGlobalNamespace();
445-
}
446-
447-
/**
448-
* @throws ExceptionFromSameNamespace
449-
*/
450-
public function ExceptionInSameNamespace() {
451-
throw new ExceptionFromSameNamespace();
452-
}
453-
454-
/**
455-
* @throws \Test\Admin\ExceptionFromSameNamespace
456-
*/
457-
public function ExceptionInSameNamespaceToo() {
458-
throw new ExceptionFromSameNamespace();
459-
}
460-
461-
/**
462-
* @throws \Different\NameSpaceName\ExceptionFromDifferentNamespace
463-
*/
464-
public function ExceptionInSameNamespaceToo() {
465-
throw new \Different\NameSpaceName\ExceptionFromDifferentNamespace();
466-
}
467-
}
464+
class NameSpacedClass {
465+
/**
466+
* @throws \ExceptionFromGlobalNamespace
467+
*/
468+
public function ExceptionInGlobalNamespace() {
469+
throw new \ExceptionFromGlobalNamespace();
470+
}
471+
472+
/**
473+
* @throws ExceptionFromSameNamespace
474+
*/
475+
public function ExceptionInSameNamespace() {
476+
throw new ExceptionFromSameNamespace();
477+
}
478+
479+
/**
480+
* @throws \Test\Admin\ExceptionFromSameNamespace
481+
*/
482+
public function ExceptionInSameNamespaceToo() {
483+
throw new ExceptionFromSameNamespace();
484+
}
485+
486+
/**
487+
* @throws \Different\NameSpaceName\ExceptionFromDifferentNamespace
488+
*/
489+
public function ExceptionInSameNamespaceToo() {
490+
throw new \Different\NameSpaceName\ExceptionFromDifferentNamespace();
491+
}
492+
}
468493
}
469494

470495
namespace {
471-
class GlobalNameSpaceClass {
472-
/**
473-
* @throws SomeGlobalException
474-
*/
475-
public function ThrowGlobalException() {
476-
throw new SomeGlobalException();
477-
}
478-
479-
/**
480-
* @throws \SomeGlobalException
481-
*/
482-
public function ThrowGlobalExceptionToo() {
483-
throw new SomeGlobalException();
484-
}
485-
}
496+
class GlobalNameSpaceClass {
497+
/**
498+
* @throws SomeGlobalException
499+
*/
500+
public function ThrowGlobalException() {
501+
throw new SomeGlobalException();
502+
}
503+
504+
/**
505+
* @throws \SomeGlobalException
506+
*/
507+
public function ThrowGlobalExceptionToo() {
508+
throw new SomeGlobalException();
509+
}
510+
}
486511
}

0 commit comments

Comments
 (0)