Cast variable

[anhskohbo]

The method just return cast $this->options as array but actually $this->options assign before still a string in some case.

Bugs will be show when I try empty cmb setting in wp_options table.

[jtsternberg]

You will need to write unit tests to prove/test your scenario.

[anhskohbo]

Added unit tests

[jtsternberg]

I just ran the tests you wrote with and without the changes you made in includes/CMB2_Options.php, and they all pass either way. To properly test your changes, at least 1 of your assertions should fail without your patch. Otherwise, there is no reason to make the change. Make sense?

[jtsternberg]

Thank you for your PR. I have actually merged it to get your unit tests (and given you props), but I reverted the Cast variable changes, as they are actually causing incorrect results. I have written another unit test (and moved them all to tests/test-cmb-options.php) that asserts that an empty option value is still cast to an array (as it should be). When I apply your patch, those tests break, so I'm not sure what the intention of your patch, but it breaks the way CMB2_Options should work.

[anhskohbo]

I miss forgot line in last commit:

public function setUp() {
    ...

    add_option( 'cmb_empty_option', '' );
}

See the unit tests failures:

The bug is because this CMB2_Option::get_options:

$this->options = get_option( $this->key, $default );

...

return (array) $this->options;

and on CMB2_Option::update() or CMB2_Option::remove() method:

public function remove( $field_id, $resave = false ) {

	$this->get_options();

	if ( isset( $this->options[ $field_id ] ) ) {
		unset( $this->options[ $field_id ] );
	}

If some reasons, I have a option with empty string (or nothing value). And call CMB2_Option::remove() will be given error Illegal string offset ....

Because you only cast when return options. Actually $this->options still is a string.

[jtsternberg]

Ah, now I understand! ed2e58f should address that specific issue. Thank you for providing more info/clarification.