Issues in NodeTwigExtension.php - Fixed Issues - Inspection of "Merge pull request #2662 from acrobat/replace-doct..." - Kunstmaan/KunstmaanBundlesCMS - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 6d6774...64f3ed )

by Jeroen

created 2020-06-25 11:41 UTC

Kunstmaan/NodeBundle/Twig/NodeTwigExtension.php (4 issues)

Labels

Bug 4

Severity

Minor 4

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

namespace Kunstmaan\NodeBundle\Twig;

use Doctrine\ORM\EntityManagerInterface;
use Kunstmaan\AdminListBundle\Entity\OverviewNavigationInterface;
use Kunstmaan\NodeBundle\Entity\Node;
use Kunstmaan\NodeBundle\Entity\NodeTranslation;
use Kunstmaan\NodeBundle\Entity\PageInterface;
use Kunstmaan\NodeBundle\Entity\StructureNode;
use Kunstmaan\NodeBundle\Helper\NodeMenu;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Twig\Extension\AbstractExtension;
use Twig\TwigFunction;

/**
 * Extension to fetch node / translation by page in Twig templates
 *
 * @final since 5.4
 */
class NodeTwigExtension extends AbstractExtension
{
    /**
     * @var EntityManagerInterface
     */
    private $em;

    /**
     * @var UrlGeneratorInterface
     */
    private $generator;

    /**
     * @var NodeMenu
     */
    private $nodeMenu;

    /**
     * @var RequestStack
     */
    private $requestStack;

    /**
     * @param \Doctrine\ORM\EntityManagerInterface                       $em
     * @param \Symfony\Component\Routing\Generator\UrlGeneratorInterface $generator
     * @param \Kunstmaan\NodeBundle\Helper\NodeMenu                      $nodeMenu
     * @param \Symfony\Component\HttpFoundation\RequestStack             $requestStack
     */
    public function __construct(
        EntityManagerInterface $em,
        UrlGeneratorInterface $generator,
        NodeMenu $nodeMenu,
        RequestStack $requestStack
    ) {
        $this->em = $em;
        $this->generator = $generator;
        $this->nodeMenu = $nodeMenu;
        $this->requestStack = $requestStack;
    }

    /**
     * Returns a list of functions to add to the existing list.
     *
     * @return array An array of functions
     */
    public function getFunctions()
    {
        return array(
            new TwigFunction(
                'get_node_for', array($this, 'getNodeFor')
            ),
            new TwigFunction(
                'get_node_translation_for',
                array($this, 'getNodeTranslationFor')
            ),
            new TwigFunction(
                'get_node_by_internal_name',
                array($this, 'getNodeByInternalName')
            ),
            new TwigFunction(
                'get_url_by_internal_name',
                array($this, 'getUrlByInternalName')
            ),
            new TwigFunction(
                'get_path_by_internal_name',
                array($this, 'getPathByInternalName')
            ),
            new TwigFunction(
                'get_page_by_node_translation',
                array($this, 'getPageByNodeTranslation')
            ),
            new TwigFunction(
                'get_node_menu',
                array($this, 'getNodeMenu')
            ),
            new TwigFunction(
                'is_structure_node',
                array($this, 'isStructureNode')
            ),
            new TwigFunction(
                'file_exists',
                array($this, 'fileExists')
            ),
            new TwigFunction(
                'get_node_trans_by_node_id',
                array($this, 'getNodeTranslationByNodeId')
            ),
            new TwigFunction(
                'getOverviewRoute',
                array($this, 'getOverviewRoute')
            ),
        );
    }

    /**
     * Get the node translation object based on node id and language.
     *
     * @param int    $nodeId
     * @param string $lang
     *
     * @return NodeTranslation
     */
    public function getNodeTranslationByNodeId($nodeId, $lang)
    {
        $repo = $this->em->getRepository('KunstmaanNodeBundle:NodeTranslation');

        return $repo->getNodeTranslationByNodeIdQueryBuilder($nodeId, $lang);
    }

    /**
     * @param NodeTranslation $nodeTranslation
     *
     * @return null|object
     */
    public function getPageByNodeTranslation(NodeTranslation $nodeTranslation)
    {
        return $nodeTranslation->getRef($this->em);
    }

    /**
     * @param PageInterface $page
     *
     * @return Node
     */
    public function getNodeFor(PageInterface $page)
    {
        return $this->em->getRepository('KunstmaanNodeBundle:Node')->getNodeFor($page);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
    }

