Weak typing fixed#629
Conversation
There was a problem hiding this comment.
Code Review
This pull request modernizes the MslsAdminIcon and MslsAdminIconTaxonomy classes by implementing PHP 7.4+ typed properties and updating logic to handle non-nullable types. Feedback identifies critical risks regarding uninitialized typed properties and potential TypeErrors when assigning WP_Error objects to string-typed properties, suggesting explicit initialization and error checking.
| protected string $icon_type = 'action'; | ||
| protected string $language; | ||
| public string $origin_language; | ||
| protected string $src; | ||
| protected string $href; | ||
| protected int $blog_id; | ||
| protected string $type; | ||
| protected string $path = 'post-new.php'; | ||
| protected int $id; |
There was a problem hiding this comment.
The newly added typed properties are not initialized with default values. In PHP 7.4+, accessing an uninitialized typed property before it has been assigned a value will throw a TypeError. Since these properties (like $language, $src, $href, and $id) are not initialized in the constructor but are accessed in methods such as get_img(), get_a(), and get_icon(), this will lead to runtime crashes if the setter methods are not called first. Initializing them with safe defaults (e.g., '' or 0) ensures the object is always in a valid state.
protected string $icon_type = 'action';
protected string $language = '';
public string $origin_language = '';
protected string $src = '';
protected string $href = '';
protected int $blog_id = 0;
protected string $type = '';
protected string $path = 'post-new.php';
protected int $id = 0;| $object_type = MslsTaxonomy::instance()->get_post_type(); | ||
|
|
||
| $this->href = get_edit_term_link( $id, $this->type, $object_type ); | ||
| $this->href = get_edit_term_link( $id, $this->type, $object_type ) ?? ''; |
There was a problem hiding this comment.
The get_edit_term_link function can return a WP_Error object. Since $this->href is now strictly typed as a string, assigning a WP_Error object to it will cause a TypeError at runtime. The current null-coalescing operator (??) only handles null values, not WP_Error. You should verify that the returned value is not an error before assignment.
$link = get_edit_term_link( $id, $this->type, $object_type );
$this->href = ! is_wp_error( $link ) ? (string) $link : '';
🔍 WordPress Plugin Check Report
📊 Report
|
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
86 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
📁 includes/MslsRestApi.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
405 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
📁 includes/MslsTranslationPickerTable.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
180 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
📁 includes/MslsWidget.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
58 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ALTERNATIVE_CONTENT_HOOK". |
📁 includes/MslsAdmin.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
247 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_REGISTER_ACTION". |
347 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ACTION_PREFIX . $section". |
📁 includes/Map/HrefLang.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
69 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_HEAD_HREFLANG_HOOK". |
📁 includes/MslsBlog.php (3 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
143 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_GET_PERMALINK_HOOK". |
147 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_GET_PERMALINK_HOOK". |
201 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::WP_ADMIN_BAR_SHOW_SITE_ICONS_HOOK". |
📁 includes/Component/Input/Select.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
39 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::RENDER_FILTER". |
📁 includes/MslsPostTagClassic.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
39 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ADD_INPUT_ACTION". |
71 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_EDIT_INPUT_ACTION". |
📁 includes/MslsMain.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
143 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_SAVE_ACTION". |
📁 includes/MslsPostTag.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
135 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ADD_INPUT_ACTION". |
170 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_EDIT_INPUT_ACTION". |
📁 includes/MslsOutput.php (6 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
96 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_GET_HOOK". |
125 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ALTERNATE_LINKS_HOOK". |
141 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ALTERNATE_LINKS_DEFAULT_HOOK". |
144 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_ALTERNATE_LINKS_ARR_HOOK". |
157 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_NO_TRANSLATION_FOUND_HOOK". |
188 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_GET_TAGS_HOOK". |
📁 includes/ContentImport/ContentImporter.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
295 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_BEFORE_IMPORT_ACTION". |
356 |
WordPress.NamingConventions.PrefixAllGlobals.DynamicHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "self::MSLS_AFTER_IMPORT_ACTION". |
🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check
No description provided.