From 1b91c1ebae989588c478e424f374879c8d20d086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 22 Apr 2024 14:58:44 +0200 Subject: [PATCH 1/4] Revert "Get rid of phpstan/phpstan-strict-rules" This reverts commit ff59ade31a2d904fe5f6ef763f2d1a244aa33ee4. --- composer.json | 1 + phpstan-baseline.neon | 36 +++++++++++++++++++++++++++++ phpstan.neon.dist | 16 +++++++++++++ plugins/managesieve/managesieve.php | 4 ++-- program/actions/contacts/delete.php | 1 + program/actions/contacts/move.php | 1 + program/actions/contacts/save.php | 1 + program/actions/mail/compose.php | 2 +- program/include/rcmail.php | 2 +- program/include/rcmail_sendmail.php | 1 + program/lib/Roundcube/rcube.php | 1 + 11 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 phpstan-baseline.neon diff --git a/composer.json b/composer.json index e855cf02411..76c9d060b00 100644 --- a/composer.json +++ b/composer.json @@ -23,6 +23,7 @@ "phpstan/extension-installer": "^1.1", "phpstan/phpstan": "^1.2", "phpstan/phpstan-deprecation-rules": "^1.0", + "phpstan/phpstan-strict-rules": "^1.3", "phpunit/phpunit": "^9.6 || ^10.0 || ^11.0", "roundcube/acl": "*", "roundcube/additional_message_headers": "*", diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon new file mode 100644 index 00000000000..86f7713ad14 --- /dev/null +++ b/phpstan-baseline.neon @@ -0,0 +1,36 @@ +parameters: + ignoreErrors: + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\App\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/App.php + + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Dialog\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/Dialog.php + + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\HtmlEditor\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/HtmlEditor.php + + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Popupmenu\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/Popupmenu.php + + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\RecipientInput\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/RecipientInput.php + + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Taskmenu\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/Taskmenu.php + + - + message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Toolbarmenu\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" + count: 1 + path: tests/Browser/Components/Toolbarmenu.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index df1f28db05d..2535502cd63 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,5 +1,6 @@ includes: - phar://phpstan.phar/conf/bleedingEdge.neon + - phpstan-baseline.neon parameters: level: 4 @@ -11,6 +12,21 @@ parameters: - vendor ignoreErrors: + # relax strict rules + - '~^Only booleans are allowed in .+, .+ given( on the (left|right) side)?\.~' + - '~^Variable (static )?(property access|method call) on .+\.~' + - '~^Variable variables are not allowed.~' + - '~^Variable .* might not be defined\.~' + - '~Call to function array_filter\(\) requires parameter #2 to be passed to avoid loose comparison semantics.~' + # relax strict rules - move to phpstan baseline once almost all l6 errors are fixed + - '~^Dynamic call to static method .+\.~' # TODO in https://github.com/roundcube/roundcubemail/pull/9314 + - '~^Construct empty\(\) is not allowed\. Use more strict comparison\.~' + - '~^Loose comparison via "[=!]=" is not allowed\.~' + - '~^Casting to .+ something that''s already .+\.~' + - '~^Short ternary operator is not allowed\. Use null coalesce operator if applicable or consider using long ternary\.~' + - '~^Call to function (array_search|in_array)\(\) requires parameter #3 to be set\.~' + - '~^Call to function base64_decode\(\) requires parameter #2 to be (set|true).~' + # https://github.com/php/pecl-authentication-krb5 - path: 'program/lib/Roundcube/rcube_imap_generic.php' diff --git a/plugins/managesieve/managesieve.php b/plugins/managesieve/managesieve.php index 1e160b60f83..910e285cb61 100644 --- a/plugins/managesieve/managesieve.php +++ b/plugins/managesieve/managesieve.php @@ -60,8 +60,8 @@ public function init() $this->register_action('plugin.managesieve-save', [$this, 'managesieve_save']); $this->register_action('plugin.managesieve-saveraw', [$this, 'managesieve_saveraw']); - $task = $this->rc->task ?? null; - $action = $this->rc->action ?? null; + $task = $this->rc->task ?? null; // @phpstan-ignore-line + $action = $this->rc->action ?? null; // @phpstan-ignore-line if ($task == 'settings') { $this->add_hook('settings_actions', [$this, 'settings_actions']); diff --git a/program/actions/contacts/delete.php b/program/actions/contacts/delete.php index 3933e17555d..5657bfad825 100644 --- a/program/actions/contacts/delete.php +++ b/program/actions/contacts/delete.php @@ -41,6 +41,7 @@ public function run($args = []) foreach ($cids as $source => $cid) { $CONTACTS = self::contact_source($source); + // @phpstan-ignore-next-line if ($CONTACTS->readonly && empty($CONTACTS->deletable)) { // more sources? do nothing, probably we have search results from // more than one source, some of these sources can be readonly diff --git a/program/actions/contacts/move.php b/program/actions/contacts/move.php index 0649d94b409..399520456cf 100644 --- a/program/actions/contacts/move.php +++ b/program/actions/contacts/move.php @@ -63,6 +63,7 @@ public function run($args = []) break; } + // @phpstan-ignore-next-line if (!$CONTACTS || !$CONTACTS->ready || ($CONTACTS->readonly && empty($CONTACTS->deletable))) { continue; } diff --git a/program/actions/contacts/save.php b/program/actions/contacts/save.php index cf4bd237780..0cf5100e1aa 100644 --- a/program/actions/contacts/save.php +++ b/program/actions/contacts/save.php @@ -245,6 +245,7 @@ public static function process_input() foreach ($subtypes as $i => $subtype) { $suffix = $subtype ? ":{$subtype}" : ''; + // @phpstan-ignore-next-line if (!empty($values[$i])) { $record[$col . $suffix][] = $values[$i]; } diff --git a/program/actions/mail/compose.php b/program/actions/mail/compose.php index 00a19e45491..0739aeb7312 100644 --- a/program/actions/mail/compose.php +++ b/program/actions/mail/compose.php @@ -1652,7 +1652,7 @@ public static function save_attachment($message, $pid, $compose_id, $params = [] $filename = !empty($params['filename']) ? $params['filename'] : self::attachment_name($part); } elseif ($message instanceof rcube_message) { // the whole message requested - $size = $message->size ?? null; + $size = $message->size ?? null; // @phpstan-ignore-line $mimetype = 'message/rfc822'; $filename = !empty($params['filename']) ? $params['filename'] : 'message_rfc822.eml'; } elseif (is_string($message)) { diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 515b77b760b..f8da1c9940b 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -56,7 +56,7 @@ class rcmail extends rcube public $oauth; /** @var rcmail_output_cli|rcmail_output_html|rcmail_output_json|null Output handler */ - public $output; + public $output; // @phpstan-ignore-line private $address_books = []; private $action_args = []; diff --git a/program/include/rcmail_sendmail.php b/program/include/rcmail_sendmail.php index aa0f0f8653e..bdd522e920d 100644 --- a/program/include/rcmail_sendmail.php +++ b/program/include/rcmail_sendmail.php @@ -457,6 +457,7 @@ public function deliver_message($message, $disconnect = true) return false; } + // @phpstan-ignore-next-line if ($mailbody_file) { $this->temp_files[$message->headers()['Message-ID']] = $mailbody_file; } diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php index dd6d0537875..159a34ef986 100644 --- a/program/lib/Roundcube/rcube.php +++ b/program/lib/Roundcube/rcube.php @@ -659,6 +659,7 @@ public function text_exists($name, $domain = null, &$ref_domain = null) // any of loaded domains (plugins) if ($domain == '*') { + // @phpstan-ignore-next-line foreach ($this->plugins->loaded_plugins() as $domain) { if (isset($this->texts[$domain . '.' . $name])) { $ref_domain = $domain; From 8d1564ad18dcd6f04a6f355f6796805fabf1b959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 22 Apr 2024 15:05:14 +0200 Subject: [PATCH 2/4] drop phpstan baseline --- phpstan-baseline.neon | 36 ------------------------------------ phpstan.neon.dist | 8 ++++++-- 2 files changed, 6 insertions(+), 38 deletions(-) delete mode 100644 phpstan-baseline.neon diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon deleted file mode 100644 index 86f7713ad14..00000000000 --- a/phpstan-baseline.neon +++ /dev/null @@ -1,36 +0,0 @@ -parameters: - ignoreErrors: - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\App\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/App.php - - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Dialog\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/Dialog.php - - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\HtmlEditor\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/HtmlEditor.php - - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Popupmenu\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/Popupmenu.php - - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\RecipientInput\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/RecipientInput.php - - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Taskmenu\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/Taskmenu.php - - - - message: "#^Parameter \\#1 \\$browser \\(Tests\\\\Browser\\\\Browser\\) of method Tests\\\\Browser\\\\Components\\\\Toolbarmenu\\:\\:assert\\(\\) should be contravariant with parameter \\$browser \\(Laravel\\\\Dusk\\\\Browser\\) of method Laravel\\\\Dusk\\\\Component\\:\\:assert\\(\\)$#" - count: 1 - path: tests/Browser/Components/Toolbarmenu.php diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 2535502cd63..1ce9ab2b8d9 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,6 +1,5 @@ includes: - phar://phpstan.phar/conf/bleedingEdge.neon - - phpstan-baseline.neon parameters: level: 4 @@ -18,7 +17,7 @@ parameters: - '~^Variable variables are not allowed.~' - '~^Variable .* might not be defined\.~' - '~Call to function array_filter\(\) requires parameter #2 to be passed to avoid loose comparison semantics.~' - # relax strict rules - move to phpstan baseline once almost all l6 errors are fixed + # TODO - '~^Dynamic call to static method .+\.~' # TODO in https://github.com/roundcube/roundcubemail/pull/9314 - '~^Construct empty\(\) is not allowed\. Use more strict comparison\.~' - '~^Loose comparison via "[=!]=" is not allowed\.~' @@ -27,6 +26,11 @@ parameters: - '~^Call to function (array_search|in_array)\(\) requires parameter #3 to be set\.~' - '~^Call to function base64_decode\(\) requires parameter #2 to be (set|true).~' + - + message: '~^Parameter #1 \$browser \(Tests\\Browser\\Browser\) of method Tests\\Browser\\Components\\\w+::assert\(\) should be contravariant with parameter \$browser \(Laravel\\Dusk\\Browser\) of method Laravel\\Dusk\\Component::assert\(\)$~' + count: 7 + path: tests/Browser/Components/*.php + # https://github.com/php/pecl-authentication-krb5 - path: 'program/lib/Roundcube/rcube_imap_generic.php' From 47eb39389059fda26041b5b7db27f721a60c2412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 22 Apr 2024 15:22:49 +0200 Subject: [PATCH 3/4] fix foreach phpstan issue --- program/lib/Roundcube/rcube.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php index 159a34ef986..ee3bf8ba293 100644 --- a/program/lib/Roundcube/rcube.php +++ b/program/lib/Roundcube/rcube.php @@ -659,10 +659,9 @@ public function text_exists($name, $domain = null, &$ref_domain = null) // any of loaded domains (plugins) if ($domain == '*') { - // @phpstan-ignore-next-line - foreach ($this->plugins->loaded_plugins() as $domain) { - if (isset($this->texts[$domain . '.' . $name])) { - $ref_domain = $domain; + foreach ($this->plugins->loaded_plugins() as $domain2) { + if (isset($this->texts[$domain2 . '.' . $name])) { + $ref_domain = $domain2; return true; } } From 019a8ce4a54179391a941229e87e3a0b0828bb10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 17 Jun 2024 13:12:54 +0200 Subject: [PATCH 4/4] adjust for rebase --- .github/workflows/ci.yml | 2 +- phpstan.neon.dist | 2 +- program/lib/Roundcube/rcube_imap.php | 1 - tests/Framework/MessageTest.php | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75445c43d3b..9a6442248cd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,4 +78,4 @@ jobs: - name: Run Static Analysis run: | rm -r public_html # remove once https://github.com/phpstan/phpstan/issues/10321 is fixed - vendor/bin/phpstan analyse + vendor/bin/phpstan analyse -v diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 1ce9ab2b8d9..edf25b4a0c5 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -27,7 +27,7 @@ parameters: - '~^Call to function base64_decode\(\) requires parameter #2 to be (set|true).~' - - message: '~^Parameter #1 \$browser \(Tests\\Browser\\Browser\) of method Tests\\Browser\\Components\\\w+::assert\(\) should be contravariant with parameter \$browser \(Laravel\\Dusk\\Browser\) of method Laravel\\Dusk\\Component::assert\(\)$~' + message: '~^Parameter #1 \$browser \(Roundcube\\Tests\\Browser\\Browser\) of method Roundcube\\Tests\\Browser\\Components\\\w+::assert\(\) should be contravariant with parameter \$browser \(Laravel\\Dusk\\Browser\) of method Laravel\\Dusk\\Component::assert\(\)$~' count: 7 path: tests/Browser/Components/*.php diff --git a/program/lib/Roundcube/rcube_imap.php b/program/lib/Roundcube/rcube_imap.php index fbfe4b3391b..858d27c333a 100644 --- a/program/lib/Roundcube/rcube_imap.php +++ b/program/lib/Roundcube/rcube_imap.php @@ -4358,7 +4358,6 @@ public function sort_folder_list($a_folders, $skip_special = false) } // Force the type of folder name variable (#1485527) - /** @var array $folders */ $folders = array_map('strval', $folders); $count = count($folders); diff --git a/tests/Framework/MessageTest.php b/tests/Framework/MessageTest.php index 876771b5329..0b29a8d0234 100644 --- a/tests/Framework/MessageTest.php +++ b/tests/Framework/MessageTest.php @@ -94,7 +94,7 @@ class rcube_message_test extends \rcube_message { private $part_bodies = []; - public function __construct($uid, $folder = null, $is_safe = false) + public function __construct($uid, $folder = null, $is_safe = false) // @phpstan-ignore constructor.missingParentCall { $this->uid = $uid; $this->folder = $folder;