    /**
     * @param PageInterface $page
     *
     * @return NodeTranslation
     */
    public function getNodeTranslationFor(PageInterface $page)
    {
        return $this->em->getRepository('KunstmaanNodeBundle:NodeTranslation')->getNodeTranslationFor($page);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
    }

    /**
     * @param string $internalName
     * @param string $locale
     *
     * @return Node|null
     */
    public function getNodeByInternalName($internalName, $locale)
    {
        $nodes = $this->em->getRepository('KunstmaanNodeBundle:Node')
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
            ->getNodesByInternalName($internalName, $locale);
        if (!empty($nodes)) {
            return $nodes[0];
        }

        return null;
    }

    /**
     * @param string $internalName Internal name of the node
     * @param string $locale       Locale
     * @param array  $parameters   (optional) extra parameters
     * @param bool   $relative     (optional) return relative path?
     *
     * @return string
     */
    public function getPathByInternalName($internalName, $locale, $parameters = array(), $relative = false)
    {
        $routeParameters = $this->getRouteParametersByInternalName($internalName, $locale, $parameters);

        return $this->generator->generate(
            '_slug',
            $routeParameters,
            $relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH
        );
    }

    /**
     * @param string $internalName   Internal name of the node
     * @param string $locale         Locale
     * @param array  $parameters     (optional) extra parameters
     * @param bool   $schemeRelative (optional) return relative scheme?
     *
     * @return string
     */
    public function getUrlByInternalName($internalName, $locale, $parameters = array(), $schemeRelative = false)
    {
        $routeParameters = $this->getRouteParametersByInternalName($internalName, $locale, $parameters);

        return $this->generator->generate(
            '_slug',
            $routeParameters,
            $schemeRelative ? UrlGeneratorInterface::NETWORK_PATH : UrlGeneratorInterface::ABSOLUTE_URL
        );
    }

    /**
     * @param string $locale
     * @param Node   $node
     * @param bool   $includeHiddenFromNav
     *
     * @return NodeMenu
     */
    public function getNodeMenu($locale, Node $node = null, $includeHiddenFromNav = false)
    {
        $request = $this->requestStack->getMasterRequest();
        $isPreview = $request->attributes->has('preview') && $request->attributes->get('preview') === true;
        $this->nodeMenu->setLocale($locale);
        $this->nodeMenu->setCurrentNode($node);
        $this->nodeMenu->setIncludeOffline($isPreview);
        $this->nodeMenu->setIncludeHiddenFromNav($includeHiddenFromNav);

        return $this->nodeMenu;
    }

    public function isStructureNode($page)
    {
        return $page instanceof StructureNode;
    }

    public function fileExists($filename)
    {
        return file_exists($filename);
    }

    /**
     * @param string $internalName
     * @param string $locale
     * @param array  $parameters
     *
     * @return array
     */
    private function getRouteParametersByInternalName($internalName, $locale, $parameters = array())
    {
        $url = '';
        $translation = $this->em->getRepository('KunstmaanNodeBundle:NodeTranslation')
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
            ->getNodeTranslationByLanguageAndInternalName($locale, $internalName);

        if (!\is_null($translation)) {
            $url = $translation->getUrl();
        }

        return array_merge(
            array(
                'url' => $url,
                '_locale' => $locale,
            ),
            $parameters
        );
    }

    public function getOverviewRoute($node)
    {
        if ($node instanceof OverviewNavigationInterface) {
            return $node->getOverViewRoute();
        }

        return null;
    }
}


Push — master ( 6d6774...64f3ed )

Kunstmaan/NodeBundle/Twig/NodeTwigExtension.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Available Fixes

Available Fixes

Available Fixes

