register_taxonomy() should call a _doing_it_wrong() when called with an empty taxonomy name

[tyxla]

If you misuse register_taxonomy() with an empty string as a first parameter, this will currently not throw any notice.

Anyway, it is definitely wrong and can lead to some very weird results both in the administration and in the frontend.

I suggest that a _doing_it_wrong() should be called to cover this case.

[tyxla]

Adding a _doing_it_wrong() when register_taxonomy() is called with an empty taxonomy name.

[DrewAPicture]

Hi @tyxla, thanks for the patch, that's a good catch. I think we can probably modify the existing _doing_it_wrong() call to account for both cases, maybe "Taxonomies must be between 1 and 32 characters in length." We also need a unit test covering the empty string case.

[tyxla]

@DrewAPicture totally agree, just like on #31134. Adding a patch in a minute.

[tyxla]

Updating the first patch. Now using a single _doing_it_wrong() call for both taxonomy min and max, also adding unit tests for both cases.

[DrewAPicture]

Just a couple of things on 31135.2.patch​:

• Rather than tacking tests on to the main test_register_taxonomy() method, you should split them out to more specifically-named tests. For instance, the "> 32 characters" case is already covered by test_register_long_taxonomy() a few lines below your changes
• Realistically, we're checking for the returned WP_Error object instead of a _doing_it_wrong() to confirm the test, see below for an example of this:

  <?php
  /**
   * @ticket 21593
   *
   * @expectedIncorrectUsage register_taxonomy
   */
  function test_register_long_taxonomy() {
          $this->assertInstanceOf( 'WP_Error', register_taxonomy( 'abcdefghijklmnopqrstuvwxyz0123456789', 'post', array() ) );
  }
  

• So perhaps creating a test_register_short_taxonomy() block might be the way to go. I'd follow the same syntax as for long

[tyxla]

Thanks for making things more clear on how _doing_it_wrong() cases are tested. And I really missed the test_register_long_taxonomy() test.

Updating the patch with your suggestions right away.

[tyxla]

Updating unit tests as per @DrewAPicture's suggestions.

[DrewAPicture]

Related: #28683

[MikeHansenMe]

uses empty instead of < 1

[MikeHansenMe]

skip assigning length to variable since we only need it once

[mattheweppelsheimer]

+1 to this ticket as well as #31134. #28683 didn't go far enough.

[johnbillion]

In 31449:

Return a WP_Error if an empty name is provided when registering a taxonomy.

Fixes #31135
Props tyxla, MikeHansenMe

[SergeyBiryukov]

In 31457:

Use more descriptive names for register_post_type() and register_taxonomy() tests with too long and too short names.

see #31134, #31135.