Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#46327 closed defect (bug) (fixed)

Normalize class loading

Reported by: szepeviktor's profile szepe.viktor Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.2 Priority: normal
Severity: trivial Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:

Description

While trying to autoload core classes I've found these 3 irregularities:

wp-admin/includes/ajax-actions.php:3259: require_once ABSPATH . '/wp-admin/includes/class-wp-site-icon.php';
wp-admin/update.php:220: include_once( ABSPATH . 'wp-admin/includes/class-wp-upgrader.php' ); //for themes_api..
wp-includes/update.php:559: include_once( ABSPATH . '/wp-admin/includes/class-wp-upgrader.php' );
  • remove leading slash as ABSPATH contains it
  • remove trailing comment

Thank you.

Attachments (2)

46327a.patch (3.8 KB) - added by dmsnell 5 years ago.
Removes duplicated leading slashes of file imports
46327-2.diff (5.6 KB) - added by birgire 5 years ago.

Download all attachments as: .zip

Change History (11)

#1 @szepe.viktor
5 years ago

If anyone interested: https://github.com/szepeviktor/wordpress-autoloaded

WordPress without unconditional class loading + Composer

The shell script could be rewritten in PHP!

Last edited 5 years ago by szepe.viktor (previous) (diff)

@dmsnell
5 years ago

Removes duplicated leading slashes of file imports

#2 @dmsnell
5 years ago

@szepeviktor thanks for finding these duplicated slashes in the include references. I found a few more and added a patch to strip them out.

I'm not sure I understand why you want to remove the comment in update.php though - can you explain that a bit more?

#3 @szepe.viktor
5 years ago

I'm not sure I understand why you want to remove the comment

My regexp did not match :)

Maybe we can move the comment from the end of line to the previous line. What do you think?

#4 @dmsnell
5 years ago

My regexp did not match :)

Ah. That makes sense. Let's not move the comment though - there's nothing wrong with it being there.

You will definitely have better luck doing what you are wanting to do if you work from a PHP parser instead of from a RegExp. There aren't many choices but you shouldn't have any problem with nikic's php-parser.

If you want to catch trailing comments though I don't see why your pattern should be failing. Did you debug the matches to see why?

#5 @szepe.viktor
5 years ago

Okay, let's not touch the comment.

I proceed with class autoloading in core here: https://github.com/szepeviktor/wordpress-autoloaded

This ticket was mentioned in Slack in #core by dmsnell. View the logs.


5 years ago

#7 @SergeyBiryukov
5 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 5.2
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

@birgire
5 years ago

#8 @birgire
5 years ago

46327-2.diff removes some other cases of duplicated leading slashes within:

  • src/wp-admin/includes/class-wp-site-health-auto-updates.php
  • src/wp-admin/includes/class-wp-debug-data.php
  • src/index.php

#9 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 45190:

Bootstrap/Load: Remove duplicate leading slashes on inclusion of various files under ABSPATH.

Props dmsnell, birgire, szepe.viktor.
Fixes #46327.

Note: See TracTickets for help on using tickets.