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..5773b72 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,14 @@ +# 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: 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 new file mode 100644 index 0000000..5773b72 --- /dev/null +++ b/phpstan.neon.example @@ -0,0 +1,14 @@ +# 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: 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 3f1a187..9c7e861 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,14 +78,14 @@ class ConvertKit_API /** * Debug * - * @var boolean + * @var \Monolog\Logger */ protected $debug_logger; /** * Guzzle Http Client * - * @var object + * @var \GuzzleHttp\Client */ protected $client; @@ -135,7 +135,6 @@ private function create_log(string $message) } } - /** * Gets the current account * @@ -151,7 +150,6 @@ public function get_account() ); } - /** * Gets all sequences * @@ -167,7 +165,6 @@ public function get_sequences() ); } - /** * Gets subscribers to a sequence * @@ -187,7 +184,6 @@ public function get_sequence_subscriptions(int $sequence_id, string $sort_order ); } - /** * Adds a subscriber to a sequence by email address * @@ -207,12 +203,11 @@ 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 +215,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(); } @@ -233,7 +231,6 @@ public function add_tag(int $tag, array $options) ); } - /** * Gets a resource index * Possible resources: forms, landing_pages, subscription_forms, tags @@ -244,7 +241,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) { @@ -361,12 +358,11 @@ public function get_resources(string $resource) return $this->resources[$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 +370,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(); } @@ -387,11 +386,10 @@ 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. * @@ -409,7 +407,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. @@ -422,7 +419,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(); } @@ -449,7 +449,6 @@ public function get_subscriber_id(string $email_address) return $subscribers->subscribers[0]->id; } - /** * Get subscriber by id * @@ -473,7 +472,6 @@ public function get_subscriber(int $subscriber_id) ); } - /** * Get a list of the tags for a subscriber. * @@ -481,7 +479,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) { @@ -497,11 +495,10 @@ 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. * @@ -519,11 +516,10 @@ public function list_purchases(array $options) return $this->get('purchases', $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. * @@ -541,7 +537,6 @@ public function create_purchase(array $options) return $this->post('purchases', $options); } - /** * Get markup from ConvertKit for the provided $url. * @@ -552,12 +547,16 @@ 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 */ 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(); } @@ -594,8 +593,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); @@ -609,31 +607,38 @@ 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. * - * @param \DOMNodeList $elements Elements. - * @param string $attribute HTML Attribute. - * @param string $url Absolute URL to prepend to relative URLs. + * @param \DOMNodeList<\DOMElement> $elements Elements. + * @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) { @@ -658,7 +663,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(). @@ -685,8 +689,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 +708,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 +727,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 +746,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,17 +765,24 @@ 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 */ 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(); } @@ -785,6 +796,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); @@ -813,7 +829,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; }