Skip to content

Conversation

@sreichel
Copy link
Contributor

Copilot AI review requested due to automatic review settings October 26, 2025 20:05
@sreichel sreichel added bug PHP 8.5 Relatee to PHP 8.5 labels Oct 26, 2025
@github-actions github-actions bot added Component: Core Relates to Mage_Core Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: lib/Varien Relates to lib/Varien Component: Index Relates to Mage_Index Component: lib/* Relates to lib/* labels Oct 26, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses null array offset deprecation warnings in PHP 8.5 by adding null checks and type coercion before array access operations. The changes prevent potential issues where null values are used as array keys or offsets.

Key Changes:

  • Added null checks with empty string fallback before array key access in multiple methods
  • Added explicit string type casting for array keys to prevent null offset warnings
  • Removed a potentially problematic curl_close() call that could fail if the resource was already null

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/Varien/Http/Adapter/Curl.php Removed curl_close call to prevent errors when resource is null
lib/Varien/Data/Collection.php Added null check converting null to empty string before array access
app/code/core/Mage/Index/Model/Indexer/Abstract.php Added null check converting null to empty string before array access
app/code/core/Mage/Core/Controller/Varien/Router/Standard.php Added string cast to prevent null as array key
app/code/core/Mage/CatalogInventory/Model/Stock/Status.php Added string cast to prevent null as array key
app/code/core/Mage/Catalog/Model/Resource/Product/Indexer/Price.php Added null check converting null to empty string before array access
app/code/core/Mage/Catalog/Model/Product/Type.php Added string cast to prevent null as array key
Comments suppressed due to low confidence (1)

app/code/core/Mage/Catalog/Model/Resource/Product/Indexer/Price.php:1

  • Converting null to empty string will result in a less informative error message on line 319 ('Unsupported product type ""'). Consider checking for null explicitly and throwing a more specific exception like 'Product type ID cannot be null' to aid debugging.
<?php

$this->load();

if ($idValue === null) {
$idValue = '';
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting null to an empty string could mask logic errors where null IDs are passed unintentionally. Consider adding validation to throw an exception or log a warning when null is passed, or return null immediately instead of treating it as a valid key. This would make debugging easier and prevent unexpected behavior where null and empty string IDs are treated as equivalent.

Suggested change
$idValue = '';
return null;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 110 to +114
*/
public function matchEntityAndType($entity, $type)
{
if ($entity === null) {
$entity = '';
Copy link

Copilot AI Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting null entity to an empty string could hide incorrect method calls. Consider adding type hints (e.g., ?string $entity) and either returning early with a default value or throwing an exception when null is passed, as null likely indicates invalid usage rather than a valid empty entity name.

Suggested change
*/
public function matchEntityAndType($entity, $type)
{
if ($entity === null) {
$entity = '';
* @throws InvalidArgumentException if $entity is null
*/
public function matchEntityAndType(?string $entity, string $type)
{
if ($entity === null) {
throw new InvalidArgumentException('Entity name must not be null.');

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2025

Comment on lines +312 to +314
if ($productTypeId === null) {
$productTypeId = '';
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not like this?

$productTypeId ??= '';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: Core Relates to Mage_Core Component: Index Relates to Mage_Index Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* PHP 8.5 Relatee to PHP 8.5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants