Pull to refresh

Comments 62

Мы «придумали свой», но по факту он очень слабо отличается от PSR-*. В данный момент мы не предоставляем готового стиля для PSR-*, но мы работаем над этим.
Жаль, что не PSR, с удовольствием использовал бы. А есть какой-то документ, где кракто описаны отличия?
В данный момент такого документа нет, к сожалению. Из «значимых» отличий я могу вспомнить, к примеру, про то, что после type-cast операторов у нас убирается пробел:

<?php
$a = (int)$b; // phpcf "default" style
$a = (int) $b; // PSR
Если нет документа который описывает различия, то может есть документ который описывает весь стиль кодирования?
Почему у вас запрещен импорт классов?
Вы имеете в виду использование «use SomeClass as AnotherClass»?
Нет, я имею ввиду почему вас в принципе запрещен импорт классов?
К классу, находящемуся внутри неймспейса, следует обращаться как \Namescape\ClassName и никак иначе, за исключением случая, когда код находится внутри того же неймспейса, в этом случае можно опустить \Namespace.
На самом деле, phpcf это не проверяет, это наше внутреннее соглашение. Такое соглашение принято для того, чтобы облегчить grep по коду.
А что вы грепаете так часто, что пренебрегаете удобствами? Ну то есть usage-статистику если считать, можно и скриптик написать, а для чего еще?
Я почему так удивляюсь — потому что вот это на мой взгляд немного жесть)
Как мне кажется, вы сильно недооцениваете необходимость в использовании утилиты grep в проекте с парой миллионов строк ;).
Find usages в PhpStorm не пробовали? Говорят, лучше подходит для таких целей.
Для этого нужно ещё все тесты переписать на использования MyClass::$class вместо 'MyClass' в моках, например.
Абсолютно верно. Еще попадаются редкие места, помимо тестов, где классы тоже в виде строки. В основном это call_user_function.
Тем не менее почти всегда стараемся в новом коде избегать таких вещей.

Кстати важный момент насчет MyClass::$class — класс должен существовать, те в случае автолоада он сразу подгружается. Т.е. есть вдруг кому захочется прописать где-нибудь некий маппинг классов в массиве, то лучше уж использовать строки, чем подгружать их всех разом.
Кстати важный момент насчет MyClass::$class — класс должен существовать, те в случае автолоада он сразу подгружается.

Это неверное утверждение.
Я тоже могу кинуть фразу, что вы не правы и сделать крутое лицо.
$a = Classname::$class; PHP Fatal error: Class 'Classname' not found in php shell code on line 1
Возможно я где-то не прав. Приведите пример — это был бы более конструктивный комментарий.
Перечитал первоисточник(документацию) и понял в чем я не прав.
Да вы правы, погорячился я. Хотя можно было с вашей стороны поправить нас с неправильным синтаксисом:
ClassName::class действительно работает без подгрузки класса.
Я подумал, что это просто опечатка. $class — это ж просто обращение к статическому свойству, подобный magic был бы адом и ломал бы BC. А так реюзается уже имеющийся keyword.
Почему не настраиваемо? (тот же phpcs умеет принимать файлы с стандартами, что позволило его включить в PHPStorm в качестве штатного инспектора)
Точнее, в комментариях написано, что конфиг есть, но в гитхабе/в посте — мимо.
Стандарты кода описываются здесь github.com/badoo/phpcf/tree/master/phpcf-src/styles
Например можно описать требуемые отличия от стандарта по умолчанию
github.com/eelf/phpcf/commit/76c8c7205f4c316cb9f557ea960c8ed15a3a1eba#diff-d41d8cd98f00b204e9800998ecf8427e
Работает так
$ ~/phpcf/phpcf --style=compact apply classes/Db.php
или так
$ ~/phpcf/phpcf --style=minified apply classes/Db.php
Для публичного инструмента конфиг ужасен :)
Согласен, выглядит несколько пугающе, но при рассмотрении вариантов:

1.) То же самое, в XML (плюсов с ходу не вижу, из минусов — потребить больше памяти, написать парсер конфига)
2.) Инструкции, подобно тому, как PHP: lxr.php.net/xref/PHP_5_5/Zend/zend_language_parser.y (опять же, подсистему парсинга, но теперь уже — псевдокода конфига)
3.) Сделать OOP-интерфейс для правил переходов
$State ->when($ContextPredicate) ->then($Transition) ->done($ReverseTransition); // (а-ля Promises, не обещает быть меннее монстроузным)
и для правил форматирования
$ClassToken ->inContext($ClassDefinitionContext)->do($IndentIncreateAction) ->inContext($ClassReferenceContext)->do($IndentIncreateAction);
4.) Текущая реализация (минимальный memory footprint, возможность программирования конфига на языке приложения)

Последний вариант мне кажется наиболее оптимальным.
добавление в packagist планируется?
Через небольшое количество времени мы планируем написать статью также на английском языке (не для Хабра), и мы постараемся учесть пожелания по формату распространения. Пока «формат распространения» у phpcf — это github, но идея добавить и в composer мне лично нравится.
Когда вожусь в чужом коде иногда очень хочется простой инструмент, который

