Skip to content

Commit

Permalink
Check for named constants
Browse files Browse the repository at this point in the history
Eg remove $six = 6

or $week = 7

These sorts of numbers are still magic, fixes povils#83
  • Loading branch information
exussum12 committed Jan 28, 2019
1 parent 6b726e5 commit ebd6a7e
Show file tree
Hide file tree
Showing 8 changed files with 239 additions and 1 deletion.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ The ``--allow-array-mapping`` option allow keys as strings when using "array" ex

The ``--xml-output`` option will generate an report in an Xml format to the path specified by the option.

The ``--whitelist`` option will only process the files listed in the file specified. This is useful for incremental anaysis.
The ``--whitelist`` option will only process the files listed in the file specified. This is useful for incremental analysis.

The ``--check-naming`` option will check for names for numbers being used, eg `$six = 6`; You also need to pass in a csv of supported languages you wish to check (e.g. en)

**By default it analyses conditions, return statements, and switch cases.**

Expand Down
8 changes: 8 additions & 0 deletions src/Console/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ protected function configure(): void
'Link to a file containing filenames to search',
''
)
->addOption(
'check-naming',
null,
InputOption::VALUE_REQUIRED,
'Check the names of variables to ensure they are not numeric',
''
)
;
}

Expand Down Expand Up @@ -211,6 +218,7 @@ private function createOption(InputInterface $input): Option
$option = new Option;
$option->setIgnoreNumbers(array_map([$this, 'castToNumber'], $this->getCSVOption($input, 'ignore-numbers')));
$option->setIgnoreFuncs($this->getCSVOption($input, 'ignore-funcs'));
$option->setCheckNaming($this->getCSVOption($input, 'check-naming'));
$option->setIncludeStrings($input->getOption('strings'));
$option->setIncludeNumericStrings($input->getOption('include-numeric-string'));
$option->setIgnoreStrings($this->getCSVOption($input, 'ignore-strings'));
Expand Down
28 changes: 28 additions & 0 deletions src/Console/Option.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Povils\PHPMND\Console;

use Povils\PHPMND\Extension\Extension;
use Povils\PHPMND\Language;

/**
* @package Povils\PHPMND\Console
Expand Down Expand Up @@ -49,6 +50,11 @@ class Option
*/
private $allowArrayMapping = false;

/**
* @var array
*/
private $checkNaming = [];

public function setExtensions(array $extensions)
{
$this->extensions = $extensions;
Expand Down Expand Up @@ -128,4 +134,26 @@ public function setAllowArrayMapping(?bool $allowArrayMapping)
{
$this->allowArrayMapping = $allowArrayMapping;
}

/**
* @return Language[]
*/
public function checkNaming(): array
{
return $this->checkNaming;
}

public function setCheckNaming(array $checkNaming)
{
$languages = [];
foreach ($checkNaming as $language) {
$language = ucfirst($language);
$className = '\Povils\PHPMND\Languages\\' . $language;

if (class_exists($className)) {
$languages[] = new $className();
}
}
$this->checkNaming = $languages;
}
}
12 changes: 12 additions & 0 deletions src/Language.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Povils\PHPMND;


interface Language
{
/*
* Returns an array of words which
*/
public function parse(int $number): array;
}
105 changes: 105 additions & 0 deletions src/Languages/En.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?php
namespace Povils\PHPMND\Languages;

use Povils\PHPMND\Language;

class En implements Language
{
protected $specialNumbers = [
2 => [
'half',
],
3 => [
'third',
],
7 => [
'week',
],
10 => [
'tenth',
'decile',
],
24 => [
'hours',
],
28 => [
'February',
],
60 => [
'second',
'minute',
],
100 => [
'percent',
'centile'
],
];

protected $numberMapping = [
'zero',
'one',
'two',
'three',
'four',
'five',
'six',
'seven',
'eight',
'nine',
'ten',
'eleven',
'twelve',
'thirteen',
'fourteen',
'fifteen',
'sixteen',
'seventeen',
'eighteen',
'nineteen',
'twenty',
30 => 'thirty',
40 => 'forty',
50 => 'fifty',
60 => 'sixty',
70 => 'seventy',
80 => 'eighty',
90 => 'ninety',
100 => 'hundred',
1000 => 'thousand',
1000000 => 'million',
];

public function parse(int $number): array
{

end($this->numberMapping);
$final = $this->specialNumbers[$number] ?? [];

if ($number < 0) {
$final [] = 'minus';
$final [] = 'negative';

$number = -$number;
}

while (prev($this->numberMapping) !== false && $number > 0) {
$key = key($this->numberMapping);

if ($number < $key) {
continue;
}
$multiple = 1;

if ($key * 2 < $number && $key > 0) {
$multiple = floor($number / $key);

$final = array_merge($final, $this->parse($multiple));
}

$final[] = current($this->numberMapping);
$number -= $key * $multiple;
}

return $final;
}
}
42 changes: 42 additions & 0 deletions src/Visitor/DetectorVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,28 @@ public function __construct(FileReport $fileReport, Option $option)
public function enterNode(Node $node): ?int
{
if ($this->isIgnoreableConst($node)) {
if ($this->checkNameContainsLanguage(
$node->name->name,
$node->value->value ?? 0
)) {
$this->fileReport->addEntry($node->getLine(), $node->value->value);
}

return NodeTraverser::DONT_TRAVERSE_CHILDREN;
}


if ($this->isNumber($node) || $this->isString($node)) {
/** @var LNumber|DNumber|String_ $scalar */
$scalar = $node;

if ($this->checkNameContainsLanguage(
$node->getAttribute('parent')->var->name ?? '',
$node->value
)) {
$this->fileReport->addEntry($node->getLine(), $scalar->value);
}

if ($this->hasSign($node)) {
$node = $node->getAttribute('parent');
if ($this->isMinus($node)) {
Expand Down Expand Up @@ -119,4 +135,30 @@ private function isValidNumeric(Node $node): bool
is_numeric($node->value) &&
false === $this->ignoreString($node);
}

/**
* @param string $name
* @param string|int $value
* @return bool
*/
private function checkNameContainsLanguage(string $name, $value): bool
{
foreach ($this->option->checkNaming() as $language) {
$generatedNumbers = $language->parse($value);

$regex = '/^';
foreach ($generatedNumbers as $word) {
$regex .= "(?:{$word}[\s_-]*)?";
}

$regex .= '$/i';
$match = preg_match($regex, $name);

if ($match) {
return true;
}
}

return false;
}
}
30 changes: 30 additions & 0 deletions tests/DetectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,36 @@ public function testDetectReadingNumber(): void
);
}

public function testNamesCorrectlyFound()
{

$option = $this->createOption();
$option->setCheckNaming(['en']);
$detector = $this->createDetector($option);

$fileReport = $detector->detect(FileReportTest::getTestFile('check_names'));

$this->assertSame(
[
[
'line' => 4,
'value' => 27,
],
[
'line' => 8,
'value' => 98,
],
[
'line' => 9,
'value' => 7,
],

],
$fileReport->getEntries()
);

}

public function testAllowArrayMappingWithArrayExtension(): void
{
$option = $this->createOption();
Expand Down
11 changes: 11 additions & 0 deletions tests/files/check_names.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php
class a
{
const TWENTY_SEVEN = 27;

public function testVariables()
{
$ninetyEight = 98;
$week = 7;
}
}

0 comments on commit ebd6a7e

Please sign in to comment.