Issues in ExternalFiles.php - Fixed Issues - Inspection of "Merge pull request #11088 from mrclay/5790_invalid..." - Elgg/Elgg - Measure and Improve Code Quality continuously with Scrutinizer

Test Failed

Push — master ( 8c47c2...3acf9f )

by Steve

created 2017-08-07 13:14 UTC

engine/classes/Elgg/Assets/ExternalFiles.php (5 issues)

Labels

Coding Style 5

Severity

Minor 5

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 Elgg\Assets;


/**
 * WARNING: API IN FLUX. DO NOT USE DIRECTLY.
 *
 * @access private
 *
 * @package    Elgg.Core
 * @subpackage Assets
 * @since      1.10.0
 */
class ExternalFiles {
	/**
	 * Global Elgg configuration
	 *
	 * @var \stdClass
	 */
	private $CONFIG;

	/**
	 * Constructor
	 *
	 * @param \stdClass $config Predefined configuration in format used by global $CONFIG variable
	 */
	public function __construct(\stdClass $config = null) {
		if (!($config instanceof \stdClass)) {
			$config = new \stdClass();
		}
		$this->CONFIG = $config;
	}
	
	/**
	 * Core registration function for external files
	 *
	 * @param string $type     Type of external resource (js or css)
	 * @param string $name     Identifier used as key
	 * @param string $url      URL
	 * @param string $location Location in the page to include the file
	 * @param int    $priority Loading priority of the file
	 *
	 * @return bool
	 */
	public function register($type, $name, $url, $location, $priority = 500) {
// 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);
    }
}
		
	
		if (empty($name) || empty($url)) {
			return false;
		}
	
		$url = elgg_normalize_url($url);

		$this->bootstrap($type);
	
		$name = trim(strtolower($name));
	
		// normalize bogus priorities, but allow empty, null, and false to be defaults.
		if (!is_numeric($priority)) {
			$priority = 500;
		}
	
		// no negative priorities right now.
		$priority = max((int) $priority, 0);
	
		$item = elgg_extract($name, $GLOBALS['_ELGG']->externals_map[$type]);
	
		if ($item) {
			// updating a registered item
			// don't update loaded because it could already be set
			$item->url = $url;
			$item->location = $location;
	
			// if loaded before registered, that means it hasn't been added to the list yet
			if ($GLOBALS['_ELGG']->externals[$type]->contains($item)) {
				$priority = $GLOBALS['_ELGG']->externals[$type]->move($item, $priority);
			} else {
				$priority = $GLOBALS['_ELGG']->externals[$type]->add($item, $priority);
			}
		} else {
			$item = new \stdClass();
			$item->loaded = false;
			$item->url = $url;
			$item->location = $location;
	
			$priority = $GLOBALS['_ELGG']->externals[$type]->add($item, $priority);
		}

		$GLOBALS['_ELGG']->externals_map[$type][$name] = $item;
	
		return $priority !== false;
	}
	
	/**
	 * Unregister an external file
	 *
	 * @param string $type Type of file: js or css
	 * @param string $name The identifier of the file
	 *
	 * @return bool
	 */
	public function unregister($type, $name) {
// 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);
    }
}

		$this->bootstrap($type);
	
		$name = trim(strtolower($name));
		$item = elgg_extract($name, $GLOBALS['_ELGG']->externals_map[$type]);
	
		if ($item) {
			unset($GLOBALS['_ELGG']->externals_map[$type][$name]);
			return $GLOBALS['_ELGG']->externals[$type]->remove($item);
		}
	
		return false;
	}
	
	/**
	 * Load an external resource for use on this page
	 *
	 * @param string $type Type of file: js or css
	 * @param string $name The identifier for the file
	 *
	 * @return void
	 */
	public function load($type, $name) {
// 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);
    }
}
		
	
		$this->bootstrap($type);
	
		$name = trim(strtolower($name));
	
		$item = elgg_extract($name, $GLOBALS['_ELGG']->externals_map[$type]);
	
		if ($item) {
			// update a registered item
			$item->loaded = true;
		} else {
			$item = new \stdClass();
			$item->loaded = true;
			$item->url = '';
			$item->location = '';
			
			if (elgg_view_exists($name)) {
				$item->url = elgg_get_simplecache_url($name);
				$item->location = ($type == 'js') ? 'foot' : 'head';
			}

			$GLOBALS['_ELGG']->externals[$type]->add($item);
			$GLOBALS['_ELGG']->externals_map[$type][$name] = $item;
		}
	}
	
	/**
	 * Get external resource descriptors
	 *
	 * @param string $type     Type of file: js or css
	 * @param string $location Page location
	 *
	 * @return array
	 */
	public function getLoadedFiles($type, $location) {
// 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);
    }
}
		
	
		if (
			isset($GLOBALS['_ELGG']->externals)
			&& isset($GLOBALS['_ELGG']->externals[$type])
			&& $GLOBALS['_ELGG']->externals[$type] instanceof \ElggPriorityList
		) {
			$items = $GLOBALS['_ELGG']->externals[$type]->getElements();
	
			$items = array_filter($items, function($v) use ($location) {
				return $v->loaded == true && $v->location == $location;
			});
			if ($items) {
				array_walk($items, function(&$v, $k){
					$v = $v->url;
				});
			}
			return $items;
		}
		return [];
	}
	
	/**
	 * Bootstraps the externals data structure in $_ELGG.
	 *
	 * @param string $type The type of external, js or css.
	 * @return null
	 */
	protected function bootstrap($type) {
// 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);
    }
}

		if (!isset($GLOBALS['_ELGG']->externals)) {
			$GLOBALS['_ELGG']->externals = [];
		}
	
		if (!isset($GLOBALS['_ELGG']->externals[$type]) || !$GLOBALS['_ELGG']->externals[$type] instanceof \ElggPriorityList) {
			$GLOBALS['_ELGG']->externals[$type] = new \ElggPriorityList();
		}
	
		if (!isset($GLOBALS['_ELGG']->externals_map)) {
			$GLOBALS['_ELGG']->externals_map = [];
		}
	
		if (!isset($GLOBALS['_ELGG']->externals_map[$type])) {
			$GLOBALS['_ELGG']->externals_map[$type] = [];
		}
	}
}


