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

Updates for strict-types #6157

Merged
merged 7 commits into from
Jan 31, 2024
Merged

Conversation

drbyte
Copy link
Member

@drbyte drbyte commented Jan 27, 2024

Fixes #6082

@torvista
Copy link
Member

But then in strict types mode, explode on the next line complains when it is fed an integer...I've got functions_products and the shopping cart class using strict types mode and there are ("only") half a dozen places where casting is needed to clear all the errors (when a product is added to the cart).

@drbyte
Copy link
Member Author

drbyte commented Jan 27, 2024

Right.

Remember, coercive typing is one of the reasons people choose PHP over other languages like Java and C that are heavily strictly-typed.

@drbyte drbyte requested a review from zcwilt January 27, 2024 19:59
@torvista
Copy link
Member

While I may the only one beating ZC with this stick, there are really very few places left where ZC falls foul of this/it's nearly ready for a stricter world. Be nice to clear them.

@drbyte
Copy link
Member Author

drbyte commented Jan 29, 2024

Tested with several non-attribute products, with fractional quantities. Errors not thrown.

You probably have some specific products that warrant testing this with, though.

@torvista
Copy link
Member

Simple products give no errors.

Product with attributes:
PHP Fatal error: Uncaught TypeError: strpos(): Argument #1 ($haystack) must be of type string, int given in includes\classes\shopping_cart.php:316

if (strpos($option, TEXT_PREFIX) === 0) {

@drbyte
Copy link
Member Author

drbyte commented Jan 29, 2024 via email

@drbyte
Copy link
Member Author

drbyte commented Jan 30, 2024

Lots more updates made.
There's a HUGE risk of introducing rounding errors due to the loss of precision that occurs when trying to implement strict-typing with numeric data when it could legitimately be an integer or a float interchangeably ... such as with fractional quantities and with taxes.
I really hope this didn't break anything.

@torvista
Copy link
Member

In my limited testing, with the shopping_cart class, functions_products and functions_general_shared all using strict types, I could checkout with simple and attributed products with no rounding errors.

Only two simple errors:

$limit being passed as string instead of int.

$get_url .= rawurlencode(stripslashes($key)) . '=' . rawurlencode(stripslashes($value)) . '&';
}
} else {
if (IS_ADMIN_FLAG) continue; // admin (and maybe catalog?) doesn't support passing arrays by GET, so skipping any arrays here
foreach (array_filter($value) as $arr) {
if (is_array($arr)) continue;
$get_url .= rawurlencode(stripslashes($key)) . '[]=' . rawurlencode(stripslashes($arr)) . '&';

$key being passed to stripslashes as an int instead of a string

@drbyte
Copy link
Member Author

drbyte commented Jan 31, 2024

$key being passed to stripslashes as an int instead of a string

This is because elsewhere (some of) those values were cast to int for other reasons.

@drbyte drbyte changed the title Declare parameter as string|int for zen_get_prid Updates for strict-types Jan 31, 2024
@drbyte drbyte merged commit efad267 into zencart:master Jan 31, 2024
6 checks passed
@drbyte drbyte deleted the hybrid-param-zen_get_prid branch January 31, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

functions_products: zen_get_prid(): Argument #1 ($uprid) must be of type string, int given
2 participants