CMSEditLinkAPI - Code Metrics - Inspection of "MAJOR: adding sanitize_class_name of ModelName for..." - sunnysideup/silverstripe-cms_edit_link_field - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( d66b65...126a7d )

by Nicolaas

created 2017-03-30 03:33 UTC

CMSEditLinkAPI A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	117
Duplicated Lines	0 %

Coupling/Cohesion

Components	0
Dependencies	9

Importance

Changes

Metric	Value
wmc	21
lcom	0
cbo	9
dl	0
loc	117
rs	10
c	0
b	0
f	0

2 Methods

Rating	Name	Duplication	Size	Complexity
D	find_edit_link_for_object()	0	88	20
A	sanitize_class_name()	0	3	1

<?php


class CMSEditLinkAPI extends Object
namespace YourVendor;

class YourClass { }
{

    private static $_cache = array();


    /**
     * common usage ...
     *
     *         public function CMSEditLink($action = null)
     *         {
     *             return CMSEditLinkAPI::find_edit_link_for_object($this, $action);
     *         };
     *
     * @param  DataObject | string $objectOrClassName
     * @param  string              $action

     *
     * @return string

     */
    public static function find_edit_link_for_object($objectOrClassName, $action = null, $modelAdminURLOverwrite = '')
    {
        if(is_string($objectOrClassName)) {
            $modelNameToEdit = $objectOrClassName;
            $objectToEdit = Injector::inst()->get($modelNameToEdit);
            $id = 0;
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
        } else {
            $modelNameToEdit = $objectOrClassName->ClassName;
            $objectToEdit = $objectOrClassName;
        }
        if($objectToEdit instanceof DataObject) {
            if($objectToEdit->exists()) {
                $id = $objectOrClassName->ID;
            } else {
                $id = 0;
            }
        } else {
            user_error('$objectOrClassName is not set correctly.', E_USER_NOTICE);
            return;
        }
        if($objectToEdit instanceof Member) {
            return Controller::join_links(
                Director::baseURL(),
                '/admin/security/EditForm/field/Members/item/'.$objectToEdit->ID.'/edit/'
            );
        }
        if($objectToEdit instanceof Group) {
            return Controller::join_links(
                Director::baseURL(),
                '/admin/security/EditForm/field/Groups/item/'.$objectToEdit->ID.'/edit/'
            );
        }
        if($modelAdminURLOverwrite) {
            $classFound = true;
        } else {
            $cachekey = $modelNameToEdit.'_'.$action;
            $cache = SS_Cache::factory('cms_edit_link_cache');
            $myAdminClassName = $cache->load($cachekey);
            if ($myAdminClassName) {
                $myModelAdminclassObject = Injector::inst()->get($myAdminClassName);
                if($myModelAdminclassObject instanceof ModelAdmin) {
                    $classFound = true;
                }
            }
            else {
                $classFound = false;
                foreach(ClassInfo::subclassesFor('ModelAdmin') as $i => $myAdminClassName) {
$collection = json_decode($data, true);
if ( ! is_array($collection)) {
    throw new \RuntimeException('$collection must be an array.');
}

foreach ($collection as $item) { /** ... */ }
                    if($myAdminClassName == 'ModelAdmin') {continue;}
                    if(ClassInfo::classImplements($myAdminClassName, 'TestOnly')) {continue;}
                    $myModelAdminclassObject = Injector::inst()->get($myAdminClassName);
                    $models = $myModelAdminclassObject->getManagedModels();
                    foreach($models as $key => $model) {
                        if($key === $modelNameToEdit || (is_string($model) && $model === $modelNameToEdit)) {
                            $classFound = true;
                            $cache->save($myAdminClassName, $cachekey);

                            break 2;
                        }
                    }
                }
            }
        }
        if($classFound) {
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
            $modelNameToEdit = $this->sanitiseClassName($modelNameToEdit);
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
            if($id === 0) {
                $id = 'new';
            }
            if(!$action) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                $action = $modelNameToEdit.'/EditForm/field/'.$modelNameToEdit.'/item/'.$id.'/';
            }
            if($modelAdminURLOverwrite) {
                $link = '/admin/'.$modelAdminURLOverwrite.'/'.$action;
            } else {
                $link = $myModelAdminclassObject->Link($action);
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
            }
            return Controller::join_links(
                Director::baseURL(),
                $link
            );
        }
        else {
            return Controller::join_links(
                Director::baseURL(),
                'admin/not-found'
            );
        }

    }


    /**
     * Sanitise a model class' name for inclusion in a link
     * @return string
     */
    protected static function sanitize_class_name($className) {
        return str_replace('\\', '-', $className);
    }

}


1			<?php
2
3
4			class CMSEditLinkAPI extends Object
			0 ignored issues – show Coding Style Compatibility introduced 2017-03-15 02:19 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...
