Use something other than a single underscore for hook namespacing

subscribing. While this is a bit ugly, and would take some getting used to, I think it's a very good idea for a number of reasons.

Could someone please enumerate at least some of the "countless/numerous" reasons that make this patch compelling so close to freeze? We've managed to get 13 releases out without this, and no one has died so far, to my knowledge.

The number of contrib modules has doubled every release or something, module namespaces are going to get more crowded.

Whether we keep the registry or remove it, knowing that a hook implementation is some_module_name_hook() instead of some_module_name_hook() is going to get more and more necessary.

Latest attempt (curious for test bot to tell me what's still broken ;))

Should not the hook name be declared in the comment placed before the function body? Every hook implementation should have Implement hook_hook_name().

CNR for the bot.

Arg, talk about a painful patch without a real benefit. The current system works, it used alot and all this patch really does is make busy work for module developers.

Add also that there could be some module developers who, seeing the hooks being named in this way, will start to name other function so; if that really happens (and I am sure it would happen, as Drupal hooks are simply PHP functions), then it would not be possible to know if a function is a hook by watching its name.

The reason I think some developers would start to name their module functions module__function_name() is that the module functions are named module_function_name() because it is the name convention used for Drupal hooks; once you change the hooks name convention, then also the normal functions could start to follow the same convention.

Nothing have stopped anybody to use two underscores after the module name until now; once they note they can use two underscores also for the normal functions (and they see it works), they will start to use the two underscores for all the functions their modules define.

Hm... I would tend to agree that a double underscore is confusing and awkward.

If we're going to do this at all, maybe consider the possibility of a more descriptive separator? For example:

/**
 * Implement hook_form_alter().
 */
function mymodule_implement_hook_form_alter() {
...
}

actually... using double underscore between the module name and the actual function name, is in my opinion NOT detrimental, as it still creates more properly namespaced function names.

- views_menu_alter
v.s.
- views__menu_alter
- views_menu__alter

also, views_menu implementing hook_alter, while views implements hook_menu_alter ensures some pretty fatal errors.

in addition, it follows PHP precedent as that already uses the double underscore at the start (and end) of a string (or funtion/method) for magic stuff. (consider PHP as module '')

@#18: That would not apply for implementations of hook_form_FORM-ID_alter(); what would the hook name be, in that case?

@#19: The purpose of the change should be to make clearer when a function is the implementation of a hook, not to make more clearly namespaced functions.

As also reported from nevets, it is not worth to change the way the hooks are named, and it doesn't take anything good.

while it might not be the main intention, it does play on the background (see:#367355: Module names should not contain an underscore)

also, views_menu implementing hook_alter, while views implements hook_menu_alter ensures some pretty fatal errors.

Drupal doesn't define a hook_alter() hook; if there would be a third-party module to define such hook, then the fault would be of that module.
If then view_menu.module would implement hook_alter(), and both view_menu.module and views.module would be installed, the problem would be to have a function defined twice; even using two underscores for the hook functions, the problem would still present for other functions defined from the modules.

@#18: That would not apply for implementations of hook_form_FORM-ID_alter(); what would the hook name be, in that case?

Wouldn't it just be mymodule_implement_hook_form_FORM-ID_alter()?

@#22
my bad, was thinking of these:
- hook_prepare
- hook_insert
- hook_delete
- hook_update
- hook_validate

conflicting modulename endings, when prefixed with any other module name:
- actions
- comment
- file
- image_style
- node
- node_type
- taxanomy_term
- taxanomy_vocabulary
- user

for example, the module field_actions can not exist due to namespace conflicts.
(field_delete & field_actions_delete vs. field_actions_delete & field_actions_actions_delete)
even if only one module implements the field_actions_delete, wrong hooks get fired at the wrong time.

field_file_delete anyone?

@#23: Then, for a function that would alter the content type form, the function name should be mymodule_implement_hook_form_node_type_form_alter(). It seems that the name is getting too long, IMO.

mymodule__form_node_type_form_alter() would be a lot better

for example, the module field_actions can not exist due to namespace conflicts.

Which conflict should that be?

EDIT: The conflict would be for field_actions_delete(), which could be interpreted as implementation of hook_delete() made from field_actions.module, or as implementation of hook_actions_delete() made from field.module.
Probably field_actions_delete() would be interpreted as implementation of hook_delete() only if there is another hook implementation; still the conflict could exist for some hooks, and some modules.

I should have tested the patch better before I submitted, HEAD is totally broken visually with this patch.

However, I do think namespacing hook implementations better IS a win, but I'm not arguing that it will create work for existing module maintainers. This issue was highlighted as one that was desirable to get in at the code sprint in Paris, however it may take a bit to get it in... (too long?)

Neither one is clearer, it is a matter of convention and changing the convention with so much existing code generates busy work. The hooks are already "properly" namespaced, because it is a convention any choice allows for accidental function naming that conflicts with a hook name. I for one would like to see a stop to changes in Drupal core that have not real benefit but make work for developers.

Personnaly, i prefer :

function views_hook_menu() { }

than :

function views__menu() { }

I agree with nevets.
It's just a convention, and nothing would stop somebody to call a module views__menu.module; in that case there would still be a conflict between two modules. As Drupal is not forcing a module to not use the underscore in the module name (which is a good thing, IMO), then it will not force a module to not use two underscores in its name.

Unless there's a really good reason why not (and modules with 'hook' in name isn'n't one), mymodule_hook_hookname() seems pretty good - since it'd help searching within module code as well and we could possibly drop the 'Implements' from PHPdoc one day.

mymodule_hook_hookname() is much better, IMO; it avoids conflict problems, and it allows a module to have a function like mymodule_delete(), which would not be confused with hook_delete().
In this case, it would make sense to not accept modules that have a name ending in _hook; it makes more sense than to not accept a module with a name containing an underscore (which has never been done).

The problem is when to implement this change; maybe it would be better to postpone it for Drupal 8.x. Although, I am not contrary on implementing it on Drupal 7.x.

It's just a convention, and nothing would stop somebody to call a module views__menu.module; in that case there would still be a conflict between two modules.

do you have a usecase for a modulename with two underscores?
single underscores are common replacements of spaces.

In this case, it would make sense to not accept modules that have a name ending in _hook; it makes more sense than to not accept a module with a name containing an underscore (which has never been done).

it would be similar easy to deny modulenames with double underscores.

actually, denying modulenames with double underscores is one of the more easier things to do.

actually, denying modulenames with double underscores is one of the more easier things to do.

As module names has never been forced to not have a single underscore, I don't see why modules should be forced to not have two underscores.

do you have a usecase for a modulename with two underscores?

Do you have an example of module containing the word _hook in its name?
I find more logical to explain that a function name should contain _hook_ only when it is a hook implementation, rather than explaining that a function name should not contain two underscores.

Do you have an example of module containing the word _hook in its name?

how about a captain_hook module??? oh wait... that woudn't pass the GNU requirement ;) but seriously, how about a plot_hook module

in addition, I prefer generic solutions over specific solutions.

remember that PHP developpers normally don't use double underscores due to being reserved for PHP when used at the start (or end) of an string (__FILE__, __CLASS__, __construct())

Well hooks is a specific system, so using the word 'hook' in a function name doesn't seem any more specific than what we have now. Just a note that _hook_ would also simplify #521838: Clean up drupal_get_schema_versions() a bit.

A _hook_ would be awesome. I am going to come up with a script. Surely you do not want a patch -- only at the latest minute, said patch would break all the time. No, a script is what we need here. Set to CNR to attract more opinions esp from Dries/webchick.

Note that if you have page_hook module then your menu would be page_hook_hook_menu.

And a script.

I'm really not fond of changing something this fundamental so close to freeze. However, if we must do so then I think I find modulename_hook_foo() less offensive than modulename__foo(). It is more self-documenting and helps set "real" hooks apart from normal namespaced functions.

I prefer "__" instead of "_hook_". Not everything is a hook, and we have to figure out how to have better namespacing given that many contribute modules use a single underscore in their name. In other words, I don't think "_hook_" solves the actual problem. _hook_ would only solve part of the problem.

"__" is quite nice because it makes me think of "::".

Using "__" does not solve the complete problem either, because it does not address the issue of a module which accidentally implements another module's hook.

To solve the complete problem would require a mix of both solutions; for example, something like this:

function mymodule__hook_form_alter()
function mymodule__some_random_function()

I'd therefore suggest maybe just going with _hook_ for now, since it's necessary either way, and solves the biggest part of the overall problem.

Although it's not clear to me if we're even talking about Drupal 7 anymore at this point, or whether this issue should already be moved to Drupal 8?

To be honest, this does not sit in the top 20 things to in the next 4 weeks of the "Drupal 7 code slush". I think we should be focus on other things, and recommend moving this to Drupal 8.

done.

The real long-term solution is actually to use \, which is the PHP 5.3 namespace separator. Anything else we do here is just a stopgap until we can require PHP 5.3. :-)

subscribe

+1 to #35

having double underscores as namespace separator would serve for more than just hooks
for instead:
- hook names (which might have __hook or hook__ as well)
- constant names (like define('MYMODULE__MYSUBNAMESPACE__DEFAULT_VALUE', ...))
- submodules (like cck, cck__nodereference, etc)

@Crell
actually... you would have to use \\ or else... let's just say drupal\node would become

drupal
ode

really... that's the biggest phpWTF of all.

@arhak
sub-modules are also just regular modules, and still need to confirm to the naming convention, so they would be cck_submodule__hookname()

My opinion:

If we want this in D7:
modulename_[_]hook_[_]hookname
modulename__some_function as a recommendation.
and dedicated conventions for _alter_ and _preprocess_ etc.

If we want to wait for D8:
Use PHP namespaces and require PHP 5.3 (and see what PHP 6 brings).

@alexanderpas (#54):
Totally phpWTF, very disappointing.

I think that prefixing all hooks with _hook_ (implementation hook_nodeapi would be mymodule_hook_nodeapi() seems good; Nobody will never call his module mymodule_hook, plus, it's also really convienent when you use an IDE capable of outlining your code, it means that with a single look you can find the hooks implementations in anybody's code in a really easy way, which sometimes would be really usefull, when you try to understand a non documented module.
I really don't like the double underscore stuff.
Namespacing isn't so great, this is far away from the original template methods pattern (the hook system), which is more like specialization (not quite, but implementations uses specialization) in OOP. A lot of OOP languages use namespacing, but not to solve this kind of problems, it will be a total mess; but this is still my opinion. A lot of OOP discussions says that the right way to deal with conflicts is to have strict naming conventions (even in OOP code!).
All of this was IMHO.

This would be a fundamental improvement to the Drupal environment.

I think this is a critical improvement - I'd still favor something more terse "__" or "_h_" or something.

_hook_ is easier to google than _h_.
And I would prefer if '__' could be used for namespacing of regular functions.

If you like half-assed solutions, we could make it a convention that new hooks are called
hook_hook_xyz() or hook__hook_xyz.
This would force the format on all new hook implementations.

A simple separation of $module and $hook is not sufficient, because $hook is namespaced too and is actually the more pressing namespacing issue we need to fix.

As outlined in #588766-66: Standardize lazy-loading of optional include files:

views_comment_load()

becomes

views__comment__load()

That way, we can auto-determine the module name the hook belongs to. Therefore, all

hook__comment__*()

hook implementations can live in

$module.comment.inc

(auto-loaded)

However, if PHP 5.3 supports

views\\comment\\load()

then this would of course be the preferred solution.

This would allow us to entirely kill http://api.drupal.org/api/function/hook_hook_info/7

We should schedule this major change very early for the D8 cycle.

We all take it for granted that PHP namespaces would be the superior solution, if available. But what exactly are the benefits? Shorter function declarations?

Some possible problems of namespaces:
- don't work for PHP < 5.3 (but that would be acceptable for Drupal 8)
- the backslash sucks. well, this doesn't kill us.
- A lot of work for module maintainers (but that will be the same with _hook_).
- If you call a function without the namespace, it will be difficult for a reader to find out if you are calling a global function or a local one.

And what about the possible consequences for text search? Will it become harder or easier to find all implementations of a specific hook? Or just one specific implementation? Or what about Drupal API search in google?

I don't want to kill the idea, I just think we need better arguments. And if we don't find any, we can ditch the idea and go back to _hook_ or __.

Agree with donquixote, I'd prefer nice full OOP code or some function name prefix like _hook_ rather than using namespaces.

The Drupal core code has many different pieces using totally different designs, some new OOP parts (which I love, when OOP is clean designed, which is not the case for all new code, but this is not the place to discuss this) and old procedural code because of its history (which is well structured, and nice to develop with I'd say, and well documented too), you should not add usage of namespaces for only one use case, it will do an (OOP / PHP 5.3 namespaces / procuderal) combination which will be horrible to maintain and will loose all the module/core developers.

I think the '_hook_' prefixing idea still is best until now, except if you plan to do a full OOP core (which would a really really hard work we I look at the actual codebase).

This is my opinion as an every day module developer.

EDIT: because Drupal core is evolving really fast, you should prefer fix strong convention than try to use all new fancy stuff if you wan't to keep it stable and maintainable. Be carefull with design, else you will have big surprises, and may be loose a lot of users/developpers, especially if you modify so deeply the visible part of the code which is the module developers hooks with these new technos.

You seem to ignore the second part of the namespacing issue we need to solve.

views_hook_comment_hook_load()

...is not really what you want to suggest, or do you?

@sun:
Obviously you would use _hook_ for only one of the two.

views_hook_comment__load()
views__hook_comment__load
views__comment_hook_load (ugly ugly ugly)
vs
views__comment__load

The benefit of _hook_ is that it allows a more targetted search than __, and is more obvious for beginners.

I was not aware of the second part of the namespacing issue before you came up with it. Is it really that severe?

OOP is entirely unrelated here. Using classes to replace hooks is not even remotely on the table. There's plenty of other places where we should be using OOP, but hooks is not it.

That said, wha? Why do we need to use the separator twice, whatever it is? That makes no sense to me.

And really, can we leave this issue alone until Drupal 7 is done?

OOP is unrelated yes, but mixing so much coding styles is dangerous, whatever you choose as solution here, please keep coherentconsistent with what already exists in core.

There is a little of confusion on this.
If the hook is hook_comment_load(), then Views should implement that hook by declaring the function views_hook_comment_load(). The difference is that to get the name of the function that needs to be declared you now replace the word hook (used in the hook name) with the module name without extension; if this suggestion would be implemented, then you would prefix the hook name with the module name (always without extension), and an underscore.

There is no reason to name a function implementing a hook views_hook_comment_hook_load(), if the hook name is hook_comment_load().

Again, we have 2 issues:

1) Namespace conflict in module names: Given two hooks, hook_links() and hook_attach_links(), to which module belongs views_attach_links()? views.module or views_attach.module, and which module's hook implementation is it?

2) Namespace problem in hook names: Given the hook implementation views_admin_menu_alter(), to which module belongs the hook? admin.module or admin_menu.module?

While 1) deals with a major namespace problem and related function name clashes (fatal errors), we absolutely want to take 2) into account when considering any changes to hook implementation names. That is, because only a proper change for 2) is able to fix our broken auto-loading.