Из такого:
<?php

$result = a(b(c(d($data))));

?>

делает примерно такое
<?php

$result4 = d($data);

$result3 = c($result4);

$result2 = b($result3);

$result = a($result2);

?>


А то названия функций или классы+методы очень длинные
и это все в одну строку...


На самом деле, phpcf намеренно сделан таким, какой он есть, то есть, трогающим только пробелы, чтобы его можно было достаточно легко конфигурировать (то есть, не меняя исходный код, а изменяя конфиг). Мы «just for fun» писали простенькие утилиты по, к примеру, конвертации «старого синтаксиса массивов» в «новый», но более сложный анализ кода требует большой аккуратности, и пока что таких утилит у нас нет :).
Я только За простые утилиты.
Это просто к теме читабельности кода. Возможно как в старом анекдоте, где из-за студента профессор с бородой не мог уснуть, вы также однажды решите что пришло время еще одной простой утилиты :)
Интересно, а существует ли плагин к PHPStorm (например), который анализирует выбранный проект на используемый стиль (думаю данных должно хватить по всем пунктам, если проект не мелкий), и сохраняет его как шаблон. А вот делаю Pull Request например куда-нибудь, так и хочется нажать хоткей форматирования, то все порушится)
Не уверен, существует ли, но на основе phpcf такой плагин сделать вполне реально :).
В Project Settings -> PHP -> Code Sniffer настройки PHPCS. Это самое близкое к требуемому.
Я говорил про Project Settings -> Code Style -> PHP, чтобы на основе загруженного проекта можно было сформировать правильный стиль, который используется в данном проекте и без проблем форматировать свой код не заботясь о «Code Style».
И еще Mess Detector там же. Щелкаете по директории — Inspect Code… и PHPStorm проверит по всем инспекциям, что у него есть (настроенные phpcs, phpmd, jshint, инспекции из Inspections, плюс разное — нераспознанные идешкой переменные, например).
Имхо неверно поставлена задача. Принимаете плохой код и правите его автоматически.
А если просто не принимать плохой код, проверяя его через git с помощью PHP Code Sniffer?
Наш pre-receive хук проверяет плохое форматирование, используя phpcf. Т.е. производится форматирование измененных строк, и, если там код не отформатирован, коммит не проходит.
Не понял по тексту, а возможно ли этой утилите скормить в пайп просто кусок кода вместо имени файла и номеров строк начала/конца? А то, насколько я помню, в эклипсе можно получить выделенный кусок кода для внешней тулзы, а номера строк нельзя…
Действие «phpcf stdin» должно это делать. Помимо этого, вы можете воспользоваться функциональностью, описанной в секции «Использование классов phpcf напрямую»
А что мешает в шторме импортировать удобный лично Вашей компании конфиг форматера(который более чем подробный в конфигурации) и пожамкать им в два клика целый проект?
«Несколько лет назад компания Badoo начала значительно расти по числу сотрудников, с 20 до 100 и более»

Заставить под угрозой увольнения все сто человек выбросить любимый vim/emacs/netbeans? Это какой-то фашизм :)
Причин несколько, на самом деле. Помимо того, что не все используют PHPStorm, мы хотели поддерживать «отформатированность» кода с помощью git-хуков, настроенных на отклонение неотформатированных изменений, с возможностью легко исправить форматирование. Поэтому отдельная утилита представляется намного более удобным и гибким решением, чем использование форматирующей части одной из популярных IDE, тем более проприетарной :).
Как уже выше сказали, авто-форматирование в PhpStorm хорошая фича для небольших проектов с малым количеством задействованных разработчиков. Для большого проекта она бесполезна. Это всё нужно делать централизованно.
> пожамкать им в два клика целый проект?
именно этого нам и хотелось избежать. Изначально (примерно 6-8 лет назад), у нас не было никакого стандарта. Точнее он был, но скорее как рекомендации и во многих местах он не был соблюден. Компания росла и в какой-то момент вопросы стандарта кодирования стали возникать всё чаще.
Было 2 варианта — отформатировать разом весь репозиторий, или сделать так чтобы весь новый код был по стандарту, а старый код постепенно бы менялся/рефакторился и тем самым попал бы под новое форматирование.
Полный реформат репозитория не понравился тем что сильно загрязнится история в git-е — много строк кода будут ссылаться на комит в котором был сделан реформат. Это неудобно потому что нам довольно часто приходится читать старый код, искать кто и почему написал ту или иную строку кода. Поэтому был выбран второй вариант
насколько я понимаю с вопросом хука запрещающего коммиты «не по-формату» прекрасно справляется phpcs
вопрос автоформата в истории коммитов — его можно делать отдельным коммитом.

