Problem/Motivation

Increasing the PHPStan level further uncovers some valuable suggestions for improving the module's code.

Proposed resolution

Increase PHPStan level past level 1 to a level that strikes a reasonable balance between adding value and causing developer pain.

Issue fork orange_dam-3403239

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

markdorison created an issue. See original summary.

markdorison’s picture

Status: Active » Needs work

25 issues still outstanding.

------ ----------------------------------------------------------------------- 
  Line   src/Commands/OrangeDamCommands.php                                     
 ------ ----------------------------------------------------------------------- 
  766    Cannot access property $RecordID on array.                             
  946    PHPDoc tag @param for parameter $timeUpdatedUntil with type            
         Drupal\Core\Datetime\DrupalDateTime|null is not subtype of native      
         type Drupal\Core\Datetime\DrupalDateTime.                              
  998    Call to an undefined method                                            
         Drupal\migrate\Plugin\MigrateDestinationInterface::getDerivativeId().  
 ------ ----------------------------------------------------------------------- 
 ------ -------------------------------------------------------------------------- 
  Line   src/EventSubscriber/OrangeDamMigrationSubscriber.php                      
 ------ -------------------------------------------------------------------------- 
  82     Call to an undefined method                                               
         Drupal\migrate\Plugin\MigrateDestinationInterface::getDerivativeId().     
  103    Access to an undefined property                                           
         Drupal\Core\Entity\EntityInterface::$field_system_id.                     
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
 ------ -------------------------------------------------------------------------- 
 ------ -------------------------------------------------------------------------- 
  Line   src/OrangeDamApi.php                                                      
 ------ -------------------------------------------------------------------------- 
  66     Interface Drupal\Core\Logger\LoggerChannelInterface referenced with       
         incorrect case: Drupal\Core\Logger\LoggerchannelInterface.                
  195    Method Drupal\orange_dam\OrangeDamApi::getDataTableData() has invalid     
         return type Object.                                                       
  202    Call to an undefined method Chromatic\OrangeDam\Factory::dataTable().     
  240    Method Drupal\orange_dam\OrangeDamApi::getSearchApiData() has invalid     
         return type Ojbect.                                                       
  428    Access to an undefined property object::$GlobalInfo.                      
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
  518    Call to an undefined method Chromatic\OrangeDam\Factory::assetLink().     
  595    Call to an undefined method                                               
         Chromatic\OrangeDam\Factory::ObjectManagement().                          
  690    Call to an undefined method Chromatic\OrangeDam\Factory::dataTable().     
  737    Access to an undefined property object::$ResponseSummary.                 
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
  801    Call to an undefined method Chromatic\OrangeDam\Factory::mediaFile().     
  878    Call to an undefined method Chromatic\OrangeDam\Factory::dataTable().     
  962    Call to an undefined method Chromatic\OrangeDam\Factory::dataTable().     
  1011   Call to an undefined method                                               
         Chromatic\OrangeDam\Http\Client::getClient().                             
  1050   Call to an undefined method                                               
         Chromatic\OrangeDam\Http\Client::oAuth2().                                
  1055   Call to an undefined method                                               
         Chromatic\OrangeDam\Http\Client::getClient().                             
 ------ -------------------------------------------------------------------------- 
 ------ ---------------------------------------------- 
  Line   src/OrangeDamMigrationDataManager.php         
 ------ ---------------------------------------------- 
  220    Cannot access property $data on object|true.  
 ------ ---------------------------------------------- 
 ------ ----------------------------------------------------------------------- 
  Line   src/OrangeDamQueueDataManager.php                                      
 ------ ----------------------------------------------------------------------- 
  337    Call to an undefined method                                            
         Drupal\migrate\Plugin\MigrateDestinationInterface::getDerivativeId().  
 ------ ----------------------------------------------------------------------- 
 ------ --------------------------------------------------------------------------------------- 
  Line   src/Plugin/migrate/process/OrangeDamProcessBase.php                                    
 ------ --------------------------------------------------------------------------------------- 
  28     Constructor of class                                                                   
         Drupal\orange_dam\Plugin\migrate\process\OrangeDamProcessBase has an                   
         unused parameter $loggerChannelFactory.                                                
  38     Access to an undefined property                                                        
         Drupal\orange_dam\Plugin\migrate\process\OrangeDamProcessBase::$loggerChannelFactory.  
         💡 Learn more:                                                                         
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property               
 ------ --------------------------------------------------------------------------------------- 
 ------ --------------------------------------------------------------------- 
  Line   src/Plugin/migrate/process/OrangeDamRecordIdSystemId.php             
 ------ --------------------------------------------------------------------- 
  32     Access to property $SystemID on an unknown class Object.             
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols  
 ------ --------------------------------------------------------------------- 
 [ERROR] Found 25 errors                                                   
apotek’s picture

On this warning, it is interesting that the method works and returns data:

 ------ ----------------------------------------------------------------------- 
  Line   src/Commands/OrangeDamCommands.php                                     
 ------ ----------------------------------------------------------------------- 
  998    Call to an undefined method                                            
         Drupal\migrate\Plugin\MigrateDestinationInterface::getDerivativeId().  
 

getDerivativeId() is a method on PluginBase.

interface MigrateDestinationInterface extends PluginInspectionInterface

and


abstract class PluginBase implements PluginInspectionInterface, DerivativeInspectionInterface

DerivativeInspectionInterface contains method getDerivativeId().

So I am not sure why phpstan is complaining here. Code runs fine.

markdorison’s picture

getDerivativeId() is a method on PluginBase.

DerivativeInspectionInterface contains method getDerivativeId().

So I am not sure why phpstan is complaining here. Code runs fine.

It feels like PHPStan is confused and looking in the wrong place or not able to find these methods. 🤔

markdorison’s picture

getDerivativeId() is a method on PluginBase.

DerivativeInspectionInterface contains method getDerivativeId().

So I am not sure why phpstan is complaining here. Code runs fine.

$migration->getDestinationPlugin()->getDerivativeId();

getDestinationPlugin() returns an object conforming to MigrateDestinationInterface which does not specify getDestinationPlugin(); as you noted, it is in DerivativeInspectionInterface. Is it possible that in our case it is returning an object that conforms to both, but in theory, we could receive an object that does not have this method?

If the above is accurate, this is why PHPStan is complaining.

markdorison’s picture

How do we feel about the state of the MR? There are 14 issues outstanding. If it is not worthwhile to push further on these, we can drop the PHPStan level back down to 1 and commit these improvements as is.

adamzimmermann’s picture

I would like to push this forward if possible still. @apotek what are your thoughts on some of the issues you raised?

markdorison’s picture

Status: Needs work » Needs review
markdorison’s picture

Status: Needs review » Reviewed & tested by the community

  • markdorison committed 702cfb5c on 1.x
    Issue #3403239 by markdorison, apotek, adamzimmermann: Increase PHPStan...
markdorison’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.