Skip to content

Conversation

@pyatnitsev
Copy link

I add JakubOnderka\PhpVarDumpCheck integration, please take a look

@pyatnitsev pyatnitsev changed the title Feature/add php vardump checker Add php vardump checker Oct 17, 2019
@AntonAcc
Copy link
Contributor

В доке либы не нашел, на какие кейсы реагирует и что считает нормальным. Хотелось бы где-то это увидеть что бы показать разработчикам как можно, а как теперь нельзя.

На каких кейсах сам тестировал?

@pyatnitsev
Copy link
Author

А можешь посмотреть реферес в configuration.ini.dist

[phpvardumpcheck]
className='PhpCsBitBucket\Checker\PhpVarDumpCheck'
mode='--symfony'

вот такой новый конфиг добавился. + я добавил его в секцию checkers

Можно задавать mode, у библиотеки в readme описаны допустимые значения. Если не задавать mode - тогда будет проверять на стандартные var_dump, var_export, print_r

Если задать как у меня - то добавятся еще Symfony-дампы - dump().

В текущей версии основной библиотеки не поддерживаются dd() для symfony, запросил выпустить релиз.

@pyatnitsev
Copy link
Author

Тестировал на своем репозитории, примерно такой вызов будет для личного репозитория пользователя в Bitbucjet:

php app.php test ~DPYATNITSEV dkron

где test - ветка, ~DPYATNITSEV - проект (это имя пользователя начинающееся с ~), dkron - название репозитория.

Я сделал следующее:

  • Создал отдельную ветку,
  • Сделал туда commit с var_dump и dump в разных строчках
  • Сделал PR в Master
  • Запустил Tool из-под себя

Получил 2 сообщения - что забыл dump там и там.

@AntonAcc
Copy link
Contributor

AntonAcc commented Oct 17, 2019

Т.е. например https://git.finam.ru/projects/WT/repos/sparta/browse/comon/lib/generic/RequestHandlerSwitcher.php#272 больше не возможно будет закоммитеть?

А как на var_export($var, true) отреагирует?

Беспокоит ложное срабатывание на полезное использование и отсутствие возможности обойти или игнорировать.

@pyatnitsev
Copy link
Author

Там можно задавать исключения, в виде путей на файлы,
Могу это добавить в саму либу,

но как отличить хорошее срабатывание от плохого - можно подумать

@pyatnitsev
Copy link
Author

Можно добавить разрешенные функции - и добавить туда как раз var_export и обработать это на стороне тула

@AntonAcc
Copy link
Contributor

AntonAcc commented Oct 17, 2019

Предлагаю описать, что считаем злом и не пропускаем, а что полезное использование и добавить в исключения. Обсудить и согласовать это с коммандой WTT. Получив коду с описание того, как больше нельзя, а как можно, загоняем ее тестовый коммит и настраиваем либу, что бы на этот коммит не ругалась. После этого готов добавить это в мастер. А пока эти изменения для меня черный ящик, возможно таящий много подводных камней.

@pyatnitsev
Copy link
Author

Добавил возможность исключать конкретные функции из проверки.

То, что я добавил - дополнительный функционал - его можно включить/выключить в конфиге.

Какое согласование с командой WTT требуется?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants