fix Using null as an array offset is deprecated, use an empty string instead by mambax7 · Pull Request #1596 · XOOPS/XoopsCore25

mambax7 · 2026-01-01T22:19:55Z

No description provided.

…instead fix offset is deprecated, use an empty string instead replacing extract()

Copilot

Pull request overview

This PR addresses PHP deprecation warnings related to using null as an array offset by replacing extract() calls with explicit variable extraction and adding validation checks before using variables as array keys.

Key changes:

Replaced extract() calls in config() and write() methods with explicit array key checking and assignment
Added string type and empty string validation before using variables as array offsets
Updated parameter documentation to reflect that null values are accepted

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-01T22:22:34Z

htdocs/class/cache/xoopscache.php

+            if (isset($name['name']) && is_string($name['name'])) {
+                $name = $name['name'];
+            }
+            if (isset($name['settings']) && is_array($name['settings'])) {
+                $settings = array_merge($settings, $name['settings']);


After reassigning $name to a string on line 113, the code attempts to access $name['settings'] on line 115. This will cause a "Cannot use a scalar value as an array" error. The original array should be preserved in a separate variable before extracting values from it.

Suggested change

if (isset($name['name']) && is_string($name['name'])) {

$name = $name['name'];

}

if (isset($name['settings']) && is_array($name['settings'])) {

$settings = array_merge($settings, $name['settings']);

$config = $name;

if (isset($config['name']) && is_string($config['name'])) {

$name = $config['name'];

}

if (isset($config['settings']) && is_array($config['settings'])) {

$settings = array_merge($settings, $config['settings']);

Copilot · 2026-01-01T22:22:34Z

htdocs/class/cache/xoopscache.php

+
        $config = $_this->config($config);

+


Extra blank line. There are two consecutive blank lines here which is inconsistent with the rest of the codebase.

Suggested change

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T00:26:41Z

htdocs/class/cache/xoopscache.php

+
        $config = $_this->config($config);

+


Extra blank line should be removed for code consistency.

Suggested change

Copilot · 2026-01-02T00:26:42Z

htdocs/class/cache/xoopscache.php

+            }
+
+            if (isset($config['settings']) && is_array($config['settings'])) {
+                $settings = array_merge($settings, $config['settings']);


The behavior change from using extract to array_merge could introduce subtle bugs. The old code would completely replace settings with the value from the config array, but the new code merges them. If the intention is to maintain backward compatibility, line 119 should be changed to assign the settings directly rather than merging: $settings = $config['settings']; instead of $settings = array_merge($settings, $config['settings']);

Suggested change

$settings = array_merge($settings, $config['settings']);

$settings = $config['settings'];

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

htdocs/class/cache/xoopscache.php:435

Potential null array offset issue: When $_this->name is null (which is its initial value), accessing $_this->configs[$_this->name] will use null as an array key. This should be guarded with a null check on $_this->name before attempting array access, similar to the pattern used in the updated config method at lines 156-161.

        if (!$engine && isset($_this->configs[$_this->name]['engine'])) {
            $engine = $_this->configs[$_this->name]['engine'];
        }

htdocs/class/cache/xoopscache.php:451

Potential null array offset issue: When $_this->name is null (which is its initial value), accessing $_this->configs[$_this->name] will use null as an array key. This should be guarded with a null check on $_this->name before attempting array access, similar to the pattern used in the updated config method at lines 156-161.

        if (!$engine && isset($_this->configs[$_this->name]['engine'])) {
            $engine = $_this->configs[$_this->name]['engine'];

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T14:19:17Z

htdocs/class/cache/xoopscache.php

+
+

Excessive blank lines: There are three consecutive blank lines (lines 99-100) which is inconsistent with the rest of the codebase that typically uses single blank lines between methods.

Suggested change

Copilot · 2026-01-02T14:19:18Z

htdocs/class/cache/xoopscache.php

+        $config = $_this->resolveEngineConfig($config);
+        if ($config === false) {
+            return false;
        }


Inconsistent return type behavior: The method returns false when config resolution fails (line 338), but the PHPDoc at line 328 states the return type is mixed (cached data or false). While this is technically correct, it creates an ambiguity where false could mean either "config failed" or "cached data is false". Consider documenting this nuance or using a different sentinel value for configuration failures.

Copilot · 2026-01-02T14:19:18Z

htdocs/class/cache/xoopscache.php

+        }
+
+        $engine = $config['engine'];
+


Missing null check: The code assumes $_this->engine[$engine] exists and is not null before calling gc() on it. If the engine initialization failed (line 224 sets it to null), this will cause a fatal error. An isInitialized check should be added before accessing the engine, similar to the pattern used in write, read, delete, and clear methods.

Suggested change

if (!isset($_this->engine[$engine]) || $_this->engine[$engine] === null) {

return false;

}

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T15:40:34Z

htdocs/class/cache/xoopscache.php

+     * @var array<string, XoopsCacheEngine|null>
     * @access protected
     */
+    protected $engine = [];


The $engine property is now initialized as an empty array, but throughout the codebase it's used with string keys (e.g., $_this->engine[$name]). Consider adding a check in the engine() method to ensure that $_this->engine[$name] is not just isset() but also an instance of the expected cache engine class before calling methods on it, to prevent potential type errors if the array is manipulated incorrectly elsewhere.

Copilot · 2026-01-02T15:40:35Z

htdocs/class/cache/xoopscache.php

+     */
+    private function resolveEngineName($engine)
+    {
+        if (is_string($engine) && $engine !== '') {


The validation $engine !== '' is checking for empty strings, but the parameter type allows string|null. Consider explicitly checking for null first ($engine !== null) before the string checks for clearer intent and to handle the null case distinctly from empty strings.

Suggested change

if (is_string($engine) && $engine !== '') {

if ($engine !== null && is_string($engine) && $engine !== '') {

Copilot · 2026-01-02T15:40:35Z

htdocs/class/cache/xoopscache.php

+            $config = $name;
+
+            if (isset($config['name']) && is_string($config['name'])) {
+                $name = $config['name'];
+            }


When $name is an array, it's assigned to $config, but if $config['name'] is not set or not a string, $name remains an array. This could lead to type errors in subsequent code that expects $name to be a string. Consider setting a default string value (e.g., 'default') if the name cannot be extracted from the config array.

Copilot · 2026-01-02T15:40:35Z

htdocs/class/cache/xoopscache.php

+            }
+
+            if (isset($config['settings']) && is_array($config['settings'])) {
+                // Preserve legacy behavior: overwrite settings, do NOT merge


The comment uses 'do NOT merge' for emphasis, but the standard style would be 'do not merge' without capitalization. Consider using consistent casing for better readability.

Suggested change

// Preserve legacy behavior: overwrite settings, do NOT merge

// Preserve legacy behavior: overwrite settings, do not merge

Copilot · 2026-01-02T15:40:35Z

htdocs/class/cache/xoopscache.php

+        // Check if engine is initialized before calling gc()
+        if (!$_this->isInitialized($engine)) {
+            trigger_error("Cache engine {$engine} not initialized for garbage collection", E_USER_WARNING);
+            return false;
+        }
+


The isInitialized() method now returns false if $engine is null (from resolveEngineName()). However, at line 274, $engine is extracted from the validated config, so it should always be a string. The check at line 277 is redundant given that resolveEngineConfig() already validates the engine. Consider removing this check or documenting why it's necessary despite the earlier validation.

Suggested change

// Check if engine is initialized before calling gc()

if (!$_this->isInitialized($engine)) {

trigger_error("Cache engine {$engine} not initialized for garbage collection", E_USER_WARNING);

return false;

}

Copilot · 2026-01-02T15:40:36Z

htdocs/class/cache/xoopscache.php

+        return is_string($engine)
+            && isset($_this->engine[$engine])
+            && $_this->engine[$engine] !== null;


The triple condition check is_string($engine) && isset($_this->engine[$engine]) && $_this->engine[$engine] !== null could be simplified. If $engine is null or not a string, the isset() check would return false anyway. Consider reordering to is_string($engine) && !empty($_this->engine[$engine]) for conciseness, though this changes semantics slightly if the engine value is falsy but not null.

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T17:49:08Z

htdocs/class/cache/xoopscache.php

+
+        $engine = $config['engine'];
+
+// Engine configuration is valid, but runtime initialization may have failed


Comment formatting is inconsistent with project standards. There should be a space after the double slashes in inline comments.

Suggested change

// Engine configuration is valid, but runtime initialization may have failed

// Engine configuration is valid, but runtime initialization may have failed

Copilot · 2026-01-02T17:49:09Z

htdocs/class/cache/xoopscache.php

+ * @package    core
+ * @subpackage cache
+ * @since      2.5.12
+ * @deprecated 4.0.0 Use Xoops\Core\Cache\EngineInterface instead


The deprecation notice refers to version "4.0.0" which appears to be a hypothetical future version. While this is acceptable for deprecation notices indicating long-term plans, consider whether a more concrete version number should be used if the deprecation timeline is known, or add a note explaining this is a planned future major version.

Suggested change

* @deprecated 4.0.0 Use Xoops\Core\Cache\EngineInterface instead

* @deprecated 4.0.0 (planned future major version) Use Xoops\Core\Cache\EngineInterface instead

fix Using null as an array offset is deprecated, use an empty string …

116ebac

…instead fix offset is deprecated, use an empty string instead replacing extract()

mambax7 requested review from Copilot and removed request for Copilot January 1, 2026 22:19

Copilot started reviewing on behalf of mambax7 January 1, 2026 22:20 View session

mambax7 requested a review from Copilot January 1, 2026 22:20

Copilot started reviewing on behalf of mambax7 January 1, 2026 22:20 View session

Copilot AI reviewed Jan 1, 2026

View reviewed changes

fix for potential "Cannot use a scalar value as an array" error

0e13b73

mambax7 requested a review from Copilot January 2, 2026 00:23

Copilot started reviewing on behalf of mambax7 January 2, 2026 00:24 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

cleanup

90599b6

mambax7 requested a review from Copilot January 2, 2026 14:14

Copilot started reviewing on behalf of mambax7 January 2, 2026 14:15 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

minor improvements

a120f66

mambax7 requested a review from Copilot January 2, 2026 15:34

Copilot started reviewing on behalf of mambax7 January 2, 2026 15:35 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

adding XoopsCacheEngineInterface, improvements, cosmetics

cbd83ff

mambax7 requested a review from Copilot January 2, 2026 17:44

Copilot started reviewing on behalf of mambax7 January 2, 2026 17:45 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

	$settings = array_merge($settings, $config['settings']);
	$settings = $config['settings'];

	if (is_string($engine) && $engine !== '') {
	if ($engine !== null && is_string($engine) && $engine !== '') {

	// Preserve legacy behavior: overwrite settings, do NOT merge
	// Preserve legacy behavior: overwrite settings, do not merge


		$engine = $config['engine'];

		// Engine configuration is valid, but runtime initialization may have failed

	* @deprecated 4.0.0 Use Xoops\Core\Cache\EngineInterface instead
	* @deprecated 4.0.0 (planned future major version) Use Xoops\Core\Cache\EngineInterface instead

Uh oh!

Conversation

mambax7 commented Jan 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant