TopEditsController - Code Metrics - Inspection of "Abstract XtoolsController for refactoring code in..." - x-tools/xtools - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#111)

by MusikAnimal

created 2017-10-15 23:00 UTC

TopEditsController A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	179
Duplicated Lines	12.29 %

Coupling/Cohesion

Components	1
Dependencies	8

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
wmc	16
lcom	1
cbo	8
dl	22
loc	179
rs	10
c	1
b	0
f	0

5 Methods

Rating	Name	Duplication	Size	Complexity
A	getToolShortname()	0	4	1
B	namespaceTopEdits()	0	46	4
B	singlePageTopEdits()	0	40	5
A	indexAction()	22	22	3
A	resultAction()	0	16	3

How to fix Duplicated Code

<?php
/**
 * This file contains only the TopEditsController class.
 */

namespace AppBundle\Controller;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Xtools\Page;
use Xtools\PagesRepository;
use Xtools\Project;
use Xtools\ProjectRepository;
use Xtools\User;
use Xtools\UserRepository;
use Xtools\TopEdits;
use Xtools\TopEditsRepository;
use Xtools\Edit;

/**
 * The Top Edits tool.
 */
class TopEditsController extends XtoolsController
{

    /**
     * Get the tool's shortname.
     * @return string
     * @codeCoverageIgnore
     */
    public function getToolShortname()
    {
        return 'topedits';
    }

    /**
     * Display the form.
     * @Route("/topedits", name="topedits")
     * @Route("/topedits", name="TopEdits")
     * @Route("/topedits/", name="topEditsSlash")
     * @Route("/topedits/index.php", name="TopEditsIndex")
     * @Route("/topedits/{project}", name="TopEditsProject")
     * @param Request $request
     * @return Response
     */
    public function indexAction(Request $request)

    {
        $params = $this->parseQueryParams($request);

        // Redirect if at minimum project and username are provided.
        if (isset($params['project']) && isset($params['username'])) {
            return $this->redirectToRoute('TopEditsResults', $params);
        }

        // Convert the given project (or default project) into a Project instance.
        $params['project'] = $this->getProjectFromQuery($params);

        return $this->render('topedits/index.html.twig', array_merge([
            'xtPageTitle' => 'tool-topedits',
            'xtSubtitle' => 'tool-topedits-desc',
            'xtPage' => 'topedits',

            // Defaults that will get overriden if in $params.
            'namespace' => 0,
            'article' => '',
        ], $params));
    }

    /**
     * Display the results.
     * @Route("/topedits/{project}/{username}/{namespace}/{article}", name="TopEditsResults",
     *     requirements={"article"=".+"})
     * @param Request $request The HTTP request.
     * @param int $namespace
     * @param string $article
     * @return RedirectResponse|Response
     * @codeCoverageIgnore
     */
    public function resultAction(Request $request, $namespace = 0, $article = '')
    {
        // Second parameter causes it return a Redirect to the index if the user has too many edits.
        $ret = $this->validateProjectAndUser($request, 'topedits');
        if ($ret instanceof RedirectResponse) {
            return $ret;
        } else {
            list($projectData, $user) = $ret;
        }

        if ($article === '') {
            return $this->namespaceTopEdits($request, $user, $projectData, $namespace);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
        } else {
            return $this->singlePageTopEdits($user, $projectData, $namespace, $article);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
        }
    }

    /**
     * List top edits by this user for all pages in a particular namespace.
     * @param Request $request The HTTP request.
     * @param User $user The User.
     * @param Project $project The project.
     * @param integer|string $namespace The namespace ID or 'all'
     * @return \Symfony\Component\HttpFoundation\Response
     * @codeCoverageIgnore
     */
    public function namespaceTopEdits(Request $request, User $user, Project $project, $namespace)
    {
        $isSubRequest = $request->get('htmlonly')
            || $this->container->get('request_stack')->getParentRequest() !== null;

        // Make sure they've opted in to see this data.
        if (!$project->userHasOptedIn($user)) {
            $optedInPage = $project
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends 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.
    }
}
                ->getRepository()
                ->getPage($project, $project->userOptInPage($user));

