Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files/FileName: check file extensions and stricter file name checking ? #2164

Closed
jrfnl opened this issue Dec 20, 2022 · 4 comments
Closed

Files/FileName: check file extensions and stricter file name checking ? #2164

jrfnl opened this issue Dec 20, 2022 · 4 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Dec 20, 2022

Originally posted by @GaryJones in #2154 (comment):

I can see there are some test files with extra . in (and not just underscores), but I was wondering if an incorrect file name like class.my-class.php might be detected, as the checking for hyphenation only replaces underscores.

I also see substr( $file_name, 0, -4 ), which is assuming that a PHP-rendered file will always have a three letter file extension, but I wonder if someone could break that assumption with --extensions=php3,myphp or similar.

Response by @jrfnl :

I can see there are some test files with extra . in (and not just underscores), but I was wondering if an incorrect file name like class.my-class.php might be detected, as the checking for hyphenation only replaces underscores.

I don't think it is, but did you test it ? If a test would proof it not to be handled, it might be good to open an issue about that.

I also see substr( $file_name, 0, -4 ), which is assuming that a PHP-rendered file will always have a three letter file extension, but I wonder if someone could break that assumption with --extensions=php3,myphp or similar.

They most definitely could, but is that likely for a WP file or plugin ? I mean, for that to work on a server, either additional .htaccess rules or nginx rules (or similar) would need to be set up to support that.

All the same, I agree it may be good to clarify the file name rule in the handbook to be explicit about file extensions. Once that is done, this sniff would need to be updated.

@jrfnl
Copy link
Member Author

jrfnl commented Jul 4, 2023

The most pertinent points from this issue have now been addressed in PR #2285.

The only thing left is whether or not to enforce that files use a .php file extension. As this is not a (handbook) rule, this would need a post on Make to decide whether that rule should be added or not.

@GaryJones Do you want to leave this issue open for that ? Or shall we close this issue as fixed and open a separate issue for the file extension check ?

@GaryJones
Copy link
Member

shall we close this issue as fixed and open a separate issue for the file extension check ?

Let's do this.

@jrfnl jrfnl added this to the 3.0.0 milestone Jul 4, 2023
@jrfnl
Copy link
Member Author

jrfnl commented Jul 4, 2023

@GaryJones 👍🏻 As the file extension check was your suggestion, will you open the issue ?

@GaryJones
Copy link
Member

As the file extension check was your suggestion, will you open the issue ?

Done: #2292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants