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

Updated - WP.AlternativeFunctions: Add more functions to filesystem functions list #2108

Merged
merged 15 commits into from
Dec 19, 2022

Conversation

sandeshjangam
Copy link
Contributor

@sandeshjangam sandeshjangam commented Nov 12, 2022

Closes #1265

  • Check the current state of WP_File_System and remove any functions from the list which don't have a WP_File_System counterpart.
    If there is no replacement function, the sniff would throw a non-actionable error/warning, which effectively is just noise.
  • For functions which do have a WP counter-functions (outside WP_File_System), set those up in separate groups with an informative warning/error message.
  • Remove file_get_contents() from the list as it is already checked in another group in the same sniff (with logic to prevent triggering on opening of local files) and we don't want to throw duplicate notices.

@jrfnl jrfnl added this to the 3.0.0 milestone Dec 2, 2022
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this update @sandeshjangam ! Looking good.

I've left some small remarks. Other than that, could you please add some tests for the new functions ?

Adding tests for this, is basically just a case of adding a line with a function call to each of these functions in the WordPress/Tests/WP/AlternativeFunctionsUnitTest.inc file and then adding the line numbers for the lines containing those function calls to the WordPress/Tests/WP/AlternativeFunctionsUnitTest.php file.

WordPress/Sniffs/WP/AlternativeFunctionsSniff.php Outdated Show resolved Hide resolved
WordPress/Sniffs/WP/AlternativeFunctionsSniff.php Outdated Show resolved Hide resolved
WordPress/Sniffs/WP/AlternativeFunctionsSniff.php Outdated Show resolved Hide resolved
WordPress/Sniffs/WP/AlternativeFunctionsSniff.php Outdated Show resolved Hide resolved
@jrfnl jrfnl added Component: Extra Type: Enhancement Status: Awaiting feedback Focus: WP interoperability Doing things the WP way, prefer using WP functionality over userland/PHP native labels Dec 3, 2022
@jrfnl
Copy link
Member

jrfnl commented Dec 15, 2022

@sandeshjangam Just checking - will you have a change to update the PR in the next few days ? I'd like to include this in WPCS 3.0.0, but if not, I'll just move it out of the milestone.

@sandeshjangam
Copy link
Contributor Author

@jrfnl Trying to finish it today itself. I will update you here.

@sandeshjangam
Copy link
Contributor Author

@jrfnl Changes done as per suggestions and added test cases also.

@sandeshjangam sandeshjangam requested a review from jrfnl December 16, 2022 16:43
WordPress/Sniffs/WP/AlternativeFunctionsSniff.php Outdated Show resolved Hide resolved
WordPress/Sniffs/WP/AlternativeFunctionsSniff.php Outdated Show resolved Hide resolved
WordPress/Tests/WP/AlternativeFunctionsUnitTest.inc Outdated Show resolved Hide resolved
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than those things pointed out by @GaryJones, this PR looks good to me. Thanks for making those updates @sandeshjangam !

Co-authored-by: Gary Jones <github@garyjones.io>
sandeshjangam and others added 2 commits December 19, 2022 11:44
Co-authored-by: Gary Jones <github@garyjones.io>
Co-authored-by: Gary Jones <github@garyjones.io>
@sandeshjangam
Copy link
Contributor Author

Thanks for the feedback. All changes are done now.

@GaryJones GaryJones merged commit a88f0eb into WordPress:develop Dec 19, 2022
GaryJones added a commit to Automattic/VIP-Coding-Standards that referenced this pull request Dec 20, 2022
The ruleset tests were failing mainly due to WordPress/WordPress-Coding-Standards#2108 which renamed some violation codes that we were excluding.

The other change to account for was the replacement of a WPCS StrictComparisons sniff with a PHPCSExtras Universal sniff. The sniff in PHPCSExtra contains a fixer. As this is a risky fixer, this fixer is turned off for WPCS. The sniff in PHPCSExtra will provide metrics about loose versus strict comparisons.
GaryJones added a commit to Automattic/VIP-Coding-Standards that referenced this pull request Jun 20, 2023
The ruleset tests were failing mainly due to WordPress/WordPress-Coding-Standards#2108 which renamed some violation codes that we were excluding.

The other change to account for was the replacement of a WPCS StrictComparisons sniff with a PHPCSExtras Universal sniff. The sniff in PHPCSExtra contains a fixer. As this is a risky fixer, this fixer is turned off for WPCS. The sniff in PHPCSExtra will provide metrics about loose versus strict comparisons.
GaryJones added a commit to Automattic/VIP-Coding-Standards that referenced this pull request Jun 22, 2023
The ruleset tests were failing mainly due to WordPress/WordPress-Coding-Standards#2108 which renamed some violation codes that we were excluding.

The other change to account for was the replacement of a WPCS StrictComparisons sniff with a PHPCSExtras Universal sniff. The sniff in PHPCSExtra contains a fixer. As this is a risky fixer, this fixer is turned off for WPCS. The sniff in PHPCSExtra will provide metrics about loose versus strict comparisons.
GaryJones added a commit to Automattic/VIP-Coding-Standards that referenced this pull request Jun 22, 2023
The ruleset tests were failing mainly due to WordPress/WordPress-Coding-Standards#2108 which renamed some violation codes that we were excluding.

The other change to account for was the replacement of a WPCS StrictComparisons sniff with a PHPCSExtras Universal sniff. The sniff in PHPCSExtra contains a fixer. As this is a risky fixer, this fixer is turned off for WPCS. The sniff in PHPCSExtra will provide metrics about loose versus strict comparisons.
GaryJones added a commit to Automattic/VIP-Coding-Standards that referenced this pull request Jun 22, 2023
The ruleset tests were failing mainly due to WordPress/WordPress-Coding-Standards#2108 which renamed some violation codes that we were excluding.

The other change to account for was the replacement of a WPCS StrictComparisons sniff with a PHPCSExtras Universal sniff. The sniff in PHPCSExtra contains a fixer. As this is a risky fixer, this fixer is turned off for WPCS. The sniff in PHPCSExtra will provide metrics about loose versus strict comparisons.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Extra Focus: WP interoperability Doing things the WP way, prefer using WP functionality over userland/PHP native Type: Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants