Change return type hint of JsonCast::get method to mixed

[ChibuezeAgwuDennis]

Description

The JsonCast::get method currently has a return type hint of array|stdClass. However, considering the nature of the method, which decodes JSON strings into PHP types, the return value can theoretically be any PHP type, including scalar values, arrays, or stdClass objects.

Suggested Change

Change the return type hint of the get method from array|stdClass to mixed, to accurately reflect the potential return types.

Code Example

public static function get(
    mixed $value,
    array $params = [],
    ?object $helper = null
): mixed {
    // Existing logic...
}

Rationale

This change will prevent any type mismatches and make the method's expected behavior clearer to developers using the framework.

Additional Context

If the method is used with a JSON string representing a scalar value (like a string or number), the current type hints may cause confusion or type errors.

References

Relevant section in the codebase:

class JsonCast extends BaseCast {
    //...
}

Thank you for considering this improvement.

[neznaika0]

Tests for mixed values?
The normal behavior is an array or stdClass. Maybe string:
var_dump(json_encode(null)); // "null"

[ChibuezeAgwuDennis]

Response to "Tests for Mixed Values?"

The suggestion to use mixed as the return type for the JsonCast::get method is meant to capture the full range of potential return types that can result from decoding a JSON string. This includes not only arrays and stdClass objects but also scalar values (like strings and numbers) and even null. Here's how to address this:

1. Explain the Justification:

   • The json_decode function in PHP indeed returns various types based on the JSON input. By setting the return type to mixed, the method signature would accurately reflect all possible outcomes of decoding JSON strings, including edge cases where the JSON might represent a scalar or null.

2. Address the Current Use Cases:

   • While the common cases involve arrays or stdClass objects, there are legitimate scenarios where JSON might represent a simple value or null. For example:

     // JSON string representing a number
     $jsonNumber = "42";
     
     // JSON string representing a string
     $jsonString = "\"Hello, World\"";
     
     // JSON string representing null
     $jsonNull = "null";

3. Propose Testing Strategy:

   • To ensure that the method handles all possible return types correctly, below is how I implemented a series of tests aligned with CodeIgniter 4's testing standards. These tests covers:
     • Array and stdClass objects: to ensure that JSON objects and arrays are correctly decoded.
     • Scalar Values: to test cases for JSON strings representing numbers, strings, and booleans.
     • Null Values: to verify that JSON strings representing null are handled without errors.
     • Invalid JSON: to ensure that exceptions are thrown for invalid JSON strings.

4. Illustrate Test Cases:

   public function testJsonCastGetWithVariousJsonTypes()
   {
       // Test for array
       $jsonArray = '[{"key": "value"}]';
       $this->assertIsArray(JsonCast::get($jsonArray, ['array']));
   
       // Test for stdClass
       $jsonObject = '{"key": "value"}';
       $this->assertInstanceOf(stdClass::class, JsonCast::get($jsonObject));
   
       // Test for scalar value (number)
       $jsonNumber = '42';
       $this->assertIsInt(JsonCast::get($jsonNumber));
   
       // Test for scalar value (string)
       $jsonString = '"Hello, World"';
       $this->assertIsString(JsonCast::get($jsonString));
   
       // Test for null
       $jsonNull = 'null';
       $this->assertNull(JsonCast::get($jsonNull));
   
       // Test for invalid JSON
       $this->expectException(CastException::class);
       JsonCast::get('invalid json');
   }

In summary, by adopting mixed as the return type and implementing comprehensive tests, we can ensure that the JsonCast::get method is robust, flexible, accommodating the diverse nature of JSON data. This approach not only addresses potential type mismatches but also enriches the framework's reliability and developer experience.

[neznaika0]

int|null|bool|float It is not returned

[paulbalandan]

While json_decode may return scalars in some cases, do we have valid use cases for such inputs that would cause such returns within the context of CI4?

By virtue of YAGNI, if the use case for widening the return is because the underlying implementation theoretically covers such wide return but realistically no one ever enter into such scenario, I think it is not worth it.

[ChibuezeAgwuDennis]

@neznaika0:

You raised a valid point regarding the specific types int|null|bool|float not being explicitly returned by the current implementation. However, the intention behind proposing the use of mixed as the return type is to future-proof the method against any potential scenarios where these types might be encountered.

By leveraging mixed, we ensure that the method can handle any valid JSON input without any type-related constraints, thus enhancing its robustness. While the current use cases primarily involve arrays and stdClass objects, it’s prudent to consider the full spectrum of JSON data types.

@paulbalandan:

I appreciate your thoughtful consideration of the YAGNI principle, and I agree that practicality should guide our decisions. However, it’s worth noting that JSON data, especially when dealing with external APIs or user-generated content, can often be unpredictable. The use of mixed as a return type is a precaution to handle any eventuality gracefully.

While it might seem that scalar returns are not common within the current context of CI4, one could envision scenarios such as settings storage or configuration management where JSON-encoded scalar values are not only plausible but also practical. For example:

Scenarios in a CodeIgniter 4 Model Cast for Key-Value Options:

