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)

Showing only issues like (Show All)

super-globals are not used.

Coding Style Minor

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)

Showing only issues like (Show All)

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like