[TYPO3-dev] duplicated lines in TYPO3 detected by phpcpd

Fri Apr 17 19:55:02 CEST 2009

Hey girls and guys,

for some weeks i`ve read a german blog post [1] about outsourcing some 
features of PHPUnit to small own programms.
One of this small own programms is "phpcpd" aka "PHP copy paste 
detector" [2] from Sebastian Bergmann (the author of PHPUnit [3]).
This topic is very interisting for me. So i want to test it. But to test 
this tool i need a existing project. So i chose TYPO3 :) (i`ve export 
the subversion trunk (revision 5327)).

I want to discuss the following results with you. I dont want to create 
more and more work for core devs.
I want to discuss with you for the optimal solutions. If there are some 
good ideas to solve this i will try to create some patches to minimize 
the dublicate lines.
And every developer knows: If you could erase dublicate lines, do this, 
because of the maintainance!

My first run with this tool on TYPO3 ouputs the following results:

-------------- Linux shell output start --------------
webdev:/home/andy# phpcpd /var/www/typo3_4.3_r5327/
phpcpd 1.1.1 by Sebastian Bergmann.

Found 63 exact clones with 2907 duplicated lines in 56 files:

   - /var/www/typo3_4.3_r5327/t3lib/class.t3lib_querygenerator.php:1421-1436
     /var/www/typo3_4.3_r5327/t3lib/class.t3lib_fullsearch.php:773-788

   - /var/www/typo3_4.3_r5327/typo3/mod/tools/em/class.nusoap.php:3964-3973
     /var/www/typo3_4.3_r5327/typo3/mod/tools/em/class.nusoap.php:4043-4052

   - /var/www/typo3_4.3_r5327/typo3/mod/web/info/index.php:196-202
     /var/www/typo3_4.3_r5327/typo3/mod/web/func/index.php:191-197

   - /var/www/typo3_4.3_r5327/typo3/class.filelistfoldertree.php:235-258
     /var/www/typo3_4.3_r5327/typo3/class.webpagetree.php:277-300

   - /var/www/typo3_4.3_r5327/t3lib/class.t3lib_treeview.php:353-360
     /var/www/typo3_4.3_r5327/typo3/class.webpagetree.php:331-338

   - /var/www/typo3_4.3_r5327/typo3/file_edit.php:110-132
     /var/www/typo3_4.3_r5327/typo3/file_rename.php:112-134

   - /var/www/typo3_4.3_r5327/typo3/file_upload.php:129-137
     /var/www/typo3_4.3_r5327/typo3/file_newfolder.php:129-137

   - /var/www/typo3_4.3_r5327/typo3/file_edit.php:123-132
     /var/www/typo3_4.3_r5327/typo3/file_newfolder.php:137-146

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlparser.php:61-69

/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlengine.php:61-69

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlparser.php:97-122

/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlengine.php:97-122

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlparser.php:151-335

/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlengine.php:151-335

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlparser.php:351-363

/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_sqlengine.php:344-356

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_db.php:738-748

/var/www/typo3_4.3_r5327/typo3/sysext/dbal/class.ux_t3lib_db.php:809-819

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/cms/tslib/media/scripts/gmenu_layers.php:88-450

/var/www/typo3_4.3_r5327/typo3/sysext/cms/tslib/media/scripts/tmenu_layers.php:88-450

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/cms/tslib/class.tslib_menu.php:1881-1922