1		<?php
2
3		namespace Kunstmaan\NodeBundle\Twig;
4
5		use Doctrine\ORM\EntityManagerInterface;
6		use Kunstmaan\AdminListBundle\Entity\OverviewNavigationInterface;
7		use Kunstmaan\NodeBundle\Entity\Node;
8		use Kunstmaan\NodeBundle\Entity\NodeTranslation;
9		use Kunstmaan\NodeBundle\Entity\PageInterface;
10		use Kunstmaan\NodeBundle\Entity\StructureNode;
11		use Kunstmaan\NodeBundle\Helper\NodeMenu;
12		use Symfony\Component\HttpFoundation\RequestStack;
13		use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
14		use Twig\Extension\AbstractExtension;
15		use Twig\TwigFunction;
16
17		/**
18		* Extension to fetch node / translation by page in Twig templates
19		*
20		* @final since 5.4
21		*/
22		class NodeTwigExtension extends AbstractExtension
23		{
24		/**
25		* @var EntityManagerInterface
26		*/
27		private $em;
28
29		/**
30		* @var UrlGeneratorInterface
31		*/
32		private $generator;
33
34		/**
35		* @var NodeMenu
36		*/
37		private $nodeMenu;
38
39		/**
40		* @var RequestStack
41		*/
42		private $requestStack;
43
44		/**
45		* @param \Doctrine\ORM\EntityManagerInterface $em
46		* @param \Symfony\Component\Routing\Generator\UrlGeneratorInterface $generator
47		* @param \Kunstmaan\NodeBundle\Helper\NodeMenu $nodeMenu
48		* @param \Symfony\Component\HttpFoundation\RequestStack $requestStack
49		*/
50		public function __construct(
51		EntityManagerInterface $em,
52		UrlGeneratorInterface $generator,
53		NodeMenu $nodeMenu,
54		RequestStack $requestStack
55		) {
56		$this->em = $em;
57		$this->generator = $generator;
58		$this->nodeMenu = $nodeMenu;
59		$this->requestStack = $requestStack;
60		}
61
62		/**
63		* Returns a list of functions to add to the existing list.
64		*
65		* @return array An array of functions
66		*/
67		public function getFunctions()
68		{
69		return array(
70		new TwigFunction(
71		'get_node_for', array($this, 'getNodeFor')
72		),
73		new TwigFunction(
74		'get_node_translation_for',
75		array($this, 'getNodeTranslationFor')
76		),
77		new TwigFunction(
78		'get_node_by_internal_name',
79		array($this, 'getNodeByInternalName')
80		),
81		new TwigFunction(
82		'get_url_by_internal_name',
83		array($this, 'getUrlByInternalName')
84		),
85		new TwigFunction(
86		'get_path_by_internal_name',
87		array($this, 'getPathByInternalName')
88		),
89		new TwigFunction(
90		'get_page_by_node_translation',
91		array($this, 'getPageByNodeTranslation')
92		),
93		new TwigFunction(
94		'get_node_menu',
95		array($this, 'getNodeMenu')
96		),
97		new TwigFunction(
98		'is_structure_node',
99		array($this, 'isStructureNode')
100		),
101		new TwigFunction(
102		'file_exists',
103		array($this, 'fileExists')
104		),
105		new TwigFunction(
106		'get_node_trans_by_node_id',
107		array($this, 'getNodeTranslationByNodeId')
108		),
109		new TwigFunction(
110		'getOverviewRoute',
111		array($this, 'getOverviewRoute')
112		),
113		);
114		}
115
116		/**
117		* Get the node translation object based on node id and language.
118		*
119		* @param int $nodeId
120		* @param string $lang
121		*
122		* @return NodeTranslation
123		*/
124		public function getNodeTranslationByNodeId($nodeId, $lang)
125		{
126		$repo = $this->em->getRepository('KunstmaanNodeBundle:NodeTranslation');
127
128		return $repo->getNodeTranslationByNodeIdQueryBuilder($nodeId, $lang);
129		}
130
131		/**
132		* @param NodeTranslation $nodeTranslation
133		*
134		* @return null\|object
135		*/
136		public function getPageByNodeTranslation(NodeTranslation $nodeTranslation)
137		{
138		return $nodeTranslation->getRef($this->em);
139		}
140
141		/**
142		* @param PageInterface $page
143		*
144		* @return Node
145		*/
146		public function getNodeFor(PageInterface $page)
147		{
148		return $this->em->getRepository('KunstmaanNodeBundle:Node')->getNodeFor($page);
		0 ignored issues – show Bug introduced 2020-01-09 14:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a concrete implementation and not the interface `Doctrine\Persistence\ObjectRepository` as the method `getNodeFor()` does only exist in the following implementations of said interface: `Kunstmaan\NodeBundle\Repository\NodeRepository`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
149		}
150
151		/**
152		* @param PageInterface $page
153		*
154		* @return NodeTranslation
155		*/
156		public function getNodeTranslationFor(PageInterface $page)
157		{
158		return $this->em->getRepository('KunstmaanNodeBundle:NodeTranslation')->getNodeTranslationFor($page);
		0 ignored issues – show Bug introduced 2020-01-09 14:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a concrete implementation and not the interface `Doctrine\Persistence\ObjectRepository` as the method `getNodeTranslationFor()` does only exist in the following implementations of said interface: `Kunstmaan\NodeBundle\Rep...deTranslationRepository`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
159		}
160
161		/**
162		* @param string $internalName
163		* @param string $locale
164		*
165		* @return Node\|null
166		*/
167		public function getNodeByInternalName($internalName, $locale)
168		{
169		$nodes = $this->em->getRepository('KunstmaanNodeBundle:Node')
		0 ignored issues – show Bug introduced 2020-01-09 14:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a concrete implementation and not the interface `Doctrine\Persistence\ObjectRepository` as the method `getNodesByInternalName()` does only exist in the following implementations of said interface: `Kunstmaan\NodeBundle\Repository\NodeRepository`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
170		->getNodesByInternalName($internalName, $locale);
171		if (!empty($nodes)) {
172		return $nodes[0];
173		}
174
175		return null;
176		}
177
178		/**
179		* @param string $internalName Internal name of the node
180		* @param string $locale Locale
181		* @param array $parameters (optional) extra parameters
182		* @param bool $relative (optional) return relative path?
183		*
184		* @return string
185		*/
186	View Code Duplication	public function getPathByInternalName($internalName, $locale, $parameters = array(), $relative = false)
187		{
188		$routeParameters = $this->getRouteParametersByInternalName($internalName, $locale, $parameters);
189
190		return $this->generator->generate(
191		'_slug',
192		$routeParameters,
193		$relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH
194		);
195		}
196
197		/**
198		* @param string $internalName Internal name of the node
199		* @param string $locale Locale
200		* @param array $parameters (optional) extra parameters
201		* @param bool $schemeRelative (optional) return relative scheme?
202		*
203		* @return string
204		*/
205	View Code Duplication	public function getUrlByInternalName($internalName, $locale, $parameters = array(), $schemeRelative = false)
206		{
207		$routeParameters = $this->getRouteParametersByInternalName($internalName, $locale, $parameters);
208
209		return $this->generator->generate(
210		'_slug',
211		$routeParameters,
212		$schemeRelative ? UrlGeneratorInterface::NETWORK_PATH : UrlGeneratorInterface::ABSOLUTE_URL
213		);
214		}
215
216		/**
217		* @param string $locale
218		* @param Node $node
219		* @param bool $includeHiddenFromNav
220		*
221		* @return NodeMenu
222		*/
223		public function getNodeMenu($locale, Node $node = null, $includeHiddenFromNav = false)
224		{
225		$request = $this->requestStack->getMasterRequest();
226		$isPreview = $request->attributes->has('preview') && $request->attributes->get('preview') === true;
227		$this->nodeMenu->setLocale($locale);
228		$this->nodeMenu->setCurrentNode($node);
229		$this->nodeMenu->setIncludeOffline($isPreview);
230		$this->nodeMenu->setIncludeHiddenFromNav($includeHiddenFromNav);
231
232		return $this->nodeMenu;
233		}
234
235		public function isStructureNode($page)
236		{
237		return $page instanceof StructureNode;
238		}
239
240		public function fileExists($filename)
241		{
242		return file_exists($filename);
243		}
244
245		/**
246		* @param string $internalName
247		* @param string $locale
248		* @param array $parameters
249		*
250		* @return array
251		*/
252		private function getRouteParametersByInternalName($internalName, $locale, $parameters = array())
253		{
254		$url = '';
255		$translation = $this->em->getRepository('KunstmaanNodeBundle:NodeTranslation')
		0 ignored issues – show Bug introduced 2020-01-09 14:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a concrete implementation and not the interface `Doctrine\Persistence\ObjectRepository` as the method `getNodeTranslationByLanguageAndInternalName()` does only exist in the following implementations of said interface: `Kunstmaan\NodeBundle\Rep...deTranslationRepository`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
256		->getNodeTranslationByLanguageAndInternalName($locale, $internalName);
257
258		if (!\is_null($translation)) {
259		$url = $translation->getUrl();
260		}
261
262		return array_merge(
263		array(
264		'url' => $url,
265		'_locale' => $locale,
266		),
267		$parameters
268		);
269		}
270
271		public function getOverviewRoute($node)
272		{
273		if ($node instanceof OverviewNavigationInterface) {
274		return $node->getOverViewRoute();
275		}
276
277		return null;
278		}
279		}
280

Kunstmaan / KunstmaanBundlesCMS

Push — master ( 6d6774...64f3ed )

Kunstmaan/NodeBundle/Twig/NodeTwigExtension.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like