In other words: We need to know to which module a hook belongs, and we currently do not. If we would, then all modules could properly move support code for optional other modules into

$module.$someothermodule.inc

This include file is then only loaded if $someothermodule is actually available and installed. In D7, we at least allow for manual registration and grouping of hooks due to #588766: Standardize lazy-loading of optional include files.

Drupal core contains plenty of optional modules that may not be installed, but support code is loaded for them anyways, which is plain silly. #624324: Do not load support code for optional core modules tries to solve this with the tools at hand in D7.

The same is true for a multitude of contributed modules.

Hence, we should resolve both issues by doing a proper hook naming change in D8.

@sun:

The $module.$someothermodule.inc pattern makes a lot of sense if you include all of these files (for all enabled modules) with every request.

If you want to reduce the number of PHP files per request, then it will still help you - but other grouping patterns with $module.$groupname.inc might be more useful.

For instance, there are a bunch of hooks (hook_menu, hook_perm, hook_theme etc) that are only needed to rebuild some type of registry, so you don't need to load them for average requests. These hooks might be defined by different modules, but the module author doesn't want to have a separate file for each of them. (The examples are not the best, they all live in core)

If we don't use the $module.$someothermodule.inc pattern, the second "__" will become less necessary, except for some rare nameclash problems.

---------

