Skip to content

Converting warnings to exceptions in array related functions. #4566

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

Closed
wants to merge 4 commits into from

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 19, 2019

Converted various warnings from array functions to Error/TypeError exceptions.

The big one here is count()/sizeof() which even affected the run-test.php file.

@cmb69
Copy link
Member

cmb69 commented Aug 20, 2019

which even affected the run-test.php file.

Which shows that these changes need to be prominently documented (it seems that even https://wiki.php.net/rfc/consistent_type_errors is not mentioned in UPGRADING yet).

@Girgias
Copy link
Member Author

Girgias commented Aug 20, 2019

which even affected the run-test.php file.

Which shows that these changes need to be prominently documented (it seems that even https://wiki.php.net/rfc/consistent_type_errors is not mentioned in UPGRADING yet).

What would be the best way? A list of functions affected or just a broad statement? I suppose for TypeError it's not necessary needed but for warnings promoted to Error a list could be convenient.

@cmb69
Copy link
Member

cmb69 commented Aug 20, 2019

If we manage to convert all these warnings to errors accross the codebase (what we should), I think a general statement is sufficient.

@Girgias
Copy link
Member Author

Girgias commented Aug 20, 2019

But not all warnings are converted to Error (c.f. Out of bounds offsets in string functions in #4554 ) So I suppose if I'm not mistaken the general statement should specify that Warnings have been promoted to Errors in case of Empty parameters and invalid mode/flag values (which is the current way I'm processing on how to promote to Error)

@theodorejb
Copy link
Contributor

I will probably need to make changes to #4543 once this is merged.

@Girgias
Copy link
Member Author

Girgias commented Aug 20, 2019

I will probably need to make changes to #4543 once this is merged.

Yes for more return types o/

@Girgias
Copy link
Member Author

Girgias commented Aug 21, 2019

Split into smaller PRs

@Girgias Girgias closed this Aug 21, 2019
@Girgias Girgias deleted the array-convert-warnings-to-error branch November 20, 2019 19:25
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.

4 participants