Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 5 방식의 생성자 (__construct) 사용 #1363

Closed
wants to merge 2 commits into from
Closed

PHP 5 방식의 생성자 (__construct) 사용 #1363

wants to merge 2 commits into from

Conversation

kijin
Copy link
Contributor

@kijin kijin commented Apr 1, 2015

올해 안에 공식 발표될 PHP 7에서는 클래스와 같은 이름의 메소드를 생성자로 사용할 경우 E_DEPRECATED 오류가 발생합니다. 또한 차차기 버전에서는 아예 이런 방식의 생성자를 지원하지 않고 __construct()만 인정할 계획이라고 합니다.

참고: https://wiki.php.net/rfc/remove_php4_constructors

당장 문제가 있는 것은 아니지만, 간단하게 고칠 수 있는 것이기에 PR을 넣습니다.

  • 클래스와 같은 이름의 메소드를 생성자로 사용하는 경우를 모두 찾아 __construct()로 변경
  • parent::부모클래스이름() 문법을 사용하는 경우도 모두 parent::__construct()로 변경

@ghost ghost modified the milestone: next-2015-19 Apr 17, 2015
@ghost ghost modified the milestones: next-2015-19, next-2015-24 May 4, 2015
@ghost ghost modified the milestones: next-2015-24, next-2015-28 Jun 9, 2015
@ghost ghost modified the milestones: next-2015-32, 1.8.4 Jul 6, 2015
@misol
Copy link
Contributor

misol commented Jul 20, 2015

이 PR을 아파치, PHP/5.5.15 기반, 그리고 nginx, PHP/5.5.9 기반에서 테스트 했을 때 서버 스크립트 문제가 발생하지 않았습니다.

클라우드 플레어를 이용했을 때 문제가 발생하지 않았습니다.

Window7 Internet Explorer 11.0.9600, Chrome 43.0.2357.134 m, Firefox 39.0 에서 테스트 했을때 클라이언트단 오류가 발생하지 않았습니다.
안드로이드 폰과 아이폰에서 테스트 했을때 클라이언트단 문제가 발생하지 않았습니다.
안드로이드 버전 5.0.1, 갤럭시S5, Chrome 브라우저 최신버전(2015년 7월 20일 기준)에서 테스트 했습니다.
아이폰은 아이폰5 iOS8 최신버전(2015년 7월 20일 기준)에서 테스트 했습니다.

테스트 도중 구성한 페이지들에서 문제가 발생하지 않았습니다. 구성한 페이지는 게시판 목록, 게시판의 본문글, 위젯 페이지, 외부 페이지 등이 포함 되었습니다.

@ghost ghost modified the milestones: next-2015-32, next-2015-37 Aug 18, 2015
@misol
Copy link
Contributor

misol commented Aug 19, 2015

@kijin ./classes/extravars/Extravar.class.php 의 160번째 줄 ExtraItem class 의 ExtraItem 메소드도 손봐주세요!

@kijin
Copy link
Contributor Author

kijin commented Aug 20, 2015

아... 한 파일에 클래스 2개 들어있는거 정말 싫어요 ㅠㅠ

@misol
Copy link
Contributor

misol commented Aug 20, 2015

@kijin 그런게 좀 있는 모양이에요.. ㅎㅎ Travis-CI 에서 ./modules/autoinstall/autoinstall.class.php on line 60 에도 있다고 알려주고 있습니다..

  • autoinstall 클래스의 autoinstall 메소드가 있습니다. (./modules/autoinstall/autoinstall.class.php)

@misol
Copy link
Contributor

misol commented Aug 20, 2015

@kijin 그 외에 발견하는 대로 댓글로 계속 남기겠습니다.

  • ./modules/autoinstall/autoinstall.lib.php 파일의 320 번째 줄 정의된 SFTPModuleInstaller class 에도 SFTPModuleInstaller 메소드가 존재합니다.
  • 같은 파일 (./modules/autoinstall/autoinstall.lib.php) 489번째 줄에 정의된 PHPFTPModuleInstaller 클래스에도 PHPFTPModuleInstaller 메소드가 존재합니다.
  • 역시 같은 파일 (./modules/autoinstall/autoinstall.lib.php) 703번째 줄에 정의된 FTPModuleInstaller 클래스에도 FTPModuleInstaller 메소드가 존재합니다.
  • 한번 더 같은 파일 (./modules/autoinstall/autoinstall.lib.php) 876번째 줄에 정의된 DirectModuleInstaller class에도 DirectModuleInstaller 메소드가 존재합니다.

