Problem

  • The different casing in Drupal\entity\EntityController looks just... poor and wonky.

Goal

  • Sane PSR-0 namespace names for extensions.

Details

  • For simplicity and making progress, we decided to make the class loader register a module's namespace using the raw original module name (in lowercase).
  • The unusual lowercase component in module namespaces is confusing for developers and against common PHP class naming practices outside of Drupal.
  • Previous discussions mentioned that an automated lowercase_underscore to UpperCamelCase conversion would be problematic for modules that consist of multiple words but do not use underscores in their names; e.g.:
    • mediaelement » Mediaelement
    • i18nviews » I18nviews
    • xmlsitemap » Xmlsitemap
    • imageapi » Imageapi
  • However, a nondeterministic evaluation of module names in sites reveals that these are edge-cases (e.g., 1 out of 65 modules). And. Who really cares?
  • The vast majority of modules will work out perfectly:
    • admin_menu » AdminMenu
    • entity_translation » EntityTranslation
    • feeds_querypath_parser » FeedsQuerypathParser
    • git_deploy » GitDeploy
    • hierarchical_select » HierarchicalSelect
    • l10n_update » L10nUpdate
    • media_browser_plus » MediaBrowserPlus
    • menu_block » MenuBlock
    • views_bulk_operations » ViewsBulkOperations

Proposed solution

  1. Automatically convert the default PSR-0 namespace for extensions into upper CamelCase.