5			{
6
7			private static $_cache = array();
			0 ignored issues – show Unused Code introduced 2017-03-21 02:46 UTC by Report Bug Copy Issue Report The property `$_cache` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
8
9			/**
10			* common usage ...
11			*
12			* public function CMSEditLink($action = null)
13			* {
14			* return CMSEditLinkAPI::find_edit_link_for_object($this, $action);
15			* };
16			*
17			* @param DataObject \| string $objectOrClassName
18			* @param string $action
			0 ignored issues – show Documentation introduced 2017-03-15 02:19 UTC by Report Bug Copy Issue Report Should the type for parameter `$action` not be `string\|null`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
19			*
20			* @return string
			0 ignored issues – show Documentation introduced 2017-03-21 02:46 UTC by Report Bug Copy Issue Report Should the return type not be `null\|string`? 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...
21			*/
22			public static function find_edit_link_for_object($objectOrClassName, $action = null, $modelAdminURLOverwrite = '')
23			{
24			if(is_string($objectOrClassName)) {
25			$modelNameToEdit = $objectOrClassName;
26			$objectToEdit = Injector::inst()->get($modelNameToEdit);
27			$id = 0;
			0 ignored issues – show Unused Code introduced 2017-03-21 02:46 UTC by Report Bug Copy Issue Report `$id` 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...
28			} else {
29			$modelNameToEdit = $objectOrClassName->ClassName;
30			$objectToEdit = $objectOrClassName;
31			}
32			if($objectToEdit instanceof DataObject) {
33			if($objectToEdit->exists()) {
34			$id = $objectOrClassName->ID;
35			} else {
36			$id = 0;
37			}
38			} else {
39			user_error('$objectOrClassName is not set correctly.', E_USER_NOTICE);
40			return;
41			}
42			if($objectToEdit instanceof Member) {
43			return Controller::join_links(
44			Director::baseURL(),
45			'/admin/security/EditForm/field/Members/item/'.$objectToEdit->ID.'/edit/'
46			);
47			}
48			if($objectToEdit instanceof Group) {
49			return Controller::join_links(
50			Director::baseURL(),
51			'/admin/security/EditForm/field/Groups/item/'.$objectToEdit->ID.'/edit/'
52			);
53			}
54			if($modelAdminURLOverwrite) {
55			$classFound = true;
56			} else {
57			$cachekey = $modelNameToEdit.'_'.$action;
58			$cache = SS_Cache::factory('cms_edit_link_cache');
59			$myAdminClassName = $cache->load($cachekey);
60			if ($myAdminClassName) {
61			$myModelAdminclassObject = Injector::inst()->get($myAdminClassName);
62			if($myModelAdminclassObject instanceof ModelAdmin) {
63			$classFound = true;
64			}
65			}
66			else {
67			$classFound = false;
68			foreach(ClassInfo::subclassesFor('ModelAdmin') as $i => $myAdminClassName) {
			0 ignored issues – show Bug introduced 2017-03-15 02:19 UTC by Report Bug Copy Issue Report The expression `\ClassInfo::subclassesFor('ModelAdmin')` of type `null\|array` is not guaranteed to be traversable. How about adding an additional type check? There are different options of fixing this problem. If you want to be on the safe side, you can add an additional type-check: $collection = json_decode($data, true); if ( ! is_array($collection)) { throw new \RuntimeException('$collection must be an array.'); } foreach ($collection as $item) { /** ... / } If you are sure that the expression is traversable, you might want to add a doc comment cast* to improve IDE auto-completion and static analysis: /** @var array $collection / $collection = json_decode($data, true); foreach ($collection as $item) { /* .. / } Mark the issue as a false-positive*: Just hover the remove button, in the top-right corner of this issue for more options. Loading history...
69			if($myAdminClassName == 'ModelAdmin') {continue;}
70			if(ClassInfo::classImplements($myAdminClassName, 'TestOnly')) {continue;}
71			$myModelAdminclassObject = Injector::inst()->get($myAdminClassName);
72			$models = $myModelAdminclassObject->getManagedModels();
73			foreach($models as $key => $model) {
74			if($key === $modelNameToEdit \|\| (is_string($model) && $model === $modelNameToEdit)) {
75			$classFound = true;
76			$cache->save($myAdminClassName, $cachekey);
77
78			break 2;
79			}
80			}
81			}
82			}
83			}
84			if($classFound) {
			0 ignored issues – show Bug introduced 2017-03-21 02:46 UTC by Report Bug Copy Issue Report The variable `$classFound` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
85			$modelNameToEdit = $this->sanitiseClassName($modelNameToEdit);
			0 ignored issues – show Bug introduced 2017-03-30 03:34 UTC by Report Bug Copy Issue Report The variable `$this` does not exist. Did you forget to declare it? This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug. Loading history... Documentation Bug introduced 2017-03-30 03:34 UTC by Report Bug Copy Issue Report The method `sanitiseClassName` does not exist on `object<CMSEditLinkAPI>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
86			if($id === 0) {
87			$id = 'new';
88			}
89			if(!$action) {
			0 ignored issues – show Bug Best Practice introduced 2017-03-15 02:19 UTC by Report Bug Copy Issue Report The expression `$action` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
90			$action = $modelNameToEdit.'/EditForm/field/'.$modelNameToEdit.'/item/'.$id.'/';
91			}
92			if($modelAdminURLOverwrite) {
93			$link = '/admin/'.$modelAdminURLOverwrite.'/'.$action;
94			} else {
95			$link = $myModelAdminclassObject->Link($action);
			0 ignored issues – show Bug introduced 2017-03-21 02:46 UTC by Report Bug Copy Issue Report The variable `$myModelAdminclassObject` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
96			}
97			return Controller::join_links(
98			Director::baseURL(),
99			$link
100			);
101			}
102			else {
103			return Controller::join_links(
104			Director::baseURL(),
105			'admin/not-found'
106			);
107			}
108
109			}
110
111
112			/**
113			* Sanitise a model class' name for inclusion in a link
114			* @return string
115			*/
116			protected static function sanitize_class_name($className) {
117			return str_replace('\\', '-', $className);
118			}
119
120			}
121

sunnysideup / silverstripe-cms_edit_link_field

Push — master ( d66b65...126a7d )

CMSEditLinkAPI A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

2 Methods

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like