Mark all simple wrappers in bootstrap.inc and common.inc as deprecated

All functions in the issue description are marked deprecated and instructions are added on what to use instead. Only _current_path I'm not sure what to use, so didn't add instructions there.

Hmm, I see that I named the patch incorrectly. Sorry for that.

Lets leave t() there. Procedural code should still be able to translate stuff in a way that the string extractor finds them. Even if we end up with 0 usages in core (which i doubt) we cant force everyone to give up procedural code style
Besides that patch looks good

I think we need to leave them in until 9.x, or make this beta deadline.

@ianthomas_uk, thanks!

Two points of feedback:

1. _current_path() doesn't indicate what you should do instead, and there's no indication to me that "@todo This is a temporary function pending refactoring Drupal to use Symfony's Request object exclusively" was anything more than wishful thinking. Is there an issue for this?
2. For the others (like format_string()) that have clear replacements, we should have filed the issue that will convert all remaining core usages to the replacement, and reference that issue link in the deprecation note.

Thanks @rpayanm and @ianthomas_uk!

Since I last reviewed this issue, we've entered beta. Reviewing in terms of the allowed beta changes policy (https://www.drupal.org/core/beta-changes), this patch would definitely qualify as disruptive (because hundreds of lines of core code would be using deprecated functions as a result). It is not a prioritized type of change, and it is correctly marked as a normal task.

So, now that we are in the beta phase, we should instead be marking functions for removal for 9.0.0 if they were not already deprecated when the first beta was released. (In general, and in this issue in particular.) In addition, some of the deprecations below do not have finalized replacements in HEAD, so we should not deprecate them here. Specific notes below:

1. +++ b/core/includes/bootstrap.inc
   @@ -539,6 +539,9 @@ function t($string, array $args = array(), array $options = array()) {
   + * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.0.
   + *   Use \Drupal\Component\Utility\String::format().
     */
    function format_string($string, array $args = array()) {
   

   Over 600 uses of this still in core. Let's deprecate this for removal in 9.0.0 instead.

2. +++ b/core/includes/bootstrap.inc
   @@ -569,6 +572,9 @@ function format_string($string, array $args = array()) {
   + * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.0.
   + *   Use \Drupal\Component\Utility\Unicode::validateUtf8().
     */
    function drupal_validate_utf8($text) {
   

   This is not used so we should have an issue to just remove it in a separate issue. Can you file a separate followup issue and reference it on this issue? Actually, even if core doesn't use it, contrib might, so let's apply the same logic and deprecate it for 9.0.0 instead.

3. +++ b/core/includes/bootstrap.inc
   @@ -1168,8 +1174,13 @@ function request_path() {
   + * Internal use only
   + *
     * @todo This is a temporary function pending refactoring Drupal to use
     *   Symfony's Request object exclusively.
   + *
   + * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.0.
   + *   by https://www.drupal.org/node/2237001.
   

   This is a whole can of worms. We also should not mark things deprecated that do already have replacements in HEAD. I'd remove this change from the patch entirely.

4. +++ b/core/includes/bootstrap.inc
   @@ -1353,6 +1364,9 @@ function drupal_static_reset($name = NULL) {
   + *
   + * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.0.
   + *   Use \Drupal\Component\Utility\String::placeholder().
     */
    function drupal_placeholder($text) {
   

   3 uses in HEAD. Let's deprecate this for removal in 9.0.0 instead.

5. +++ b/core/includes/common.inc
   @@ -323,6 +323,10 @@ function valid_email_address($mail) {
   + *   Use \Drupal\Component\Utility\String::checkPlain().
   ...
    function check_url($uri) {
   

   16 uses in HEAD. Looks like this is incorrect text? Or at least we should be providing more information. Let's deprecate this for removal in 9.0.0 instead.

6. +++ b/core/includes/common.inc
   @@ -508,6 +512,9 @@ function format_size($size, $langcode = NULL) {
   + *
   + * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.
   + *   Use \Drupal::service('date.formatter')->format().
     */
    function format_date($timestamp, $type = 'medium', $format = '', $timezone = NULL, $langcode = NULL) {
   

   112 uses remaining. Let's deprecate this for removal in 9.0.0 instead.

Edit: removed comment that was about lines not in the patch. :P

Oh, regarding #19, it is out of scope to remove any code within this issue, but it looks like the latest patch in #21 reverted that.

Needs reroll.

$ git apply 2221771-21.patch 
error: patch failed: core/includes/bootstrap.inc:539
error: core/includes/bootstrap.inc: patch does not apply
error: patch failed: core/includes/common.inc:508
error: core/includes/common.inc: patch does not apply

Please review.

Just the fact that core currently uses a function doesn't mean it should not be deprecated for D9. I have a problem with deprecating for D9, because it really means "we won't actually deprecate this in any of our lifetimes."

Anyway, does this patch do what it says on the tin?

Yes.

Thanks @rpayanm, deprecating this for 9.0.0 is correct during the beta. Looks good. One more thing though:

+++ b/core/includes/common.inc
@@ -280,6 +280,9 @@ function valid_email_address($mail) {
+ *
+ * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
+ *   Use \Drupal\Component\Utility\SafeMarkup::checkPlain().
  */

As per #22 point 5, we need to add more detail to this documentation because checkPlain() is not a direct replacement for check_url().

Also, just a note for @rpayanm -- you can include an interdiff when updating patches so that reviewer can spot the changes you've made from the previous patch easily. Thanks!

+++ b/core/includes/common.inc
@@ -280,6 +280,9 @@ function valid_email_address($mail) {
+ *
+ * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
+ *   Use \Drupal\Component\Utility\SafeMarkup::checkPlain().

This can't be deprecated like this. I don't think we can deprecate it yet. Let's just remove the deprecation notice for check_url() and move on.

Undeprecate check_url().

Nothing to add about that.
It does what it says.

Sorry!
drupal_placeholder() is going to be marked as deprecated by #2552579: Remove SafeMarkup::placeholder(), deprecate drupal_placeholder() and stop drupal_placeholder() from marking safe. with a better comment. We should let that other issue manage this deprecation as it seems to be part of a bigger change.

Perfect, thank you :)
It applies and do the job.
Let's get it commited!

This is deprecating thing for Drupal 9 that have long been deprecated. Nice. Committed 5236486 and pushed to 8.0.x. Thanks!