Skip to content
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

strict_types for initVar() #1143

Open
mambax7 opened this issue Jan 2, 2022 · 4 comments
Open

strict_types for initVar() #1143

mambax7 opened this issue Jan 2, 2022 · 4 comments

Comments

@mambax7
Copy link
Collaborator

mambax7 commented Jan 2, 2022

I want to set the default to 1:

$this->initVar('active', \XOBJ_DTYPE_INT, 1, true);

But XOOPS Kernel Object requires null:

phpStan: Parameter #3 $value of method XoopsObject::initVar() expects null, int given.

/**
* initialize variables for the object
*
* YOU SHOULD NOT USE THE $enumeration PARAMETER
*
* @access   public
*
* @param string $key
* @param int    $data_type set to one of XOBJ_DTYPE_XXX constants (set to XOBJ_DTYPE_OTHER if no data type checking nor text sanitizing is required)
* @param null   $value
* @param bool   $required  require html form input?
* @param int    $maxlength for XOBJ_DTYPE_TXTBOX type only
* @param string $options
* @param string $enumerations
*
* @return void
*/
public function initVar($key, $data_type, $value = null, $required = false, $maxlength = null, $options = '', $enumerations = '')
{
   $this->vars[$key] = array(
       'value'       => $value,
       'required'    => $required,
       'data_type'   => $data_type,
       'maxlength'   => $maxlength,
       'changed'     => false,
       'options'     => $options,
       'enumeration' => $enumerations);
}

Should we change

 * @param null   $value

to

 * @param string|int|null  $value 

Could we also make similar adjustments for all other cases like this one?

@geekwright
Copy link
Contributor

This is purely an incorrect docblock. If we try to create a list, we'll be forever twiddling it. The appropriate fix is:

* @param mixed $value

@mambax7
Copy link
Collaborator Author

mambax7 commented Feb 19, 2022

Are you going to do it, or should I make the adjustments for all the cases with "null" and submit PR?

@geekwright
Copy link
Contributor

Any * @param null lines are obviously wrong. Why have a parameter that only accepts null? I count about 43 occurances.

They are not all 'mixed' so some investigation is required. There are a lot that should be string|null and some array|null.

If you would like to do make the adjustments, please do. Otherwise I'll do it.

@mambax7
Copy link
Collaborator Author

mambax7 commented Feb 19, 2022

Whatever works better for you - I'm happy to help, but if you prefer to do it yourself because of potential errors on my part, I perfectly fine with that.
So just let me know...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants