Allow DROP TABLE schema change in uninstall.php

[Asgaros]

Plugins often require their own tables - therefore those tables naturally should be removed during the uninstall process of the plugin. This brings the requirement to execute a query like DROP TABLE IF EXISTS wp_my_table; which is placed in the uninstall.php file.

Currently, the CodeSniffer creates warnings for those database queries, like:

Attempting a database schema change is discouraged. (WordPress.DB.DirectDatabaseQuery.SchemaChange)

Since deleting unnecessary data during the uninstall process is a proper way to avoid leaving trash behind, I suggest to add an exception which allows the DROP TABLE query within the uninstall.php file.

[dingo-d]

You can just silence the direct database query sniffs in your plugin's ruleset

<rule ref="WordPress.DB.DirectDatabaseQuery.SchemaChange">
    <severity>0</severity>
</rule>

I recommend you read about modifying your ruleset, and about customizing WPCS ruleset.

[Asgaros]

Yes, I know that. Basically, I can silence every annoying warning, so at the end my code will be perfectly fine.

My point is more: This ruleset makes perfectly sense in general, so I want to keep and test code against it to ensure no schema changes are happening in the main plugin files. But at the same time, it should be a good idea to allow this kind of query during the uninstall process because it is an essential part of cleaning up and therefore should not trigger a warning there.

[dingo-d]

@jrfnl @GaryJones Could we add this exclusion to the sniff? I mean, the way I'd approach this would be to just add an exclusion for this one file in my ruleset and that'd be it.

But uninstall.php is a plugin-specific file, so it could be used as a special case.

[GaryJones]

I think checking if the file name is uninstall.php and exiting early makes sense. We already have file-specific conditions for other things (like for template files).

[smileBeda]

I do not understand why this sniff is in place.

1. It is very common for plugins to create, drop, or else manipulate database tables. Look at WooCommerce, Learndash, wpDiscuz, and a hundreds more
2. There is no other way to create database tables than doing what it is flagging
3. Silencing errors is forbidden by WP Plugin Reviewers and yet at the same time if you do not silence it, for example GitHub WPCS workflows can fail.

It is the very suggested way of doing it (https://codex.wordpress.org/Creating_Tables_with_Plugins) As there is literally no other suggested method to create or else manipulate tables, this sniff is very much breaking stuff in CI implementations, without workaround, because as said, you are not allowed to silence errors in WP Plugins if you want them to pass review.

This does not only relate to delete tables in uninstall, it also relates to create tables, which might happen about anywhere in the code. Sniffers should not try to force us what our code does but how it does what it does This is very much asking developers to simply not create or else work with database tables, in a simplistic point of view.

Do I miss something?

[jrfnl]

@smileBeda I'm going to hide your comment as "Off Topic". Your remark and the discussion you are trying to spark does not belong in this ticket as this ticket is about a specific issue with the sniff, not about the principle of the sniff itself. For background on why the sniff exists: see the actual coding standards: https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#database-queries