Remove FileSystemInterface::basename() and use PHP native basename()

Would be good to have a subsystem maintainer sign-off before any effort is put into code on this.

Pinging @kimb0 (Kim Pepper) on Slack.

Looks like you are correct. I'd be happy to deprecate/remove.

Confirmed that:

• the bug linked in the comment is no longer relevant.
• Directory separators are handled natively
• The behavior of php basename() seems to handle directories ending in slash just fine.

   * PHP's basename() does not properly support streams or filenames beginning
   * with a non-US-ASCII character.

@see http://bugs.php.net/bug.php?id=37738

Here's an 3v4l snippet to test the standard basename() behavior:

https://3v4l.org/5oTmX

I reviewed the changes and confirmed that all use of FileSystemInterface::basename() have been converted to basename().

My only question is on which version this will be removed in.

@deprecated in drupal:11.3.0 and is removed from drupal:13.0.0.

Is it too late to have it removed in drupal 12.0.0 ?

@kim.pepper Thanks for the quick review. Was typing up the following while your review came in:

This was actually not as embedded in core as I expected it to be.

Set deprecation for 11.3 removal in 13.0 based on (the not yet accepted) discussion in 📌 Defer disruptive 11.3 deprecations for removal until 13.0 Active out of concern it might be considered 'disruptive' (removing a method).

As an argument it is not disruptive: basename()is PHP native and works in PHP8.1 which is the minimum supported version for any D10.x release. A module could easily support ^10 || ^11 || ^12 with just the native function.

Above my position to make the final call on D12 vs D13 (willing to edit to D12 if it is approved).

This does impact a deprecation added in 🐛 Separate MIME type mapping from ExtensionMimeTypeGuesser Needs work the file_system was added to the ExtensionMimeTypeGuesser as optional in D11.2 and would have been required in D12 will now be removed in D11.3.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Back to NR after the branch was rebased.

Never received a merge conflict email, did this actually have a conflict or was this a bot FP?

It was a bot triggered comment that sent an email. I just rebased it.

I think this can be deprecated for removal in 12.

Disruptive deprecations are defined here: https://www.drupal.org/about/core/policies/core-change-policies/allowed-... →

I think a deprecation that is literally a find and replace and the replacement supports 11+ is not disruptive.

Further since this removes a deprecation for the negotiator I think you need to update and insert the message there and say it's not accepted, some people may have updated.

We can't just remove that argument.

I would also update the original cr with a note linking to this issue to follow since it's something that will only exist for a minor point

No conflict rebased on orgin/11.x
Deprecation set to D12.0
Deprecation added for constructor argument
Original related CR referenced to this issue

Reviewed this again. I think it is good to go, but will leave as NR for others.

Removing the sub-maintainer tag since this was worked on by the actual sub-maintainer :)

Did a search for "->basename(" and only instance is coming from the deprecation test
Issue summary is clear why this change can happen.
Reviewed both CRs and short and sweet. Kinda funny we made it required in 11.2 and not needed in 11.3 lol

Seems like a good replacements and no issues that I can see.