/var/www/typo3_4.3_r5327/typo3/sysext/cms/tslib/class.tslib_menu.php:2425-2466

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/install/mod/class.tx_install_ajax.php:33-42
     /var/www/typo3_4.3_r5327/typo3/sysext/cms/tslib/showpic.php:57-66

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/datadict/datadict-mssql.inc.php:159-242

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/datadict/datadict-sybase.inc.php:121-204

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-db2.inc.php:458-468

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-odbc.inc.php:333-343

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-db2.inc.php:758-769

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-odbc.inc.php:658-669

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysql.inc.php:391-444

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-pdo_mysql.inc.php:75-128

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysql.inc.php:546-562

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysqli.inc.php:476-492

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysql.inc.php:413-419

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysqli.inc.php:532-538

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysql.inc.php:437-446

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mysqli.inc.php:553-562

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mssql.inc.php:164-173

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado_mssql.inc.php:105-114

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-mssql.inc.php:203-243

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-odbc_mssql.inc.php:223-263

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-odbc.inc.php:143-152

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-sqlite.inc.php:213-222

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-oci8.inc.php:466-505

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-postgres64.inc.php:260-299

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-pdo_pgsql.inc.php:134-167

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-postgres64.inc.php:486-519

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado.inc.php:15-32

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado5.inc.php:15-32

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado.inc.php:103-173

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado5.inc.php:126-196

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado.inc.php:210-230

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado5.inc.php:231-251

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado.inc.php:251-264

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado5.inc.php:279-292

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado.inc.php:295-606

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/drivers/adodb-ado5.inc.php:328-639

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:105-172

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:87-154

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:276-289

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:251-264

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:508-525

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:466-483

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:550-843

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:508-801

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:1048-1282

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:948-1182

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:1650-1752

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:1491-1593

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:1809-1816

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:1650-1657

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:1908-2033

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:1747-1872

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:2200-2218

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:2027-2045

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema03.inc.php:2295-2360

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-xmlschema.inc.php:2113-2178

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-lib.inc.php:181-190

/var/www/typo3_4.3_r5327/typo3/sysext/adodb/adodb/adodb-lib.inc.php:267-276

   - /var/www/typo3_4.3_r5327/typo3/class.browse_links.php:298-307

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:60-69

   - /var/www/typo3_4.3_r5327/typo3/class.browse_links.php:531-542

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:137-148

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_dam_browse_links.php:266-276

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:421-431

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_dam_browse_links.php:517-536

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:611-630

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_dam_browse_links.php:547-560

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:641-654

   - /var/www/typo3_4.3_r5327/typo3/class.browse_links.php:1361-1378

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:755-772

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_dam_browse_links.php:649-664

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:782-797

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_dam_browse_links.php:912-923

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php:1067-1078

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/class.tx_rtehtmlarea_base.php:267-276

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/pi2/class.tx_rtehtmlarea_pi2.php:143-152

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_dam_browse_links.php:459-479

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod4/class.tx_rtehtmlarea_dam_browse_media.php:650-670

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod3/browse_links.php:75-82

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod4/select_image.php:66-73

   - /var/www/typo3_4.3_r5327/typo3/class.browse_links.php:2355-2407

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/mod4/class.tx_rtehtmlarea_select_image.php:1006-1058

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/extensions/TYPO3HtmlParser/class.tx_rtehtmlarea_typo3htmlparser.php:39-47

/var/www/typo3_4.3_r5327/typo3/sysext/rtehtmlarea/extensions/DefaultClean/class.tx_rtehtmlarea_defaultclean.php:39-47

   - 
/var/www/typo3_4.3_r5327/typo3/sysext/belog/class.tx_belog_webinfo.php:181-191
     /var/www/typo3_4.3_r5327/typo3/sysext/belog/mod/index.php:275-285

   - /var/www/typo3_4.3_r5327/typo3/mod/user/ws/class.wslib_gui.php:973-1077
     /var/www/typo3_4.3_r5327/typo3/sysext/version/cm1/index.php:1256-1360

   - 
/var/www/typo3_4.3_r5327/typo3/mod/user/ws/class.wslib_gui.php:1120-1135
     /var/www/typo3_4.3_r5327/typo3/sysext/version/cm1/index.php:1402-1417

   - /var/www/typo3_4.3_r5327/typo3/mod/user/ws/class.wslib_gui.php:685-737
     /var/www/typo3_4.3_r5327/typo3/sysext/version/cm1/index.php:1493-1545

   - 
