Skip to content

DB/PreparedSQL: add tests for namespaced names#2708

Draft
rodrigoprimo wants to merge 2 commits intoWordPress:developfrom
rodrigoprimo:db-prepared-sql-tests
Draft

DB/PreparedSQL: add tests for namespaced names#2708
rodrigoprimo wants to merge 2 commits intoWordPress:developfrom
rodrigoprimo:db-prepared-sql-tests

Conversation

@rodrigoprimo
Copy link
Copy Markdown
Collaborator

Description

In preparation for PHPCS 4.0, which changes the tokenization of namespaced names, this PR adds tests with all forms of namespaced function calls (partially qualified, fully qualified, and namespace-relative using the namespace keyword) as well as fully qualified global function calls to the WordPress.DB.PreparedSQL sniff.

Suggested changelog entry

N/A

@rodrigoprimo rodrigoprimo marked this pull request as draft March 19, 2026 17:29
@rodrigoprimo
Copy link
Copy Markdown
Collaborator Author

Moved this to draft as I missed that the tests were not failing in the PHPCS 4.0 preparation branch, but are failing here due to a commit that I did not include in this PR.

@rodrigoprimo
Copy link
Copy Markdown
Collaborator Author

The tests are failing because of #2710. I will fix that issue in a separate PR and then move this one as ready to review once the tests pass.

@rodrigoprimo rodrigoprimo force-pushed the db-prepared-sql-tests branch from ae0b611 to 799b3e7 Compare March 23, 2026 17:40
@rodrigoprimo
Copy link
Copy Markdown
Collaborator Author

The tests are failing because of #2710. I will fix that issue in a separate PR and then move this one as ready to review once the tests pass.

I opted to change the approach: I updated the tests in this PR to clearly document the false positive behavior and reference #2710. The bug itself will be fixed in a separate PR, which will also update the test expectations. This PR is now ready for review.

@rodrigoprimo
Copy link
Copy Markdown
Collaborator Author

As discussed with @jrfnl in a chat conversation, I converted this PR to draft, and I will first create another PR fixing #2710 before marking this one as ready for review again.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant