Problem/Motivation

Gitlab is still raising PHPCS issues and has started to raise PHPStan issues.

We should fix these to make it easier to spot when new issues are flagged by Gitlab CI.

Steps to reproduce

Read the output at https://git.drupalcode.org/project/flysystem_s3/-/jobs/501985 and https://git.drupalcode.org/project/flysystem_s3/-/jobs/501986.

Proposed resolution

Fix the things.

Remaining tasks

All the things.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork flysystem_s3-3409514

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:

About issue forks

Comments

Eli-T created an issue. See original summary.

Eli-T opened merge request !15

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Active » Needs work

MR 15 fixes the PHPCS issues.

I'm thinking about splitting this issue as fixing all the PHPStan issues will necessitate potentially breaking backwards compatibility.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

With respect to #3409514-3: Fix PHPCS and PHPStan issues reported by Gitlab CI and having read Drupal's BC policy especially concerning constructors being considered internal, I think we can resolve all the PHPCS and PHPStan issues without forcing a new release.

thakurnishant_06’s picture

thakurnishant_06 commented

Hello @Eli-T,
I ran the pipeline, and no errors were reported. However, when I ran PHPStan locally with level 0, it highlighted many errors and warnings.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

Thanks @thakurnishant_06

Are you sure you are running phpstan against the fork used by the merge request?

Running locally with level 0 I get

~/code/d10stans3 via 🐘 v8.1.13 on ☁️  (eu-west-1) took 4s
❯ vendor/bin/phpstan
Note: Using configuration file /Users/eli/code/d10stans3/phpstan.neon.
 11/11 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

@thakurnishant_06 If you are definitely running against the correct code can you please share your full phpstan.neon and your output of running phpstan?

thakurnishant_06’s picture

thakurnishant_06 commented

Hi @Eli-t,

Apologies for the delayed response. Here are the warnings and errors reported by PHPStan when i ran it locally.

