Showing 77 of 86 total issues
Define a constant instead of duplicating this literal "en_US" 16 times. Confirmed
$this->from($dateArray, 'en_US', null, $timezone);
- Read upRead up
- Exclude checks
Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.
On the other hand, constants can be referenced from many places, but only need to be updated in a single place.
Noncompliant Code Example
With the default threshold of 3:
function run() { prepare('action1'); // Non-Compliant - 'action1' is duplicated 3 times execute('action1'); release('action1'); }
Compliant Solution
ACTION_1 = 'action1'; function run() { prepare(ACTION_1); execute(ACTION_1); release(ACTION_1); }
Exceptions
To prevent generating some false-positives, literals having less than 5 characters are excluded.
Argument 1 (hour)
is string
but \mktime()
takes int
Confirmed
$timestamp = mktime(date('H', $timestamp), date('i', $timestamp), date('s', $timestamp), date('n', $timestamp), date('j', $timestamp), date('Y', $timestamp));
- Exclude checks
Argument 5 (day)
is string
but \mktime()
takes int
Confirmed
$timestamp = mktime(date('H', $timestamp), date('i', $timestamp), date('s', $timestamp), date('n', $timestamp), date('j', $timestamp), date('Y', $timestamp));
- Exclude checks
Argument 3 (calendar)
is null
but \meysampg\intldate\IntlDateTrait::from()
takes string
defined at /code/IntlDateTrait.php:111
Confirmed
$this->from($dateArray, 'en_US', null, $timezone);
- Exclude checks
Refactor this function to reduce its Cognitive Complexity from 19 to the 15 allowed. Confirmed
private function parseDateTime($datetimeArray)
- Read upRead up
- Exclude checks
Cognitive Complexity is a measure of how hard the control flow of a function is to understand. Functions with high Cognitive Complexity will be difficult to maintain.
See
This branch's code block is the same as the block for the branch on line 548. Open
} elseif ($hour > 24 || $hour < 0) {
return false;
} elseif ($minute > 60 || $minute < 0) {
- Read upRead up
- Exclude checks
Having two cases
in a switch
statement or two branches in an if
chain with the same implementation is at
best duplicate code, and at worst a coding error. If the same logic is truly needed for both instances, then in an if
chain they should
be combined, or for a switch
, one should fall through to the other.
Noncompliant Code Example
switch ($i) { case 1: doSomething(); break; case 2: doSomethingDifferent(); break; case 3: // Noncompliant; duplicates case 1's implementation doSomething(); break; default: doTheRest(); } if ($a >= 0 && $a < 10) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else if ($a >= 20 && $a < 50) { doTheThing(); // Noncompliant; duplicates first condition } else { doTheRest(); } if ($b == 0) { doOneMoreThing(); } else { doOneMoreThing(); // Noncompliant; duplicates then-branch } var b = a ? 12 > 4 : 4; // Noncompliant; always results in the same value
Compliant Solution
switch ($i) { case 1: case 3: doSomething(); break; case 2: doSomethingDifferent(); break; default: doTheRest(); } if (($a >= 0 && $a < 10) || ($a >= 20 && $a < 50)) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else { doTheRest(); } doOneMoreThing(); b = 4;
or
switch ($i) { case 1: doSomething(); break; case 2: doSomethingDifferent(); break; case 3: doThirdThing(); break; default: doTheRest(); } if ($a >= 0 && $a < 10) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else if ($a >= 20 && $a < 50) { doTheThirdThing(); } else { doTheRest(); } if ($b == 0) { doOneMoreThing(); } else { doTheRest(); } int b = a ? 12 > 4 : 8;
Exceptions
Blocks in an if
chain that contain a single line of code are ignored, as are blocks in a switch
statement that contain a
single line of code with or without a following break
.
Remove the code after this "return". Open
return false;
- Read upRead up
- Exclude checks
Jump statements (return
, break
, continue
, and goto
) and throw
expressions move
control flow out of the current code block. Typically, any statements in a block that come after a jump or throw
are simply wasted
keystrokes lying in wait to confuse the unwary.
Rarely, as illustrated below, code after a jump or throw
is reachable. However, such code is difficult to understand, and should be
refactored.
Noncompliant Code Example
function fun($a) { $i = 10; return $i + $a; $i++; // this is never executed } function foo($a) { if ($a == 5) { goto error; } else { // do the job } return; error: printf("don't use 5"); // this is reachable but unreadable }
Compliant Solution
function fun($a) { $i = 10; return $i + $a; } function foo($a) { if ($a == 5) { handleError(); } else { // do the job } return; }
See
- MISRA C:2004, 14.1 - There shall be no unreachable code
- MISRA C++:2008, 0-1-1 - A project shall not contain unreachable code
- MISRA C++:2008, 0-1-9 - There shall be no dead code
- MISRA C:2012, 2.1 - A project shall not contain unreachable code
- MISRA C:2012, 2.2 - There shall be no dead code
- MITRE, CWE-561 - Dead Code
- CERT, MSC56-J. - Detect and remove superfluous code and values
- CERT, MSC12-C. - Detect and remove code that has no effect or is never executed
- CERT, MSC07-CPP. - Detect and remove dead code
Call with 7 arg(s) to \meysampg\intldate\IntlDateTrait::setIntlDateFormatter()
which only takes 6 arg(s) defined at /code/IntlDateTrait.php:635
Confirmed
$this->setIntlDateFormatter(
- Exclude checks
Argument 3 (sec)
is string
but \mktime()
takes int
Confirmed
$timestamp = mktime(date('H', $timestamp), date('i', $timestamp), date('s', $timestamp), date('n', $timestamp), date('j', $timestamp), date('Y', $timestamp));
- Exclude checks
Call with 7 arg(s) to \meysampg\intldate\IntlDateTrait::setIntlDateFormatter()
which only takes 6 arg(s) defined at /code/IntlDateTrait.php:635
Confirmed
$this->setIntlDateFormatter(
- Exclude checks
Reduce the number of returns of this function 6, down to the maximum allowed 3. Confirmed
public function guessDateTime($timestring)
- Read upRead up
- Exclude checks
Having too many return statements in a function increases the function's essential complexity because the flow of execution is broken each time a return statement is encountered. This makes it harder to read and understand the logic of the function.
Noncompliant Code Example
With the default threshold of 3:
function myFunction(){ // Noncompliant as there are 4 return statements if (condition1) { return true; } else { if (condition2) { return false; } else { return true; } } return false; }
Define and throw a dedicated exception instead of using a generic one. Open
throw new Exception('DateTime information must be an array in [year, month, day, hours, minutes, seconds] format.');
- Read upRead up
- Exclude checks
If you throw a general exception type, such as ErrorException, RuntimeException, or Exception in a library or framework, it forces consumers to catch all exceptions, including unknown exceptions that they do not know how to handle.
Instead, either throw a subtype that already exists in the Standard PHP Library, or create your own type that derives from Exception.
Noncompliant Code Example
throw new Exception(); // Noncompliant
Compliant Solution
throw new InvalidArgumentException(); // or throw new UnexpectedValueException();
See
- MITRE, CWE-397 - Declaration of Throws for Generic Exception
- CERT, ERR07-J. - Do not throw RuntimeException, Exception, or Throwable
This branch's code block is the same as the block for the branch on line 548. Open
} elseif ($second > 60 || $second < 0) {
return false;
}
- Read upRead up
- Exclude checks
Having two cases
in a switch
statement or two branches in an if
chain with the same implementation is at
best duplicate code, and at worst a coding error. If the same logic is truly needed for both instances, then in an if
chain they should
be combined, or for a switch
, one should fall through to the other.
Noncompliant Code Example
switch ($i) { case 1: doSomething(); break; case 2: doSomethingDifferent(); break; case 3: // Noncompliant; duplicates case 1's implementation doSomething(); break; default: doTheRest(); } if ($a >= 0 && $a < 10) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else if ($a >= 20 && $a < 50) { doTheThing(); // Noncompliant; duplicates first condition } else { doTheRest(); } if ($b == 0) { doOneMoreThing(); } else { doOneMoreThing(); // Noncompliant; duplicates then-branch } var b = a ? 12 > 4 : 4; // Noncompliant; always results in the same value
Compliant Solution
switch ($i) { case 1: case 3: doSomething(); break; case 2: doSomethingDifferent(); break; default: doTheRest(); } if (($a >= 0 && $a < 10) || ($a >= 20 && $a < 50)) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else { doTheRest(); } doOneMoreThing(); b = 4;
or
switch ($i) { case 1: doSomething(); break; case 2: doSomethingDifferent(); break; case 3: doThirdThing(); break; default: doTheRest(); } if ($a >= 0 && $a < 10) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else if ($a >= 20 && $a < 50) { doTheThirdThing(); } else { doTheRest(); } if ($b == 0) { doOneMoreThing(); } else { doTheRest(); } int b = a ? 12 > 4 : 8;
Exceptions
Blocks in an if
chain that contain a single line of code are ignored, as are blocks in a switch
statement that contain a
single line of code with or without a following break
.
Call with 7 arg(s) to \meysampg\intldate\IntlDateTrait::setIntlDateFormatter()
which only takes 6 arg(s) defined at /code/IntlDateTrait.php:635
Confirmed
$this->setIntlDateFormatter(
- Exclude checks
This branch's code block is the same as the block for the branch on line 548. Open
} elseif ($minute > 60 || $minute < 0) {
return false;
} elseif ($second > 60 || $second < 0) {
- Read upRead up
- Exclude checks
Having two cases
in a switch
statement or two branches in an if
chain with the same implementation is at
best duplicate code, and at worst a coding error. If the same logic is truly needed for both instances, then in an if
chain they should
be combined, or for a switch
, one should fall through to the other.
Noncompliant Code Example
switch ($i) { case 1: doSomething(); break; case 2: doSomethingDifferent(); break; case 3: // Noncompliant; duplicates case 1's implementation doSomething(); break; default: doTheRest(); } if ($a >= 0 && $a < 10) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else if ($a >= 20 && $a < 50) { doTheThing(); // Noncompliant; duplicates first condition } else { doTheRest(); } if ($b == 0) { doOneMoreThing(); } else { doOneMoreThing(); // Noncompliant; duplicates then-branch } var b = a ? 12 > 4 : 4; // Noncompliant; always results in the same value
Compliant Solution
switch ($i) { case 1: case 3: doSomething(); break; case 2: doSomethingDifferent(); break; default: doTheRest(); } if (($a >= 0 && $a < 10) || ($a >= 20 && $a < 50)) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else { doTheRest(); } doOneMoreThing(); b = 4;
or
switch ($i) { case 1: doSomething(); break; case 2: doSomethingDifferent(); break; case 3: doThirdThing(); break; default: doTheRest(); } if ($a >= 0 && $a < 10) { doTheThing(); else if ($a >= 10 && $a < 20) { doTheOtherThing(); } else if ($a >= 20 && $a < 50) { doTheThirdThing(); } else { doTheRest(); } if ($b == 0) { doOneMoreThing(); } else { doTheRest(); } int b = a ? 12 > 4 : 8;
Exceptions
Blocks in an if
chain that contain a single line of code are ignored, as are blocks in a switch
statement that contain a
single line of code with or without a following break
.
Argument 2 (min)
is string
but \mktime()
takes int
Confirmed
$timestamp = mktime(date('H', $timestamp), date('i', $timestamp), date('s', $timestamp), date('n', $timestamp), date('j', $timestamp), date('Y', $timestamp));
- Exclude checks
Call with 7 arg(s) to \meysampg\intldate\IntlDateTrait::setIntlDateFormatter()
which only takes 6 arg(s) defined at /code/IntlDateTrait.php:635
Confirmed
$this->setIntlDateFormatter(
- Exclude checks