[TASK] Remove superfluous isset checks 81/63481/6
authorAlexander Schnitzler <git@alexanderschnitzler.de>
Fri, 28 Feb 2020 14:21:59 +0000 (15:21 +0100)
committerBenni Mack <benni@typo3.org>
Mon, 2 Mar 2020 11:18:48 +0000 (12:18 +0100)
phpstan reported superfluous usages of isset checks for
variables that are defined and whose type is more
specifically checked in the same condition.

Some reported errors of phpstan have been ignored for now
as they are considered false positives and which will be
tackled once there is more type safety in the context of
those reported errors to be able to properly replace the
isset checks.

Releases: master
Resolves: #90576
Change-Id: I028a928bcbc18185f69f7bda4358aaf08cc016b3
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63481
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack <benni@typo3.org>
phpstan.neon
typo3/sysext/backend/Classes/Form/FieldWizard/LocalizationStateSelector.php
typo3/sysext/backend/Classes/Utility/BackendUtility.php
typo3/sysext/core/Classes/Localization/Parser/AbstractXmlParser.php
typo3/sysext/core/Classes/Localization/Parser/LocallangXmlParser.php
typo3/sysext/core/Classes/Resource/Index/Indexer.php
typo3/sysext/core/Classes/Resource/ResourceFactory.php
typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
typo3/sysext/scheduler/Classes/Controller/SchedulerModuleController.php

index f8af5e3..4ae2d00 100644 (file)
@@ -19,6 +19,7 @@ parameters:
     - %currentWorkingDirectory%/typo3/sysext/*/Configuration/*
 
   ignoreErrors:
+    # ignored errors for level 0
     - '#Undefined variable: \$_EXTKEY#'
     -
       message: '#Instantiated class Composer\\Util\\Filesystem not found\.#'
@@ -66,3 +67,17 @@ parameters:
       message: "#^Call to an undefined static method TYPO3Fluid\\\\Fluid\\\\Core\\\\Rendering\\\\RenderingContext\\:\\:getParserConfiguration\\(\\)\\.$#"
       count: 1
       path: typo3/sysext/fluid/Classes/Core/Rendering/RenderingContext.php
+
+    # ignored errors for level 1
+    -
+      message: "#^Variable \\$value in isset\\(\\) always exists and is not nullable\\.$#"
+      count: 1
+      path: typo3/sysext/backend/Classes/Utility/BackendUtility.php
+    -
+      message: "#^Variable \\$pidConf in isset\\(\\) always exists and is not nullable\\.$#"
+      count: 1
+      path: typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
+    -
+      message: "#^Variable \\$column in isset\\(\\) always exists and is not nullable\\.$#"
+      count: 1
+      path: typo3/sysext/workspaces/Classes/Service/GridDataService.php
index c4edaac..5a9b4ee 100644 (file)
@@ -36,10 +36,7 @@ class LocalizationStateSelector extends AbstractNode
         $result = $this->initializeResultArray();
 
         $fieldName = $this->data['fieldName'];
-        $l10nStateFieldName = '';
-        if (isset($l10nStateFieldName)) {
-            $l10nStateFieldName = 'l10n_state';
-        }
+        $l10nStateFieldName = 'l10n_state';
         if (
             !$l10nStateFieldName
             || !isset($this->data['defaultLanguageRow'])
index 2cc0594..8a6785c 100644 (file)
@@ -1953,6 +1953,9 @@ class BackendUtility
                 break;
             case 'input':
                 // Hide value 0 for dates, but show it for everything else
+                // todo: phpstan states that $value always exists and is not nullable. At the moment, this is a false
+                //       positive as null can be passed into this method via $value. As soon as more strict types are
+                //       used, this isset check must be replaced with a more appropriate check.
                 if (isset($value)) {
                     $dateTimeFormats = QueryHelper::getDateTimeFormats();
 
index f2c85bc..126ff41 100644 (file)
@@ -76,7 +76,7 @@ abstract class AbstractXmlParser implements LocalizationParserInterface
         $previousValueOfEntityLoader = libxml_disable_entity_loader(true);
         $rootXmlNode = simplexml_load_string($xmlContent, \SimpleXMLElement::class, LIBXML_NOWARNING);
         libxml_disable_entity_loader($previousValueOfEntityLoader);
-        if (!isset($rootXmlNode) || $rootXmlNode === false) {
+        if ($rootXmlNode === false) {
             $xmlError = libxml_get_last_error();
             throw new InvalidXmlFileException(
                 'The path provided does not point to existing and accessible well-formed XML file. Reason: ' . $xmlError->message . ' in ' . $this->sourcePath . ', line ' . $xmlError->line,
index 93d47ea..050789b 100644 (file)
@@ -186,7 +186,7 @@ class LocallangXmlParser extends AbstractXmlParser
             $rootXmlNode = simplexml_load_string($xmlContent, \SimpleXMLElement::class, LIBXML_NOWARNING);
             libxml_disable_entity_loader($previousValueOfEntityLoader);
         }
-        if (!isset($rootXmlNode) || $rootXmlNode === false) {
+        if ($rootXmlNode === false) {
             $xmlError = libxml_get_last_error();
             throw new InvalidXmlFileException(
                 'The path provided does not point to existing and accessible well-formed XML file. Reason: ' . $xmlError->message . ' in ' . $targetPath . ', line ' . $xmlError->line,
index 9b66454..3794276 100644 (file)
@@ -70,7 +70,7 @@ class Indexer implements LoggerAwareInterface
      */
     public function createIndexEntry($identifier): File
     {
-        if (!isset($identifier) || !is_string($identifier) || $identifier === '') {
+        if (!is_string($identifier) || $identifier === '') {
             throw new \InvalidArgumentException(
                 'Invalid file identifier given. It must be of type string and not empty. "' . gettype($identifier) . '" given.',
                 1401732565
index 5ae3564..5cec4d2 100644 (file)
@@ -382,7 +382,7 @@ class ResourceFactory implements ResourceFactoryInterface, SingletonInterface
      */
     public function getFileObjectFromCombinedIdentifier($identifier)
     {
-        if (!isset($identifier) || !is_string($identifier) || $identifier === '') {
+        if (!is_string($identifier) || $identifier === '') {
             throw new \InvalidArgumentException('Invalid file identifier given. It must be of type string and not empty. "' . gettype($identifier) . '" given.', 1401732564);
         }
         $parts = GeneralUtility::trimExplode(':', $identifier);
index 8d0781c..6116b1a 100644 (file)
@@ -1010,6 +1010,9 @@ class ContentObjectRenderer implements LoggerAwareInterface
      */
     public function getSlidePids($pidList, $pidConf)
     {
+        // todo: phpstan states that $pidConf always exists and is not nullable. At the moment, this is a false positive
+        //       as null can be passed into this method via $pidConf. As soon as more strict types are used, this isset
+        //       check must be replaced with a more appropriate check like empty or count.
         $pidList = isset($pidConf) ? trim($this->stdWrap($pidList, $pidConf)) : trim($pidList);
         if ($pidList === '') {
             $pidList = 'this';
index 8e37f51..5e1c0bf 100644 (file)
@@ -687,7 +687,7 @@ class SchedulerModuleController
                 $additionalFieldsStyle = ' style="display: none"';
             }
             // Add each field to the display, if there are indeed any
-            if (isset($fields) && is_array($fields)) {
+            if (is_array($fields)) {
                 foreach ($fields as $fieldID => $fieldInfo) {
                     $htmlClassName = strtolower(str_replace('\\', '-', $class));
                     $field = [];