Rector: Create CodeQuality level

[carusogabriel]

New Ideas

• [ ] early return

   if ($this->hasDocBlock($tokens, $index)) {
  -    $docToken = $tokens[$this->getDocBlockIndex($tokens, $index)];
  +    return $tokens[$this->getDocBlockIndex($tokens, $index)];
  -} else {
  -    $docToken = null;
  -}
  -return $docToken;
  +return null;
  

• [ ] if/else array value set
  Maybe

  -if (empty($value)) {
  -   $this->arrayBuilt[][$key] = true;
  -} else {
  -  $this->arrayBuilt[][$key] = $value;
  -}
  +$this->arrayBuilt[][$key] = empty($value) ?: $value;
  

  Would be tricky for the value inside if

Done

• [x] return value if not null

   $newNode = $this->refactor($node);
  -if ($newNode !== null) {
  -    return $newNode;
  -}
  -return null;
  +return $newNode;
  

  • can be applied for any value

  ```diff
  $newNode = $this->refactor($node);
  -if ($newNode !== 25) {

  • return $newNode;
    -}
    -return 25;
    +return $newNode;

• [x] eliminate dead code

  -$eachFuncCall->args[0];
  

• [x] Simplify checks:

  -is_array($values) && empty($values)
  +$values === []
  

• [x] simplify If

  -if (strpos($docToken->getContent(), "\n") === false) {
  -    return true;
  -}
  -return false;
  +return strpos($docToken->getContent(), "\n") === false;
  

• [x] short return

  -$nodeClasses = array_keys($robotLoader->getIndexedClasses());
  -return $nodeClasses;
  +return array_keys($robotLoader->getIndexedClasses());
  

• [x] simplify foreach to coala null

  -foreach ($this->oldToNewFunctions as $oldFunction => $newFunction) {
  -    if ($currentFunction === $oldFunction) {
  -        return $newFunction;
  -    }
  -}
  -
  -return null;
  +return $this->oldToNewFunctions[$currentFunction] ?? null;
  

• [x] binary switch
  switch statements with one case and default can be simplified to if/else:

  -switch ($foo) {
  -    case 'my string':
  -        $result = 'ok';
  -    break;
  -    
  -    default:
  -        $result = 'not ok';
  -}
  
  +if ($foo == 'my string') {
  +    $result = 'ok;
  +} else {
  +    $result = 'not ok';
  +}
  

  We must use == as PHP's switch's case are weak compared

• [x] Short array_ callbacks
  Some array_* functions that receive a callback, sometimes don't necessarily need it:

  -$paths = array_filter($paths, function ($path): bool {
  -    return is_dir($path);
  -});
  +$paths = array_filter($paths, 'is_dir');
  

• [x] simplify condition

  -if ((count($methodCallNode->args) !== 1) === false) {
  +if (count($methodCallNode->args) === 1) {
  

• [x] Array values cleanup
  Another idea: array_values isn't necessary with in_array, as it already searches in the array values.

  -in_array('key', array_values($array), true);
  +in_array('key', $array, true);
  

• [x] negative left

  -if ($this->isClassFullyQualifiedName($node) === false) {
  +if (! $this->isClassFullyQualifiedName($node)) {
  

• [x] double negative bool

  -if (! $this->isClassFullyQualifiedName($node) === false) {
  +if ($this->isClassFullyQualifiedName($node)) {
  

• [x] mirrors

  -$methodCallNode = $methodCallNode;
  

• [x] array_search
  array_search + negative comparison can be simplified to just a in_array call:

  -array_search('searching', $array) !== false;
  +in_array('searching', $array);
  

• [x] func_num_args

  -count(func_get_args()) === 1);
  +func_num_args() === 1
  

• [x] Also, stripos instead of strpos + strtolower:

  -strpos(strtolower($string), 'string');
  +stripos($string, 'string')
  

• [x] GetClassToInstanceOfRector

  -if (EventsListener::class === get_class($event->job)) { ...
  +if ($event->job instanceof EventsListener) {...
  

• [x] CombinedAssignRector

  -$value = $value + 5;
  +$value += 5;
  

• [x] Simplify conditions

---

As suggested to @TomasVotruba at Hangouts, we'd like to introduce a new level to Rector: Code Quality :tada:

I believe the name speaks for itself. The first one:

[mssimi]

Great anti-shitcode features, I really like this.

[carusogabriel]

@TomasVotruba The https://github.com/kalessil/phpinspectionsea gave me a lot of ideas. Gonna work and list them this weekend :tada:

[TomasVotruba]

• [x] There case with double ! should be skipped - resolved by #797

-if (null !== $this->expiresAt && $this->expiresAt < new \DateTime()) {
-    return false;
-}
-
-return true;
+return !(null !== $this->expiresAt && $this->expiresAt < new \DateTime());

Or completely inversed

-        return !(null !== $this->expiresAt && $this->expiresAt < new \DateTime());
+        return null === $this->expiresAt || $this->expiresAt >= new \DateTime());

[carusogabriel]

This change is wrong: https://github.com/yiisoft/yii-core/compare/master...TomasVotruba:cleanup?expand=1#diff-3cf2383d527ad795412ec15f9079f50dR445

[TomasVotruba]

There are plenty, it's just a source to fix them.

[TomasVotruba]

Moved to standalone issues to keep better track of new ideas and clear way to close them.