Since you changed the code sniffer

UM4BZSK7T

https://github.com/spryker/code-sniffer/compare/0.17.6...master Since you changed the code sniffer check for method annotations I can't commit any more. As you can see now our annotations are not accepted anymore and I see no reason why. I fixed the modules version to 0.17.5 and it runs fine again. I guess that is a bug or what should we change on project level?

Unknown

The annotations are correct and have been missing so far. A customer based contribution made this now visible.

UM4BZSK7T

Sorry I don't understand your reply. Code sniffer now says the annotations are not correct. And what has been made visible now?

Unknown

The annotations it will add are correct. You need to execution them the way the tool tells you to.

UM4BZSK7T

Then it will break with any method that is not part of spryker's core.

UM4BZSK7T

And it's simply not correct if the provided objects are from our namespaces

Unknown

Can u show or screenshot what it would fix it to?

UM4BZSK7T

Sure
So I set everything to the Spryker namepsace

/**
 * @method \Spryker\Zed\Sales\Business\SalesFacadeInterface getFacade()
 * @method \Spryker\Zed\Sales\Communication\SalesCommunicationFactory getFactory()
 * @method \Spryker\Zed\Sales\SalesConfig getConfig()
 */

And now I get

  50     Call to an undefined method                                      
         Spryker\Zed\Sales\Business\SalesFacadeInterface::orderExists().  

because of

$isSuccess = !$this->getFacade()->orderExists($quoteTransfer->getOrderReference());

As I wrote above now the problem is that not all methods can be found in the base class from the Spryker namespace.

Unknown

Ah, sry, I was kind of out of the door already when I answered. I didnt see that you are using a custom non-standard project setup (and namespace).

With this you need to add some config in order to keep BC, as this was not the default expectation for customer projects.

https://github.com/spryker/code-sniffer#configure-custom-namespaces

Someone could try to PR a better auto detection of namespaces maybe. That would also help.

UKTSRTD5M

Or perhaps it can use the namespace-Configurations by default? Or ist there any specific logic that forces these namespaces to differ from the available ones by default?

U018XELUZS9

Could work like so, did not test that yet:

diff --git a/Spryker/Sniffs/AbstractSniffs/AbstractMethodAnnotationSniff.php b/Spryker/Sniffs/AbstractSniffs/AbstractMethodAnnotationSniff.php
index 30fa8e4..45a9b3b 100644
--- a/Spryker/Sniffs/AbstractSniffs/AbstractMethodAnnotationSniff.php
+++ b/Spryker/Sniffs/AbstractSniffs/AbstractMethodAnnotationSniff.php
@@ -9,6 +9,7 @@

 use PHP_CodeSniffer\Files\File;
 use SlevomatCodingStandard\Helpers\DocCommentHelper;
+use Spryker\Shared\Config\Config;

 abstract class AbstractMethodAnnotationSniff extends AbstractClassDetectionSprykerSniff
 {
@@ -27,11 +28,6 @@ abstract class AbstractMethodAnnotationSniff extends AbstractClassDetectionSpryk
      */
     protected const LAYER_BUSINESS = 'Business';

-    /**
-     * @var string
-     */
-    public $namespaces = 'Pyz,SprykerEco,SprykerMiddleware,SprykerSdk,Spryker';
-
     /**
      * @var bool
      */
@@ -116,7 +112,7 @@ protected function runSniffer(File $phpCsFile, int $stackPointer): void
      */
     protected function getNamespaceForFilename(File $phpCsFile): ?string
     {
-        $namespaces = explode(',', $this->namespaces);
+        $namespaces = $this->getNamespaces();
         foreach ($namespaces as $namespace) {
             if (
                 $this->fileExists(
@@ -132,6 +128,47 @@ protected function getNamespaceForFilename(File $phpCsFile): ?string
         return null;
     }

+    protected function getNamespaces(): array
+    {
+        $namespaces = [];
+
+        $this->defineFakeConstants();
+        try {
+            $config = new Config();
+            $namespaces = $config->get('CORE_NAMESPACES');
+        }
+        catch (\Exception $exception) {
+            // Do nothing
+        }
+
+        return $namespaces;
+    }
+
+    protected function defineFakeConstants(): void
+    {
+        if (!defined('APPLICATION_ROOT_DIR')) {
+            define('APPLICATION_ROOT_DIR', __DIR__);
+        }
+        if (!defined('APPLICATION_VENDOR_DIR')) {
+            define('APPLICATION_VENDOR_DIR', APPLICATION_ROOT_DIR . '/vendor');
+        }
+        if (!defined('APPLICATION_SOURCE_DIR')) {
+            define('APPLICATION_SOURCE_DIR', APPLICATION_ROOT_DIR . '/src');
+        }
+        if (!defined('APPLICATION')) {
+            define('APPLICATION', 'FAKE');
+        }
+        if (!defined('APPLICATION_ENV')) {
+            define('APPLICATION_ENV', 'devtest');
+        }
+        if (!defined('APPLICATION_STORE')) {
+            define('APPLICATION_STORE', 'MDE');
+        }
+        if (!defined('APPLICATION_CODE_BUCKET')) {
+            define('APPLICATION_CODE_BUCKET', 'MDE');
+        }
+    }
+
     /**
      * Checks if the '@method' annotation for the specific method already exists
      * in the class. When $strictCheck is set to true, the method also checks

UM4BZSK7T

Looks great 😁 maybe create a pull request on GitHub? I can help you doing that against the Spryker repo if you never done that before. That's easy.

Unknown

Yes, please do so.
We plan on making another bugfix release this week, so it would fit in perfectly.

U018XELUZS9

I can make a PR on Github, I'm already familiar with that since I contributed these changes to 0.17.6

U018XELUZS9

Here you go: https://github.com/spryker/code-sniffer/pull/330

UM4BZSK7T

❤

Unknown

Instead of the config and constant overhead I would rather recommend looking at composer json psr4
What do u think?

U01LKKBK97T

Hasn't been released yet, right? 0.17.7 doesn't have it yet it seems.

U018XELUZS9

The PR is still open, feel free to have a look at it and finalize it, since I'm not sure if I finish that. I stopped working with Spryker and I'm looking for a new role currently.

UM4BZSK7T

Why is there now a blank line between all constants? Is this supposed to increas readability for some strange reason?

Unknown

Is there any alternative PR or alike to be finished as improvement towards more autodetection here?