Complex classes like ModuleScanner often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use ModuleScanner, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 41 | class ModuleScanner{ |
||
| 42 | private $manifestMap = array( |
||
| 43 | 'pre_execute'=>'pre_execute', |
||
| 44 | 'install_mkdirs'=>'mkdir', |
||
| 45 | 'install_copy'=>'copy', |
||
| 46 | 'install_images'=>'image_dir', |
||
| 47 | 'install_menus'=>'menu', |
||
| 48 | 'install_userpage'=>'user_page', |
||
| 49 | 'install_dashlets'=>'dashlets', |
||
| 50 | 'install_administration'=>'administration', |
||
| 51 | 'install_connectors'=>'connectors', |
||
| 52 | 'install_vardefs'=>'vardefs', |
||
| 53 | 'install_layoutdefs'=>'layoutdefs', |
||
| 54 | 'install_layoutfields'=>'layoutfields', |
||
| 55 | 'install_relationships'=>'relationships', |
||
| 56 | 'install_languages'=>'language', |
||
| 57 | 'install_logichooks'=>'logic_hooks', |
||
| 58 | 'post_execute'=>'post_execute', |
||
| 59 | |||
| 60 | ); |
||
| 61 | |||
| 62 | /** |
||
| 63 | * config settings |
||
| 64 | * @var array |
||
| 65 | */ |
||
| 66 | private $config = array(); |
||
| 67 | private $config_hash; |
||
| 68 | |||
| 69 | private $blackListExempt = array(); |
||
| 70 | private $classBlackListExempt = array(); |
||
| 71 | |||
| 72 | // Bug 56717 - adding hbs extension to the whitelist - rgonzalez |
||
| 73 | private $validExt = array('png', 'gif', 'jpg', 'css', 'js', 'php', 'txt', 'html', 'htm', 'tpl', 'pdf', 'md5', 'xml', 'hbs'); |
||
| 74 | private $classBlackList = array( |
||
| 75 | // Class names specified here must be in lowercase as the implementation |
||
| 76 | // of the tokenizer converts all tokens to lowercase. |
||
| 77 | 'reflection', |
||
| 78 | 'reflectionclass', |
||
| 79 | 'reflectionzendextension', |
||
| 80 | 'reflectionextension', |
||
| 81 | 'reflectionfunction', |
||
| 82 | 'reflectionfunctionabstract', |
||
| 83 | 'reflectionmethod', |
||
| 84 | 'reflectionobject', |
||
| 85 | 'reflectionparameter', |
||
| 86 | 'reflectionproperty', |
||
| 87 | 'reflector', |
||
| 88 | 'reflectionexception', |
||
| 89 | 'lua', |
||
| 90 | 'ziparchive', |
||
| 91 | 'splfileinfo', |
||
| 92 | 'splfileobject', |
||
| 93 | 'pclzip', |
||
| 94 | |||
| 95 | ); |
||
| 96 | private $blackList = array( |
||
| 97 | 'popen', |
||
| 98 | 'proc_open', |
||
| 99 | 'escapeshellarg', |
||
| 100 | 'escapeshellcmd', |
||
| 101 | 'proc_close', |
||
| 102 | 'proc_get_status', |
||
| 103 | 'proc_nice', |
||
| 104 | 'passthru', |
||
| 105 | 'clearstatcache', |
||
| 106 | 'disk_free_space', |
||
| 107 | 'disk_total_space', |
||
| 108 | 'diskfreespace', |
||
| 109 | 'dir', |
||
| 110 | 'fclose', |
||
| 111 | 'feof', |
||
| 112 | 'fflush', |
||
| 113 | 'fgetc', |
||
| 114 | 'fgetcsv', |
||
| 115 | 'fgets', |
||
| 116 | 'fgetss', |
||
| 117 | 'file_exists', |
||
| 118 | 'file_get_contents', |
||
| 119 | 'filesize', |
||
| 120 | 'filetype', |
||
| 121 | 'flock', |
||
| 122 | 'fnmatch', |
||
| 123 | 'fpassthru', |
||
| 124 | 'fputcsv', |
||
| 125 | 'fputs', |
||
| 126 | 'fread', |
||
| 127 | 'fscanf', |
||
| 128 | 'fseek', |
||
| 129 | 'fstat', |
||
| 130 | 'ftell', |
||
| 131 | 'ftruncate', |
||
| 132 | 'fwrite', |
||
| 133 | 'glob', |
||
| 134 | 'is_dir', |
||
| 135 | 'is_file', |
||
| 136 | 'is_link', |
||
| 137 | 'is_readable', |
||
| 138 | 'is_uploaded_file', |
||
| 139 | 'opendir', |
||
| 140 | 'parse_ini_string', |
||
| 141 | 'pathinfo', |
||
| 142 | 'pclose', |
||
| 143 | 'readfile', |
||
| 144 | 'readlink', |
||
| 145 | 'realpath_cache_get', |
||
| 146 | 'realpath_cache_size', |
||
| 147 | 'realpath', |
||
| 148 | 'rewind', |
||
| 149 | 'readdir', |
||
| 150 | 'set_file_buffer', |
||
| 151 | 'tmpfile', |
||
| 152 | 'umask', |
||
| 153 | 'ini_set', |
||
| 154 | 'set_time_limit', |
||
| 155 | 'eval', |
||
| 156 | 'exec', |
||
| 157 | 'system', |
||
| 158 | 'shell_exec', |
||
| 159 | 'passthru', |
||
| 160 | 'chgrp', |
||
| 161 | 'chmod', |
||
| 162 | 'chwown', |
||
| 163 | 'file_put_contents', |
||
| 164 | 'file', |
||
| 165 | 'fileatime', |
||
| 166 | 'filectime', |
||
| 167 | 'filegroup', |
||
| 168 | 'fileinode', |
||
| 169 | 'filemtime', |
||
| 170 | 'fileowner', |
||
| 171 | 'fileperms', |
||
| 172 | 'fopen', |
||
| 173 | 'is_executable', |
||
| 174 | 'is_writable', |
||
| 175 | 'is_writeable', |
||
| 176 | 'lchgrp', |
||
| 177 | 'lchown', |
||
| 178 | 'linkinfo', |
||
| 179 | 'lstat', |
||
| 180 | 'mkdir', |
||
| 181 | 'mkdir_recursive', |
||
| 182 | 'parse_ini_file', |
||
| 183 | 'rmdir', |
||
| 184 | 'rmdir_recursive', |
||
| 185 | 'stat', |
||
| 186 | 'tempnam', |
||
| 187 | 'touch', |
||
| 188 | 'unlink', |
||
| 189 | 'getimagesize', |
||
| 190 | 'call_user_func', |
||
| 191 | 'call_user_func_array', |
||
| 192 | 'create_function', |
||
| 193 | |||
| 194 | |||
| 195 | //mutliple files per function call |
||
| 196 | 'copy', |
||
| 197 | 'copy_recursive', |
||
| 198 | 'link', |
||
| 199 | 'rename', |
||
| 200 | 'symlink', |
||
| 201 | 'move_uploaded_file', |
||
| 202 | 'chdir', |
||
| 203 | 'chroot', |
||
| 204 | 'create_cache_directory', |
||
| 205 | 'mk_temp_dir', |
||
| 206 | 'write_array_to_file', |
||
| 207 | 'write_encoded_file', |
||
| 208 | 'create_custom_directory', |
||
| 209 | 'sugar_rename', |
||
| 210 | 'sugar_chown', |
||
| 211 | 'sugar_fopen', |
||
| 212 | 'sugar_mkdir', |
||
| 213 | 'sugar_file_put_contents', |
||
| 214 | 'sugar_file_put_contents_atomic', |
||
| 215 | 'sugar_chgrp', |
||
| 216 | 'sugar_chmod', |
||
| 217 | 'sugar_touch', |
||
| 218 | |||
| 219 | // Functions that have callbacks can circumvent our security measures. |
||
| 220 | // List retrieved through PHP's XML documentation, and running the |
||
| 221 | // following script in the reference directory: |
||
| 222 | |||
| 223 | // grep -R callable . | grep -v \.svn | grep methodparam | cut -d: -f1 | sort -u | cut -d"." -f2 | sed 's/\-/\_/g' | cut -d"/" -f4 |
||
| 224 | |||
| 225 | // AMQPQueue |
||
| 226 | 'consume', |
||
| 227 | |||
| 228 | // PHP internal - arrays |
||
| 229 | 'array_diff_uassoc', |
||
| 230 | 'array_diff_ukey', |
||
| 231 | 'array_filter', |
||
| 232 | 'array_intersect_uassoc', |
||
| 233 | 'array_intersect_ukey', |
||
| 234 | 'array_map', |
||
| 235 | 'array_reduce', |
||
| 236 | 'array_udiff_assoc', |
||
| 237 | 'array_udiff_uassoc', |
||
| 238 | 'array_udiff', |
||
| 239 | 'array_uintersect_assoc', |
||
| 240 | 'array_uintersect_uassoc', |
||
| 241 | 'array_uintersect', |
||
| 242 | 'array_walk_recursive', |
||
| 243 | 'array_walk', |
||
| 244 | 'uasort', |
||
| 245 | 'uksort', |
||
| 246 | 'usort', |
||
| 247 | |||
| 248 | // EIO functions that accept callbacks. |
||
| 249 | 'eio_busy', |
||
| 250 | 'eio_chmod', |
||
| 251 | 'eio_chown', |
||
| 252 | 'eio_close', |
||
| 253 | 'eio_custom', |
||
| 254 | 'eio_dup2', |
||
| 255 | 'eio_fallocate', |
||
| 256 | 'eio_fchmod', |
||
| 257 | 'eio_fchown', |
||
| 258 | 'eio_fdatasync', |
||
| 259 | 'eio_fstat', |
||
| 260 | 'eio_fstatvfs', |
||
| 261 | 'eio_fsync', |
||
| 262 | 'eio_ftruncate', |
||
| 263 | 'eio_futime', |
||
| 264 | 'eio_grp', |
||
| 265 | 'eio_link', |
||
| 266 | 'eio_lstat', |
||
| 267 | 'eio_mkdir', |
||
| 268 | 'eio_mknod', |
||
| 269 | 'eio_nop', |
||
| 270 | 'eio_open', |
||
| 271 | 'eio_read', |
||
| 272 | 'eio_readahead', |
||
| 273 | 'eio_readdir', |
||
| 274 | 'eio_readlink', |
||
| 275 | 'eio_realpath', |
||
| 276 | 'eio_rename', |
||
| 277 | 'eio_rmdir', |
||
| 278 | 'eio_sendfile', |
||
| 279 | 'eio_stat', |
||
| 280 | 'eio_statvfs', |
||
| 281 | 'eio_symlink', |
||
| 282 | 'eio_sync_file_range', |
||
| 283 | 'eio_sync', |
||
| 284 | 'eio_syncfs', |
||
| 285 | 'eio_truncate', |
||
| 286 | 'eio_unlink', |
||
| 287 | 'eio_utime', |
||
| 288 | 'eio_write', |
||
| 289 | |||
| 290 | // PHP internal - error functions |
||
| 291 | 'set_error_handler', |
||
| 292 | 'set_exception_handler', |
||
| 293 | |||
| 294 | // Forms Data Format functions |
||
| 295 | 'fdf_enum_values', |
||
| 296 | |||
| 297 | // PHP internal - function handling |
||
| 298 | 'call_user_func_array', |
||
| 299 | 'call_user_func', |
||
| 300 | 'forward_static_call_array', |
||
| 301 | 'forward_static_call', |
||
| 302 | 'register_shutdown_function', |
||
| 303 | 'register_tick_function', |
||
| 304 | |||
| 305 | // Gearman |
||
| 306 | 'setclientcallback', |
||
| 307 | 'setcompletecallback', |
||
| 308 | 'setdatacallback', |
||
| 309 | 'setexceptioncallback', |
||
| 310 | 'setfailcallback', |
||
| 311 | 'setstatuscallback', |
||
| 312 | 'setwarningcallback', |
||
| 313 | 'setworkloadcallback', |
||
| 314 | 'addfunction', |
||
| 315 | |||
| 316 | // Firebird/InterBase |
||
| 317 | 'ibase_set_event_handler', |
||
| 318 | |||
| 319 | // LDAP |
||
| 320 | 'ldap_set_rebind_proc', |
||
| 321 | |||
| 322 | // LibXML |
||
| 323 | 'libxml_set_external_entity_loader', |
||
| 324 | |||
| 325 | // Mailparse functions |
||
| 326 | 'mailparse_msg_extract_part_file', |
||
| 327 | 'mailparse_msg_extract_part', |
||
| 328 | 'mailparse_msg_extract_whole_part_file', |
||
| 329 | |||
| 330 | // Memcache(d) functions |
||
| 331 | 'addserver', |
||
| 332 | 'setserverparams', |
||
| 333 | 'get', |
||
| 334 | 'getbykey', |
||
| 335 | 'getdelayed', |
||
| 336 | 'getdelayedbykey', |
||
| 337 | |||
| 338 | // MySQLi |
||
| 339 | 'set_local_infile_handler', |
||
| 340 | |||
| 341 | // PHP internal - network functions |
||
| 342 | 'header_register_callback', |
||
| 343 | |||
| 344 | // Newt |
||
| 345 | 'newt_entry_set_filter', |
||
| 346 | 'newt_set_suspend_callback', |
||
| 347 | |||
| 348 | // OAuth |
||
| 349 | 'consumerhandler', |
||
| 350 | 'timestampnoncehandler', |
||
| 351 | 'tokenhandler', |
||
| 352 | |||
| 353 | // PHP internal - output control |
||
| 354 | 'ob_start', |
||
| 355 | |||
| 356 | // PHP internal - PCNTL |
||
| 357 | 'pcntl_signal', |
||
| 358 | |||
| 359 | // PHP internal - PCRE |
||
| 360 | 'preg_replace_callback', |
||
| 361 | |||
| 362 | // SQLite |
||
| 363 | 'sqlitecreateaggregate', |
||
| 364 | 'sqlitecreatefunction', |
||
| 365 | 'sqlite_create_aggregate', |
||
| 366 | 'sqlite_create_function', |
||
| 367 | |||
| 368 | // RarArchive |
||
| 369 | 'open', |
||
| 370 | |||
| 371 | // Readline |
||
| 372 | 'readline_callback_handler_install', |
||
| 373 | 'readline_completion_function', |
||
| 374 | |||
| 375 | // PHP internal - session handling |
||
| 376 | 'session_set_save_handler', |
||
| 377 | |||
| 378 | // PHP internal - SPL |
||
| 379 | 'construct', |
||
| 380 | 'iterator_apply', |
||
| 381 | 'spl_autoload_register', |
||
| 382 | |||
| 383 | // Sybase |
||
| 384 | 'sybase_set_message_handler', |
||
| 385 | |||
| 386 | // PHP internal - variable handling |
||
| 387 | 'is_callable', |
||
| 388 | |||
| 389 | // XML Parser |
||
| 390 | 'xml_set_character_data_handler', |
||
| 391 | 'xml_set_default_handler', |
||
| 392 | 'xml_set_element_handler', |
||
| 393 | 'xml_set_end_namespace_decl_handler', |
||
| 394 | 'xml_set_external_entity_ref_handler', |
||
| 395 | 'xml_set_notation_decl_handler', |
||
| 396 | 'xml_set_processing_instruction_handler', |
||
| 397 | 'xml_set_start_namespace_decl_handler', |
||
| 398 | 'xml_set_unparsed_entity_decl_handler', |
||
| 399 | 'simplexml_load_file', |
||
| 400 | 'simplexml_load_string', |
||
| 401 | |||
| 402 | // unzip |
||
| 403 | 'unzip', |
||
| 404 | 'unzip_file', |
||
| 405 | ); |
||
| 406 | private $methodsBlackList = array('setlevel', 'put' => array('sugarautoloader'), 'unlink' => array('sugarautoloader')); |
||
| 407 | |||
| 408 | public function printToWiki(){ |
||
| 421 | |||
| 422 | public function __construct() |
||
| 458 | |||
| 459 | private $issues = array(); |
||
| 460 | private $pathToModule = ''; |
||
| 461 | |||
| 462 | /** |
||
| 463 | *returns a list of issues |
||
| 464 | */ |
||
| 465 | public function getIssues(){ |
||
| 468 | |||
| 469 | /** |
||
| 470 | *returns true or false if any issues were found |
||
| 471 | */ |
||
| 472 | public function hasIssues(){ |
||
| 475 | |||
| 476 | /** |
||
| 477 | *Ensures that a file has a valid extension |
||
| 478 | */ |
||
| 479 | public function isValidExtension($file) |
||
| 491 | |||
| 492 | public function isConfigFile($file) |
||
| 503 | |||
| 504 | /** |
||
| 505 | *Scans a directory and calls on scan file for each file |
||
| 506 | **/ |
||
| 507 | public function scanDir($path){ |
||
| 525 | |||
| 526 | /** |
||
| 527 | * Check if the file contents looks like PHP |
||
| 528 | * @param string $contents File contents |
||
| 529 | * @return boolean |
||
| 530 | */ |
||
| 531 | public function isPHPFile($contents) |
||
| 545 | |||
| 546 | /** |
||
| 547 | * Given a file it will open it's contents and check if it is a PHP file (not safe to just rely on extensions) if it finds <?php tags it will use the tokenizer to scan the file |
||
| 548 | * $var() and ` are always prevented then whatever is in the blacklist. |
||
| 549 | * It will also ensure that all files are of valid extension types |
||
| 550 | * |
||
| 551 | */ |
||
| 552 | public function scanFile($file){ |
||
| 652 | |||
| 653 | /** |
||
| 654 | * checks files.md5 file to see if the file is from sugar |
||
| 655 | * ONLY WORKS ON FILES |
||
| 656 | * |
||
| 657 | * @param string $path |
||
| 658 | * @return bool |
||
| 659 | */ |
||
| 660 | public function sugarFileExists($path) |
||
| 676 | |||
| 677 | /** |
||
| 678 | * Normalize a path to not contain dots & multiple slashes |
||
| 679 | * |
||
| 680 | * @param string $path |
||
| 681 | * @return string false |
||
| 682 | */ |
||
| 683 | public function normalizePath($path) |
||
| 706 | |||
| 707 | /** |
||
| 708 | *This function will scan the Manifest for disabled actions specified in $GLOBALS['sugar_config']['moduleInstaller']['disableActions'] |
||
| 709 | *if $GLOBALS['sugar_config']['moduleInstaller']['disableRestrictedCopy'] is set to false or not set it will call on scanCopy to ensure that it is not overriding files |
||
| 710 | */ |
||
| 711 | public function scanManifest($manifestPath) |
||
| 763 | |||
| 764 | /** |
||
| 765 | * Takes in where the file will is specified to be copied from and to |
||
| 766 | * and ensures that there is no official sugar file there. |
||
| 767 | * If the file exists it will check |
||
| 768 | * against the MD5 file list to see if Sugar Created the file |
||
| 769 | * @param string $from source filename |
||
| 770 | * @param string $to destination filename |
||
| 771 | */ |
||
| 772 | public function scanCopy($from, $to) |
||
| 798 | |||
| 799 | |||
| 800 | /** |
||
| 801 | *Main external function that takes in a path to a package and then scans |
||
| 802 | *that package's manifest for disabled actions and then it scans the PHP files |
||
| 803 | *for restricted function calls |
||
| 804 | * |
||
| 805 | */ |
||
| 806 | public function scanPackage($path){ |
||
| 813 | |||
| 814 | /** |
||
| 815 | *This function will take all issues of the current instance and print them to the screen |
||
| 816 | **/ |
||
| 817 | public function displayIssues($package='Package'){ |
||
| 844 | |||
| 845 | /** |
||
| 846 | * Lock config settings |
||
| 847 | */ |
||
| 848 | public function lockConfig() |
||
| 854 | |||
| 855 | /** |
||
| 856 | * Check if config was modified. Return |
||
| 857 | * @param string $file |
||
| 858 | * @return array Errors if something wrong, false if no problems |
||
| 859 | */ |
||
| 860 | public function checkConfig($file) |
||
| 869 | |||
| 870 | } |
||
| 871 | |||
| 885 |