Skip to content

Commit

Permalink
Add static analysis with phpstan
Browse files Browse the repository at this point in the history
  • Loading branch information
stof committed Jun 14, 2023
1 parent 46a97c0 commit f8c2121
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 59 deletions.
13 changes: 13 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
root = true

[*]
charset = utf-8
end_of_line = lf
indent_size = 4
indent_style = space
insert_final_newline = true
tab_width = 4
trim_trailing_whitespace = true

[.github/workflows/*.yml]
indent_size = 2
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/.editorconfig export-ignore
/.gitattributes export-ignore
/.github export-ignore
/.gitignore export-ignore
/phpunit.xml.dist export-ignore
/phpunit.http_client.xml export-ignore
/phpstan*.neon export-ignore
/tests export-ignore
23 changes: 23 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,29 @@ defaults:
shell: bash

jobs:
check_composer:
name: Check composer.json
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: shivammathur/setup-php@v2
with:
coverage: none
php-version: '8.2'
- run: composer validate --strict --no-check-lock

static_analysis:
name: Static analysis
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: shivammathur/setup-php@v2
with:
coverage: none
php-version: '8.2'
- name: Install dependencies
run: composer update --ansi --no-progress --prefer-dist --no-interaction
- run: vendor/bin/phpstan analyze

tests:
name: Tests on PHP ${{ matrix.php }} with ${{ matrix.implementation }}${{ matrix.name_suffix }}
Expand Down
5 changes: 4 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,21 @@

"require": {
"php": ">=7.2",
"ext-dom": "*",
"behat/mink": "^1.9.0@dev",
"symfony/browser-kit": "^4.4 || ^5.0 || ^6.0",
"symfony/dom-crawler": "^4.4 || ^5.0 || ^6.0"
},

"require-dev": {
"mink/driver-testsuite": "dev-master",
"phpstan/phpstan": "^1.10",
"phpstan/phpstan-phpunit": "^1.3",
"phpunit/phpunit": "^8.5 || ^9.5",
"symfony/error-handler": "^4.4 || ^5.0 || ^6.0",
"symfony/http-client": "^4.4 || ^5.0 || ^6.0",
"symfony/http-kernel": "^4.4 || ^5.0 || ^6.0",
"symfony/mime": "^4.4 || ^5.0 || ^6.0",
"phpunit/phpunit": "^8.5 || ^9.5",
"yoast/phpunit-polyfills": "^1.0"
},

Expand Down
12 changes: 12 additions & 0 deletions phpstan.dist.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
parameters:
level: 8
paths:
- src
- tests
checkMissingIterableValueType: false
ignoreErrors:
- '#^Method Behat\\Mink\\Tests\\Driver\\Custom\\[^:]+Test(Case)?\:\:test\w*\(\) has no return type specified\.$#'

includes:
- vendor/phpstan/phpstan-phpunit/extension.neon
- vendor/phpstan/phpstan-phpunit/rules.neon
97 changes: 58 additions & 39 deletions src/BrowserKitDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use Behat\Mink\Exception\DriverException;
use Behat\Mink\Exception\UnsupportedDriverActionException;
use Symfony\Component\BrowserKit\AbstractBrowser;
use Symfony\Component\BrowserKit\Client;
use Symfony\Component\BrowserKit\Cookie;
use Symfony\Component\BrowserKit\Exception\BadMethodCallException;
use Symfony\Component\BrowserKit\Response;
Expand All @@ -32,27 +31,40 @@
*/
class BrowserKitDriver extends CoreDriver
{
/**
* @var AbstractBrowser
*/
private $client;

/**
* @var Form[]
* @var array<string, Form>
*/
private $forms = array();
/**
* @var array<string, string>
*/
private $serverParameters = array();
/**
* @var bool
*/
private $started = false;

/**
* Initializes BrowserKit driver.
*
* @param string|null $baseUrl Base URL for HttpKernel clients
*/
public function __construct(AbstractBrowser $client, $baseUrl = null)
public function __construct(AbstractBrowser $client, ?string $baseUrl = null)
{
$this->client = $client;
$this->client->followRedirects(true);

if ($baseUrl !== null && $client instanceof HttpKernelBrowser) {
$client->setServerParameter('SCRIPT_FILENAME', parse_url($baseUrl, PHP_URL_PATH));
$basePath = parse_url($baseUrl, PHP_URL_PATH);

if (\is_string($basePath)) {
$client->setServerParameter('SCRIPT_FILENAME', $basePath);
}
}
}

Expand Down Expand Up @@ -219,7 +231,7 @@ public function setCookie($name, $value = null)
*
* @param string $name Cookie name.
*/
private function deleteCookie($name)
private function deleteCookie(string $name): void
{
$path = $this->getCookiePath();
$jar = $this->client->getCookieJar();
Expand All @@ -235,13 +247,15 @@ private function deleteCookie($name)

/**
* Returns current cookie path.
*
* @return string
*/
private function getCookiePath()
private function getCookiePath(): string
{
$path = parse_url($this->getCurrentUrl(), PHP_URL_PATH);

if ($path === null || $path === false || $path === '') {
$path = '/';

Check warning on line 256 in src/BrowserKitDriver.php

View check run for this annotation

Codecov / codecov/patch

src/BrowserKitDriver.php#L256

Added line #L256 was not covered by tests
}

if ('\\' === DIRECTORY_SEPARATOR) {
$path = str_replace('\\', '/', $path);
}
Expand Down Expand Up @@ -394,7 +408,18 @@ public function getValue($xpath)
*/
public function setValue($xpath, $value)
{
$this->getFormField($xpath)->setValue($value);
$field = $this->getFormField($xpath);

if ($field instanceof ChoiceFormField) {
$field->setValue($value);
return;
}

if (!\is_string($value) && null !== $value) {
throw new DriverException('Textual form fields don\'t support array or boolean values');

Check warning on line 419 in src/BrowserKitDriver.php

View check run for this annotation

Codecov / codecov/patch

src/BrowserKitDriver.php#L419

Added line #L419 was not covered by tests
}

$field->setValue($value);
}

/**
Expand Down Expand Up @@ -554,6 +579,7 @@ protected function prepareUrl($url)
* @return FormField
*
* @throws DriverException
* @throws \InvalidArgumentException when the field does not exist in the BrowserKit form
*/
protected function getFormField($xpath)
{
Expand All @@ -568,7 +594,11 @@ protected function getFormField($xpath)
}

if (is_array($this->forms[$formId][$fieldName])) {
return $this->forms[$formId][$fieldName][$this->getFieldPosition($fieldNode)];
$positionField = $this->forms[$formId][$fieldName][$this->getFieldPosition($fieldNode)];

\assert($positionField instanceof FormField);

return $positionField;
}

return $this->forms[$formId][$fieldName];
Expand Down Expand Up @@ -605,6 +635,7 @@ private function getFormNode(\DOMElement $element)
{
if ($element->hasAttribute('form')) {
$formId = $element->getAttribute('form');
\assert($element->ownerDocument !== null);
$formNode = $element->ownerDocument->getElementById($formId);

if (null === $formNode || 'form' !== $formNode->nodeName) {
Expand All @@ -623,6 +654,8 @@ private function getFormNode(\DOMElement $element)
}
} while ('form' !== $formNode->nodeName);

\assert($formNode instanceof \DOMElement);

return $formNode;
}

Expand All @@ -633,11 +666,9 @@ private function getFormNode(\DOMElement $element)
* When multiple fields have the same name (checkboxes for instance), it will return
* an array of elements in the order they appear in the DOM.
*
* @param \DOMElement $fieldNode
*
* @return integer
* @throws DriverException
*/
private function getFieldPosition(\DOMElement $fieldNode)
private function getFieldPosition(\DOMElement $fieldNode): int
{
$elements = $this->getCrawler()->filterXPath('//*[@name=\''.$fieldNode->getAttribute('name').'\']');
Expand All @@ -655,7 +686,7 @@ private function getFieldPosition(\DOMElement $fieldNode)
return 0;
}
private function submit(Form $form)
private function submit(Form $form): void
{
$formId = $this->getFormNodeId($form->getFormNode());
Expand All @@ -675,21 +706,14 @@ private function submit(Form $form)
$this->forms = array();
}

private function resetForm(\DOMElement $fieldNode)
private function resetForm(\DOMElement $fieldNode): void
{
$formNode = $this->getFormNode($fieldNode);
$formId = $this->getFormNodeId($formNode);
unset($this->forms[$formId]);
}

/**
* Determines if a node can submit a form.
*
* @param \DOMElement $node Node.
*
* @return boolean
*/
private function canSubmitForm(\DOMElement $node)
private function canSubmitForm(\DOMElement $node): bool
{
$type = $node->hasAttribute('type') ? $node->getAttribute('type') : null;

Expand All @@ -700,14 +724,7 @@ private function canSubmitForm(\DOMElement $node)
return 'button' === $node->nodeName && (null === $type || 'submit' === $type);
}

/**
* Determines if a node can reset a form.
*
* @param \DOMElement $node Node.
*
* @return boolean
*/
private function canResetForm(\DOMElement $node)
private function canResetForm(\DOMElement $node): bool
{
$type = $node->hasAttribute('type') ? $node->getAttribute('type') : null;

Expand Down Expand Up @@ -754,7 +771,7 @@ private function getOptionValue(\DOMElement $option)
* @param Form $to merging target
* @param Form $from merging source
*/
private function mergeForms(Form $to, Form $from)
private function mergeForms(Form $to, Form $from): void
{
foreach ($from->all() as $name => $field) {
$fieldReflection = new \ReflectionObject($field);
Expand All @@ -768,25 +785,27 @@ private function mergeForms(Form $to, Form $from)
in_array($nodeReflection->getValue($field)->getAttribute('type'), array('submit', 'button', 'image'), true);

if (!$isIgnoredField) {
$valueReflection->setValue($to[$name], $valueReflection->getValue($field));
$targetField = $to[$name];

\assert($targetField instanceof FormField);

$valueReflection->setValue($targetField, $valueReflection->getValue($field));
}
}
}

/**
* Returns DOMElement from crawler instance.
*
* @param Crawler $crawler
*
* @return \DOMElement
*
* @throws DriverException when the node does not exist
*/
private function getCrawlerNode(Crawler $crawler)
private function getCrawlerNode(Crawler $crawler): \DOMElement
{
$node = $crawler->getNode(0);

if (null !== $node) {
\assert($node instanceof \DOMElement);

return $node;
}

Expand Down
9 changes: 8 additions & 1 deletion tests/AbstractBrowserKitConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@

abstract class AbstractBrowserKitConfig extends AbstractConfig
{
final public function __construct()
{
}

/**
* @return static
*/
public static function getInstance()
{
return new static();
Expand All @@ -25,7 +32,7 @@ public function skipMessage($testCase, $test): ?string
return parent::skipMessage($testCase, $test);
}

protected function supportsJs()
protected function supportsJs(): bool
{
return false;
}
Expand Down
Loading

0 comments on commit f8c2121

Please sign in to comment.