            return $this->render('topedits/result_namespace.html.twig', [
                'xtPage' => 'topedits',
                'xtTitle' => $user->getUsername(),
                'project' => $project,
                'user' => $user,
                'namespace' => $namespace,
                'edits' => [],
                'content_title' => '',
                'opted_in_page' => $optedInPage,
                'is_sub_request' => $isSubRequest,
            ]);
        }

        /**
         * Max number of rows per namespace to show. `null` here will cause to
         * use the TopEdits default.
         * @var int
         */
        $limit = $isSubRequest ? 10 : null;

        $topEdits = new TopEdits($project, $user, $namespace, $limit);
        $topEditsRepo = new TopEditsRepository();
        $topEditsRepo->setContainer($this->container);

        $topEdits->setRepository($topEditsRepo);

        return $this->render('topedits/result_namespace.html.twig', [
            'xtPage' => 'topedits',
            'xtTitle' => $user->getUsername(),
            'project' => $project,
            'user' => $user,
            'namespace' => $namespace,
            'te' => $topEdits,
            'is_sub_request' => $isSubRequest,
        ]);
    }

    /**
     * List top edits by this user for a particular page.
     * @param User $user The user.
     * @param Project $project The project.
     * @param int $namespaceId The ID of the namespace of the page.
     * @param string $pageName The title (without namespace) of the page.
     * @return RedirectResponse|\Symfony\Component\HttpFoundation\Response
     * @codeCoverageIgnore
     */
    protected function singlePageTopEdits(User $user, Project $project, $namespaceId, $pageName)
    {
        // Get the full page name (i.e. no namespace prefix if NS 0).
        $namespaces = $project->getNamespaces();
        $fullPageName = $namespaceId ? $namespaces[$namespaceId].':'.$pageName : $pageName;

        $page = $this->getAndValidatePage($project, $fullPageName);

        if (is_a($page, 'Symfony\Component\HttpFoundation\RedirectResponse')) {
            return $page;
        }

        // Get all revisions of this page by this user.
        $revisionsData = $page->getRevisions($user);
class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

        // Loop through all revisions and format dates, find totals, etc.
        $totalAdded = 0;
        $totalRemoved = 0;
        $revisions = [];
        foreach ($revisionsData as $revision) {
            if ($revision['length_change'] > 0) {
                $totalAdded += $revision['length_change'];
            } else {
                $totalRemoved += $revision['length_change'];
            }
            $revisions[] = new Edit($page, $revision);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
        }

        // Send all to the template.
        return $this->render('topedits/result_article.html.twig', [
            'xtPage' => 'topedits',
            'xtTitle' => $user->getUsername() . ' - ' . $page->getTitle(),
class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}
            'project' => $project,
            'user' => $user,
            'page' => $page,
            'total_added' => $totalAdded,
            'total_removed' => $totalRemoved,
            'revisions' => $revisions,
            'revision_count' => count($revisions),
        ]);
    }
}


