feat: add php sniffer for contact methods

[leogermani]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

In the effort of consolidating our data flows, we want to enforce that every contact update goes through the same route, starting at Newspack_Newsletters_Contacts class to make sure we have the propper logging.

However, we can't make all the internal methods private or protected.

This PHP Sniffer helps us avoid adding any new code that will bypass our intended flow. We'll need to add this sniffers to other Newspack plugins as well. - TBD how to do that

How to test the changes in this Pull Request:

1. checkout this branch
2. run ./vendor/bin/phpcs
3. Confirm you see some error in 2 files. (These errors are fixed in Add/logging #1574)
4. Does the message look good?
5. Try to add a method to one of the forbidden methods anywhere in the code.
6. Make sure the sniffer catches it

PS - the linting errors in the CI job are expected until we merge #1574

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1207942575891805

[miguelpeixe]

Super cool!

One issue caught by the linter is from src/blocks/subscribe/index.php, which doesn't call the forbidden method from a class. This causes the sniffer to throw a misleading error:

FILE: /home/circleci/project/src/blocks/subscribe/index.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 501 | ERROR | Method
     |       | Newspack_Newsletters_Subscription::add_contact()
     |       | should not be called from class
     |       | NewspackNewslettersContactsMethodsSniff. These methods
     |       | are for internal use only. Use methods in
     |       | Newspack_Newsletters_Contacts class instead.
     |       | (..NewspackNewslettersContactsMethods.ForbiddenContactsMethods)
----------------------------------------------------------------------

Given it includes the filename, it's not too bad, but is there a way to catch that and adjust the messaging?

[leogermani]

Super cool!

One issue caught by the linter is from src/blocks/subscribe/index.php, which doesn't call the forbidden method from a class. This causes the sniffer to throw a misleading error:

FILE: /home/circleci/project/src/blocks/subscribe/index.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 501 | ERROR | Method
     |       | Newspack_Newsletters_Subscription::add_contact()
     |       | should not be called from class
     |       | NewspackNewslettersContactsMethodsSniff. These methods
     |       | are for internal use only. Use methods in
     |       | Newspack_Newsletters_Contacts class instead.
     |       | (..NewspackNewslettersContactsMethods.ForbiddenContactsMethods)
----------------------------------------------------------------------

Given it includes the filename, it's not too bad, but is there a way to catch that and adjust the messaging?

Thanks!

Yes, we could get the filename and add to the error message if we are not inside a class. But looking at it, I think it makes more sense to make it simpler and more generic. 99425a4 does it

[leogermani]

We forgot to merge this to the epic before merging the epic to trunk and now, because of Squash, it was easier to rebuild this PR in a new branch -> #1611