Skip to content

False positive - Method return typing changed when class in the same namespace #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ihor-sviziev opened this issue Jun 3, 2020 · 3 comments

Comments

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Jun 3, 2020

I've seen few times when SVC check was complaining about changed return type, when actually it wasn't.

Example taken from magento/magento2#27980 here https://github.com/magento/magento2/pull/27980/files#diff-fe16896d208c3ab2d52fccbbf7a5eaebR113:

diff --git a/lib/internal/Magento/Framework/DB/Select.php b/lib/internal/Magento/Framework/DB/Select.php
index 075aa6b24faa..4f79b0d314f1 100644
--- a/lib/internal/Magento/Framework/DB/Select.php
+++ b/lib/internal/Magento/Framework/DB/Select.php
@@ -7,6 +7,7 @@
 
 use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\DB\Adapter\AdapterInterface;
+use Magento\Framework\DB\Sql\Expression;
 
 /**
  * Class for SQL SELECT generation and results.
@@ -107,19 +108,19 @@ public function __construct(
      * </code>
      *
      * @param string $cond The WHERE condition.
-     * @param string|array|null $value OPTIONAL An optional single or array value to quote into the condition.
-     * @param string|int|null $type OPTIONAL The type of the given value
-     * @return \Magento\Framework\DB\Select
+     * @param array|null|int|string|float|Expression|Select|\DateTimeInterface $value The value to quote.
+     * @param int|string|null $type OPTIONAL SQL datatype of the given value e.g. Zend_Db::FLOAT_TYPE or "INT"
+     * @return Select
      */
     public function where($cond, $value = null, $type = null)
     {
         if ($value === null && $type === null) {
             $value = '';
-        } elseif ($type == self::TYPE_CONDITION) {
+        } elseif ((string)$type === self::TYPE_CONDITION) {
             $type = null;
         }
         if (is_array($value)) {
-            $cond = $this->getConnection()->quoteInto($cond, $value);
+            $cond = $this->getConnection()->quoteInto($cond, $value, $type);
             $value = null;
         }
         return parent::where($cond, $value, $type);

Actual result

Result is following:
image

Level Target/Location Code/Reason
MAJOR Magento\Framework\DB\Select::where/lib/internal/Magento/Framework/DB/Select.php:115 M120 [public] Method return typing changed.

Expected result

Return type isn't changed.

Test complaining that return type has changed, but it actually not, as class Magento\Framework\DB\Select actually is THE SAME class as Select as it in the current namespace.

Additional examples:
magento/magento2#27129

magento/magento2#26355 (comment)

@m2-assistant
Copy link

m2-assistant bot commented Jun 3, 2020

Hi @ihor-sviziev. Thank you for your report.
To help us process this issue please make sure that you provided sufficient information.

Please, add a comment to assign the issue: @magento I am working on this


@okorshenko
Copy link

Hi @ihor-sviziev
Looks like this issue was fixed in #51

Could you please let us know if this issue is still relevant?

@ihor-sviziev
Copy link
Contributor Author

Yeah, looks like it was already fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants