Broaden detection for plugin entrypoint files

[kadamwhite]

In #337 we added support to opt single-file MU-plugins out from rules prohibiting namespaces outside of inc, and requiring namespaced code to live within a namespace.php file — both rules would discourage namespacing within plugin entrypoint files.

This conclusion also applies to nested plugins, such as traditionally-structured plugin folders within mu-plugins/ and also the default plugins/ folder. If we're working on a first party plugin within a project that uses HMCS, we want to be consistent about how we treat plugin entrypoints throughout first party code.

This PR refactors the MU-Plugin detection trait to recognize all plugin entrypoint files, even those within wrapping folders. By convention, we support <plugin-name>/plugin.php and <plugin-name>/<plugin-name>.php.

[rmccue]

I'm not sure I understand this change; are we allowing my-plugin/plugin.php to contain function definitions?

If so, that is not the same as allowing mu-plugins/single-file.php. The use case for allowing that pattern is that mu-plugins requires being in single files (although we can and do build our own loader on top of it), so we want to allow that as an exception.

Requiring function definitions inside plugin directories to be in inc/ is intentional however - separating definitions from imperative code. The main plugin file must contain imperative code, so we intentionally force separation between this and definitions (which always live in inc/); this ensures the codebase is testable by having no side-effects.

What's the use case for loosening this restriction?

[kadamwhite]

@rmccue I hadn't intended to alter the side-effects change, the nested plugin files are definitely meant to be held to the same standard — declarations or side-effects, never both.

What's happened here is that I ended up conflating NamespaceDirectoryNameSniff with FunctionFileNameSniff, because in the case of mu-plugins neither is desirable. In this case, the only thing I was actually trying to add was an exception for NamespaceDirectoryNameSniff, so that we can declare a namespace in the entrypoint itself. A pattern I see fairly often is to have the plugin entrypoint do things like this,

<?php
/**
 * Plugin Name: Whatever
 */

namespace HM\Whatever;

// maybe some `const`s.

require_once __DIR__ . '/inc/namespace.php';

bootstrap();

Now, you can argue this is just a convenience over HM\Whatever\bootstrap(); and that we should omit the namespace on the entrypoint, but I do see this a lot — and see devs routinely assume that all code should be in a namespace, so it gets confusing when you're told to rename your file if that file is following another convention like plugin.php.

I'll adjust or recreate the PR to back out the loosening of FunctionFileNameSniff, but what are your thoughts on the allow-namespace-in-entrypoint change

[rmccue]

Got it, that makes sense - I think it's fine to declare a namespace and potentially some consts too in that main file.

eg It's often most convenient to set a file/dir const directly from that file:

<?php
namespace HM\Whatever;

const PLUGIN_FILE = __FILE__;
const PLUGIN_DIR = __DIR__;

I'm surprised this isn't allowed already given we use it frequently 🤔 Did we regress?

[kadamwhite]

Did we regress...

Maybe...? It does feel surprising. We excluded load.php in the past, for sure, and that still works I believe

[rmccue]

Maybe...? It does feel surprising. We excluded load.php in the past, for sure, and that still works I believe

Yeah, this updated the sniff to add load.php in NamespaceDirectoryNameSniff but plugin.php and functions.php (plugins and themes respectively) were already: https://github.com/humanmade/coding-standards/pull/131/changes

[kadamwhite]

@rmccue Created a much more targeted new PR: #344