Rare name clash problems:
The other theoretical argument for 2) would be possible hook name clashes: Both admin_menu.module and admin.module can define a hook_admin_menu_alter, which are supposed to do two separate things. Boom, nameclash.

This above name clash is far less likely than a regular function name clash, such as
a_b_foo() in a.module
vs
a_b_foo() in a_b.module
simply because there are more regular function names than there are hook names.

This is why I would appreciate a __ for regular function names, and a _hook_ (+ the second __, not about that) for hook names.

__ as of Drupal 7 is now how the theme system flags magic fallback behavior of a theme key, a la Views. Having that mean "namespace" elsewhere would be a serious WTF. Hence I favor _hook_ as the separator, if not true namespaces. Anything besides _ should solve #1.

As donquixote points out, there are plenty of groupings of hooks that we could want to use that do not map to the name of the module that "owns" them. So a tight coupling to module name there is not a good approach. I also would not bake the group name directly into the hook name. That seems like it would just make hook names unnecessarily longer, and for what benefit? For the ability to define group inline rather than in hook_hook_info()? There's lots of other things that we want to do with that hook anyway (per-hook weighting, etc.), so it is unlikely to go away entirely. So I don't see any serious benefit to moving the grouping inline. The logic in module.inc wouldn't get any appreciably nicer.

Off-topic note

Why do you still want to reduce the loaded code base per request? This is a noble goal but adding such overhead for autoloading is I think nonsens. I'd prefer to see few huge files rather than numerous small ones, because everyone uses op cache now.

With or without op code cache, it's the I/O's which are the worst performance killer (using a classic LAMP), I'd prefer big files loaded in op code cache, performant and low memory footprint algorithmes in real code, and no useless autoloading overhead (which is one of the Zend Framework performance killer).

Besides, module_load_include() or such functions still uses relatives pathes (in D6, don't know in D7), which makes PHP try to resolve in which include path are the files, which adds another overhead, which also happens when trying to load op code cached code, because the op code cache will have to resolve the real absolute path to check if the file exists in cache. Every file loaded or modified should always be accessed through an absolute path for maximum performance. So what's the point about autoloading if it's a performance killer?

The real goal of cuting a module into many files is more about code readability and maintenability, and may be a coding convention, but the more you cut your code into files, the more the performances are poor.

Non off-topic note

The _hook_ stuff should resolve the views_attach_links() and views_attach_links() story, the result would be views_hook_attach_links() and views_attach_hook_links(), seems that with this convention should solve the major function collision name.
For the "rare name clash" problem, I don't think there is any real solution until there is no code encapsulation possible (oop programing or namespace use). Even if I'm against namespaces usage, I must admit that without real name scope encapsulation, it does not exists any solution, naming collision will be still a problem and will remain a problem.

For the "rare name clash" problem, I don't think there is any real solution until there is no code encapsulation possible (oop programing or namespace use).

There is/are:
1) "__" + "__" as suggested by sun: "{$module}__{$other_module}__{$hookname}"
2) "_hook_" + "__" as suggested by myself: "{$module}_hook_{$other_module}__{$hookname}".
3) Rename your hooks, if you notice a name clash.

For published modules you can do a google search to check if the hook name is already taken. The bigger problem will be custom module hooks that clash with future contrib hook names. Still, this is far less likely than custom module names or function names clashing with future contrib.

@pounard: We've been through this a couple of times already. All loaded code is cached, not executed code. See this comment: #588766-14: Standardize lazy-loading of optional include files. Further reading: #345118: Performance: Split .module files by moving hooks. There's zero point in loading and caching code that is never ever executed (because the hook-exposing module is not installed). Aside from that, files are loaded using absolute paths in D7.

You people all seem to have an awful lot of time on your hands, given that you're debating something that can't be put into Drupal core for months.

How would you like to help prepare Drupal 7 for alpha 1 release, which is in < 72 hours?