vendor/bin/phpstan
Note: Using configuration file /home/svinfotech/Pictures/drupal9/phpstan.neon.
 11/11 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -------------------------------------------------------------------------------------------- 
  Line   src/AwsCacheAdapter.php                                                                     
 ------ -------------------------------------------------------------------------------------------- 
  11     Class Drupal\flysystem_s3\AwsCacheAdapter implements unknown interface Aws\CacheInterface.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                          
 ------ -------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   src/Controller/S3CorsUploadAjaxController.php                                                                                                                        
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  64     Parameter $flysystem_factory of method Drupal\flysystem_s3\Controller\S3CorsUploadAjaxController::__construct() has invalid type Drupal\flysystem\FlysystemFactory.  
  111    Instantiated class Aws\S3\PostObjectV4 not found.                                                                                                                    
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                   
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   src/Flysystem/Adapter/S3Adapter.php                                                                                                                                        
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  17     Class Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter extends unknown class League\Flysystem\AwsS3v3\AwsS3Adapter.                                                         
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  26     Parameter $client of method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::__construct() has invalid type Aws\S3\S3ClientInterface.                                      
  34     Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::__construct() calls parent::__construct() but Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter does not extend any class.  
  42     Call to an undefined method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::applyPathPrefix().                                                                            
  44     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$bucket.                                                                                  
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  44     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$options.                                                                                 
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  44     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$s3Client.                                                                                
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  47     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$bucket.                                                                                  
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  47     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$s3Client.                                                                                
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  51     Call to an undefined method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::doesDirectoryExist().                                                                         
  54     Caught class Aws\S3\Exception\S3Exception not found.                                                                                                                       
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  63     Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::getMetadata() calls parent::getMetadata() but Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter does not extend any class.  
  70     Access to constant VISIBILITY_PUBLIC on an unknown class League\Flysystem\AdapterInterface.                                                                                
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  80     Parameter $config of method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::upload() has invalid type League\Flysystem\Config.                                            
  81     Call to an undefined method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::applyPathPrefix().                                                                            
  82     Call to an undefined method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::getOptionsFromConfig().                                                                       
  87     Call to static method guessMimeType() on an unknown class League\Flysystem\Util.                                                                                           
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  90     Call to static method detectByFilename() on an unknown class League\Flysystem\Util\MimeType.                                                                               
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  95     Call to static method contentSize() on an unknown class League\Flysystem\Util.                                                                                             
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  95     Call to static method getStreamSize() on an unknown class League\Flysystem\Util.                                                                                           
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                                         
  98     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$bucket.                                                                                  
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  98     Access to an undefined property Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::$s3Client.                                                                                
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property                                                                                        
  100    Call to an undefined method Drupal\flysystem_s3\Flysystem\Adapter\S3Adapter::normalizeResponse().                                                                          
 ------ --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   src/Flysystem/S3.php                                                                                                                                 
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 
         Class Drupal\flysystem\Plugin\FlysystemUrlTrait was not found while trying to analyse it - discovering symbols is probably not configured properly.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                   
  99     Class Drupal\flysystem\Plugin\FlysystemUrlTrait was not found while trying to analyse it - discovering symbols is probably not configured properly.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                   
  114    Class Drupal\flysystem\Plugin\FlysystemUrlTrait was not found while trying to analyse it - discovering symbols is probably not configured properly.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                   
  122    Class Drupal\flysystem\Plugin\FlysystemUrlTrait was not found while trying to analyse it - discovering symbols is probably not configured properly.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                   
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   tests/src/Functional/ModuleInstallUninstallWebTest.php                                                                                                           
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------- 
  12     Class Drupal\Tests\flysystem_s3\Functional\ModuleInstallUninstallWebTest extends unknown class Drupal\Tests\flysystem\Functional\ModuleInstallUninstallWebTest.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   tests/src/Unit/AwsCacheAdapterTest.php                                                                      
 ------ ------------------------------------------------------------------------------------------------------------ 
  14     Class Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest extends unknown class PHPUnit\Framework\TestCase.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                          
  24     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertSame().               
  27     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertSame().               
  28     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertSame().               
  31     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertNull().               
  33     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertFalse().              
  48     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertSame().               
  51     Call to an undefined method Drupal\Tests\flysystem_s3\Unit\AwsCacheAdapterTest::assertSame().               
 ------ ------------------------------------------------------------------------------------------------------------ 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 
  Line   tests/src/Unit/Flysystem/S3Test.php                                                                                                                  
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 
         Class Drupal\flysystem\Plugin\FlysystemUrlTrait was not found while trying to analyse it - discovering symbols is probably not configured properly.  
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols                                                                                   
 ------ ----------------------------------------------------------------------------------------------------------------------------------------------------- 

 -- ----------------------------------------------------------------------------------------- 
     Error                                                                                    
 -- ----------------------------------------------------------------------------------------- 
     Ignored error pattern #^Unsafe usage of new static# was not matched in reported errors.  
 -- ----------------------------------------------------------------------------------------- 

                                                                                                                        
 [ERROR] Found 41 errors
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

@thakurnishant_06 looks like you haven't installed the composer dependencies?

thakurnishant_06’s picture

thakurnishant_06 commented

@Eli-T, I have installed all the dependencies required for PHPStan, but I'm still encountering errors in my local setup.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

It's not the dependencies for PHPStan that are the issue here, it's the dependencies for flysystem_s3 that are missing.

thakurnishant_06’s picture

thakurnishant_06 commented

Added the missing dependencies, executed the test, and verified that it doesn't raise any new errors.

svinfotech1@svinfotech1:/opt/lampp/htdocs/testing$ vendor/bin/phpstan analyze web/modules/flysystem/ -c web/modules/flysystem/phpstan.neon
 11/11 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors

The last pipe line run was also clean .
Moving it to RTBC.
Thank you.

thakurnishant_06’s picture

thakurnishant_06 commented
Status: Needs review » Reviewed & tested by the community
eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Reviewed & tested by the community » Needs review

Moving back to needs review as I'd like someone using the module to check it for regression. There's no evidence that the module has actually been tested yet.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented

Tested with no regressions found. Merged to 2.1.x.

eli-t’s picture

eli-t
he/him
Primary language English
Location Manchester
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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