Pull Request — master (#111)

TopEditsController A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

5 Methods

How to fix Duplicated Code

Duplicated Code

Available Fixes

Available Fixes

Available Fixes

1		<?php
2		/**
3		* This file contains only the TopEditsController class.
4		*/
5
6		namespace AppBundle\Controller;
7
8		use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
9		use Symfony\Bundle\FrameworkBundle\Controller\Controller;
10		use Symfony\Component\HttpFoundation\RedirectResponse;
11		use Symfony\Component\HttpFoundation\Request;
12		use Symfony\Component\HttpFoundation\Response;
13		use Xtools\Page;
14		use Xtools\PagesRepository;
15		use Xtools\Project;
16		use Xtools\ProjectRepository;
17		use Xtools\User;
18		use Xtools\UserRepository;
19		use Xtools\TopEdits;
20		use Xtools\TopEditsRepository;
21		use Xtools\Edit;
22
23		/**
24		* The Top Edits tool.
25		*/
26		class TopEditsController extends XtoolsController
27		{
28
29		/**
30		* Get the tool's shortname.
31		* @return string
32		* @codeCoverageIgnore
33		*/
34		public function getToolShortname()
35		{
36		return 'topedits';
37		}
38
39		/**
40		* Display the form.
41		* @Route("/topedits", name="topedits")
42		* @Route("/topedits", name="TopEdits")
43		* @Route("/topedits/", name="topEditsSlash")
44		* @Route("/topedits/index.php", name="TopEditsIndex")
45		* @Route("/topedits/{project}", name="TopEditsProject")
46		* @param Request $request
47		* @return Response
48		*/
49	View Code Duplication	public function indexAction(Request $request)
		0 ignored issues – show Duplication introduced 2017-10-13 05:14 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
50		{
51		$params = $this->parseQueryParams($request);
52
53		// Redirect if at minimum project and username are provided.
54		if (isset($params['project']) && isset($params['username'])) {
55		return $this->redirectToRoute('TopEditsResults', $params);
56		}
57
58		// Convert the given project (or default project) into a Project instance.
59		$params['project'] = $this->getProjectFromQuery($params);
60
61		return $this->render('topedits/index.html.twig', array_merge([
62		'xtPageTitle' => 'tool-topedits',
63		'xtSubtitle' => 'tool-topedits-desc',
64		'xtPage' => 'topedits',
65
66		// Defaults that will get overriden if in $params.
67		'namespace' => 0,
68		'article' => '',
69		], $params));
70		}
71
72		/**
73		* Display the results.
74		* @Route("/topedits/{project}/{username}/{namespace}/{article}", name="TopEditsResults",
75		* requirements={"article"=".+"})
76		* @param Request $request The HTTP request.
77		* @param int $namespace
78		* @param string $article
79		* @return RedirectResponse\|Response
80		* @codeCoverageIgnore
81		*/
82		public function resultAction(Request $request, $namespace = 0, $article = '')
83		{
84		// Second parameter causes it return a Redirect to the index if the user has too many edits.
85		$ret = $this->validateProjectAndUser($request, 'topedits');
86		if ($ret instanceof RedirectResponse) {
87		return $ret;
88		} else {
89		list($projectData, $user) = $ret;
90		}
91
92		if ($article === '') {
93		return $this->namespaceTopEdits($request, $user, $projectData, $namespace);
		0 ignored issues – show Bug introduced 2017-10-13 17:45 UTC by Report Bug Copy Issue Report It seems like `$user` can be `null`; however, `namespaceTopEdits()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
94		} else {
95		return $this->singlePageTopEdits($user, $projectData, $namespace, $article);
		0 ignored issues – show Bug introduced 2017-10-13 17:45 UTC by Report Bug Copy Issue Report It seems like `$user` can be `null`; however, `singlePageTopEdits()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
96		}
97		}
98
99		/**
100		* List top edits by this user for all pages in a particular namespace.
101		* @param Request $request The HTTP request.
102		* @param User $user The User.
103		* @param Project $project The project.
104		* @param integer\|string $namespace The namespace ID or 'all'
105		* @return \Symfony\Component\HttpFoundation\Response
106		* @codeCoverageIgnore
107		*/
108		public function namespaceTopEdits(Request $request, User $user, Project $project, $namespace)
109		{
110		$isSubRequest = $request->get('htmlonly')
111		\|\| $this->container->get('request_stack')->getParentRequest() !== null;
112
113		// Make sure they've opted in to see this data.
114		if (!$project->userHasOptedIn($user)) {
115		$optedInPage = $project
		1 ignored issue – show Bug introduced 2017-06-22 03:59 UTC by Report Bug Copy Issue Report It seems like you code against a specific sub-type and not the parent class `Xtools\Repository` as the method `getPage()` does only exist in the following sub-classes of `Xtools\Repository`: `Xtools\ProjectRepository`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends 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 sub-classes 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 parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
116		->getRepository()
117		->getPage($project, $project->userOptInPage($user));
118
119		return $this->render('topedits/result_namespace.html.twig', [
120		'xtPage' => 'topedits',
121		'xtTitle' => $user->getUsername(),
122		'project' => $project,
123		'user' => $user,
124		'namespace' => $namespace,
125		'edits' => [],
126		'content_title' => '',
127		'opted_in_page' => $optedInPage,
128		'is_sub_request' => $isSubRequest,
129		]);
130		}
131
132		/**
133		* Max number of rows per namespace to show. `null` here will cause to
134		* use the TopEdits default.
135		* @var int
136		*/
137		$limit = $isSubRequest ? 10 : null;
138
139		$topEdits = new TopEdits($project, $user, $namespace, $limit);
140		$topEditsRepo = new TopEditsRepository();
141		$topEditsRepo->setContainer($this->container);
		1 ignored issue – show Compatibility introduced 2017-10-10 16:55 UTC by Report Bug Copy Issue Report `$this->container` of type `object<Symfony\Component...ion\ContainerInterface>` is not a sub-type of `object<Symfony\Component...ncyInjection\Container>`. It seems like you assume a concrete implementation of the interface `Symfony\Component\Depend...tion\ContainerInterface` to be always present. This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass. Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type. Loading history...
142		$topEdits->setRepository($topEditsRepo);
143
144		return $this->render('topedits/result_namespace.html.twig', [
145		'xtPage' => 'topedits',
146		'xtTitle' => $user->getUsername(),
147		'project' => $project,
148		'user' => $user,
149		'namespace' => $namespace,
150		'te' => $topEdits,
151		'is_sub_request' => $isSubRequest,
152		]);
153		}
154
155		/**
156		* List top edits by this user for a particular page.
157		* @param User $user The user.
158		* @param Project $project The project.
159		* @param int $namespaceId The ID of the namespace of the page.
160		* @param string $pageName The title (without namespace) of the page.
161		* @return RedirectResponse\|\Symfony\Component\HttpFoundation\Response
162		* @codeCoverageIgnore
163		*/
164		protected function singlePageTopEdits(User $user, Project $project, $namespaceId, $pageName)
165		{
166		// Get the full page name (i.e. no namespace prefix if NS 0).
167		$namespaces = $project->getNamespaces();
168		$fullPageName = $namespaceId ? $namespaces[$namespaceId].':'.$pageName : $pageName;
169
170		$page = $this->getAndValidatePage($project, $fullPageName);
		0 ignored issues – show Bug Compatibility introduced 2017-10-12 18:26 UTC by Report Bug Copy Issue Report The expression `$this->getAndValidatePag...roject, $fullPageName);` of type `Symfony\Component\HttpFo...ectResponse\|Xtools\Page` adds the type `Xtools\Page` to the return on line 172 which is incompatible with the return type documented by `AppBundle\Controller\Top...ler::singlePageTopEdits` of type `Symfony\Component\HttpFoundation\Response`. Loading history...
171		if (is_a($page, 'Symfony\Component\HttpFoundation\RedirectResponse')) {
172		return $page;
173		}
174
175		// Get all revisions of this page by this user.
176		$revisionsData = $page->getRevisions($user);
		0 ignored issues – show Bug introduced 2017-10-12 18:26 UTC by Report Bug Copy Issue Report The method `getRevisions` does only exist in `Xtools\Page`, but not in `Symfony\Component\HttpFoundation\RedirectResponse`. It seems like the method you are trying to call exists only in some of the possible types. Let’s take a look at an example: class A { public function foo() { } } class B extends A { public function bar() { } } /** * @param A\|B $x / function someFunction($x) { $x->foo(); // This call is fine as the method exists in A and B. $x->bar(); // This method only exists in B and might cause an error. } Available Fixes Add an additional type-check: /* * @param A\|B $x / function someFunction($x) { $x->foo(); if ($x instanceof B) { $x->bar(); } } Only allow a single type to be passed if the variable comes from a parameter: function someFunction(B $x) { /* ... */ } Loading history...
177
178		// Loop through all revisions and format dates, find totals, etc.
179		$totalAdded = 0;
180		$totalRemoved = 0;
181		$revisions = [];
182		foreach ($revisionsData as $revision) {
183		if ($revision['length_change'] > 0) {
184		$totalAdded += $revision['length_change'];
185		} else {
186		$totalRemoved += $revision['length_change'];
187		}
188		$revisions[] = new Edit($page, $revision);
		0 ignored issues – show Bug introduced 2017-10-12 16:59 UTC by Report Bug Copy Issue Report It seems like `$page` defined by `$this->getAndValidatePag...project, $fullPageName)` on line `170` can also be of type `object<Symfony\Component...ation\RedirectResponse>`; however, `Xtools\Edit::__construct()` does only seem to accept `object<Xtools\Page>`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
189		}
190
191		// Send all to the template.
192		return $this->render('topedits/result_article.html.twig', [
193		'xtPage' => 'topedits',
194		'xtTitle' => $user->getUsername() . ' - ' . $page->getTitle(),
		0 ignored issues – show Bug introduced 2017-10-12 18:26 UTC by Report Bug Copy Issue Report The method `getTitle` does only exist in `Xtools\Page`, but not in `Symfony\Component\HttpFoundation\RedirectResponse`. It seems like the method you are trying to call exists only in some of the possible types. Let’s take a look at an example: class A { public function foo() { } } class B extends A { public function bar() { } } /** * @param A\|B $x / function someFunction($x) { $x->foo(); // This call is fine as the method exists in A and B. $x->bar(); // This method only exists in B and might cause an error. } Available Fixes Add an additional type-check: /* * @param A\|B $x / function someFunction($x) { $x->foo(); if ($x instanceof B) { $x->bar(); } } Only allow a single type to be passed if the variable comes from a parameter: function someFunction(B $x) { /* ... */ } Loading history...
195		'project' => $project,
196		'user' => $user,
197		'page' => $page,
198		'total_added' => $totalAdded,
199		'total_removed' => $totalRemoved,
200		'revisions' => $revisions,
201		'revision_count' => count($revisions),
202		]);
203		}
204		}
205

x-tools / xtools

Pull Request — master (#111)

TopEditsController A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

5 Methods

How to fix Duplicated Code

Duplicated Code

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like