Собственно что хотелось бы понять из всей этой предистории — почему решили писать что-то свое а не адаптировать существующее. Ну я просто не верю что шторм настолько проприетарен что нельзя на отдельном компьютере(например на машине с TeamCity или PhpCI) поднять его и делать автоформат по хукам, или даже написать свою java-оберточку с форматером и запускать именно ее.
Если используете именно php как extension то почему не пойти с азов начиная с форка tokenizer'а или даже собственного bison-парсера?

Сейчас не вдаваясь в подробности я вижу что-то вида «ну мы тут чтото удобное нам накодили на php и немножко на zend'е — оно правит пробелы и еще что-то полезное. Profit»
Давайте лучше системный подход применим? Пойдем с понимания целей(в первую очередь) и задач?
А извините, с форматированием Unix-style строк и sed прекрасно справляется!
1. В статье сказано почему нам не подошёл phpcs — он мог проверить формат, но не мог отформатировать код.
2. Про автоформат.
Вы настаиваете что автоформат всего репозитория в нашем случае было бы правильным решением? Почему?
Очевидно что если бы мы решили его делать, то это был бы отдельный комит, но всё равно это сделало бы работу с репозиторием менее удобной — менее удобно было бы получать историю по всем строкам, задетым в момент форматирования. Баду репозиторий существует наверное более 8 лет, вопрос с форматированием кода начали решать где-то 3-4 года назад (могу ошибаться, точно не помню). То есть большАя часть кода в репозитории за первые 4-5 лет в git blame указывала бы на коммит автоформата, вместо того чтобы указывать на реальные «боевые» коммиты. С таким репозиторием неудобно работать.
И не стоит думать что мы сразу кинулись писать php экстеншен, сначало конечно был простенький скриптик на чистом php.

На остальные вопросы затрудняюсь ответить — я лишь пользователь phpcf, а не его разработчик.
Причина, по которой мы написали phpcf на PHP, а не стали «раздраконивать PHPStorm» очень простая — мы (100+ разработчиков) умеем писать и готовить PHP, и не умеем готовить Java :). Первую версию phpcf написал fisher за пару вечеров, а с любыми другими утилитами на Java так бы не получилось :), даже если там всё на самом деле просто — мы не Java-программисты
Полный реформат репозитория не понравился тем что сильно загрязнится история в git-е — много строк кода будут ссылаться на комит в котором был сделан реформат.

Это ж гит, в нем если нельзя, но хочется — то можно. :) Сделать реформат с сохранением истории, скажем, можно с помощью filter-branch.
После filter-branch изменятся хеши всех коммитов, и придется _везде_ переклонировать репозитории :). А разработчиков у нас немало, плюс ещё деплой, teamcity, и прочие места, где есть репозиторий — это очень большой гемморой был бы для нас.
Понятно, что изменятся. :) Один раз, кмк, такое терпимо, это проще, чем, скажем, переезд с svn ;)
Попробовал интегрировать в PHP Storm но вот что выводит в консоле при попытке обработать файл:
Internal formatter error: final state must be CTX_PHP or CTX_DEFAULT, got 'CTX_PHP / CTX_DEFAULT'

Проверяю на ОС Win8

А вот ошибка при попытке обработать выделенную область:
Internal formatter error: final state must be CTX_PHP or CTX_DEFAULT, got 'CTX_PHP / CTX_PHP / CTX_PHP / CTX_PHP / CTX_GENERIC_BLOCK / CTX_PHP / CTX_PHP / CTX_GENERIC_BLOCK / CTX_GENERIC_BLOCK / CTX_PHP / CTX_DEFAULT'
Проблема форматирования связана с комбинированным использованием PHP + HTML в файле.
По инциденту заведен Issue на github, спасибо за репорт.
Та-же ошибка при работе из cli 5.5.15 ubuntu
Если вы не используете смесь HTML + PHP (то есть, случай, по которому мы уже завели тикет на гитхабе), будьте добры пожалуйста привести полный текст ошибки и содержимое файла, на котором у вас происходит ошибка (можно сразу на github, или мне или alexkrash в личку)
php+html, комментарии писали одновременно.
А разве не проще настроить подобные простые замены в редакторе кода? Лично у меня это есть в VIM, а всякие там PHPStorm должны это всё уметь по-умолчанию. Или я не прав?
В PHPUnit есть известный bug с покрытием таких конструкций:
$var = false;
if($var == true)
    return false;
if($var == true) return false;

If в данном случае не отработает, однако Xdebug считает его покрытым. Как выход можно заключить операторы после if в фигурные скобки:
$var = false;
if($var == true) {
    return false;
}
if($var == true) { return false; }

Тогда Xdebug будет нормально считать покрытие, но только для первой конструкции. Вторую он все так же будет считать покрытой.

Вопрос вот в чем, как в badoo справляетесь с этой проблемой, если ваш форматтер как раз убирает фигурные скобки?
Все просто: наш форматтер их не убирает :)
А я вот немного не понял. Не поясните, что имеется в виду под «считает покрытым»? Не помечает к удалению второй if?
Нет, просто отмечает строку, что программа туда зашла, а на самом деле захода не было.
Only those users with full accounts are able to leave comments. Log in, please.