WordPress / plugin-check

A repository for the new Plugin Check plugin from the WordPress Performance and Plugins Team.
https://wordpress.org/plugins/plugin-check/
GNU General Public License v2.0
266 stars 54 forks source link

[Bug] `TextDomainMismatch` if checking a plugin via path #774

Open hirasso opened 1 week ago

hirasso commented 1 week ago

Expected behavior

When running wp plugin check path/to/my-plugin, the checked text domain should be my-plugin

Actual behavior

When running wp plugin check path/to/my-plugin, the checked text domain actually is the full path: wp plugin check path/to/my-plugin

Context

After the explanation in #773 that a local zip file is not supported yet, I now tried to check my plugin via path:

  1. I am creating a scoped package of my plugin using php-scoper. The scoped package is created in a subfolder of my plugin with the same name as my plugin. Seen from the plugins folder it looks like this: ./my-plugin/my-plugin
  2. I make sure plugin-checker is available in wp-env
  3. I check the subfolder in wp-env
    {
    "composer scoped:create",
    "npx wp-env run cli wp plugin install plugin-check --activate",
    "npx wp-env run cli wp plugin check wp-content/plugins/my-plugin/my-plugin"
    }

The plugin check works this time, except for one issue: The expected text domain contains the full path wp-content/plugins/my-plugin/my-plugin instead of simply my-plugin, which produces a lot of WordPress.WP.I18n.TextDomainMismatch errors.

hirasso commented 1 week ago

Another issue: Due to the fact that my path happens to begin with wp-content, I'm also getting a trademarked_term warning.

swissspidy commented 1 week ago

I‘d suggest moving your folder to a different location. That just asks for trouble.

If a plugin is in wp-content/plugins we expect to jusf pass the slug. But nested plugins are not really a scenario we support in Plugin Check nor in the plugin directory.

Have you tried moving it to wp-content/plugins directly? That would be the best solution. This way we can also actuvate the plugin during runtime checks.

Have you tried moving it to a tmp folder?

Alternatively, have you tried wp plugin check my-plugin/my-plugin?

hirasso commented 1 week ago

Have you tried moving it to wp-content/plugins directly? That would be the best solution. This way we can also actuvate the plugin during runtime checks.

That's a chicken-and-egg problem. I already have the development version of my plugin under the /plugins folder. So I can' really name my built version the same. I'll try

...I have just both your other suggestions. In all of them, I'm getting errors with the text domain. This is my script now:

{
      "npx wp-env run cli wp plugin install plugin-check --activate",
      "npx wp-env run cli mv wp-content/plugins/my-plugin/my-plugin my-plugin",
      "npx wp-env run cli wp plugin check ./my-plugin"
}

The errors:

Mismatched text domain. Expected './my-plugin' but got 'my-plugin'

Same with a tmp folder:

Mismatched text domain. Expected '/tmp/my-plugin' but got 'my-plugin'

...so there is definitely something I'm missing. I found the --slug option for the WP_CLI command in the docs and thought that I could control the text domain that way, but it doesn't work:

hirasso commented 1 week ago

...OMG I think I have it! Passing the --slug actually solves all my issues!! This is what works for me now:

{
  "plugin:check": [
      "composer scoped:create",
      "npx wp-env run cli wp plugin install plugin-check --activate",
      "SLUG=$(basename \"$(pwd)\"); npx wp-env run cli wp plugin check wp-content/plugins/$SLUG/$SLUG --slug=$SLUG"
  ],
}

This works because in my setup I

  1. Am inside wp-content/plugins/my-plugin/my-plugin when I run the script
  2. Build my plugin into wp-content/plugins/my-plugin/my-plugin
  3. Set the slug to the current basename(pwd), so it's my-plugin

I found the elegant trick with $(basename \"$(pwd)\") in plugin checker's package.json. Thanks for that! :)

ernilambar commented 1 week ago
"npx wp-env run cli wp plugin check ./my-plugin"

AFAIR, relative path is not supported. Full absolute path is expected.

--slug was introduced to fix similar issue of incorrect calculation of plugin slug. What was the error message when --slug was passed?

hirasso commented 1 week ago

@ernilambar you guys are just too quick for me ☺️ ...see my previous comment – --slug actually does the trick! 👏

hirasso commented 1 week ago

AFAIR, relative path is not supported. Full absolute path is expected.

I just tested this statement. It doesn't work with absolute paths:

{
    "plugin:check": [
      "composer scoped:create",
      "npx wp-env run cli wp plugin install plugin-check --activate",
      "SLUG=$(basename \"$(pwd)\"); npx wp-env run cli wp plugin check /var/www/html/wp-content/plugins/$SLUG/$SLUG"
    ],
}

The expected text domain becomes the full path: /var/www/html/wp-content/plugins/my-plugin/my-plugin

ernilambar commented 1 week ago

Hmm. Makes sense. That support for path was implemented keeping in mind that plugin will be outside of the WordPress setup, somewhere in the same server.

hirasso commented 1 week ago

It also doesn't work if the plugin is in /tmp/my-plugin. Outside of WP.

I would consider this a bug, after all 🐛

ernilambar commented 1 week ago

I tried wp plugin check /tmp/sample-plugin --slug=sample-plugin and it is working as expected.

hirasso commented 1 week ago

Yes! With the --slug option it's also working if the plugin resides inside the wordpress plugin folder. Without the --slug option, it doesn't work, no matter where the plugin lies.

This won't work:

wp plugin check /tmp/sample-plugin

davidperezgar commented 1 week ago

Yes, the best approach is to enforce language checks by giving the slug.

Anyway, I think we should check that command, as it should run with slug as sample-plugin.

swissspidy commented 1 week ago

Agreed, this is a bug.