- Issue created by @scott_euser
- 🇬🇧United Kingdom scott_euser
Are there any consequences to this? Is there a reason we are not doing it by default?
- 🇩🇪Germany marcus_johansson
There shouldn't be anyting, we don't use any flags anywhere asfaik and always opt for getting arrays on decode. The reasoning in my code is that I didn't know it existed :)
In theory having more options on json_decode is nice, like the JSON_THROW_ON_ERROR would be a nicer solution than json_last_error? But until we need the flags, having consistent Drupal way of code is better, so we should change.
- 🇬🇪Georgia aivazashvilit
I think this issue needs to be closed as it was fixed here https://www.drupal.org/project/ai/issues/3477735 📌 Switch to Json::decode() from Drupal core Active
- 🇩🇪Germany marcus_johansson
@aivazashvilit - I think that issue just was for AI Search, not for the whole module in general. There are at least 20+ json_encode and 30+ json_decode left and I'm guessing most of them doesn't have any flags or any reason not to use Drupals built in function.
Will set this back to active.
- 🇬🇧United Kingdom MrDaleSmith
I've gone a little beyond the original scope on this to also include updating all the files in any /src folder to have proper php8 create statements and to document as many of the functions and methods as I can possibly can, in the hope that this will make the project a little more manageable.
Tests are passing, but I'd recommend a couple of people test their own specific areas to make sure the code was actually doing what they though - there were a lot of base classes and interfaces that specified return types for messages in doc comments (so not enforced) that conflicted with with implementations were actually returning, so I may have chosen the wrong side in those battles.
As part of this, I *think* that the test in tests/src/Kernel/OperationType/ImageClassification/ImageClassificationInterfaceTest::testImageClassificationBroken() may be giving a false positive: working through it, it is supposed to check for a specific error if a binary isn't provided to a provider doing image classification, but it is also not specifying a model in its call: if you do set a model, then the test fails. I've amended the code to replicate the existing test pass, but it may need someone who knows the intention behind the test to rejig it to be sure that's what is being tested.
It would be good for this to get merged ASAP as it's going to go stale very quickly.