1. Configuration Settings:

   • A model that stores various configuration settings where each setting might be a different type, such as a boolean for feature toggles, a float for precision settings, or an integer for limits.

   class SettingsModel extends \CodeIgniter\Model
   {
       protected $table = 'settings';
       protected $casts = [
           'value' => 'json',
       ];
   }
   
   // Example JSON values:
   // {"feature_enabled": true}
   // {"timeout": 30}
   // {"version": "1.2.3"}

2. User Preferences:

   • A scenario where user preferences are stored in a JSON format, allowing for varied data types like strings, integers, and booleans to represent different settings.

   // Saving user preferences
   $preferences = [
       'theme' => 'dark',
       'notifications' => true,
       'font_size' => 14
   ];
   
   $userModel->update($userId, ['preferences' => json_encode($preferences)]);

3. Dynamic Form Data:

   • For applications that use dynamic forms, the data might be stored in JSON, where each form element can be a different type, such as a dropdown value (string), a checkbox (boolean), or a slider (integer).

   // Example JSON structure:
   // {"dropdown": "option1", "checkbox": false, "slider_value": 25}

The current framework usage might not heavily rely on scalar JSON types, adopting a mixed return type could provide a flexible foundation for future developments and integrations. This approach ensures that the framework remains adaptable to a broader range of use cases, safeguarding against unforeseen needs without introducing significant overhead.

[paulbalandan]

You are not giving a valid use case for scenarios where inputs are purely scalar. In your example 1, the JSON values are actually returned as json objects (or json arrays if required).

$json = '{"featured_enabled": true}';

// this is an object
$value = JsonCast::get($json);
var_dump($value->feature_enabled); // bool(true)

[paulbalandan]

Please stop using ChatGPT for giving us examples. Use a redacted version of your real world projects (or needs) where such scenarios to return null|bool is needed to be returned.

[ChibuezeAgwuDennis]

Example:

Array (
    [id] => 9
    [key] => site_name
    [value] => "Sarovus CMS" // This was JsonCast::set(...) which is a string
    [type] => string
    [group] => general_settings
    [created_at] => 2024-12-15 04:50:23
    [updated_at] => 2024-12-15 04:50:23
)

@paulbalandan, these are not ChatGPT examples. Does that mean one cannot delve into details with examples without chatgpt? It seems like you're not understanding me.

[paulbalandan]

Hi @ChibuezeAgwuDennis I do not mean to offend you in any way. But to me or maybe other readers that your previous responses are made entirely by chatgpt. I understand if you're not fluent in English and want the AI to help you construct your messages but the way I see it you type in the question and let the AI answer for you and that's it. No changes or anything.

[paulbalandan]

Now, my question on your example. Why is the cast you set for value is json when it is just a string?

[ChibuezeAgwuDennis]

@paulbalandan

Example in Model:

<?php

namespace App\Models;

use CodeIgniter\Model;

class Setting extends Model
{
    protected $table = 'settings';
    
    protected $allowedFields = [
        'key',
        'value',
        'type',
    ];
    
    protected array $casts = [
        'value' => 'json',
    ];
}

From the example above, our goal is to allow the value field to accommodate various data formats, including arrays, objects, and scalar types such as strings and numbers. This approach ensures that diverse types of data can be efficiently stored and retrieved from the value column in the settings table.

My Modification on the Core Code:

public static function get(
        mixed $value,
        array $params = [],
        ?object $helper = null
    ): mixed {
        if (!is_string($value)) {
            self::invalidTypeValueError($value);
        }

        $associative = in_array('array', $params, true);

        $output = ($associative ? [] : new stdClass());

        try {
            $output = json_decode($value, $associative, 512, JSON_THROW_ON_ERROR);
        } catch (JsonException $e) {
            throw CastException::forInvalidJsonFormat($e->getCode());
        }

        return $output;
    }

I changed the return hint type to mixed, which now works seamlessly for any data types without encountering any data type return errors.

I understand if this detail might be overwhelming for some readers, but my aim is to gain a simple yet clear understanding. The current implementation uses json_encode and json_decode, yet doesn't fully leverage these functions' capabilities within the ->get() method.

While the set method allows all data types when casting JSON, as shown here:

public static function set(
        mixed $value, // Any data type can be passed 
        array $params = [],
        ?object $helper = null
    ): string

The get method was restricted to returning only array or object. Please review this aspect of the code thoroughly.

I prefer not to modify the core code directly, which is why I'm sharing my suggestions here. If there is a more effective way to store values of any data type, please feel free to suggest it. I am willing to adapt to a better solution.

Although I refrain from comparing frameworks, I believe Laravel handles this scenario effectively. However, as a devoted user of CodeIgniter, I am eager to find an optimal solution within this framework as well.

[paulbalandan]

Thank you for your patience. Now I understand your use case.

As BaseCast::get() return type is mixed, I think we can safely change the JsonCast::get() to mixed as well. If you can send a PR to the 4.6 branch along with a few tests showing your use cases and some docs, that would be great. Thanks!