1		<?php
2		namespace Elgg\Assets;
3
4
5		/**
6		* WARNING: API IN FLUX. DO NOT USE DIRECTLY.
7		*
8		* @access private
9		*
10		* @package Elgg.Core
11		* @subpackage Assets
12		* @since 1.10.0
13		*/
14		class ExternalFiles {
15		/**
16		* Global Elgg configuration
17		*
18		* @var \stdClass
19		*/
20		private $CONFIG;
21
22		/**
23		* Constructor
24		*
25		* @param \stdClass $config Predefined configuration in format used by global $CONFIG variable
26		*/
27	3	public function __construct(\stdClass $config = null) {
28	3	if (!($config instanceof \stdClass)) {
29	1	$config = new \stdClass();
30		}
31	3	$this->CONFIG = $config;
32	3	}
33
34		/**
35		* Core registration function for external files
36		*
37		* @param string $type Type of external resource (js or css)
38		* @param string $name Identifier used as key
39		* @param string $url URL
40		* @param string $location Location in the page to include the file
41		* @param int $priority Loading priority of the file
42		*
43		* @return bool
44		*/
45	1	public function register($type, $name, $url, $location, $priority = 500) {
		0 ignored issues – show Coding Style introduced 2017-07-31 10:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `register` uses the super-global variable `$GLOBALS` 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...
46
47
48	1	if (empty($name) \|\| empty($url)) {
49	1	return false;
50		}
51
52	1	$url = elgg_normalize_url($url);
53
54	1	$this->bootstrap($type);
55
56	1	$name = trim(strtolower($name));
57
58		// normalize bogus priorities, but allow empty, null, and false to be defaults.
59	1	if (!is_numeric($priority)) {
60	1	$priority = 500;
61		}
62
63		// no negative priorities right now.
64	1	$priority = max((int) $priority, 0);
65
66	1	$item = elgg_extract($name, $GLOBALS['_ELGG']->externals_map[$type]);
67
68	1	if ($item) {
69		// updating a registered item
70		// don't update loaded because it could already be set
71	1	$item->url = $url;
72	1	$item->location = $location;
73
74		// if loaded before registered, that means it hasn't been added to the list yet
75	1	if ($GLOBALS['_ELGG']->externals[$type]->contains($item)) {
76	1	$priority = $GLOBALS['_ELGG']->externals[$type]->move($item, $priority);
77		} else {
78	1	$priority = $GLOBALS['_ELGG']->externals[$type]->add($item, $priority);
79		}
80		} else {
81	1	$item = new \stdClass();
82	1	$item->loaded = false;
83	1	$item->url = $url;
84	1	$item->location = $location;
85
86	1	$priority = $GLOBALS['_ELGG']->externals[$type]->add($item, $priority);
87		}
88
89	1	$GLOBALS['_ELGG']->externals_map[$type][$name] = $item;
90
91	1	return $priority !== false;
92		}
93
94		/**
95		* Unregister an external file
96		*
97		* @param string $type Type of file: js or css
98		* @param string $name The identifier of the file
99		*
100		* @return bool
101		*/
102	1	public function unregister($type, $name) {
		0 ignored issues – show Coding Style introduced 2017-07-31 10:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `unregister` uses the super-global variable `$GLOBALS` 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...
103
104	1	$this->bootstrap($type);
105
106	1	$name = trim(strtolower($name));
107	1	$item = elgg_extract($name, $GLOBALS['_ELGG']->externals_map[$type]);
108
109	1	if ($item) {
110	1	unset($GLOBALS['_ELGG']->externals_map[$type][$name]);
111	1	return $GLOBALS['_ELGG']->externals[$type]->remove($item);
112		}
113
114	1	return false;
115		}
116
117		/**
118		* Load an external resource for use on this page
119		*
120		* @param string $type Type of file: js or css
121		* @param string $name The identifier for the file
122		*
123		* @return void
124		*/
125	2	public function load($type, $name) {
		0 ignored issues – show Coding Style introduced 2017-07-31 10:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `load` uses the super-global variable `$GLOBALS` 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...
126
127
128	2	$this->bootstrap($type);
129
130	2	$name = trim(strtolower($name));
131
132	2	$item = elgg_extract($name, $GLOBALS['_ELGG']->externals_map[$type]);
133
134	2	if ($item) {
135		// update a registered item
136	2	$item->loaded = true;
137		} else {
138	1	$item = new \stdClass();
139	1	$item->loaded = true;
140	1	$item->url = '';
141	1	$item->location = '';
142
143	1	if (elgg_view_exists($name)) {
144		$item->url = elgg_get_simplecache_url($name);
145		$item->location = ($type == 'js') ? 'foot' : 'head';
146		}
147
148	1	$GLOBALS['_ELGG']->externals[$type]->add($item);
149	1	$GLOBALS['_ELGG']->externals_map[$type][$name] = $item;
150		}
151	2	}
152
153		/**
154		* Get external resource descriptors
155		*
156		* @param string $type Type of file: js or css
157		* @param string $location Page location
158		*
159		* @return array
160		*/
161	2	public function getLoadedFiles($type, $location) {
		0 ignored issues – show Coding Style introduced 2017-07-31 10:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `getLoadedFiles` uses the super-global variable `$GLOBALS` 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...
162
163
164		if (
165	2	isset($GLOBALS['_ELGG']->externals)
166	2	&& isset($GLOBALS['_ELGG']->externals[$type])
167	2	&& $GLOBALS['_ELGG']->externals[$type] instanceof \ElggPriorityList
168		) {
169	2	$items = $GLOBALS['_ELGG']->externals[$type]->getElements();
170
171		$items = array_filter($items, function($v) use ($location) {
172	2	return $v->loaded == true && $v->location == $location;
173	2	});
174	2	if ($items) {
175	2	array_walk($items, function(&$v, $k){
176	2	$v = $v->url;
177	2	});
178		}
179	2	return $items;
180		}
181	1	return [];
182		}
183
184		/**
185		* Bootstraps the externals data structure in $_ELGG.
186		*
187		* @param string $type The type of external, js or css.
188		* @return null
189		*/
190	2	protected function bootstrap($type) {
		0 ignored issues – show Coding Style introduced 2017-07-31 10:07 UTC by Report Bug Copy Issue Report Show Similar Issues like this `bootstrap` uses the super-global variable `$GLOBALS` 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...
191
192	2	if (!isset($GLOBALS['_ELGG']->externals)) {
193	1	$GLOBALS['_ELGG']->externals = [];
194		}
195
196	2	if (!isset($GLOBALS['_ELGG']->externals[$type]) \|\| !$GLOBALS['_ELGG']->externals[$type] instanceof \ElggPriorityList) {
197	1	$GLOBALS['_ELGG']->externals[$type] = new \ElggPriorityList();
198		}
199
200	2	if (!isset($GLOBALS['_ELGG']->externals_map)) {
201	1	$GLOBALS['_ELGG']->externals_map = [];
202		}
203
204	2	if (!isset($GLOBALS['_ELGG']->externals_map[$type])) {
205	1	$GLOBALS['_ELGG']->externals_map[$type] = [];
206		}
207	2	}
208		}
209

Elgg / Elgg

Push — master ( 8c47c2...3acf9f )

engine/classes/Elgg/Assets/ExternalFiles.php (5 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like