@sun: I'm glad to ear it for D7, that's really good.
For the rest, non executed code is not a big deal, compiled and cached, it does not represent a big amount of memory. Drupal 6 uncompressed total size is less than 5mos, real code once modules are enabled and code compiled then op code cached is probably much less than that.
If you take account that a typical Drupal installation with views, content (fields for D7), an image handling solution, some fancy modules will needs a 64mo memory limit per PHP thread (that's pretty often the case, and that ss really huge) to ensure the site does not crash, I think's not really a problem.
Plus, this cached code size is constant once the site in production (and should not have a large footprint). I don't worry about non executed code, non executed code is much more non activated modules than dead if() because a module was not enabled nor specified in .info file. For example, making a my_module.admin.inc for 100 lines of code is really stupid, unless this is done for code readability.
Most of modules which uses casual code depending on configuration, and which have a really huge codebase (so a huge op code cache memory footprint) are modules like views, which have their own include and load methods and often does not use the core for that.

But, I'm sure that you done things right with D7, I don't want to argue on this point before I read and tested the code myself.

@donquixote: That '__' thing is right, but I think, as you says, they are rare, and as you says, "Rename your hooks, if you notice a name clash". This kind of foo_hook_bar__hello() functions names are becaming more complicated that implementing the code itself, I might sounds crazy saying that I like when the code looks good, but in fact, I really do think so, and even if it solves a rare problem, it's not really elegant, and a lot of module owners might won't respect this standard. I think it's showing some kind of mind complexity and starts looking like brainfuck language. Like, don't know, og_event_hook_views__views_data() as example, or node_hook__menu() or taxonomy_menu_hook__menu(), function names are kinda ugly. Attention, this is a 100% pure esthetical note (so not about technical stuff).

subscribing. I vote for '__' as hook seperator.

When we have this (or maybe earlier?), we can change the backport policy:
#786210: Introduction of new hooks in stable Drupal releases should be allowed and encouraged.

See also #867772: Use PHP 5.3 namespaces.

#54 this isn't correct

If you use single quotes, \ does not need to be escaped.

See http://www.php.net/manual/en/language.types.string.php#language.types.st...

I like this hook naming a lot, as it would really help in organizing module-specific functions versus hook implementations.

Not quite sure how I feel about double underscores "__" as it doesn't really accomplish much other then just confuscate the code more. Having "_hook_" gives more context to the function, defining itself as a hook.

The double underscores would be useful if we wanted to apply this to all module functions (i.e. things like menu loader callbacks). If we did that, it would allow for identifying which module a particular callback belongs to.

I originally wanted to leave that to the 5.3 namespaces issue, but namespaces looks like it would really obfuscate things, so this might be a case where we want to do it with a convention.

The double underscores would be useful if we wanted to apply this to all module functions (i.e. things like menu loader callbacks). If we did that, it would allow for identifying which module a particular callback belongs to.

I would still like to have '_hook_' or '__hook_' to identify hooks, even in addition to the '__' for regular module functions.

Typical situation: module foo defines a function foo__bar(). Later module bar decides there should be a hook_bar(). Now the foo__bar() is mistakenly identified as a hook implementation. No good.

I vote for:
- '__' for regular module functions
- '__hook_' for hook implementations
- another '__' in hooks, so it would be 'mymodule__hook_inventor__hookname()' or 'mymodule__hook_inventor()', where 'inventor' is the module that defines the hook.

All this if we discard the idea of namespaces.
I still don't see what is so bad about namespaces, but let's discuss this in the other issue.

Btw, we should also have a naming convention for classes.

I usually do
mymodule_SomeClass {..}
or
_mymodule_SomeClass {..}
for "private" classes - that is, only to be used in the same module.

With the new '__' convention it would be
mymodule__SomeClass {..}

The existing conventions for OOP code at
http://drupal.org/node/608152
just suck, they have no trace at all of a module name.
See my comment at http://drupal.org/node/608152#comment-3683598

Typical situation: module foo defines a function foo__bar(). Later module bar decides there should be a hook_bar(). Now the foo__bar() is mistakenly identified as a hook implementation. No good.

This makes no sense. Module foo would only define foo__bar(), if there was a hook__bar(). If there is no hook__bar(), it would be illegal to define foo__bar().

I'm not sure if I read this right, here's what it might look like:

// node_load()
function node__load() {};

// drupal_load()
function drupal__load() {};

// user_node_load()
function user__hook_node__load() {};

All hooks should be namespaced by module name, ones that aren't (like hook_load()) are bugs anyway, so I don't see a specific reason to add 'inventor' - we can just use MODULE__HOOKNAME as the hook name itself.

@sun I'm not sure what you mean.

mymodule defines a function - let's call it mymodule__i_want_a_banana();

Someone then creates the i_want_a module, this invokes hook_i_want_a_banana()

Now there's a hook called hook_i_want_a_banana()

So my_module__i_want_a_banana() gets invoked despite not being a hook implementation.

However, this doesn't necessarily mean we need _hook_.

Instead you could have:

function mymodule__i_want_a_banana();

module_invoke_all('i_want_a__banana');

now only

function mymodule__i_want_a__banana()

would get called as the hook implementation,

function mymodule__i_want_a_banana()

wouldn't.

Is going to be tough if you make a mistake and forget the double underscore though.

@sun (#86):
foo__bar() would be a legal regular function name, if we require '__' as a general separator for module functions.

@catch (#87, #88):

• We might want to allow hook names that are just the module name, and nothing else. So, module bar could define a hook_bar, and another hook_bar__something. If we want to allow the simple hook_bar, is a matter of taste.
• Some modules might want to use the '__' separator for things other than regular hooks. For instance, callbacks with some kind of suffix.
• As pointed out in #82, Having "_hook_" gives more context to the function, defining itself as a hook. It just looks and feels more semantic.
  

mymodule defines a function - let's call it mymodule__i_want_a_banana();

If you define such a function, then it must be a hook implementation.

I've no idea why we're discussing to have __ as a generic module namespace thing. That makes zero sense to me, has never been the idea of this issue, and I've personally never heard of function definition conflicts between modules that are not hook implementations. In turn, the proposal to have such a generic module namespace separator tries to solve a problem that does not exist.

@catch (#88):
So if we allow a module named 'i_want_a_banana' to define the hook_i_want_a_banana', and we do not have a '__hook_' separator, then 'function mymodule__i_want_a_banana' will be mistaken as a hook implementation.

@sun - why must it be a hook implementation?

node_load() is not a hook implementation, nor is drupal_load(), but poll_load() is. This only works due to the stupidity of how hook_load() is invoked, but for any real, genuine, hook you would have a collision.

hook_load is not really a hook, I would say. It is just a custom magic callback mechanic.
We should consider that these custom magic callbacks also want to use '__' separator to avoid naming conflicts.

Right, it's not, but that's somewhere in core we have the naming collision, albeit protected by magic callback mechanics. I'm sure there's plenty of examples in contrib though. At least one or two contrib modules had to change function names due to new hooks added in D7.

I've personally never heard of function definition conflicts

Me neither.
But any time I have to come up with a new function name, I am silently worried about exactly this possibility of nameclash. That's a nasty paranoia following me all around, and making me invent extra long identifiers that I otherwise would not invent.

how about the Field Actions module?

hook_delete & hook_actions_delete
field_delete & field_actions_delete
field_actions_delete & field_actions_actions_delete

Thw problem is not so much function definition, (which simply WSOD drupal) but more the wrong function run at the wrong time. (PHP is unable to see weither field_actions_delete() is an implementation of hook_delete or hook_actions_delete if the field_actions module is enabled, even when the function is only defined in one of the modules, it will fire also for the wrong implementation.)

(The problem was luckily subverted by smart module naming.)

you can read my post @24 to see some examples.

@alexanderpas (#96):
I think we agree this has to be fixed, and so far all the solutions I've seen in this thread do in fact fix this problem.
The question is now, is this enough, or do we want to go further.

These are the possibilities (some of them can be combined):

1. Simple '__' separator only for hook implementations. So, mymodule_menu() becomes mymodule__menu(). This sufficiently solves the heart of the problem. It also means we can not safely use '__' for purposes other than hooks.
2. Instead of '__', we use '_hook_' as a separator. Either for aesthetic reasons (this can be discussed), or because we already use '__' for something else.
3. We further separate the "inventor" (module that defines or "owns" the hook) and the "hook name", so that we would have mymodule__actions__delete() or mymodule_hook_actions__delete() for a hook_actions__delete() defined by the actions module.
4. We require the '__' separator for all module functions, not just hooks.
5. We go straight to PHP 5.3 namespaces.

If the hook ambiguities are the only thing we are worried about, then any of the first two solutions are sufficient.
Solutions '__' (1) and '_hook_' (2) already require a considerable amount of work, but they are still the "cheapest" options that we have, in the sense of how much code we need to change.
Also, if we are looking for a temporary solution for D8, before we introduce PHP 5.3 namespaces in D9, then we should pick the one solution that is least work.
If we have to choose between (1) and (2), I would personally prefer (2) with '_hook_'.

As a final, "future proof" solution, I would rather choose either PHP 5.3 namespaces or, if we don't get there, something equivalent (#84).

+1 for option (1) of #97, mostly because it's shorter. I like keeping code lines under 80 chars, if possible. Also, function names are often indexed in the database, and thus contribute toward the 1000-byte (333 character) index length limit.

It's 768 / 256 in fact.

EDIT: *Huge* MySQL WTF.

hook_hookname_by_modulename I think the advantage is trivial: when teaching someone how to implement a hook just say "if you implement a hook just add "by modulename". Way more intuitive than "replace hook with modulename".

Edit the MySQL index is actually 255 for InnoDB (max index prefix is 767 bytes) and 333 for MyISAM.

@#103 -- So you want to switch from a prefix grammar to a postfix grammar.

Bump. I'm not changing it to avoid status wars, but this is actually a bug report.
I just got bitten by this, when I created a custom 'image_field' module for a site, which silently implemented hook_field_schema on behalf of image module and blew everything up.

Issue summary needs to be updated to compare/contrast between the status quo and the proposed alternatives, but (meh) I can't work up enough enthusiasm to care...

The hook_something_by_whatever() only downside is that no module can be named "by" but I guess no one will do that :)

namespace hooks\node {
  function foo() {
    echo "in a previous life I was node_foo()\n";
  }
}
namespace hooks\user {
  function foo() {
    echo "in a previous life I was user_foo()\n";
  }
}
namespace {
  // I put the module calling in the global namespace to demonstrate that's still available.
  $hook = 'foo';
  foreach (array('node', 'user') as $module) {
    $function = '\\hooks\\' . $module . '\\' . $hook;
    $function();
  }
}

Considerable prior discussion: #867772: Use PHP 5.3 namespaces

In #1290658: Move all module-provided classes to PHP namespaces (PSR-0 or similar), and autoload them without the registry [policy, no patch]
it seems likely that we will agree on a namespace-per-module pattern for classes.
There are some decisions to be made over there. Everyone may feel invited to participate.

So far this other discussion has been entirely about classes.
Imo, we need to find a namespace pattern that works nicely for both classes and functions, and of course for hooks.

-------------

This said, there is also this other issue
#867772: Use PHP 5.3 namespaces
which is more about namespaces.

As PHP 5.3 namespaces have been agreed on in other places, the other alternative ("_hook_" separator) seems like out of the game.. or not?
Should we rather continue in the other issue?

See also:
http://bluedroplet.com/blog/proposal-fixing-php-namespacing-drupal-8
#1393208: Namespace all Drupal PHP code (esp. procedural)

Note that I am only suggesting doing hooks at this time. We can put the rest in the global namespace and solve at another time. I have also figured out how we can actually execute this. Etc. Namespaces here are very intuitive . Whether the rest of Drupal moves into namespaces and how, we will figure out in time.

Also, any separator we use will be a Drupalism. Backslash is the PHP native. So yes, I consider the double underscore, _hook_ etc etc suggestions obsoleted by PHP 5.3 that's why I took this over and no I don't want to enter into the neverending general namespacing discussion.

I don't like it. Namespaces are meant to organize code. They should bring clarity (following directory structure for example) not introduce more drupalisms.

Code which lives in a namespace has very different rules than code which does not.

In the global namespace, you can do things like $dom = new DOMDocument() and throw new Exception(). Inside a namespace, you can't. You either need a "\" in front of the class name or a "use" statement at the top.

So with this proposal of converting only hooks to namespaces, a typical .module file would have two different halves (one enclosed by "namespace hooks\mymodule {}" and the other enclosed by "namespace {}") with two different sets of rules, wouldn't it?

This is related to a more general issue (see #1323082: Establish standards for namespace usage [no patch]) but it seems like converting part of a typical .module file to namespaces but not the rest brings the issue much more front-and-center; it would add a fair amount of structure and complexity to .module files and it's not clear to me what benefit it would really bring.

@David_Rothstein is right but it must be noted only classes and exceptions need a \ functions fall back to global. That said, I have a strong suspicion that by the time D8 rolls around a small backslash will be the last of our problems: likely everything will go into their own namespaces so fully-pathed classes will be best practice anyways. And, while those two examples are right, what you want to throw is a FieldException which will so become \Drupal\Field\Exception at least...

I agree with #113.

#113, #116 you forget that we already have a Drupalism. This will make it less magicky I think. If this would be new, perhaps we would do it another way. But we have what we have and this is but a relatively simple fix.

Note that I am only suggesting doing hooks at this time. We can put the rest in the global namespace and solve at another time.

This sounds tempting, but I disagree.
If we postpone the general namespacing, we will likely get something that is not future-proof. As a result, we have to do a big clean-up in D9, where the hook pattern changes again.

We can decide to implement piecemeal in D8, but we should design for the big picture of "everything has a namespace".
Although, for my own taste, I'd prefer full namespaces in D8, as in #867772-78: Use PHP 5.3 namespaces

I agree with #118.

If we use the namespace in multipe different ways, developers will loose their mind; I'm just sayin'

I agree this might be an elegant way of solving the hook namespace problem, but making it happen while global discussions about namespacing usage are being done elsewhere is not really a good timing: it may happen that we end up with two different conventions/standard from each side which would definitely be a huge WTF.

We should include both parties to this discussion.

Feels to me as well that we should agree on namespacing (contrib, hooks) once (in one place) and for all.

I don't necessarily oppose against putting hook implementations into a separate namespace (e.g. Drupal/Module/name_of_module/Hook) but I do oppose against using the names of namespaces to make hooks work magically.

I know this is what we are already doing with the current hook system, but this might be a good opportunity to switch to an actual observer pattern.

E.g. through a Module class that every module implements and in which they register their hook-implemenations/observers. Then it wouldn't matter how these hook-implementations are named or in which namespace they are placed.

Of course it would be good to define standards where hooks should live. If we use an observer pattern in which you can register observers we get total freedom how hooks are named.

Instead of putting all hook implementations in a Hook namespace, now we could put them in a sub-namespace named after the module or API that defines the hook. e.g.

namespace Drupal/Module/node/Token; function token_info() in node/node.tokens.inc (functions aren't autoloaded so filenames like this for procedural hook implementations could be great; they don't clash with PSR-0)
namespace Drupal/Module/block/Form; class UserProfileFormAlteration in block/Form/UserProfileFormAlteration.inc (also using the observer pattern)

If we could agree on the design space in #867772-78: Use PHP 5.3 namespaces, we'd be one big step forward.

When doing 'new Exception()' if Exception does not exist in the local namespace, the global namespace will be checked. You do NOT need to call it new \Exception.

Unless you have created namespace\Exception, in which case you need to do it to ensure you do not get the local version.

Oh it appears I am wrong. For classes PHP won't fall back but it will for functions and consts.

PHP is braindead. :(

Yes, see the namespace basics, it explains it well.

PHP is braindead. :(

I guess that's the autoload, why it has to be like that. For a "new Exception", PHP would always need to attempt autoloading \whatever\namespace\Exception, before it can use the native one.

I believe #128 is correct about why classes and functions behave differently. The debates on -internals about it were amusing. :-) It's mostly a performance question. (Note: This does not mean PHP is not braindead.)

I'm not entirely against #108, but I'm still cautious. Certainly it has its advantages, and chx is correct that, if anything, we'd be swapping one Drupalism for another so there's no net change in Drupalism count. I guess the question is whether it's a better Drupalism or a worse Drupalism. That's the part I'm undecided about.

chx: Let me ask this. Right now we have a lazy-load mechanism for hooks, hook_hook_info(). It's not great, and it's not widely used, but at least it's something. How would this change affect that? Is there some other way we could use it to improve lazy-loading of procedural hooks? Or would there be no effect whatsoever no matter what we do?

I believe that it would not, in fact, change the lazy loading of hooks either positively or negatively. There is no way to extract file names from the namespace when the namespace is already declared for us.

Though, ahh. We could cheat and use a lazy load namespace which would be interesting. I don't know if this is a GOOD idea mind you but we could do something silly in our module file like this:

namespace \drupal\modules\mymodule\lazyload {
  const menu = 'modulename.menu.inc';
  const entity = 'includes/modulename.entity.inc';
}

It's a half-baked idea, I haven't thought it out or through. It's probably flawed some way but nothing jumps out at me yet.

Is that really any different than hook_hook_info? It seems like a funkier syntax, less capable version of it. :-)

hook_hook_info only actually works for the module that defines the hook (which is part of what's awful about it) where this works for the module that implements the hook.

I'm also very concerned about the amount of data that could end up being in hook_hook_info() if we actually used it widely. [#977052:71] is a long list from just one module, and that doesn't include dynamic hooks which can be very numerous (and how do you do that for hook_form_FORM_ID_alter() when it's currently not necessary to register all forms). Especially once you have a site with lots of modules and lots of content types etc. which means potentially thousands of hooks, most of which will never get implemented.

I regularly see Drupal 6 sites that reach 120M+ memory usage with cold caches, and 80% of this is precisely from collating and processing large info hooks.

Generally the number of hook implementations will be a lot more manageable (which is part of why module_implements() caching works - there are always several magnitudes less implementations on a site than the theoretical maximum of modules * hooks).

But I don't think we should particularly tackle that in this issue, if we can resolve name collisions then that seems like a good reason to consider this, if it opens up something like Earl's example that'd be good to look at in a followup.

Did that actually make it in? I followed some of the discussion a few months ago, but then they hit RC, and https://wiki.php.net/rfc/autofunc says still 'under discussion'.

Just to confirm that there's not really any performance benefit from autoloading: when I tested the registry hook split patches for Drupal 7 with APC, there was only maybe 100k or 200k of memory savings and no change in CPU time at all. The 100-200k is nothing compared to the rest of the memory usage during a request, and also likely less than the extra metadata we risk lugging around to track it.

Either way, needs to be considered completely separately to this proposal (we've established it doesn't change anything either way in this regard as it stands).

#131
looks equivalent to an info hook. Just, the result does not need to be stored in an array, because that "array" already exists..
Yeah, so probably faster.
If this could be declared by the hook inventor to define default locations for hooks, there'd be much less code to write. (probably has been discussed somewhere already)

Anyway, this is all about which file to include for hook x.
Unless we want to encode this location in the namespace / hook name, we don't need to talk about it here.
To put mymodule_menu() into mymodule.menu.inc, we'd have to call it ..\mymodule\..\registry\menu() or similar. This would also allow namespace-per-file for those include files.
Any other lazy load mechanism can be discussed another day.

-------

That said, I still don't know if we agree on namespace-per-module.
If we postpone that to D9, we have to think about a smooth conversion from whatever half-assed we come up with for D8. Ideally, we should already know more or less how D9 will look like, and design D8 as an intermediate soltution. Otherwise, module maintainers will be punished twice.

We now have Drupal\$modulename as the namespace root for module classes. Is there any reason to not also make this the namespace root for hook implementations? For example, if module 'node' wants to implement hook_menu(), for this to be Drupal\node\hook_menu()? Then node.module can start with namespace Drupal\node;, and contain both API/helper functions (those that do not start with "hook_") and hook implementations (those that start with "hook_").

@effulgentsia,
I've been saying this all day (in other issues), and I absolutely agree.

More thoughts are in #1428592: Proposal: PHP 5.3 Namespaces for module-provided functions and hook implementations., but it may be better to just agree on that simple pattern for a start.

1) Note that we still would not be putting hook implementations into files in $module\lib. It would be a renaming only, not reorganization. We'd likely also be forced to use the namespace {} syntax in many cases, which is generally discouraged. If we didn't, then all of a .module file would get pushed into that namespace. Just pointing out the limitations of that approach.

2) There is some movement around converting wholesale from procedural hooks to Symfony's EventDispatcher, which supports any callable and is much more flexible. I do not know if it is going to bear fruit, but we will have to support modules declaring additional listeners for WSCCI anyway, since all of the kernel events are listeners. (That means that hook_init is likely to go away, for instance, as redundant.) So that's a problem we have to solve anyway.

And if we've solved that problem, however that happens... is there a reason to not go all the way to listeners, which, by using explicit registration, would avoid this problem entirely?

If we didn't, then all of a .module file would get pushed into that namespace.

This is all fine with the approach in #138.
Those recent proposals are designed to avoid {} syntax.

go all the way to listeners, which, by using explicit registration, would avoid this problem entirely?

This would be an interesting move indeed.
I did not look into the EventDispatcher thing so far, but I imagine that "explicit registration" is quite uncomfortable compared to auto-detected hooks. Also, this means that a bunch of listener objects need to be initiated, even if most of these get never called in this request.

Hooks are the one big thing that is keeping us on procedural, and I think a lot of people need to be convinced before we throw them away.

The other big thing about procedural is that it protects us from abuse of classes as namespaces, and the overuse of static. I rather sit on some procedural code for a longer while, than being fed with poor quality OOP..

There are ways around the "instantiate everything" problem; The way Symfony full stack handles it, in general, is by registering listeners as services in the DIC and then instantiating them out of the DIC on demand. I haven't looked at the specific code for it, but it's an interesting approach that we could likely steal outright.

At this point, hooks have their own performance problems with needing to be loaded at all times, plus the bootstrap order issue. Plus testability. If we can ease the instantiation problem, then I think it would be a good net trade-off.

@Crell: With due respect, revamping Drupal's hook system needs way more analysis and planning than some arbitrary pointers at Symfony's EventDispatcher/Listener or random "discussions" in IRC. So far, neither the Symfony nor the PSR-0 conversions showed a performance improvement, but instead, (in terms of kernel patch) actually a performance decrease (recent testbot runs on the full test suite show a consistent ~3 minute decrease compared to current core, which is a lot). Even more so, Symfony's EventDispatcher/Listener does not resolve the problems we actually have with our modular hook system. It merely repeats our mistakes, only differently. If we really want to improve our hook system, then we don't need our crappy current system, nor Symfony's crappy Event design. Neither of both resolves any of our hard problems in a modular environment that Drupal is. Replacing our hook system just for the kicks of "going Symfony" is pointless and won't fly. Before thinking about possible replacements, let's make sure to talk to base system maintainers to learn about past and current issues, in order to understand the entire problem space. ;)

On the current proposal of this issue (which is only about function namespaces, not about revamping/replacing the hook system entirely):

Having Drupal\$module\Foo pointing to */modules/$module/lib/Drupal/$module/Foo,
but Drupal\$module\hook\foo implicitly pointing to */modules/$module.module
sounds very confusing.

As @Crell pointed out, namespace declarations in the form of

namespace Drupal\mymodule {
}

would be required but are officially discouraged by the PHP manual since quite some time already. We could certainly get around that by moving all hook implementations into $module.hook.inc though.

@sun, please read #138 again.
There will be no curly brackets, if we follow more recent proposals. Just
Drupal\my_module\foo() (regular function)
Drupal\my_module\hook_foo() (hook implementation)

Maybe this should be added to the issue summary.

For having classes and procedural in separate files, I think this is something we need to get used to anyway.

Wrote the whole thing up.

function Foo\bar() {
  print "World" . PHP_EOL;
}
print "Hello ";
bar();

PHP Parse error:  syntax error, unexpected T_NS_SEPARATOR, expecting '(' in test.php on line 2

No, there will be a namespace declaration, curly-brace or otherwise.

sun: A separate hook file per module is a possibility, but modules routinely implement hooks on each other's behalf, too.

Also, I am not saying "Use EventDispatcher, discuss nothing else, end!" I am saying that if we're going to make a major invasive change to the hook system, we should seriously consider making more of them all at once because just namespacing all of the hook functions buys us fairly little. All it gets us is reducing name collision in return for a more involved syntax. It does not solve the performance issues with hooks, the bootstrap order issues, the global-state-so-untestable problem, etc. We need to consider all of those. EventDispatcher is an alternative approach that we will have in core already; that's unavoidable. That makes it worth trying to make work, else we end up with two event systems in core.

No, there will be a namespace declaration, curly-brace or otherwise.

The "otherwise" is perfectly acceptable.
Put one "namspace Drupal\my_module;" at the top, done.
No curly brackets.

It does not solve the performance issues with hooks, the bootstrap order issues, the global-state-so-untestable problem, etc. We need to consider all of those.

Solving these things was never the purpose in these discussions.

Avoiding collisions between modules and also with 3rd party code, and having shorter function names (on declaration time), is benefit enough, for my taste. Noone is promising any more than that.

(and I think a lot of the problems you mention could be solved even with the currently existing hook system)

-------

It is certainly interesting to talk about listeners vs hooks, but I am not really convinced that it is an acceptable replacement.
Is this discussed anywhere?

I would rather see this as an alternative to hooks, and decide per situation which is preferable.

This is the most public discussion I know of: #1509164: Use Symfony EventDispatcher for hook system

Adding "namespace Drupal\$my_module;" to the .module files would result in a immense list of resulting changes:

- Every single function call to another module would require an explicit use. node_load() is called 170 times in 40+ files. And that's just one function.
- It would break all callbacks, from menu callbacks over form id's, to any other system that uses something like that. Everything would need to be changed to include the fully qualified name.
- Every function reference in the documentation would need to be updated to use the fully qualified name, as per our standards. This includes @see's but also any other mention of a function in a docblock or inline comment. It's already hard to catch the class references, this would be crazy to change.

So, as suggested, the IMHO only way to make this would would be to introduce a new .hook.inc that is included by default and put all non-grouped hooks in there. No, enforcing hook_hook_info() with a required group would not work, that is a hook as well, chicken egg problem...

Um, yes, that's a lot of work. Only worth it if it can stay for a very long while.
If that is too much for us, I'd rather see the _hook_ separator, or just nothing at all, than a half-namespace solution.

Um, yes, that's a lot of work. Only worth it if it can stay for a very long while.

Precisely my point. :-) I don't believe that it would say for a very long while. Whether it changes in Drupal 8 or waits until Drupal 9, I expect hooks to be radically overhauled if not replaced entirely in the not too distant future. Hence why I think it's worth exploring alternative approaches before we embark on a task that large that has a relatively small ROI for the amount of work it would entail.

It might be even more work than Berdir suggests.
Most of what Berdir says could be automated (which can still be unpractical), but a lot of the magic renaming may be undesired.

E.g. with node_load() as a magic callback for %node in the url, how would we rename that? node\\load ? What about other wildcard loaders, where the loaded thing is not identical with the module name? taxonomy\\term_load ? Then the wildcard would be %taxonomy\\term ?

Also, functions that are very short, e.g. function "look_at_people()", in a module named "look_at", would we be happy with a function "look_at\\people()", or would we rather have "look_at\\look_at_people()" ?
Duh, stupid example. Actually, look_at\\people() is not so bad.

Now I'm crossing myself, having supported namespaces for procedural quite heavily.
But I think that's ok, if the argument is valid :)

One question, if procedural and hooks are going to stay around forever, then would the conversion be worthwile?
Or, how much of "stay for a very long while" would be enough to make this worthwile?

(assuming that a lot can be automated)

It might be better worth our time to "do it right" and work towards moving our procedural code over to PSR-0 classes to gain the benefit of auto-loading.

At this point, I think hooks are staying as-is for D8, while as much as possible is moved to PSR-0 code.

Just to prove once again how fragile is the current hook naming convention: #1374116-35: Move bundle CRUD API out of Field API.

I'm going to go out on a limb and say this is way too invasive for Drupal 8 at this point in the dev cycle. We've got enough "change all the things" work going on as is. :-)

Instead of namespacing global functions, a better interim solution might be to move them to static classes via the PHPUnit Blocker issues.

It would be a nice start if module_implements() could contain arbitrary callbacks, not just module names.

foreach (module_implements('captn_ahab') as $key => $info) {
  if (isset($info['group'])) {
    // .. include the respective file
  }
  if (is_callable($callback)) {
    call_user_func_array($callback, $args);
  }
}

This would open up a new playing field.

Or it could encapsulate that stuff in an object..

Update: More recent proposals don't need curly brackets for namespaces.

Following #158, I proposed a patch to fix this in D7 (and possibly D8) by introducing a new hook.

See: #2016003-1: Prevent potential naming collisions caused by the hook naming pattern

If we fix #1972304: Add a HookEvent, collisions can be prevented by implementing an event listener for the hook, rather than implementing the hook as a procedural function.

Okay so just for the record: If hooks will be reworked in a way D7 can't follow for D9 and perhaps even D8, can we get a 'fix' in for D7 already?

I've already written a patch in #2016003-1: Prevent potential naming collisions caused by the hook naming pattern that allows your module to say:

Oh hey, I seem to have messed up by using an underscore in my module's machine name. Let me fix that for you by specifying my own hook implementation callbacks.

If people are interested in solving this problem for D7, what should they do? I don't see this getting fixed in D8 or D9 in a way that can be backported to D7.

@kristiaanvandeneynde: I am all for it. but depends what others say.

@catch: Instead of closing the other as a duplicate, we could keep this one as a meta issue without patches, and reopen the other to explore a specific implementation approach.

I did a quick test on a site I work on, to look for potential collisions.
This is just one type of collision, where one function name could mean two different hooks implemented by two different modules. And those functions do not actually exist.

pathauto_entity_query_alter() = pathauto + hook_entity_query_alter() = pathauto_entity + hook-query_alter()
search_api_views_query_alter() = search_api + hook_views_query_alter() = search_api_views + hook_query_alter()
entity_token_field_info_alter() = entity_token + hook_field_info_alter() = entity + hook_token_field_info_alter()

Personally I find this bad enough. On the other hand it is actually surprising that it is only 3 overlaps on a relatively big site, and these are only virtual overlaps, the functions do not actually exist.

I think the bigger problem is actually if a module implements a hook unintendedly, with a function that is meant to be public API. In this case we cannot even test if this was intended or not.

Wow, this one actually works!

// Include the file that defines function Drupal\procedural\hook_init()
require_once __DIR__ . '/example.namespaced.inc';

/**
 * Implements hook_module_implements_alter()
 */
function example_module_implements_alter(&$implementations, $hook) {
  if ($hook === 'init') {
    // Register Drupal\example\hook_init()
    $implementations['Drupal\example\hook'] = FALSE;
  }
}

Notes:
- The $group must be FALSE, because module_load_include() does not work on something that is not a module.
- There might be cases where this does not work, because a hook invocation uses something other than module_invoke_all() and call_user_func().

Following #162, can we reopen the other issue (#2016003: Prevent potential naming collisions caused by the hook naming pattern ) to fix this for D7 by introducing a new hook?

See "API addition" on https://drupal.org/node/1527558

Double underscores could be implemented in 8.x if we really wanted to - just a case of an extra function_exists() in hook discovery, and possibly preventing double implementation in a single module as part of that.

However downgrading to minor since given the number of hook implementations in the wild this is only a problem for 0.00001% or so.

@catch.
If the goal is to prevent false positives, then adding an alternative pattern without removing the original pattern does not help at all. If anything, it would make it worse.
Also, performance.. (of the discovery process)

Can we use PHP namespaces to avoid this?

@pwolanin:
We could, as proposed in #1428592: Proposal: PHP 5.3 Namespaces for module-provided functions and hook implementations.. Or we could use static methods. Or whatever. But:

The main problem is that within D7 and D8 we cannot for BC reasons remove the traditional hook naming pattern.
But what we can do is to allow specific modules to opt out of the regular hook syntax, or specify a whitelist of hooks.

This is where #2016003: Prevent potential naming collisions caused by the hook naming pattern was headed (although I had only a brief look at it). So maybe yes it should be reopened, since it is slightly different from what we discuss here.

A module can already do this today with hook_module_implements_alter() (D7), simply shut down all unintended implementations.
E.g. in the case of field_group_permission(), which is both field + group_permission() and field_group + permission(), one of the modules could use hook_module_implements_alter() to shut down false positives.

The question would be how to determine which implementation is intended and which is not.

Maybe 'foo' defines this function as "Implements hook_permission() on behalf of field_group module.", without adding field_group as a dependency. Because 'foo' will work without field_group, but just has an extra functionality if field_group is enabled.

But maybe 'foo' defines this function as "Implements hook_group_permission() on behalf of field module.", without adding field as a dependency.

In the existing case, I think field is in core, so we could default to that (still not without problems, but ok).

But imagine that both 'field' and 'field_group' live in contrib, and do not know each other. A site might have only field_group installed, and not know about the existence of a 'field' module. But the field_group_permission() might be intended for 'field' + 'group_permission', not for 'field_group' + 'permission'.

There are 3 parties in this debacle:
1. The author of the 'field' module, who knows nothing about 'field_group' or 'foo'.
2. The author of the 'field_group' module, who knows nothing about 'field' or 'foo' (this is the hypothetical case which we should focus on for completeness).
3. The author of the 'foo' module, who might know about either 'field' or 'field_group' or both. (Otherwise he/she would not define this function)

Each party wants to make their module safe against hook nameclashes, without relying on the other parties. So what can each party do?

Ah, yes - you could start making some such tweaks in \Drupal\Core\Extension\ModuleHandler::buildImplementationInfo() but you might need something from the module .info file?

Or you could simply look there for both the function with the "_" or some other separator or the namespace as suggested in the issue summary. Would need to cache the found implementation which seems not to be done now.

Also any change there seems like it could cause problems with hook_module_implements_alter()? Maybe not since it seems like that just lets you remove or re-oder implementations.

Given the length of this thread, I suggest moving to https://www.drupal.org/project/issues/ideas and getting agreement on some plan for 8 and then a new issue can be made for implementation

The problem has multiple parts:

1. Design a new hook syntax for D9.
   E.g. with namespaces, double underscore, static methods, etc (this has been a long discussion).
2. Introducing the new hook syntax as an alternative in D8, alongside the existing syntax. This does NOT fix any nameclash issues, but is still something we should do so modules can prepare for D9.
   The signature of hook_module_implements_alter() is designed with the traditional hook syntax in mind, so maybe we might run into problems there - which I think is what @pwolanin says in #173.
3. Reducing name clashes in D7 and D8 in a BC-friendly way.
   E.g. by allowing modules to "opt out" of the regular hook discovery, and instead specifying a "whitelist" of hooks they want to implement.
   This is, I think, already possible with hook_module_implements_alter(), but maybe we want to provide a more comfortable way to do this in core.

For this we can have 3 separate issues:

1. This issue, to design a hook syntax for D9. Most of the existing comments were already about this.
   
2. A new issue, to discuss how we can add forward support for this in D8.
   EDIT: Created an issue here, #2817439: Support alternative module hook syntax alongside the existing syntax.
3. Possibly reopen #2016003: Prevent potential naming collisions caused by the hook naming pattern for the name clashes prevention.

I have no opinion about moving to "ideas", I leave this for others to decide.

I think the ideal would be if whatever alternative syntax we decide to support in D8 should become the official and only syntax in D9.
But I wonder if we can honestly predict what D9 will be like.

Maybe it is better to be pragmatic and choose something that looks like it can work in D9, without pretending that we know that it will survive that long.

Either way, if we do this, it should be something bullet-proof, and not just a bit less fragile than what we have now. So for my taste, either namespaces or static methods. But no '__'.

Once we agree which issues should be used for which part of the discussion, we can make an overview with pros/cons of either solution. (If this does not already exist somewhere.)

Or just a *Module class (ie. FieldGroupModule or FieldModule) with instance methods, which means you could actually inject services in the module class and use them without being global state.

Or replace them using the EventDispatcher progressively, no need for 2 different event mechanisms.

@donquixote I'm impressed with your write up in #172. Looks like you really gave the suggested solution from #2016003: Prevent potential naming collisions caused by the hook naming pattern some thought and then some :)

I also agree with #176 in that we need something bullet-proof. Double underscores still falls within the realm of magic names and can therefore not be trusted.

The module class idea from #178 Is a good suggestion, but it would not make much sense to go the OO way with a new system whereas we already have the event system we could leverage. The only thing I'm not sure about is whether the event system supports all type of hooks.

If I got this right, the D8 alternatives are:

• Info hooks -> Annotation discovery (In place and functioning)
• Action hooks (events) -> Event system (In place, but not implemented)
• Alter hooks -> ???
• module_implements_alter() -> ???

Between the Symfony EventDispatcher and the proposed PSR-14 Event Manager (still in early stages), I expect by Drupal 9 having any sort of procedural hook system in place will be a liability, not a benefit. That is, the problems with the hook naming scheme discussed here will be irrelevant, since we shouldn't be using magically named functions by Drupal 9 at all. Their day has passed, let us give them an honorable burial.

@Crell: Generally yes.
But within this issue my own position is that I don't care.

Inventing a new syntax and pretending that it is for D9 will not stop us from killing it again in D9.
But hopefully it will allow us to come to conclusions with the other parts of this issue - see #175.

Or in other words: We still have plenty of time to come to the conclusion that we don't want procedural hooks in Drupal 9.

Here is an attempt in Drupal 7 contrib to allow modules to opt out of the magic naming hook discovery.
https://github.com/donquixote/drupal-clashnomore

(We might still want to open a new issue to discuss whether this should be in core)

I'd say...

For this we need...

Adding a related issue that illustrates a potential breakage in the current system.