@misol
Copy link
Contributor

misol commented Aug 20, 2015

  • ./libs/ftp.class.php 파일 44번째 줄에 정의된 ftp class 내용 중 ftp() 가 존재합니다.
  • ./libs/tar.class.php 파일 79번째 줄에 정의된 tar() 메소드는 tar 클래스의 constructor 입니다.

@kijin
Copy link
Contributor Author

kijin commented Aug 20, 2015

우왁... autoinstall 파일 하나에 대체 클래스가 몇 갠가요 ㅠ

그냥 grep으로 클래스 목록을 모조리 검색한 후, 같은 이름의 메소드를 찾아보는 게 빠르겠습니다.

#!/bin/bash
for classname in `grep -rnoEi "class\s+([0-9a-z_]+)" | grep ".php:" | grep -iv "HTMLPurifier" | cut -d' ' -f2 | sort | uniq -u`
do
    regexp="function\s+$classname\s*\("
    grep -rnEi "$regexp"
done

주의: 급조한 쉘스크립트라 검색에 10분 넘게 걸리고, false positive도 많아요.

libs/ftp.class.php:56: function ftp()
libs/tar.class.php:79: function tar() {
modules/autoinstall/autoinstall.class.php:73: function autoinstall()
modules/autoinstall/autoinstall.lib.php:347: function SFTPModuleInstaller(&$package)
modules/autoinstall/autoinstall.lib.php:510: function PHPFTPModuleInstaller(&$package)
modules/autoinstall/autoinstall.lib.php:723: function FTPModuleInstaller(&$package)
modules/autoinstall/autoinstall.lib.php:883: function DirectModuleInstaller(&$package)
tests/classes/db/classes/xml/xmlquery/queryargument/QueryArgumentTest.php:11: function QueryArgumentTest(){
tests/classes/db/classes/xml/xmlquery/tags/condition/ConditionTagTest.php:11: function ConditionTagTest(){
tests/classes/db/classes/xml/xmlquery/tags/table/TablesTagTest.php:10: function TablesTagTest(){
tests/classes/db/classes/xml/xmlquery/tags/table/TableTagTest.php:10: function TableTagTest(){
tests/classes/db/db/MockDb.php:48: function MockDBMssql(){
tests/classes/db/db/MockDb.php:59: function MockDBCubrid(){
tests/classes/db/db/MockDb.php:70: function MockDBMysql(){
tests/classes/db/db/xml_query/cubrid/CubridIndexHintTest.php:7: function CubridIndexHintTest(){
tests/classes/db/db/xml_query/cubrid/CubridSubqueryTest.php:12: function CubridSubqueryTest(){
tests/classes/db/db/xml_query/mssql/MssqlIndexHintTest.php:7: function MssqlIndexHintTest(){
tests/classes/db/db/xml_query/mysql/MysqlIndexHintTest.php:7: function MysqlIndexHintTest(){
tests/classes/db/QueryTester.class.php:8: function QueryTester(){
widgets/content/content.class.php:782: function contentItem($browser_title='')
widgets/mcontent/mcontent.class.php:674: function mcontentItem($browser_title='')

위젯을 한두 개 놓쳤고, tests 쪽에 DB클래스 사본이 상당히 많이 굴러다니네요.

@misol
Copy link
Contributor

misol commented Aug 20, 2015

역시 사람은 머리가 좋아야...ㅋㅋㅋ
제가 찾은건 제가 눈으로 확인한거니 true positive 들이에요 ㅋㅋ
많네요..

@ghost ghost modified the milestones: next-2015-37, next-2015-41 Sep 21, 2015
@ghost ghost modified the milestones: next-2015-41, next-2015-46 Oct 16, 2015
@ghost ghost removed this from the next-2015-46 milestone Jan 11, 2016
@ghost ghost added the status/keep label Feb 1, 2017
@ghost ghost closed this Feb 1, 2017
@ghost ghost mentioned this pull request Sep 21, 2018
dorami added a commit to daolcms/daolcms that referenced this pull request Oct 14, 2018
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants