Warning for using include and require

[khacoder]

[New sniff] If theme uses include(_once) or require(_once) issue warning to consider get_template_part().

Rule type:

Warning

Rule:

Custom template files should be called using get_template_part() or locate_template().

Ref: https://make.wordpress.org/themes/handbook/review/required/#templates

WARNING (manual check required) | Check if a theme uses include(_once) or require(_once) (where they should use get_template_part()). Current implementation excluded the functions.php file from this check. We may want to continue doing so.

Theme check file covering this rule:

https://github.com/Otto42/theme-check/blob/master/checks/include.php

To do:

• Create unit tests
• Create new sniff
• Adjust existing WPCS / PHPCS sniff and send in PR upstream.
• Add existing sniffname sniff to the ruleset.

[khacoder]

I have a sniff for this.

[justintadlock]

One thing I've seen as problematic with this check is that theme authors are now starting to incorrectly use get_template_part() to load non-template files. I wonder if we should consider a check specifically for functions.php that does the opposite. Thoughts?

[khacoder]

I think this needs to be improved as well. I think the warning for using require/include functions could be changed to an error if the file being loaded does not have the 'function' string.

The use of get_template_directory can also be checked in the same (or probably) a new sniff.

[justintadlock]

I can't ever see this particular check being an error. There are too many exceptions. Just a couple:

This correctly uses locate_template():

$template = locate_template( array( 'example/abc.php`, `example/xyz.php' ) );

if ( $template )
    include( $template );

This correctly uses get_page_template():

$template = get_page_template();

if ( $template )
    include( $template );

Not all files loaded will have a function string. You can have a class without any methods/functions defined. Some files will only have variables.

---

For a functions-file sniff, get_template_directory() wouldn't really be a good idea either. Just a couple of examples that break that:

Using a variable:

$dir = trailingslashit( get_template_directory() );

require_once( $dir . 'file-1.php' );
require_once( $dir . 'file-2.php' );

Including file from child theme:

if ( is_child_theme() ) {

    $dir = trailingslashit( get_stylesheet_directory() );

    if ( file_exists( $dir . 'example.php' ) )
        require_onc( $dir . 'example.php' );

These are just examples that are common in my own themes. Proper file loading is something that's going to always take a manual check as far as I can tell.

[khacoder]

OK no problem....we can move back to a warning.

However let's eliminate all the warnings that we know are OK. On testing I have seen some themes that have a significant amount of warnings. In themecheck this is handled by not checking the functions.php file, and only issuing one warning per file.

I would think we could add a few checks that would greatly reduce the number of times this warning comes up. One of course would be where functions are defined, a warning should not be issued. That would cut down the warnings significantly.

What do you think?

[grappler]

Closing as #67 has been merged.