What are the Slack Archives?
Itβs a history of our time together in the Slack Community! Thereβs a ton of knowledge in here, so feel free to search through the archives for a possible answer to your question.
Because this space is not active, you wonβt be able to create a new post or comment here. If you have a question or want to start a discussion about something, head over to our categories and pick one to post in! You can always refer back to a post from Slack Archives if needed; just copy the link to use it as a reference..
Since you changed the code sniffer
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?
Comments
-
The annotations are correct and have been missing so far. A customer based contribution made this now visible.
0 -
Sorry I don't understand your reply. Code sniffer now says the annotations are not correct. And what has been made visible now?
0 -
The annotations it will add are correct. You need to execution them the way the tool tells you to.
0 -
Then it will break with any method that is not part of spryker's core.
0 -
And it's simply not correct if the provided objects are from our namespaces
0 -
Can u show or screenshot what it would fix it to?
0 -
Sure
So I set everything to theSpryker
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.
0 -
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.
0 -
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?
0 -
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
0 -
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.
0 -
Yes, please do so.
We plan on making another bugfix release this week, so it would fit in perfectly.0 -
I can make a PR on Github, I'm already familiar with that since I contributed these changes to
0.17.6
0 -
Here you go: https://github.com/spryker/code-sniffer/pull/330
0 -
β€
0 -
Instead of the config and constant overhead I would rather recommend looking at composer json psr4
What do u think?0 -
Hasn't been released yet, right? 0.17.7 doesn't have it yet it seems.
0 -
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.
0 -
Why is there now a blank line between all constants? Is this supposed to increas readability for some strange reason?
0 -
Is there any alternative PR or alike to be finished as improvement towards more autodetection here?
0
Categories
- All Categories
- 42 Getting Started & Guidelines
- 7 Getting Started in the Community
- 8 Additional Resources
- 7 Community Ideas and Feedback
- 75 Spryker News
- 924 Developer Corner
- 783 Spryker Development
- 88 Spryker Dev Environment
- 362 Spryker Releases
- 3 Oryx frontend framework
- 35 Propel ORM
- 68 Community Projects
- 3 Community Ideation Board
- 30 Hackathon
- 3 PHP Bridge
- 6 Gacela Project
- 25 Job Opportunities
- 3.2K π Slack Archives
- 116 Academy
- 5 Business Users
- 370 Docker
- 551 Slack General
- 2K Help
- 75 Knowledge Sharing
- 6 Random Stuff
- 4 Code Testing
- 32 Product & Business Questions
- 70 Spryker Safari Questions
- 50 Random