ResetPasswordCommand::sendResetPasswordEmail() - Code Metrics - Inspection of "$php supersake security:resetpassword email-or-id" - axyr/silverstripe-console - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( d2af9d...ff4bf4 )

by Martijn van

created 2016-04-18 21:23 UTC

ResetPasswordCommand::sendResetPasswordEmail() A

↳ Parent: ResetPasswordCommand

Complexity

Conditions	2
Paths	2

Size

Total Lines	23
Code Lines	13

Duplication

Lines	0
Ratio	0 %

Importance

Changes	1
Bugs	0	Features	0

Metric	Value
c	1
b	0
f	0
dl	0
loc	23
rs	9.0856
cc	2
eloc	13
nc	2
nop	1

<?php

use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputOption;

class ResetPasswordCommand extends SilverstripeCommand
namespace YourVendor;

class YourClass { }
{
    /**
     * @var string
     */
    protected $name = 'security:resetpassword';

    /**
     * @var string
     */
    protected $description = 'Send a reset password link to an email address';

    public function fire()
    {
        $member = $this->getMemberByEmailOrID();

        if(!(bool)$member) {
            $this->error('Member not found');
        } else {
            $link = $this->sendResetPasswordEmail($member);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            $this->info('Email sent to '.$member->getName().'<'.$member->Email.'>');
            $this->line($link);
        }
    }

    /**
     * @return Member|null

     */
    protected function getMemberByEmailOrID()
    {
        $emailorid = $this->argument('emailorid');

        $member = null;
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

        if(Str::contains($emailorid, '@')) {
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
            $member = Member::get()->where("Email = '".Convert::raw2sql($emailorid)."'")->first();
        }else{
            $member = Member::get()->byID($emailorid);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        }

        return $member;
    }

    /**
     * Send the reset password email and return the generated link.
     *
     * @param Member $member
     * @return string
     */
    protected function sendResetPasswordEmail(Member $member)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        // hack ?
        global $_FILE_TO_URL_MAPPING;
function myFunction($a, $b) {
    // Do something
}

        if($_FILE_TO_URL_MAPPING[BASE_PATH]) {
            $_SERVER['REQUEST_URI'] = $_FILE_TO_URL_MAPPING[BASE_PATH];
        }

        $token = $member->generateAutologinTokenAndStoreHash();
        $link  = Security::getPasswordResetLink($member, $token);

        /* @var Member_ForgotPasswordEmail $email */
        $email = Member_ForgotPasswordEmail::create();
        $email->populateTemplate($member);
        $email->populateTemplate(array(
            'PasswordResetLink' => $link
        ));
        $email->setTo($member->Email);
        $email->send();

        return $link;
    }

    /**
     * @return array
     */
    protected function getArguments()
    {
        return [
            ['emailorid', InputArgument::REQUIRED, 'The emailaddress or ID of the Member']
        ];
    }
}


1			<?php
2
3			use Symfony\Component\Console\Input\InputArgument;
4			use Symfony\Component\Console\Input\InputOption;
5
6			class ResetPasswordCommand extends SilverstripeCommand
			0 ignored issues – show Coding Style Compatibility introduced 2016-04-17 01:01 UTC by Report Bug Copy Issue Report PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
7			{
8			/**
9			* @var string
10			*/
11			protected $name = 'security:resetpassword';
12
13			/**
14			* @var string
15			*/
16			protected $description = 'Send a reset password link to an email address';
17
18			public function fire()
19			{
20			$member = $this->getMemberByEmailOrID();
21
22			if(!(bool)$member) {
23			$this->error('Member not found');
24			} else {
25			$link = $this->sendResetPasswordEmail($member);
			0 ignored issues – show Documentation introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report `$member` is of type `object<DataObject>\|null`, but the function expects a `object<Member>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
26			$this->info('Email sent to '.$member->getName().'<'.$member->Email.'>');
27			$this->line($link);
28			}
29			}
30
31			/**
32			* @return Member\|null
			0 ignored issues – show Documentation introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report Should the return type not be `DataObject\|null`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
33			*/
34			protected function getMemberByEmailOrID()
35			{
36			$emailorid = $this->argument('emailorid');
37
38			$member = null;
			0 ignored issues – show Unused Code introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report `$member` is not used, you could remove the assignment. This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value'; $higher = false; if (rand(1, 6) > 3) { $higher = true; } else { $higher = false; } Both the `$myVar` assignment in line 1 and the `$higher` assignment in line 2 are dead. The first because `$myVar` is never used and the second because `$higher` is always overwritten for every possible time line. Loading history...
39
40			if(Str::contains($emailorid, '@')) {
			0 ignored issues – show Bug introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report It seems like `$emailorid` defined by `$this->argument('emailorid')` on line `36` can also be of type `array`; however, `Str::contains()` does only seem to accept `string`, 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...
41			$member = Member::get()->where("Email = '".Convert::raw2sql($emailorid)."'")->first();
42			}else{
43			$member = Member::get()->byID($emailorid);
			0 ignored issues – show Documentation introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report `$emailorid` is of type `string\|array`, but the function expects a `integer`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
44			}
45
46			return $member;
47			}
48
49			/**
50			* Send the reset password email and return the generated link.
51			*
52			* @param Member $member
53			* @return string
54			*/
55			protected function sendResetPasswordEmail(Member $member)
			0 ignored issues – show Coding Style introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report `sendResetPasswordEmail` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
56			{
57			// hack ?
58			global $_FILE_TO_URL_MAPPING;
			0 ignored issues – show Compatibility Best Practice introduced 2016-04-18 21:25 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
59
60			if($_FILE_TO_URL_MAPPING[BASE_PATH]) {
61			$_SERVER['REQUEST_URI'] = $_FILE_TO_URL_MAPPING[BASE_PATH];
62			}
63
64			$token = $member->generateAutologinTokenAndStoreHash();
65			$link = Security::getPasswordResetLink($member, $token);
66
67			/* @var Member_ForgotPasswordEmail $email */
68			$email = Member_ForgotPasswordEmail::create();
69			$email->populateTemplate($member);
70			$email->populateTemplate(array(
71			'PasswordResetLink' => $link
72			));
73			$email->setTo($member->Email);
74			$email->send();
75
76			return $link;
77			}
78
79			/**
80			* @return array
81			*/
82			protected function getArguments()
83			{
84			return [
85			['emailorid', InputArgument::REQUIRED, 'The emailaddress or ID of the Member']
86			];
87			}
88			}
89

axyr / silverstripe-console

Push — master ( d2af9d...ff4bf4 )

ResetPasswordCommand::sendResetPasswordEmail() A

Complexity

Size

Duplication

Importance

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like