From 59a8b09b005303516e60d2b5fff3f66bdfb3efd0 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Wed, 8 Mar 2023 14:05:16 +0000 Subject: [PATCH 1/4] Added PHPStan with level 6 --- TESTING.md | 21 ++++++++++++ phpstan.neon.dist | 9 +++++ phpstan.neon.example | 9 +++++ src/ConvertKit_API.php | 76 ++++++++++++++++++++++++++---------------- 4 files changed, 86 insertions(+), 29 deletions(-) create mode 100644 phpstan.neon.dist create mode 100644 phpstan.neon.example diff --git a/TESTING.md b/TESTING.md index 2a56209..95fc856 100644 --- a/TESTING.md +++ b/TESTING.md @@ -22,6 +22,27 @@ If your work fixes existing functionality, check if a test exists. Either update Tests are written in PHP using [PHPUnit](https://phpunit.de/), and the existing `tests/ConvertKitAPITest.php` is a good place to start as a guide. +## Run PHPStan + +[PHPStan](https://phpstan.org) performs static analysis on the Plugin's PHP code. This ensures: + +- DocBlocks declarations are valid and uniform +- DocBlocks declarations for WordPress `do_action()` and `apply_filters()` calls are valid +- Typehinting variables and return types declared in DocBlocks are correctly cast +- Any unused functions are detected +- Unnecessary checks / code is highlighted for possible removal +- Conditions that do not evaluate can be fixed/removed as necessary + +In the Plugin's directory, run the following command to run PHPStan: + +```bash +vendor/bin/phpstan --memory-limit=1G +``` + +Any errors should be corrected by making applicable code changes. + +False positives [can be excluded by configuring](https://phpstan.org/user-guide/ignoring-errors) the `phpstan.neon` file. + ## Run PHPUnit Once you have written your code and tests, run the tests to make sure there are no errors. diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..2a42ff4 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,9 @@ +# Parameters +parameters: + # Paths to scan + paths: + - src/ + + # Should not need to edit anything below here + # Rule Level: https://phpstan.org/user-guide/rule-levels + level: 6 \ No newline at end of file diff --git a/phpstan.neon.example b/phpstan.neon.example new file mode 100644 index 0000000..2a42ff4 --- /dev/null +++ b/phpstan.neon.example @@ -0,0 +1,9 @@ +# Parameters +parameters: + # Paths to scan + paths: + - src/ + + # Should not need to edit anything below here + # Rule Level: https://phpstan.org/user-guide/rule-levels + level: 6 \ No newline at end of file diff --git a/src/ConvertKit_API.php b/src/ConvertKit_API.php index 3f1a187..095b16e 100644 --- a/src/ConvertKit_API.php +++ b/src/ConvertKit_API.php @@ -57,14 +57,14 @@ class ConvertKit_API /** * API resources * - * @var array + * @var array> */ protected $resources = []; /** * Additional markup * - * @var array + * @var array */ protected $markup = []; @@ -78,7 +78,7 @@ class ConvertKit_API /** * Debug * - * @var boolean + * @var \Monolog\Logger */ protected $debug_logger; @@ -211,8 +211,8 @@ public function add_subscriber_to_sequence(int $sequence_id, string $email) /** * Adds a tag to a subscriber * - * @param integer $tag Tag ID. - * @param array $options Array of user data. + * @param integer $tag Tag ID. + * @param array $options Array of user data. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -220,7 +220,10 @@ public function add_subscriber_to_sequence(int $sequence_id, string $email) */ public function add_tag(int $tag, array $options) { - if (!is_int($tag) || !is_array($options)) { + if (!is_int($tag)) { + throw new \InvalidArgumentException(); + } + if (!is_array($options)) { throw new \InvalidArgumentException(); } @@ -244,7 +247,7 @@ public function add_tag(int $tag, array $options) * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * - * @return object API response + * @return array API response */ public function get_resources(string $resource) { @@ -365,8 +368,8 @@ public function get_resources(string $resource) /** * Adds a subscriber to a form. * - * @param integer $form_id Form ID. - * @param array $options Array of user data (email, name). + * @param integer $form_id Form ID. + * @param array $options Array of user data (email, name). * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -374,7 +377,10 @@ public function get_resources(string $resource) */ public function form_subscribe(int $form_id, array $options) { - if (!is_int($form_id) || !is_array($options)) { + if (!is_int($form_id)) { + throw new \InvalidArgumentException(); + } + if (!is_array($options)) { throw new \InvalidArgumentException(); } @@ -391,7 +397,7 @@ public function form_subscribe(int $form_id, array $options) /** * Remove subscription from a form * - * @param array $options Array of user data (email). + * @param array $options Array of user data (email). * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -422,7 +428,10 @@ public function form_unsubscribe(array $options) */ public function get_subscriber_id(string $email_address) { - if (!is_string($email_address) || !filter_var($email_address, FILTER_VALIDATE_EMAIL)) { + if (!is_string($email_address)) { + throw new \InvalidArgumentException(); + } + if (!filter_var($email_address, FILTER_VALIDATE_EMAIL)) { throw new \InvalidArgumentException(); } @@ -481,7 +490,7 @@ public function get_subscriber(int $subscriber_id) * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * - * @return false|array + * @return false|array */ public function get_subscriber_tags(int $subscriber_id) { @@ -501,7 +510,7 @@ public function get_subscriber_tags(int $subscriber_id) /** * List purchases. * - * @param array $options Request options. + * @param array $options Request options. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -523,7 +532,7 @@ public function list_purchases(array $options) /** * Creates a purchase. * - * @param array $options Purchase data. + * @param array $options Purchase data. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -557,7 +566,10 @@ public function create_purchase(array $options) */ public function get_resource(string $url) { - if (!is_string($url) || !filter_var($url, FILTER_VALIDATE_URL)) { + if (!is_string($url)) { + throw new \InvalidArgumentException(); + } + if (!filter_var($url, FILTER_VALIDATE_URL)) { throw new \InvalidArgumentException(); } @@ -625,7 +637,7 @@ public function get_resource(string $url) * Converts any relative URls to absolute, fully qualified HTTP(s) URLs for the given * DOM Elements. * - * @param \DOMNodeList $elements Elements. + * @param \DOMNodeList<\DOMElement> $elements Elements. * @param string $attribute HTML Attribute. * @param string $url Absolute URL to prepend to relative URLs. * @@ -685,8 +697,8 @@ private function strip_html_head_body_tags(string $markup) /** * Performs a GET request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -704,8 +716,8 @@ public function get(string $endpoint, array $args = []) /** * Performs a POST request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -723,8 +735,8 @@ public function post(string $endpoint, array $args = []) /** * Performs a PUT request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -742,8 +754,8 @@ public function put(string $endpoint, array $args = []) /** * Performs a DELETE request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -761,9 +773,9 @@ public function delete(string $endpoint, array $args = []) /** * Performs an API request using Guzzle. * - * @param string $endpoint API Endpoint. - * @param string $method Request method (POST, GET, PUT, PATCH, DELETE). - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param string $method Request method (POST, GET, PUT, PATCH, DELETE). + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -771,7 +783,13 @@ public function delete(string $endpoint, array $args = []) */ public function make_request(string $endpoint, string $method, array $args = []) { - if (!is_string($endpoint) || !is_string($method) || !is_array($args)) { + if (!is_string($endpoint)) { + throw new \InvalidArgumentException(); + } + if (!is_string($method)) { + throw new \InvalidArgumentException(); + } + if (!is_array($args)) { throw new \InvalidArgumentException(); } From 8ed9585088bc318650f7a2adfc8f96d47eed8896 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Wed, 8 Mar 2023 14:06:32 +0000 Subject: [PATCH 2/4] Coding standards --- src/ConvertKit_API.php | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/ConvertKit_API.php b/src/ConvertKit_API.php index 095b16e..369cbc3 100644 --- a/src/ConvertKit_API.php +++ b/src/ConvertKit_API.php @@ -211,8 +211,8 @@ public function add_subscriber_to_sequence(int $sequence_id, string $email) /** * Adds a tag to a subscriber * - * @param integer $tag Tag ID. - * @param array $options Array of user data. + * @param integer $tag Tag ID. + * @param array $options Array of user data. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -638,14 +638,14 @@ public function get_resource(string $url) * DOM Elements. * * @param \DOMNodeList<\DOMElement> $elements Elements. - * @param string $attribute HTML Attribute. - * @param string $url Absolute URL to prepend to relative URLs. + * @param string $attribute HTML Attribute. + * @param string $url Absolute URL to prepend to relative URLs. * * @since 1.0.0 * * @return void */ - private function convert_relative_to_absolute_urls(\DOMNodeList $elements, string $attribute, string $url) + private function convert_relative_to_absolute_urls(\DOMNodeList $elements, string $attribute, string $url) // phpcs:ignore Squiz.Commenting.FunctionComment.IncorrectTypeHint, Generic.Files.LineLength.TooLong { // Anchor hrefs. foreach ($elements as $element) { @@ -697,8 +697,8 @@ private function strip_html_head_body_tags(string $markup) /** * Performs a GET request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -716,8 +716,8 @@ public function get(string $endpoint, array $args = []) /** * Performs a POST request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -735,8 +735,8 @@ public function post(string $endpoint, array $args = []) /** * Performs a PUT request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -754,8 +754,8 @@ public function put(string $endpoint, array $args = []) /** * Performs a DELETE request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -773,9 +773,9 @@ public function delete(string $endpoint, array $args = []) /** * Performs an API request using Guzzle. * - * @param string $endpoint API Endpoint. - * @param string $method Request method (POST, GET, PUT, PATCH, DELETE). - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param string $method Request method (POST, GET, PUT, PATCH, DELETE). + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * From 0017306ed58673cef5f7a3ef043a25ef65735618 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Wed, 8 Mar 2023 14:33:16 +0000 Subject: [PATCH 3/4] Use PHPStan level 8 --- phpstan.neon.dist | 7 ++++- phpstan.neon.example | 7 ++++- src/ConvertKit_API.php | 67 ++++++++++++++++++++---------------------- 3 files changed, 44 insertions(+), 37 deletions(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 2a42ff4..5773b72 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -6,4 +6,9 @@ parameters: # Should not need to edit anything below here # Rule Level: https://phpstan.org/user-guide/rule-levels - level: 6 \ No newline at end of file + level: 8 + + # Ignore the following errors, as PHPStan either does not have registered symbols for them yet, + # or the symbols are inaccurate. + ignoreErrors: + - '#\$headers of class GuzzleHttp\\Psr7\\Request constructor expects#' \ No newline at end of file diff --git a/phpstan.neon.example b/phpstan.neon.example index 2a42ff4..5773b72 100644 --- a/phpstan.neon.example +++ b/phpstan.neon.example @@ -6,4 +6,9 @@ parameters: # Should not need to edit anything below here # Rule Level: https://phpstan.org/user-guide/rule-levels - level: 6 \ No newline at end of file + level: 8 + + # Ignore the following errors, as PHPStan either does not have registered symbols for them yet, + # or the symbols are inaccurate. + ignoreErrors: + - '#\$headers of class GuzzleHttp\\Psr7\\Request constructor expects#' \ No newline at end of file diff --git a/src/ConvertKit_API.php b/src/ConvertKit_API.php index 369cbc3..e7e421d 100644 --- a/src/ConvertKit_API.php +++ b/src/ConvertKit_API.php @@ -57,7 +57,7 @@ class ConvertKit_API /** * API resources * - * @var array> + * @var array> */ protected $resources = []; @@ -85,11 +85,10 @@ class ConvertKit_API /** * Guzzle Http Client * - * @var object + * @var \GuzzleHttp\Client */ protected $client; - /** * Constructor for ConvertKitAPI instance * @@ -135,7 +134,6 @@ private function create_log(string $message) } } - /** * Gets the current account * @@ -151,7 +149,6 @@ public function get_account() ); } - /** * Gets all sequences * @@ -167,7 +164,6 @@ public function get_sequences() ); } - /** * Gets subscribers to a sequence * @@ -187,7 +183,6 @@ public function get_sequence_subscriptions(int $sequence_id, string $sort_order ); } - /** * Adds a subscriber to a sequence by email address * @@ -207,7 +202,6 @@ public function add_subscriber_to_sequence(int $sequence_id, string $email) ); } - /** * Adds a tag to a subscriber * @@ -236,7 +230,6 @@ public function add_tag(int $tag, array $options) ); } - /** * Gets a resource index * Possible resources: forms, landing_pages, subscription_forms, tags @@ -247,7 +240,7 @@ public function add_tag(int $tag, array $options) * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * - * @return array API response + * @return array API response */ public function get_resources(string $resource) { @@ -364,7 +357,6 @@ public function get_resources(string $resource) return $this->resources[$resource]; } - /** * Adds a subscriber to a form. * @@ -393,7 +385,6 @@ public function form_subscribe(int $form_id, array $options) ); } - /** * Remove subscription from a form * @@ -415,7 +406,6 @@ public function form_unsubscribe(array $options) return $this->put('unsubscribe', $options); } - /** * Get the ConvertKit subscriber ID associated with email address if it exists. * Return false if subscriber not found. @@ -458,7 +448,6 @@ public function get_subscriber_id(string $email_address) return $subscribers->subscribers[0]->id; } - /** * Get subscriber by id * @@ -482,7 +471,6 @@ public function get_subscriber(int $subscriber_id) ); } - /** * Get a list of the tags for a subscriber. * @@ -506,7 +494,6 @@ public function get_subscriber_tags(int $subscriber_id) ); } - /** * List purchases. * @@ -528,7 +515,6 @@ public function list_purchases(array $options) return $this->get('purchases', $options); } - /** * Creates a purchase. * @@ -550,7 +536,6 @@ public function create_purchase(array $options) return $this->post('purchases', $options); } - /** * Get markup from ConvertKit for the provided $url. * @@ -561,6 +546,7 @@ public function create_purchase(array $options) * @param string $url URL of HTML page. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. + * @throws \Exception If parsing the legacy form or landing page failed. * * @return false|string */ @@ -606,8 +592,7 @@ public function get_resource(string $url) ); // Get just the scheme and host from the URL. - $url_scheme = parse_url($url); - $url_scheme_host_only = $url_scheme['scheme'] . '://' . $url_scheme['host']; + $url_scheme_host_only = parse_url($url, PHP_URL_SCHEME) . '://' . parse_url($url, PHP_URL_HOST); // Load the HTML into a DOMDocument. libxml_use_internal_errors(true); @@ -621,18 +606,25 @@ public function get_resource(string $url) $this->convert_relative_to_absolute_urls($html->getElementsByTagName('script'), 'src', $url_scheme_host_only); $this->convert_relative_to_absolute_urls($html->getElementsByTagName('form'), 'action', $url_scheme_host_only); + // Save HTML. + $resource = $html->saveHTML(); + + // If the result is false, return a blank string. + if (!$resource) { + throw new \Exception(sprintf('Could not parse %s', $url)); + } + // Remove some HTML tags that DOMDocument adds, returning the output. // We do this instead of using LIBXML_HTML_NOIMPLIED in loadHTML(), because Legacy Forms // are not always contained in a single root / outer element, which is required for // LIBXML_HTML_NOIMPLIED to correctly work. - $resource = $this->strip_html_head_body_tags($html->saveHTML()); + $resource = $this->strip_html_head_body_tags($resource); // Cache and return. $this->markup[$url] = $resource; return $resource; } - /** * Converts any relative URls to absolute, fully qualified HTTP(s) URLs for the given * DOM Elements. @@ -670,7 +662,6 @@ private function convert_relative_to_absolute_urls(\DOMNodeList $elements, strin } } - /** * Strips , and opening and closing tags from the given markup, * as well as the Content-Type meta tag we might have added in get_html(). @@ -697,8 +688,8 @@ private function strip_html_head_body_tags(string $markup) /** * Performs a GET request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -716,8 +707,8 @@ public function get(string $endpoint, array $args = []) /** * Performs a POST request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -735,8 +726,8 @@ public function post(string $endpoint, array $args = []) /** * Performs a PUT request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -754,8 +745,8 @@ public function put(string $endpoint, array $args = []) /** * Performs a DELETE request to the API. * - * @param string $endpoint API Endpoint. - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. * @@ -773,11 +764,12 @@ public function delete(string $endpoint, array $args = []) /** * Performs an API request using Guzzle. * - * @param string $endpoint API Endpoint. - * @param string $method Request method (POST, GET, PUT, PATCH, DELETE). - * @param array $args Request arguments. + * @param string $endpoint API Endpoint. + * @param string $method Request method (POST, GET, PUT, PATCH, DELETE). + * @param array $args Request arguments. * * @throws \InvalidArgumentException If the provided arguments are not of the expected type. + * @throws \Exception If JSON encoding arguments failed. * * @return false|mixed */ @@ -803,6 +795,11 @@ public function make_request(string $endpoint, string $method, array $args = []) $this->create_log(sprintf('%s, Request body: %s', $method, $request_body)); + // Bail if an error occured encoind the arguments. + if (!$request_body) { + throw new \Exception('Error encoding arguments'); + } + if ($method === 'GET') { if ($args) { $url .= '?' . http_build_query($args); @@ -831,7 +828,7 @@ public function make_request(string $endpoint, string $method, array $args = []) $status_code = $response->getStatusCode(); // If not between 200 and 300. - if (!preg_match('/^[2-3][0-9]{2}/', $status_code)) { + if (!preg_match('/^[2-3][0-9]{2}/', (string) $status_code)) { $this->create_log(sprintf('Response code is %s.', $status_code)); return false; } From 2535220fac3c779a952c89480ce0e9aae6378241 Mon Sep 17 00:00:00 2001 From: Tim Carr Date: Wed, 8 Mar 2023 14:38:04 +0000 Subject: [PATCH 4/4] Coding standards --- src/ConvertKit_API.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ConvertKit_API.php b/src/ConvertKit_API.php index e7e421d..9c7e861 100644 --- a/src/ConvertKit_API.php +++ b/src/ConvertKit_API.php @@ -89,6 +89,7 @@ class ConvertKit_API */ protected $client; + /** * Constructor for ConvertKitAPI instance *