Known issues

  • The conversion makes one module occupy two namespaces, namely foo_bar as well as foobar.
    • That is not the case, since PSR-0 namespaces are case-sensitive (even though PHP's native namespaces are case-insensitive).
    • The first turns into Drupal\FooBar, the second into Drupal\Foobar.
    • While there is a point of visual ambiguity, the probability for facing this scenario in the real world is close to zero.
  • The conversion is magic / unpredictable.
    • It is definitely predictable within the Drupal system. The conversion algorithm is simple and easily understood.

Notes

  • Custom defined namespaces (via .info files or the like) are not up for debate here. Separate issue. This is about the automatically registered default namespace for modules.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Status: Active » Needs review

For review, even.

Status: Needs review » Needs work

The last submitted patch, drupal8.classloader-camelcase.0.patch, failed testing.

sun’s picture

Assigned: Unassigned » sun
Status: Needs work » Needs review
FileSize
48.54 KB

Now with the actual conversions.

Status: Needs review » Needs work

The last submitted patch, drupal8.classloader-camelcase.3.patch, failed testing.

RobLoach’s picture

YesPlease!

sun’s picture

Status: Needs work » Needs review
FileSize
49.42 KB

Forgot to rename the directories as well. (see http://drupal.org/node/1542048 for how to do this in Windows)

sun’s picture

Assigned: sun » Unassigned

Oh my. git reset --hard fails to revert the renames on Windows, so I'm definitely the wrong person to roll this patch. Hope someone else not on Windows can take it up! :)

Status: Needs review » Needs work

The last submitted patch, drupal8.classloader-camelcase.6.patch, failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
47.74 KB

Was missing module_autoload_test/lib/module_autoload_test --> lib/ModuleAutoloadTest.

Status: Needs review » Needs work

The last submitted patch, 1550680.patch, failed testing.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
52.44 KB

Here you go.

Crell’s picture

I disagree. Magic name folding is a PITA. As to the "who cares?", well, I do. :-)

There are TONS of modules that do not have an underscore for multi-word names, including many I've written, precisely to avoid the hook-name-clash problem. We'd be screwing those modules over.

Also, right now we have a very nice separation: Drupal\SomethingCapitalized is implicitly reserved for core. (Right now we just have Core and Component, but that's not to say it could never grow beyond that.) Drupal\something_lowercase is implicitly reserved for extensions (modules and themes). Forcing CamelCase on modules would increase the potential for collision there. (Side note: If we had Drupal\Modules\$modulename instead, that problem would be eliminated entirely.)

There is added cognitive overhead with this approach, because you have to mentally try and mimic the code to figure out what something will get named. That's added wetware CPU cycles I do not want to spend.

KISS.

neclimdul’s picture

I've said repeatedly in different places I think this is a bad idea. Its magic and since the only thing we require is that the entire module name as-is is unique, we can't guarantee that the converted name is unique so its unpredictable and wrong. If you want an upper camel case namespace I think you should register it explicitly as an extra namespace through some method like a field in a .info file or an early hook. The registration of arbitrary namespaces with the autoloader is is functionality that we'll need anyways.

sun’s picture

Issue summary: View changes

Updated issue summary.

sun’s picture

Thanks for your opinions and concerns. I interpret and reply to them as follows:

  1. The conversion makes one module occupy two namespaces, namely foo_bar as well as foobar.
    • That is not the case, since PSR-0 namespaces are case-sensitive (even though PHP's native namespaces are case-insensitive).
    • The first turns into Drupal\FooBar, the second into Drupal\Foobar.
    • While there is a point of visual ambiguity, the probability for facing this scenario in the real world is close to zero.
  2. The conversion is magic / unpredictable.
    • It is definitely predictable within the Drupal system. The conversion algorithm is simple and easily understood.

(to not repeat mistakes of the past, I'm collecting all concerns in the summary)

Crell’s picture

pounard’s picture

I was against magic and pro adding a namespace[] = "Drupal\FooBar" entry into the info file for people that don't want to inherit from the lowercase namespace, I still think this would be a good compromise.

neclimdul’s picture

We may always be able to predict the output of the algorithm in that given a module name we can determine what it will provide as output. We can not however ensure that the logic is predictable as to what it is doing to that state of the system. I can't think of a case in the current ecosystem but I will try to build a hypothetical example.

Say I have a views module that has a dependency on so_cool for extending a views filter. That module provides Drupal\SoCool\Views\SoCoolFilter because that's the namespace core autoregistered. I'm not particularly fond of so_cool and want to use socool for some other views so I enable it and start trying to build my site. Unfortunately socool also provides Drupal\SoCool\Views\SoCoolFilter by registering the SoCool namespace and the first time I try and build a view one of the two is going to get autoloaded and that is not predictable.

Really either module could claim that namespace. socool might have been around since 4.7 and had an early PHP 5.3 release that started using the namespace and keeps with it. socool might be based on a external project and is using their classes while so_cool is a Drupal only project with its own implementation. The fact is it doesn't matter why, it matters that is that we're endorsing something that can lead to a broken environment and hoping it doesn't happen and /that/ seems like a bad idea and I don't think the gain over explicit namespace registration is large enough to warrant it.

edit: fix totally indecipherable sentence.

pounard’s picture

At some point, you have to arbitrary give a namespace name, no matter what already exists. We already have a standard, which is to put modules under Drupal\SomeThing vendor, which is already enough to avoid potential conflicts with the outside world. Regarding potential module conflicts (module so_cool and module socool both claimimg the same namespace) I'd say this will happen one day or another, whatever effort we put into avoiding that, and I'd rather count on a good community communication rather than on a non flexible hardcore convention. We could even imagine a specific section on api.drupal.org that would be automatically fed with module claimed namespaces (based upon commits or module releases) to help developers avoiding conflicts. I'd prefer that to enforcing module namespaces being lowercased.

EDIT: I'd add to this statement that having a centralized git repository, standards, release management system, etc... for community modules is already a huge step towards avoiding such conflicts, there is not that many projects being that lucky. Plus module names being so close often happens when both modules don't support the same core version or when one is obsolete or unmaintained, which greatly reduces the potential conflicts.

pounard’s picture

Issue summary: View changes

Updated issue summary.

Crell’s picture

Issue summary: View changes
Status: Needs review » Closed (won't fix)

We ended up not doing this.