Skip to content

chore(quality): migrate to PHPStan 2 — 35 findings to zero, plus a missing-manager bug #697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rubenvdlinde merged 2 commits into from
Aug 22, 2026
Merged

chore(quality): migrate to PHPStan 2 — 35 findings to zero, plus a missing-manager bug #697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bf2f5a
chore(quality): migrate to PHPStan 2 — 35 findings to zero, plus a re…
Aug 22, 2026
3223f41
style(quality): satisfy phpcs on the comments this branch added
Aug 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@
},
"require-dev": {
"conduction/coding-standard": "^1.0",
"conduction/hydra-gates": "^1.0",
"conduction/hydra-gates": "^1.8.2",
"cyclonedx/cyclonedx-php-composer": "^6.2",
"edgedesign/phpqa": "^1.27",
"guzzlehttp/guzzle": "^7.8",
"nextcloud/ocp": "^34.0",
"phpcsstandards/phpcsextra": "^1.4",
"phpmd/phpmd": "^2.15",
"phpmetrics/phpmetrics": "^2.8",
"phpstan/phpstan": "^1.10",
"phpstan/phpstan": "^2.0",
"phpunit/phpunit": "^10.5",
"roave/security-advisories": "dev-latest",
"squizlabs/php_codesniffer": "^3.9",
Expand Down
23 changes: 17 additions & 6 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion lib/Controller/OrganisationMembersController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,9 @@ private function authorizeMaintainer(string $organisationUuid): ?JSONResponse {
*
* @return \OCA\OpenRegister\Service\OrganisationService The service instance.
*
* @throws \Throwable When OpenRegister is unavailable.
* No `@throws`: the body is a plain property read. If OpenRegister is
* unavailable the failure happens in the container while CONSTRUCTING this
* controller, not here.
*/
private function getOrganisationService(): \OCA\OpenRegister\Service\OrganisationService {
return $this->organisationService;
Expand Down
8 changes: 5 additions & 3 deletions lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ public function index(): JSONResponse {

try {
$user = $this->userSession->getUser();
$isAdmin = $user !== null && $this->groupManager->isAdmin($user->getUID());
$isAdmin = $this->groupManager->isAdmin($user->getUID());

// Delegate all business logic to service.
$data = $this->settingsService->getAllSettings();
Expand Down Expand Up @@ -1577,10 +1577,12 @@ private function parseArchiMateFileUpload(): ?array {
* @spec openspec/changes/method-decomposition/tasks.md#task-3
*/
private function resolveArchiMateMethod(array $options): array {
// No method_exists() probe: ArchiMateService declares
// importArchiMateFileFromPathOptimized(), so only the request parameter
// decides which path runs.
$useOptimized = $this->request->getParam('useOptimized', 'true') === 'true';
$hasOptimized = method_exists($this->archiMateService, 'importArchiMateFileFromPathOptimized');

if ($useOptimized === true && $hasOptimized === true) {
if ($useOptimized === true) {
$this->logger->info('Using OPTIMIZED ArchiMate import method.');
return $this->archiMateService->importArchiMateFileFromPathOptimized($options);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/ArchiMateExportService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ private function addObjectDirectlyToXmlWithProperties(
$xmlData = $this->cleanObjectDataForXml(object: $object, propDefMap: $propertyDefinitionMap);
}

if (is_array($xmlData) === true && empty($xmlData) === false) {
if (empty($xmlData) === false) {
if ($sectionName === 'views') {
$this->addViewDataToXmlNode(viewNode: $objectNode, viewData: $xmlData);
} else {
Expand Down
65 changes: 32 additions & 33 deletions lib/Service/ArchiMateImportService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,11 @@ public function importArchiMateFileFromPathOptimized(array $options = []): array

// PERFORMANCE OPTIMIZATION: Clean up memory after XML parsing.
$memoryCleanupTime = 0;
if (self::PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] !== false) {
$memCleanupStart = microtime(true);
$this->cleanupMemory();
$memoryCleanupTime = microtime(true) - $memCleanupStart;
}
// PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] is a class constant set
// to true, so this was never conditional.
$memCleanupStart = microtime(true);
$this->cleanupMemory();
$memoryCleanupTime = microtime(true) - $memCleanupStart;

// STEP 2: Extract model identifier.
$modelIdStartTime = microtime(true);
Expand Down Expand Up @@ -1254,7 +1254,7 @@ private function createSectionObject(string $section, string $identifier, array
// Fallback: Use AMEF identifier as both ID and extract clean UUID for slug.
$objectId = $identifier;
// Extract clean UUID from AMEF identifier (remove "id-" prefix if present).
if ($identifier !== false && str_starts_with($identifier, 'id-') === true) {
if (str_starts_with($identifier, 'id-') === true) {
$slug = substr($identifier, 3);
// Remove "id-" prefix.
} else {
Expand Down Expand Up @@ -1735,9 +1735,10 @@ private function saveObjectsInParallelBatches(array $objects, ObjectServiceInter
}//end try

// Memory cleanup between chunks.
if (self::PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] !== false) {
$this->cleanupMemory();
}
// PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] is a class constant set
// to true, so this was never conditional. Flip the constant and the
// compiler will point you back here.
$this->cleanupMemory();
}//end foreach

// Store the aggregated result for statistics calculation.
Expand Down Expand Up @@ -2047,7 +2048,7 @@ private function getAmefRegisterId(): ?int {
}

// Validate and normalize to positive int.
if ($rawRegisterId !== null && $rawRegisterId !== '' && is_numeric((string)$rawRegisterId) === true) {
if ($rawRegisterId !== '' && is_numeric((string)$rawRegisterId) === true) {
$registerId = (int)$rawRegisterId;
if ($registerId > 0) {
return $registerId;
Expand Down Expand Up @@ -2549,10 +2550,8 @@ private function findItemsInSection(array $sectionData, string $sectionName): ar
// OPTIMIZATION: Removed debug logging from section processing.
$items = [];

// Safety check: ensure sectionData is an array.
if (is_array($sectionData) === false) {
return [];
}
// No is_array() safety check: $sectionData is declared array, so PHP
// rejects anything else at the call boundary before this could run.

// Get section structure configuration from AMEF config.
$config = $this->getSectionStructureConfig(sectionName: $sectionName);
Expand Down Expand Up @@ -4033,7 +4032,7 @@ private function processStandardVersionRelationship(
$standardId = $source;
}

if ($versionId !== false && $standardId === true) {
if ($standardId === true) {
$stdVersionRelMap[$versionId] = $standardId;
}
}//end processStandaardVersieRelationship()
Expand Down Expand Up @@ -4084,7 +4083,7 @@ private function processRelationshipImmediate(
$standardId = $source;
}

if ($refCompId !== false && $standardId === true) {
if ($standardId === true) {
// Initialize arrays if not exists.
if (isset($gemmaRelationshipMap[$refCompId]) === false) {
$gemmaRelationshipMap[$refCompId] = [
Expand Down Expand Up @@ -4800,7 +4799,7 @@ private function transformSectionObjectsBatch(
// AMEF identifier becomes slug.
} else {
// Fallback: extract clean UUID from AMEF identifier for slug.
if ($identifier !== false && str_starts_with($identifier, 'id-') === true) {
if (str_starts_with($identifier, 'id-') === true) {
$object['@self']['slug'] = substr($identifier, 3);
// Remove "id-" prefix.
} else {
Expand All @@ -4809,7 +4808,7 @@ private function transformSectionObjectsBatch(
}
} else {
// No properties to flatten, use AMEF identifier logic.
if ($identifier !== false && str_starts_with($identifier, 'id-') === true) {
if (str_starts_with($identifier, 'id-') === true) {
$object['@self']['slug'] = substr($identifier, 3);
// Remove "id-" prefix.
} else {
Expand Down Expand Up @@ -4988,7 +4987,9 @@ private function flattenPropertiesBatch(array &$object, array $properties, array
continue;
}

if ($value !== null && isset($propDefMap[$defRef]) === true) {
// No isset($propDefMap[$defRef]) re-check: the loop above only
// reaches here for a $defRef the map already has.
if ($value !== null) {
$propertyName = $propDefMap[$defRef];
$camelCaseName = $this->convertToCamelCase(propertyName: $propertyName);
$object[$camelCaseName] = $value;
Expand Down Expand Up @@ -5022,13 +5023,14 @@ private function flattenPropertiesBatch(array &$object, array $properties, array
);
}
} else {
// 'mapping_exists' is always true here — the map lookup already
// succeeded, so a null $value is the only way into this branch.
$this->logger->warning(
'Property value is null or mapping missing',
'Property value is null',
[
'object_id' => $object['identifier'] ?? 'unknown',
'property_def_ref' => $defRef,
'value' => $value,
'mapping_exists' => isset($propDefMap[$defRef]) === true,
]
);
}//end if
Expand Down Expand Up @@ -5720,10 +5722,8 @@ private function calculateObjectStatistics(array $normalizedData): array {
$sectionKey = 'elements';
}//end if

if (isset($statistics[$sectionKey]) === false) {
continue;
// Skip unknown section types.
}
// No "skip unknown section types" guard: the branch above pins
// $sectionKey to a key $statistics always has, so it never fired.

// Determine if this object was created, updated, or had errors.
$objectId = $object['@self']['id'] ?? $object['identifier'] ?? null;
Expand Down Expand Up @@ -5809,14 +5809,13 @@ private function calculateObjectStatistics(array $normalizedData): array {
'total_errors' => 0,
];

foreach ($statistics as $section => $sectionStats) {
if ($section !== 'omschrijving') {
// Skip summary section itself.
$summary['total_objects_created'] += $sectionStats['created'];
$summary['total_objects_updated'] += $sectionStats['updated'];
$summary['total_objects_unchanged'] += $sectionStats['unchanged'];
$summary['total_errors'] += count($sectionStats['errors']);
}
// No "skip the summary section" guard: `omschrijving` is written into
// $statistics on the line AFTER this loop, so the loop can never see it.
foreach ($statistics as $sectionStats) {
$summary['total_objects_created'] += $sectionStats['created'];
$summary['total_objects_updated'] += $sectionStats['updated'];
$summary['total_objects_unchanged'] += $sectionStats['unchanged'];
$summary['total_errors'] += count($sectionStats['errors']);
}

$statistics['omschrijving'] = $summary;
Expand Down
45 changes: 20 additions & 25 deletions lib/Service/ArchiMateService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,8 @@ private function findItemsInSection(array $sectionData, string $sectionName): ar
// OPTIMIZATION: Removed debug logging from section processing.
$items = [];

// Safety check: ensure sectionData is an array.
if (is_array($sectionData) === false) {
return [];
}
// No is_array() safety check: $sectionData is declared array, so PHP
// rejects anything else at the call boundary before this could run.

// Get section structure configuration from AMEF config.
$config = $this->getSectionStructureConfig(sectionName: $sectionName);
Expand Down Expand Up @@ -974,7 +972,7 @@ private function createSectionObject(string $section, string $identifier, array
} elseif (isset($data['Object ID']) === true) {
// Check if we have "Object ID" property directly.
$slug = $data['Object ID'];
} elseif ($identifier !== false && str_starts_with($identifier, 'id-') === true) {
} elseif (str_starts_with($identifier, 'id-') === true) {
// Fallback: extract from identifier (remove "id-" prefix if present).
$slug = substr($identifier, 3);
}
Expand Down Expand Up @@ -1027,9 +1025,9 @@ private function saveObjectsToDatabase(array $objects): array {

// PERFORMANCE OPTIMIZATION: Use parallel batch processing for large datasets.
$batchProcessingStartTime = microtime(true);
if (self::PERFORMANCE_OPTIMIZATIONS['parallel_processing'] === true
&& count($objects) > self::PERFORMANCE_OPTIMIZATIONS['batch_size']
) {
// PERFORMANCE_OPTIMIZATIONS['parallel_processing'] is a class constant set
// to true, so only the batch-size threshold decides this.
if (count($objects) > self::PERFORMANCE_OPTIMIZATIONS['batch_size']) {
$result = $this->saveObjectsInParallelBatches(
objects: $objects,
objectService: $objectService,
Expand Down Expand Up @@ -1169,9 +1167,9 @@ private function saveObjectsInParallelBatches(array $objects, ObjectServiceInter
}//end try

// Memory cleanup between chunks.
if (self::PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] !== false) {
$this->cleanupMemory();
}
// PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] is a class constant set
// to true, so this was never conditional.
$this->cleanupMemory();
}//end foreach

// Store the aggregated result for statistics calculation.
Expand Down Expand Up @@ -1767,7 +1765,7 @@ private function getAmefRegisterId(): ?int {
}

// Validate and normalize to positive int.
if ($rawRegisterId !== null && $rawRegisterId !== '' && is_numeric((string)$rawRegisterId) === true) {
if ($rawRegisterId !== '' && is_numeric((string)$rawRegisterId) === true) {
$registerId = (int)$rawRegisterId;
if ($registerId > 0) {
return $registerId;
Expand Down Expand Up @@ -2258,10 +2256,8 @@ private function calculateObjectStatistics(array $normalizedData): array {
// Default fallback.
};

if (isset($statistics[$sectionKey]) === false) {
continue;
// Skip unknown section types.
}
// No "skip unknown section types" guard: the branch above pins
// $sectionKey to a key $statistics always has, so it never fired.

// Determine if this object was created, updated, or had errors.
$objectId = $object['@self']['id'] ?? $object['identifier'] ?? null;
Expand Down Expand Up @@ -2346,14 +2342,13 @@ private function calculateObjectStatistics(array $normalizedData): array {
'total_errors' => 0,
];

foreach ($statistics as $section => $sectionStats) {
if ($section !== 'omschrijving') {
// Skip summary section itself.
$summary['total_objects_created'] += $sectionStats['created'];
$summary['total_objects_updated'] += $sectionStats['updated'];
$summary['total_objects_skipped'] += $sectionStats['skipped'];
$summary['total_errors'] += count($sectionStats['errors']);
}
// No "skip the summary section" guard: `omschrijving` is written into
// $statistics on the line AFTER this loop, so the loop can never see it.
foreach ($statistics as $sectionStats) {
$summary['total_objects_created'] += $sectionStats['created'];
$summary['total_objects_updated'] += $sectionStats['updated'];
$summary['total_objects_skipped'] += $sectionStats['skipped'];
$summary['total_errors'] += count($sectionStats['errors']);
}

$statistics['omschrijving'] = $summary;
Expand Down Expand Up @@ -3061,7 +3056,7 @@ private function processRelationshipImmediate(
$standardId = $source;
}

if ($refCompId !== false && $standardId === true) {
if ($standardId === true) {
// Initialize arrays if not exists.
if (isset($gemmaRelationshipMap[$refCompId]) === false) {
$gemmaRelationshipMap[$refCompId] = [
Expand Down
Loading
Loading