-
Notifications
You must be signed in to change notification settings - Fork 228
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.
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
N°7645 - PHP 8.1: Fix usage of strpos() & str_replace() with null value when compiling empty dictionary #600
N°7645 - PHP 8.1: Fix usage of strpos() & str_replace() with null value when compiling empty dictionary #600
Conversation
Hello Thomas, |
I guess it is just |
Other PHP 8.1 findings:
|
Hello, |
Already discussed with @Molkobain on Slack. Displaying a Person on the portal (e.g. click on manager on profile page) causes the The other one was when going to a User's details page (might be related to having the activity panel open) |
ormCaselog error is handled in #607 |
Having this issue as well with 3.1.1:
This is becoming a major issue since it blocks the setup while upgrading to 3.1.1 when for example the sv-display-mgmt is installed. |
Even replacing this line with the following isn't a suitable workaround: <entry id="Class:Display/Attribute:type_id+" _delta="define"></entry> Looks like there is no way anymore to define an empty dict in xml !!! |
This to workaround the bug mentioned in Combodo/iTop#600
Woops absolutely sorry I forgot this PR :( |
@Molkobain will be the new assignee for the reviews next month. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technical review: We would rather fix the root cause than the symptom:
FilterDictString()
- Method type hinting can be enforce as it is only used within the compiler:
FilterDictString(string $s): string
- Fix te one usage in the line below your proposal:
self::FilterDictString($sValue ?? '')
- Method type hinting can be enforce as it is only used within the compiler:
QuoteForPHP()
- As it is used in many places, fix its internal usages with
$s = $s ?? '';
at the begining of the method.
- As it is used in many places, fix its internal usages with
Functional review: Accepted. |
…ue when compiling empty dictionary (#600) * fix(compiler): provide empty string instead of null value * Apply review suggestions
To reproduce
Having :
Deploy this XML, that is redefining a dict key to an empty value :
Upon compilation (either by the setup or the toolkit) you'll get the following notices :
They are blocking during setup.
Analysis
The methods
FilterDictString
andQuoteForPHP
both usestrpos
orstr_replace
who give a deprecation notice when the argument isnull
:This message is repeated for each empty dictionary item, which seems to happen on a Professional instance connected to the Designer.
Logs taken from a iTop Professional 3.1.0-3. The null argument gives a deprecation notice since PHP 8.1, which is supported for this iTop version.