/var/www/typo3_4.3_r5327/typo3/mod/user/ws/class.wslib_gui.php:1266-1274
     /var/www/typo3_4.3_r5327/typo3/sysext/version/cm1/index.php:1636-1644

   - 
/var/www/typo3_4.3_r5327/tests/t3lib/cache/backend/t3lib_cache_backend_apcbackendtestcase.php:146-214

/var/www/typo3_4.3_r5327/tests/t3lib/cache/backend/t3lib_cache_backend_memcachedbackendtestcase.php:177-245

1.03% duplicated lines out of 282824 total lines of code.
-------------- Linux shell output ends --------------

After this results i`ve spend some time on it to look in these files to 
find out what "phpcpd" has detected.
Some lines are not exactly outputted by phpcpd but you could always see 
the dublicate lines.
Some files which are in the list of phpcpd are external sources (for 
example nusoap or adodb). This code is not in our hand.

So lets have a detailed look to the results:

	- t3lib/class.t3lib_querygenerator.php:1421-1436
	- t3lib/class.t3lib_fullsearch.php:773-788
	The lines are not exactly but you will see that the method "function 
getTreeList($id, $depth, $begin=0, $perms_clause)" is completly the same.
	And in this method there is no reference on "$this" (expect of the 
recursion of the method, this could be solved by self::getTreeList... 
for example).
	One solution is to delete the method body from one class and point to 
the other (making an object and execute the method from the other 
class). The disadvantage of this solution is the dependency of this two 
classes.
	Another solution is to export this method to a static class like t3lib_div.

	- typo3/mod/web/info/index.php:196-202
   - typo3/mod/web/func/index.php:191-197
   In this result, the method "getButtons()" is affected. The methods in 
this two classes are nearly the same.
   I think that could be optimized. But actually i dont know how.

   - typo3/class.filelistfoldertree.php:235-258
   - typo3/class.webpagetree.php:277-300
   In this two classes the following methods are nearly or exactly the same:
   	- wrapTitle($title,$row,$bank=0)
   	- printTree($treeArr = '')
   	- PMicon($row,$a,$c,$nextCount,$exp)
   	- PMiconATagWrap($icon, $cmd, $isExpand = true)
   A example solution is to write an base class for this which is 
extended by class.filelistfoldertree.php and class.webpagetree.php.

	- t3lib/class.t3lib_treeview.php:353-360
   - typo3/class.webpagetree.php:331-338
   The method "getBrowsableTree()" is nearly the same. For a solution 
see result one (query generator).

	- typo3/file_edit.php:110-132
   - typo3/file_rename.php:112-134
   The Method "init()" is exaclty the same.
   Solution: Create a base class or rewrite this files with very small 
methods to connect this with other classes

   - typo3/file_upload.php:129-137
   - typo3/file_newfolder.php:129-137
   The Method "init()" is nearly the same.
   Solution: Create a base class or rewrite this files with very small 
methods to connect this with other classes

   - typo3/file_edit.php:123-132
   - typo3/file_newfolder.php:137-146
   Some duplicate lines in the init() method.
   Solution: Create a base class or rewrite this files with very small 
methods to connect this with other classes

To describe all dublicate linesmake this post veeeery long, because i 
know that small maillinglist posts are very welcome :D
So whats your opinion?
Is this important or is this "task" bullshit?

My opinion is, that this is a very important point. 2907 duplicated 
lines are not few.
I think thats more than 2907, because if you look at the source code, 
some lines are written in other coding style, so phpcpd cant detect them.

Some modules are historicly grown so refactoring are a good idea i 
think. If i can, i want to help.

Have a nice day,
Andy

[1] http://www.phphatesme.com/blog/softwaretechnik/copy-and-paste-detection/
[2] http://github.com/sebastianbergmann/phpcpd/tree/master
[3] http://www.phpunit.de/