Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#12519 closed defect (bug) (fixed)

wp_unregister_GLOBALS violates with specs

Reported by: hakre Owned by: dd32
Priority: lowest Milestone: 3.0
Component: General Version: 3.0
Severity: trivial Keywords: has-patch tested
Cc:

Description

The function wp_unregister_GLOBALS contains uppercase letters which breaks the specs which call for all functions containing lowercase letters only and using underscores to separate words.

Since function names are case insensitive in PHP, this can be easily fixed.

Additionally some other of the more basic Wordpress coding standards are not followed in that function which can be fixed.

Attachments (2)

12519.patch (1.2 KB) - added by hakre 3 years ago.
12519.2.patch (1.7 KB) - added by hakre 3 years ago.

Download all attachments as: .zip

Change History (14)

hakre3 years ago

X-Ref: #11971

  • Milestone changed from Unassigned to 3.0
  • Severity changed from normal to trivial
  • Priority changed from normal to lowest

comment:4 follow-up: ↓ 5   nacin3 years ago

  • Milestone 3.0 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

The function name is a clear reference to what the function manipulates - $GLOBALS.

comment:5 in reply to: ↑ 4   hakre3 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Your argument is not very convincing, the meaning of the function does not depend on case here.

This should have propper reasons, the coding standard is very clear about how functions have to be written:

Variables, functions, file names, and operators
Use lowercase letters in variable and function names (never camelcase). Separate words via underscores

Since this function is even called internally (in a single point of invoke!), I'll extend the patch to reflect that.

Please rething your argumentation.

hakre3 years ago

comment:6   dd323 years ago

  • Owner set to dd32
  • Status changed from reopened to accepted

In this case, I do not think changing the function name would be suitable, GLOBALS is the functionality its affecting.

As for the variables, they should be renamed to stay within the coding standards

comment:7   dd323 years ago

  • Milestone set to 3.0

comment:8   dd323 years ago

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

(In [13598]) Coding Standards: no camelcase variables, use lowercase for variables. Props hakre. Fixes #12519

I'm against making exceptions. The function is well named regardless of it's case. All contributors should be treated equal.

This function is the only exception that I could find in the whole codebase (!), I see it problematic to show other developers that certain personal styles of certain developers are preferred for not obvious reason.

Background info: Function was moved in load.php in r12732. Originally it was introduce in wp-settings.php as unregister_GLOBALS() in r2784 and renamed in r4203 to it's current name wp_unregister_GLOBALS() to prevent name collisions with various functions called unregister_globals() in many other projects.

Changing the case of the function as I suggested has not negative inpact, only positive.

  • Resolution fixed deleted
  • Status changed from closed to reopened

I am pretty sure the function name is even misleading with uppercase letters.

The function is in there to turn over a PHP setting called register_globals. The case is "all lowercase". That setting is pretty well known.

The functionality of the function in question in this ticket is to "revert" the effect of a switched on register_globals PHP ini setting.

Next to this new information, even after the last commit, the function name still violates the wordpress coding standard.

The issue therefore is still unfixed.

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

Related to violating coding standard function naming: #13583

Note: See TracTickets for help on using tickets.