From 23694341c9e8d2d4ee82cd986df366c665621c59 Mon Sep 17 00:00:00 2001 From: HerrVigg Date: Sat, 19 Feb 2022 23:56:22 +0100 Subject: [PATCH] QTS: fix warnings and cleanup (#671) Several functions had missing or incompatible return values. For example, WP actions do not expect return values -> void. Proper PHPDoc tags allow tools to detect inconsistencies. Fix return tags and complete also param tags. Delete comments bloating the code, minor cleanup. --- .../includes/class-qtranslate-slug-widget.php | 2 +- .../slugs/includes/class-qtranslate-slug.php | 43 +++++--- .../qtranslate-slug-settings-options.php | 10 +- .../includes/qtranslate-slug-settings.php | 100 +++--------------- modules/slugs/slugs.php | 3 - 5 files changed, 47 insertions(+), 111 deletions(-) diff --git a/modules/slugs/includes/class-qtranslate-slug-widget.php b/modules/slugs/includes/class-qtranslate-slug-widget.php index feaa8c66..fa6d504a 100644 --- a/modules/slugs/includes/class-qtranslate-slug-widget.php +++ b/modules/slugs/includes/class-qtranslate-slug-widget.php @@ -18,7 +18,7 @@ function widget( $args, $instance ) { $title = empty( $instance['title'] ) ? __( 'Language', 'qts' ) : apply_filters( 'widget_title', $instance['title'] ); $hide_title = empty( $instance['hide-title'] ) ? false : 'on'; $type = $instance['type']; - $short_text = ( $instance['short_text'] == 'on' ) ? true : false; + $short_text = $instance['short_text'] == 'on'; if ( $type != 'text' && $type != 'image' && $type != 'both' && $type != 'dropdown' ) { $type = 'text'; diff --git a/modules/slugs/includes/class-qtranslate-slug.php b/modules/slugs/includes/class-qtranslate-slug.php index 2cd90343..0345c61c 100644 --- a/modules/slugs/includes/class-qtranslate-slug.php +++ b/modules/slugs/includes/class-qtranslate-slug.php @@ -316,17 +316,21 @@ function init() { /** * Filters menu link attribute. + * + * @param string $item_title The menu item title attribute. + * + * @return string */ - function qts_filter_attr_title( $attr_title ) { - return $this->qts_quickuse( $attr_title ); + function qts_filter_attr_title( $item_title ) { + return $this->qts_quickuse( $item_title ); } /** * Adds proper links to the content with available translations. * Fixes issue #25 * - * @global $qtranslate_slug used to convert the url - * @global $q_config available languages + * @global QtranslateSlug $qtranslate_slug used to convert the url + * @global array $q_config available languages */ public function qtranslate_slug_header_extended() { if ( is_404() ) { @@ -372,7 +376,7 @@ public function get_slug( $id, $lang ) { * @param string $text the whole text * @param string $lang (optional) get the text in this language, or if empty, the current * - * @return array the text in the required language + * @return string the text in the required language */ public function qts_quickuse( $text, $lang = '' ) { $lang = '' == $lang ? $this->current_lang : $lang; @@ -380,6 +384,8 @@ public function qts_quickuse( $text, $lang = '' ) { if ( ! empty( $parsed_text[ $lang ] ) ) { return $parsed_text[ $lang ]; } + // TODO: check what to do if lang not found in ML values + return $text; } /** @@ -592,15 +598,16 @@ public function query_vars( $query_vars ) { } } + // TODO check this call, looks bug-prone return count( array_diff( $query_vars, $wp->public_query_vars ) ) > 0 ? $query_vars : $wp->public_query_vars; } /** * Function called when query parameters are processed by Wordpress. * - * @param $query query parameters + * @param array $query query parameters * - * @return array() $query processed + * @return array $query processed */ function filter_request( $query ) { global $wp; @@ -817,11 +824,11 @@ public function get_extra_permastruct( $permastruct = false, $name = false ) { * Filter that translates the slug parts in a page link. * * @param string $link the link for the page generated by Wordpress - * @param WP_Post $post + * @param WP_Post|WP_Error $post * @param bool $leavename * @param bool $sample * - * @return string the link translated + * @return string|WP_Error the link translated */ public function post_type_link( $link, $post, $leavename, $sample ) { global $wp_rewrite; @@ -1172,7 +1179,7 @@ function get_object_terms( $terms, $obj_id, $taxonomy, $args ) { */ public function hide_quick_edit() { echo "" . PHP_EOL; - echo "" . PHP_EOL; } @@ -1275,9 +1282,9 @@ public function draw_meta_box( $post ) { */ public function validate_post_slug( $slug, $post, $lang ) { - $post_title = trim( qtranxf_use( $lang, $post->post_title) ); + $post_title = trim( qtranxf_use( $lang, $post->post_title ) ); - $post_name = get_post_meta( $post->ID, $this->get_meta_key( $lang ), true ); + $post_name = get_post_meta( $post->ID, $this->get_meta_key( $lang ), true ); if ( ! $post_name ) { $post_name = $post->post_name; } @@ -1383,6 +1390,8 @@ public function wp_unique_post_slug( $slug, $post_ID, $post_status, $post_type, * * @param $post_id int the post id * @param $post object the post object + * + * @return void */ public function save_postdata( $post_id, $post ) { $post_type_object = get_post_type_object( $post->post_type ); @@ -1391,7 +1400,7 @@ public function save_postdata( $post_id, $post ) { || ( ! isset( $_POST['post_ID'] ) || $post_id != $_POST['post_ID'] ) // check revision || ( isset( $_POST['qts_nonce'] ) && ! wp_verify_nonce( $_POST['qts_nonce'], 'qts_nonce' ) ) // verify nonce || ( ! current_user_can( $post_type_object->cap->edit_post, $post_id ) ) ) { // check permission - return $post_id; + return; } foreach ( $this->get_enabled_languages() as $lang ) { @@ -1425,7 +1434,7 @@ public function show_term_fields( $term ) { $value = ( $slug ) ? htmlspecialchars( $slug, ENT_QUOTES ) : ''; echo "" . PHP_EOL; - echo "" . PHP_EOL; + echo "" . PHP_EOL; echo "" . PHP_EOL; } @@ -1535,6 +1544,8 @@ public function unique_term_slug( $slug, $term, $lang ) { * @param $term_id int the term id * @param $tt_id int the term taxonomy id * @param $taxonomy object the term object + * + * @return void */ public function save_term( $term_id, $tt_id, $taxonomy ) { $cur_screen = get_current_screen(); @@ -1542,7 +1553,7 @@ public function save_term( $term_id, $tt_id, $taxonomy ) { || ( ! current_user_can( 'edit_posts' ) ) // check permission || ( isset( $cur_screen ) && $cur_screen->id === "nav-menus" ) ) { - return $term_id; + return; } $term = get_term( $term_id, $taxonomy ); @@ -1773,7 +1784,7 @@ public function language_menu( $type = "text", $args = array() ) { //TODO: add i18n for alt attribute //43LC: hardcoding height and width - $link_flag = "\"$language_name\""; + $link_flag = "\"$language_name\""; } echo "
  • $link_flag$link_content
  • " . PHP_EOL; endforeach; diff --git a/modules/slugs/includes/qtranslate-slug-settings-options.php b/modules/slugs/includes/qtranslate-slug-settings-options.php index 1aab2fca..958cff69 100644 --- a/modules/slugs/includes/qtranslate-slug-settings-options.php +++ b/modules/slugs/includes/qtranslate-slug-settings-options.php @@ -25,7 +25,7 @@ function get_multi_txt_choices( $name = false ) { return array(); } $choices = array(); - foreach ( $q_config['enabled_languages'] as $key => $lang ) { + foreach ( $q_config['enabled_languages'] as $lang ) { $label = sprintf( __( 'Slug (%s)', 'qts' ), $q_config['language_name'][ $lang ] ); $choices[] = "$label|$lang"; // prints: 'Slug (English)|en' ( $name = books ) } @@ -77,7 +77,7 @@ function qts_options_page_fields() { "section" => "post_types", "id" => QTS_PREFIX . "post_type_" . $post_type->name, "title" => $post_type->labels->singular_name, - "desc" => sprintf( __( 'http://example.org/%s/some-%s/', 'qts' ), $post_type->name, $post_type->name ), + "desc" => sprintf( __( 'https://example.org/%s/some-%s/', 'qts' ), $post_type->name, $post_type->name ), 'class' => 'qts-slug', "type" => "multi-text", "choices" => get_multi_txt_choices( $post_type->name ), @@ -90,7 +90,7 @@ function qts_options_page_fields() { "section" => "taxonomies", "id" => QTS_PREFIX . "taxonomy_category", "title" => __( 'Categories', 'qts' ), - "desc" => __( 'http://example.org/category/some-category/', 'qts' ), + "desc" => __( 'https://example.org/category/some-category/', 'qts' ), "type" => "multi-text", 'class' => 'qts-slug', "choices" => get_multi_txt_choices( 'category' ), @@ -101,7 +101,7 @@ function qts_options_page_fields() { "section" => "taxonomies", "id" => QTS_PREFIX . "taxonomy_post_tag", "title" => __( 'Tags', 'qts' ), - "desc" => __( 'http://example.org/tag/some-tag/', 'qts' ), + "desc" => __( 'https://example.org/tag/some-tag/', 'qts' ), "type" => "multi-text", 'class' => 'qts-slug', "choices" => get_multi_txt_choices( 'post_tag' ), @@ -116,7 +116,7 @@ function qts_options_page_fields() { "section" => "taxonomies", "id" => QTS_PREFIX . "taxonomy_" . $taxonomy->name, "title" => $taxonomy->labels->singular_name, - "desc" => sprintf( __( 'http://example.org/%s/some-%s/', 'qts' ), $taxonomy->name, $taxonomy->name ), + "desc" => sprintf( __( 'https://example.org/%s/some-%s/', 'qts' ), $taxonomy->name, $taxonomy->name ), "type" => "multi-text", 'class' => 'qts-slug', "choices" => get_multi_txt_choices( $taxonomy->name ), diff --git a/modules/slugs/includes/qtranslate-slug-settings.php b/modules/slugs/includes/qtranslate-slug-settings.php index 30693b65..85d10cc7 100644 --- a/modules/slugs/includes/qtranslate-slug-settings.php +++ b/modules/slugs/includes/qtranslate-slug-settings.php @@ -9,8 +9,7 @@ * @return array */ function qts_get_settings() { - $output = array(); - // put together the output array + $output = array(); $output['qts_option_name'] = QTS_OPTIONS_NAME; // the option name as used in the get_option() call. $output['qts_page_title'] = __( 'Qtranslate Slug options', 'qts' ); // the settings page title $output['qts_page_sections'] = qts_options_page_sections(); // the settings sections @@ -27,10 +26,9 @@ function qts_get_settings() { * * @param (array) $args The array of arguments to be used in creating the field * - * @return function call + * @return void */ function qts_create_settings_field( $args = array() ) { - // default array to overwrite when calling the function $defaults = array( 'id' => 'default_field', // the ID of the setting in our options array, and the ID of the HTML form element @@ -70,29 +68,22 @@ function qts_create_settings_field( $args = array() ) { * Register our setting, settings sections and settings fields */ function qts_register_settings() { - // get the settings sections array $settings_output = qts_get_settings(); $qts_option_name = $settings_output['qts_option_name']; - //setting register_setting( $qts_option_name, $qts_option_name, 'qts_validate_options' ); - - //sections if ( ! empty( $settings_output['qts_page_sections'] ) ) { // call the "add_settings_section" for each! foreach ( $settings_output['qts_page_sections'] as $id => $title ) { add_settings_section( $id, $title, 'qts_section_fn', __FILE__ ); } } - - //fields if ( ! empty( $settings_output['qts_page_fields'] ) ) { // call the "add_settings_field" for each! foreach ( $settings_output['qts_page_fields'] as $option ) { qts_create_settings_field( $option ); } } - //style if ( ! empty( $settings_output['qts_page_styles'] ) ) { // call the "add_settings_field" for each foreach ( $settings_output['qts_page_styles'] as $styleoption ) { @@ -152,12 +143,12 @@ function qts_section_fn( $page_section = false ) { switch ( $page_section['id'] ) { case 'post_types': - echo "

    " . __( 'For example, the post_type books, in Spanish would be displayed as http://example.org/es/libros/post-type-name/. If you leave this blank will use the default option when you registered the post_type.', 'qts' ) . "

    "; + echo "

    " . __( 'For example, the post_type books, in Spanish would be displayed as https://example.org/es/libros/post-type-name/. If you leave this blank will use the default option when you registered the post_type.', 'qts' ) . "

    "; break; case 'taxonomies': - echo "

    " . __( 'For example, the taxonomy category, in Spanish would be displayed as http://example.org/es/categoria/taxonomy-name/. If you leave this blank will use the default option when you registered the taxonomy (if you previously setup a base permastruct for categories or tags in permalinks page, these bases will be overwritten by the translated ones).', 'qts' ) . "

    "; + echo "

    " . __( 'For example, the taxonomy category, in Spanish would be displayed as https://example.org/es/categoria/taxonomy-name/. If you leave this blank will use the default option when you registered the taxonomy (if you previously setup a base permastruct for categories or tags in permalinks page, these bases will be overwritten by the translated ones).', 'qts' ) . "

    "; break; case 'styles': @@ -181,7 +172,6 @@ function qts_show_form_field( $args = array() ) { $choices = $args['choices']; $class = $args['class']; - // get the settings sections array $settings_output = qts_get_settings(); $options = $qtranslate_slug->get_options(); @@ -309,15 +299,9 @@ function qts_show_form_field( $args = array() ) { /** * Validates base slugs per 'type' (post_type | taxonomy) and 'language'. */ -function qts_sanitize_bases( $base_slugs = false ) { - - if ( ! $base_slugs || empty( $base_slugs ) ) { - return; - } - +function qts_sanitize_bases( $base_slugs ) { $base_slugs_processed = array(); $base_founded = array(); - // changing array structure foreach ( $base_slugs as $type => $base ) { foreach ( $base as $lang => $value ) { @@ -342,30 +326,25 @@ function qts_sanitize_bases( $base_slugs = false ) { * @return void echoes output */ function qts_show_settings_page() { - // get the settings sections array $settings_output = qts_get_settings(); ?>

    - -

    pretty permalinks, in this section you can translate the URLs bases for public post_types, categories, tags and taxonomies.', 'qts' ); ?>

    +

    pretty permalinks, in this section you can translate the URLs bases for public post_types, categories, tags and taxonomies.', 'qts' ); ?>

    ; @@ -384,8 +363,6 @@ function qts_show_settings_page() { * @return array */ function qts_validate_options( $input ) { - - // for enhanced security, create a new empty array $valid_input = array(); // collect only the values we expect and fill the new $valid_input array @@ -393,26 +370,20 @@ function qts_validate_options( $input ) { // get the settings sections array $settings_output = qts_get_settings(); - - $styleoptions = $settings_output['qts_page_styles']; - - $slugoptions = $settings_output['qts_page_fields']; - - $options = array_merge( $styleoptions, $slugoptions ); + $styleoptions = $settings_output['qts_page_styles']; + $slugoptions = $settings_output['qts_page_fields']; + $options = array_merge( $styleoptions, $slugoptions ); // run a foreach and switch on option type foreach ( $options as $option ): - switch ( $option['type'] ): case 'text': //switch validation based on the class! switch ( $option['class'] ) { - //for numeric case 'numeric': //accept the input only when numeric! $input[ $option['id'] ] = trim( $input[ $option['id'] ] ); // trim whitespace $valid_input[ $option['id'] ] = ( is_numeric( $input[ $option['id'] ] ) ) ? $input[ $option['id'] ] : 'Expecting a Numeric value!'; - // register error if ( is_numeric( $input[ $option['id'] ] ) == false ) { add_settings_error( @@ -423,8 +394,6 @@ function qts_validate_options( $input ) { ); } break; - - //for multi-numeric values (separated by a comma) case 'multinumeric': //accept the input only when the numeric values are comma separated $input[ $option['id'] ] = trim( $input[ $option['id'] ] ); // trim whitespace @@ -446,22 +415,16 @@ function qts_validate_options( $input ) { ); } break; - - //for no html case 'nohtml': //accept the input only after stripping out all html, extra white space etc! $input[ $option['id'] ] = sanitize_text_field( $input[ $option['id'] ] ); // need to add slashes still before sending to the database $valid_input[ $option['id'] ] = addslashes( $input[ $option['id'] ] ); break; - - //for url case 'url': //accept the input only when the url has been sanited for database usage with esc_url_raw() $input[ $option['id'] ] = trim( $input[ $option['id'] ] ); // trim whitespace $valid_input[ $option['id'] ] = esc_url_raw( $input[ $option['id'] ] ); break; - - //for email case 'email': //accept the input only after the email has been validated $input[ $option['id'] ] = trim( $input[ $option['id'] ] ); // trim whitespace @@ -470,7 +433,6 @@ function qts_validate_options( $input ) { } elseif ( $input[ $option['id'] ] == '' ) { $valid_input[ $option['id'] ] = __( 'This setting field cannot be empty! Please enter a valid email address.', 'qts' ); } - // register error if ( is_email( $input[ $option['id'] ] ) == false || $input[ $option['id'] ] == '' ) { add_settings_error( @@ -481,8 +443,6 @@ function qts_validate_options( $input ) { ); } break; - - // a "cover-all" fall-back when the class argument is not set default: // accept only a few inline html elements $allowed_html = array( @@ -510,27 +470,21 @@ function qts_validate_options( $input ) { $text_values = array(); foreach ( $option['choices'] as $k => $v ) { - // explode the connective - $pieces = explode( "|", $v ); - + $pieces = explode( "|", $v ); $text_values[] = $pieces[1]; } foreach ( $text_values as $v ) { - // Check that the option isn't empty if ( ! empty( $input[ $option['id'] . '|' . $v ] ) ) { // If it's not null, make sure it's sanitized, add it to an array switch ( $option['class'] ) { // different sanitation actions based on the class create you own cases as you need them - - //for numeric input case 'numeric': //accept the input only if is numberic! $input[ $option['id'] . '|' . $v ] = trim( $input[ $option['id'] . '|' . $v ] ); // trim whitespace $input[ $option['id'] . '|' . $v ] = ( is_numeric( $input[ $option['id'] . '|' . $v ] ) ) ? $input[ $option['id'] . '|' . $v ] : ''; break; - case 'qts-slug': // strip all html tags and white-space. $exploded_types = explode( '_', $option['id'] ); @@ -538,8 +492,6 @@ function qts_validate_options( $input ) { $input[ $option['id'] . '|' . $v ] = sanitize_title( sanitize_text_field( $input[ $option['id'] . '|' . $v ] ) ); $input[ $option['id'] . '|' . $v ] = addslashes( $input[ $option['id'] . '|' . $v ] ); break; - - // a "cover-all" fall-back when the class argument is not set default: // strip all html tags and white-space. $input[ $option['id'] . '|' . $v ] = sanitize_text_field( $input[ $option['id'] . '|' . $v ] ); // need to add slashes still before sending to the database @@ -562,10 +514,8 @@ function qts_validate_options( $input ) { case 'textarea': //switch validation based on the class! switch ( $option['class'] ) { - //for only inline html case 'inlinehtml': // accept only inline html - // trim whitespace $input[ $option['id'] ] = trim( $input[ $option['id'] ] ); // find incorrectly nested or missing closing tags and fix markup $input[ $option['id'] ] = force_balance_tags( $input[ $option['id'] ] ); @@ -574,27 +524,20 @@ function qts_validate_options( $input ) { //calls stripslashes then addslashes $valid_input[ $option['id'] ] = wp_filter_kses( $input[ $option['id'] ] ); break; - - //for no html case 'nohtml': //accept the input only after stripping out all html, extra white space etc! // need to add slashes still before sending to the database $input[ $option['id'] ] = sanitize_text_field( $input[ $option['id'] ] ); $valid_input[ $option['id'] ] = addslashes( $input[ $option['id'] ] ); break; - - //for allowlinebreaks case 'allowlinebreaks': //accept the input only after stripping out all html, extra white space etc! // need to add slashes still before sending to the database $input[ $option['id'] ] = wp_strip_all_tags( $input[ $option['id'] ] ); $valid_input[ $option['id'] ] = addslashes( $input[ $option['id'] ] ); break; - - // a "cover-all" fall-back when the class argument is not set default: // accept only limited html - //my allowed html $allowed_html = array( 'a' => array( 'href' => array(), 'title' => array() ), 'b' => array(), @@ -656,25 +599,20 @@ function qts_validate_options( $input ) { break; } break; - case 'select': // check to see if the selected value is in our approved array of values! $valid_input[ $option['id'] ] = ( in_array( $input[ $option['id'] ], $option['choices'] ) ? $input[ $option['id'] ] : '' ); break; - case 'select2': // process $select_values $select_values = array(); foreach ( $option['choices'] as $k => $v ) { - // explode the connective - $pieces = explode( "|", $v ); - + $pieces = explode( "|", $v ); $select_values[] = $pieces[1]; } // check to see if selected value is in our approved array of values! $valid_input[ $option['id'] ] = ( in_array( $input[ $option['id'] ], $select_values ) ? $input[ $option['id'] ] : '' ); break; - case 'checkbox': // if it's not set, default to null! if ( ! isset( $input[ $option['id'] ] ) ) { @@ -683,19 +621,14 @@ function qts_validate_options( $input ) { // Our checkbox value is either 0 or 1 $valid_input[ $option['id'] ] = ( $input[ $option['id'] ] == 1 ? 1 : 0 ); break; - case 'multi-checkbox': unset( $checkboxarray ); $check_values = array(); foreach ( $option['choices'] as $k => $v ) { - // explode the connective - $pieces = explode( "|", $v ); - + $pieces = explode( "|", $v ); $check_values[] = $pieces[1]; } - foreach ( $check_values as $v ) { - // Check that the option isn't null if ( ! empty( $input[ $option['id'] . '|' . $v ] ) ) { // If it's not null, make sure it's true, add it to an array @@ -709,19 +642,14 @@ function qts_validate_options( $input ) { $valid_input[ $option['id'] ] = $checkboxarray; } break; - case 'multi-radio': $valid_input[ $option['id'] ] = ( empty( $input ) || ! isset( $input[ $option['id'] ] ) ) ? $option['std'] : $input[ $option['id'] ]; break; - endswitch; if ( ! empty( $valid_input ) && $valid_input[ $option['id'] ] === "qts-slug" ) { $valid_input = qts_sanitize_bases( $valid_input ); - } else { - $valid_input = $valid_input; } - endforeach; return $valid_input; diff --git a/modules/slugs/slugs.php b/modules/slugs/slugs.php index 544a63bb..8ac5b48a 100644 --- a/modules/slugs/slugs.php +++ b/modules/slugs/slugs.php @@ -15,9 +15,6 @@ if ( ! defined( "QTS_OPTIONS_NAME" ) ) { define( "QTS_OPTIONS_NAME", 'qts_options' ); } -if ( ! defined( "PHP_EOL" ) ) { - define( "PHP_EOL", "\r\n" ); -} // Init the module global $qtranslate_slug;