Skip to content

Commit

Permalink
NEXT-37561 - Update PHPStan and its plugins
Browse files Browse the repository at this point in the history
  • Loading branch information
mitelg committed Aug 22, 2024
1 parent 3433f20 commit 09240a3
Show file tree
Hide file tree
Showing 15 changed files with 56 additions and 57 deletions.
1 change: 1 addition & 0 deletions .danger.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
'^phpstan\/phpstan.*$' => 'Even patch updates for PHPStan may lead to a red CI pipeline, because of new static analysis errors',
'^symplify\/phpstan-rules$' => 'Even patch updates for PHPStan plugins may lead to a red CI pipeline, because of new static analysis errors',
'^rector\/type-perfect$' => 'Even patch updates for PHPStan plugins may lead to a red CI pipeline, because of new static analysis errors',
'^tomasvotruba\/type-coverage$' => 'Even patch updates for PHPStan plugins may lead to a red CI pipeline, because of new static analysis errors',
'^phpat\/phpat$' => 'Even patch updates for PHPStan plugins may lead to a red CI pipeline, because of new static analysis errors',
'^dompdf\/dompdf$' => 'Patch updates of dompdf have let to a lot of issues in the past, therefore it is pinned.',
'^scssphp\/scssphp$' => 'Patch updates of scssphp might lead to UI breaks, therefore it is pinned.',
Expand Down
8 changes: 4 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@
"phpdocumentor/reflection-docblock": "^5.3.0",
"phpdocumentor/type-resolver": "^1.7.1",
"phpstan/extension-installer": "^1.4.1",
"phpstan/phpstan": "1.11.5",
"phpstan/phpstan": "1.11.11",
"phpstan/phpstan-deprecation-rules": "1.2.0",
"phpstan/phpstan-phpunit": "1.4.0",
"phpstan/phpstan-symfony": "1.4.4",
"phpstan/phpstan-symfony": "1.4.8",
"phpunit/phpunit": "^10.5",
"predis/predis": "^2.2",
"rector/type-perfect": "0.1.6",
"rector/type-perfect": "0.1.8",
"shopware/dev-tools": "^1.3",
"smalot/pdfparser": "^2.2.2",
"symfony/browser-kit": "~7.1.1",
Expand All @@ -191,7 +191,7 @@
"symfony/phpunit-bridge": "~7.1.1",
"symfony/var-dumper": "~7.1.1",
"symplify/phpstan-rules": "13.0.0",
"tomasvotruba/type-coverage": "^0.2.1"
"tomasvotruba/type-coverage": "0.3.1"
},
"suggest": {
"shopware/dev-tools": "For development tools, profiler, faker, etc",
Expand Down
18 changes: 0 additions & 18 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ parameters:

excludePaths:
- src/WebInstaller/Resources/
- src/WebInstaller/vendor
- src/WebInstaller/Tests/_fixtures
- tests/e2e/cypress

Expand All @@ -46,12 +45,6 @@ parameters:
- src/**/node_modules/*
- tests/**/node_modules/*

# class behind feature flags
- src/Core/Checkout/Cart/Exception/InvalidCartException.php

# extends final class, which is ok for mocks
- src/Core/Content/Test/ImportExport/MockRepository.php

# @todo NEXT-22697 - Remove when re-enabling cms-aware
- src/Core/System/CustomEntity/Xml/Config/CustomEntityEnrichmentService.php
- tests/integration/Core/System/CustomEntity/Xml/Config/CmsAwareAndAdminUiTest.php
Expand Down Expand Up @@ -157,11 +150,6 @@ parameters:
- src/Core/Checkout/Cart/Error/Error.php
- src/Core/Content/ProductExport/Error/Error.php

- # MockRepo extends final EntityRepository class, with is ok for tests
message: "#^Class Shopware\\Core\\Content\\Test\\ImportExport\\MockRepository extends @final class Shopware\\Core\\Framework\\DataAbstractionLayer\\EntityRepository.$#"
count: 1
path: src/Core/Content/Test/ImportExport/MockRepository.php

- # @deprecated tag:v6.7.0 Using autoload === true on associations is deprecated and must be refactored. See NEXT-25334 and child tasks
message: '#^[a-zA-Z_]+\.[a-zA-Z_]+ association has a configured autoload\=\=\=true, this is forbidden for platform integrations$#'
paths:
Expand Down Expand Up @@ -206,12 +194,6 @@ parameters:
# Breaking changes which are not worth it
- '#Method Shopware\\Core\\Framework\\DataAbstractionLayer\\EntityCollection::filterAndReduceByProperty\(\) has parameter \$value with no type specified\.#'

- # Google Cloud Storage filesystem closes the stream even though it should not
message: '#Call to function is_resource\(\) with resource will always evaluate to true#'
paths:
- src/Core/Content/Media/File/FileSaver.php
- src/Core/Content/ImportExport/ImportExport.php

- # Can not be fixed currently. See https://github.com/phpstan/phpstan/discussions/9159
message: '#Method Shopware\\Core\\Framework\\DataAbstractionLayer\\Field\\Field::getFlag\(\) should return \(TFlag of Shopware\\Core\\Framework\\DataAbstractionLayer\\Field\\Flag\\Flag\)\|null but returns Shopware\\Core\\Framework\\DataAbstractionLayer\\Field\\Flag\\Flag\|null#'
path: src/Core/Framework/DataAbstractionLayer/Field/Field.php
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,53 @@ class MediaThumbnailSizeEntity extends Entity
use EntityIdTrait;

/**
* @var int
* @deprecated tag:v6.7.0 - Will be natively typed
*
* @var int<1, max>
*/
protected $width;

/**
* @var int
* @deprecated tag:v6.7.0 - Will be natively typed
*
* @var int<1, max>
*/
protected $height;

/**
* @deprecated tag:v6.7.0 - Will be natively typed
*
* @var MediaFolderConfigurationCollection|null
*/
protected $mediaFolderConfigurations;

/**
* @return int<1, max>
*/
public function getWidth(): int
{
return $this->width;
}

/**
* @param int<1, max> $width
*/
public function setWidth(int $width): void
{
$this->width = $width;
}

/**
* @return int<1, max>
*/
public function getHeight(): int
{
return $this->height;
}

/**
* @param int<1, max> $height
*/
public function setHeight(int $height): void
{
$this->height = $height;
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Content/Media/Thumbnail/ThumbnailService.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;

/**
* @phpstan-type ImageSize array{width: int, height: int}
* @phpstan-type ImageSize array{width: int<1, max>, height: int<1, max>}
*/
#[Package('buyers-experience')]
class ThumbnailService
Expand Down
4 changes: 4 additions & 0 deletions src/Core/Content/Media/Thumbnail/ThumbnailSizeCalculator.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,17 @@ public function calculate(
}

$calculatedWidth = (int) round($imageSize['width'] * $factor);
\assert($calculatedWidth > 0);
$calculatedHeight = (int) round($imageSize['height'] * $factor);
\assert($calculatedHeight > 0);

return $this->determineValidSize($imageSize, $calculatedWidth, $calculatedHeight);
}

/**
* @param ImageSize $imageSize
* @param int<1, max> $thumbnailWith
* @param int<1, max> $thumbnailHeight
*
* @return ImageSize
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public function matchException(\Exception $e): ?\Exception
if (preg_match('/SQLSTATE\[23000\]:.*1062 Duplicate.*uniq.search_config_field.field__config_id\'/', $e->getMessage())) {
$field = [];
preg_match('/Duplicate entry \'(.*)\' for key/', $e->getMessage(), $field);
$field = substr($field[1], 0, (int) strpos($field[1], '-'));
$fieldNameMatch = $field[1] ?? '';
$field = substr($fieldNameMatch, 0, (int) strpos($fieldNameMatch, '-'));

return new DuplicateProductSearchConfigFieldException($field, $e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public function matchException(\Exception $e): ?\Exception
if (preg_match('/SQLSTATE\[23000\]:.*1062 Duplicate.*uniq.product.product_number__version_id\'/', $e->getMessage())) {
$number = [];
preg_match('/Duplicate entry \'(.*)\' for key/', $e->getMessage(), $number);
/** @var int $position */
$position = strrpos($number[1], '-');
$number = substr($number[1], 0, $position);
$numberMatch = $number[1] ?? '';
$position = (int) strrpos($numberMatch, '-');
$number = substr($numberMatch, 0, $position);

return new DuplicateProductNumberException($number, $e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function matchException(\Exception $e): ?\Exception
if (preg_match('/SQLSTATE\[23000\]:.*1062 Duplicate.*uniq.product_sorting.url_key\'/', $e->getMessage())) {
$key = [];
preg_match('/Duplicate entry \'(.*)\' for key/', $e->getMessage(), $key);
$key = $key[1];
$key = $key[1] ?? '';

return new DuplicateProductSortingKeyException($key, $e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public function matchException(\Exception $e): ?\Exception
if (preg_match('/SQLSTATE\[23000\]:.*1062 Duplicate.*file_name\'/', $e->getMessage())) {
$file = [];
preg_match('/Duplicate entry \'(.*)\' for key/', $e->getMessage(), $file);
$file = $file[1];
$file = $file[1] ?? '';

return new DuplicateFileNameException($file, $e);
}
Expand Down
11 changes: 8 additions & 3 deletions src/Core/Content/Test/ImportExport/MockRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
namespace Shopware\Core\Content\Test\ImportExport;

use Shopware\Core\Framework\Context;
use Shopware\Core\Framework\DataAbstractionLayer\Entity;
use Shopware\Core\Framework\DataAbstractionLayer\EntityCollection;
use Shopware\Core\Framework\DataAbstractionLayer\EntityDefinition;
use Shopware\Core\Framework\DataAbstractionLayer\EntityRepository;
use Shopware\Core\Framework\DataAbstractionLayer\Event\EntityWrittenContainerEvent;
Expand All @@ -20,11 +22,11 @@
#[Package('services-settings')]
class MockRepository extends EntityRepository
{
public $createCalls = 0;
public int $createCalls = 0;

public $updateCalls = 0;
public int $updateCalls = 0;

public $upsertCalls = 0;
public int $upsertCalls = 0;

public function __construct(private readonly EntityDefinition $definition)
{
Expand All @@ -50,6 +52,9 @@ public function clone(string $id, Context $context, ?string $newId = null, ?Clon
throw new \Error('MockRepository->clone: Not implemented');
}

/**
* @return EntitySearchResult<EntityCollection<Entity>>
*/
public function search(Criteria $criteria, Context $context): EntitySearchResult
{
throw new \Error('MockRepository->search: Not implemented');
Expand Down
14 changes: 4 additions & 10 deletions src/Core/Framework/Adapter/Filesystem/Plugin/CopyBatch.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
use League\Flysystem\AsyncAwsS3\AsyncAwsS3Adapter;
use League\Flysystem\Filesystem;
use League\Flysystem\FilesystemOperator;
use League\Flysystem\PathPrefixer;
use League\MimeTypeDetection\MimeTypeDetector;
use Shopware\Core\Framework\Log\Package;

#[Package('core')]
Expand All @@ -26,12 +24,11 @@ public static function copy(FilesystemOperator $filesystem, CopyBatchInput ...$f

foreach ($files as $batchInput) {
$handle = $batchInput->getSourceFile();
if (\is_string($handle)) {
$handle = fopen($handle, 'r');
}

foreach ($batchInput->getTargetFiles() as $targetFile) {
if (!\is_resource($batchInput->getSourceFile())) {
$handle = fopen($batchInput->getSourceFile(), 'r');
}

$filesystem->writeStream($targetFile, $handle);
}

Expand Down Expand Up @@ -72,13 +69,10 @@ private static function getS3Client(FilesystemOperator $operator): ?array
private static function copyS3(AsyncAwsS3Adapter $adapter, S3Client $s3Client, CopyBatchInput ...$files): void
{
// Extract the bucket name, mime type detector and path prefixer from the adapter.
/** @var string $bucketName */
$bucketName = \Closure::bind(fn () => $adapter->bucket, $adapter, $adapter::class)();

/** @var MimeTypeDetector $mimeTypeDetector */
$mimeTypeDetector = \Closure::bind(fn () => $adapter->mimeTypeDetector, $adapter, $adapter::class)();

/** @var PathPrefixer $prefixer */
$prefixer = \Closure::bind(fn () => $adapter->prefixer, $adapter, $adapter::class)();

// Copy the files in batches of 250 files. This is necessary to have open sockets and not run into the "Too many open files" error.
Expand All @@ -88,7 +82,7 @@ private static function copyS3(AsyncAwsS3Adapter $adapter, S3Client $s3Client, C
foreach ($filesBatch as $file) {
$sourceFile = $file->getSourceFile();

if (!\is_resource($sourceFile)) {
if (\is_string($sourceFile)) {
$sourceFile = @fopen($sourceFile, 'rb');

if ($sourceFile === false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,23 @@ public function matchException(\Exception $e): ?\Exception
$e->getMessage(),
$matches
)) {
if (\array_key_exists('technicalName', $matches) && \is_string($matches['technicalName'])) {
return PaymentException::duplicateTechnicalName($matches['technicalName']);
}
return PaymentException::duplicateTechnicalName($matches['technicalName']);
}

if (\preg_match(
'/SQLSTATE\[23000]: Integrity constraint violation: 1062 Duplicate entry \'(?<technicalName>.*)\' for key \'shipping_method.uniq\.technical_name\'/',
$e->getMessage(),
$matches
)) {
if (\array_key_exists('technicalName', $matches) && \is_string($matches['technicalName'])) {
return ShippingException::duplicateTechnicalName($matches['technicalName']);
}
return ShippingException::duplicateTechnicalName($matches['technicalName']);
}

if (\preg_match(
'/SQLSTATE\[23000]: Integrity constraint violation: 1062 Duplicate entry \'(?<technicalName>.*)\' for key \'import_export_profile\.uniq\.import_export_profile\.technical_name\'/',
$e->getMessage(),
$matches
)) {
if (\array_key_exists('technicalName', $matches) && \is_string($matches['technicalName'])) {
return ImportExportException::duplicateTechnicalName($matches['technicalName']);
}
return ImportExportException::duplicateTechnicalName($matches['technicalName']);
}

return null;
Expand Down
6 changes: 3 additions & 3 deletions src/Core/Migration/Test/MigrationNameAndTimestampTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ public function testMigrationNameAndTimestampAreNamedAfterOptionalConvention():
$matches = [];
$result = preg_match('/\\\\(?<name>Migration(?<timestamp>\d+)\w+)$/', (string) $className, $matches);

static::assertEquals(1, $result, \sprintf(
static::assertSame(1, $result, \sprintf(
'Invalid migration name "%s". Example for a valid format: Migration1536232684Order',
$className
));

$timestamp = (int) $matches['timestamp'];
static::assertEquals($migration->getCreationTimestamp(), $timestamp, \sprintf(
$timestamp = (int) ($matches['timestamp'] ?? 0);
static::assertSame($migration->getCreationTimestamp(), $timestamp, \sprintf(
'Timestamp in migration name "%s" does not match timestamp of method "getCreationTimestamp"',
$className
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public function testDeleteThumbnailThrowsMediaContainsNoThumbnailException(): vo

/**
* @param array<string, int> $imageSize
* @param array<string, int> $preferredThumbnailSize
* @param array<string, int<1, max>> $preferredThumbnailSize
* @param array<string, int> $expectedSize
*/
#[DataProvider('thumbnailSizeProvider')]
Expand Down

0 comments on commit 09240a3

Please sign in to comment.