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

Проверка на запросы в цикле #32

Merged

Conversation

DoublesunRUS
Copy link
Contributor

  1. Ищутся все методы, в которых есть запросы
  2. В цикле ищутся все методы, которые вызывают методы, в которых есть запросы
  3. Обходятся все циклы. Для каждого оператора цикла проверяется, что он не является выполнением запроса или вызовом метода с запросом.

Closes #17

Copy link
Collaborator

@marmyshev marmyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Было бы еще неплохо проверить это на ERP 2 (как самая большая кодовая база) - чтобы понять сколько там ошибок и все ли они достоверные (т.е. нет ли каких-то особенностей в коде).

@DoublesunRUS
Copy link
Contributor Author

Было бы еще неплохо проверить это на ERP 2 (как самая большая кодовая база) - чтобы понять сколько там ошибок и все ли они достоверные (т.е. нет ли каких-то особенностей в коде).

@marmyshev
Особенности в коде ЗУП есть. Например когда обходится порция объектов, то в тексте запроса есть "ВЫБРАТЬ ПЕРВЫЕ", и цикл бесконечный. Например "Пока Истина Цикл". Но в статье на ИТС я не нашел какого-то стандарта как делать исключения правильно.
То есть определять порционные запросы каким-то образом надо, но не хотелось бы делать много исключений.
Предлагаю сделать одно исключение опционально. Разрешить для циклов Пока Истина Цикл выполнять запрос вызывая код, но ругаться если в таком бесконечном цикле вызываются процедуры с другими запросами.

@marmyshev
Copy link
Collaborator

Предлагаю сделать одно исключение опционально. Разрешить для циклов Пока Истина Цикл выполнять запрос вызывая код, но ругаться если в таком бесконечном цикле вызываются процедуры с другими запросами.

Ну тут можно будет ставить локальные исключения // @skip-check query-in-loop - порционный запрос - но это потребует доработки кода, что "настоящие программисты" очень не любят 😂

Да, давай тогда этот кейс сделай параметром "Разрешить порционные циклы" - по умолчанию "истина", тогда кому не надо такое - тот отключит. Но в этом случае надо как-то очень точно анализировать что запрос именно "порционный", чтобы не было ложных срабатываний. Эти кейсы надо бы в документации четко описать - чтобы не было вопросов у разработчика, почему тут ошибка не нашлась.

@marmyshev
Copy link
Collaborator

Было бы очень неплохо - в реквесте публиковать сколько в типовых конфигурациях (ЗУП, БП, ERP итд) нашлось таких ошибок.

  1. С одной стороны: сразу понятна полезность проверки
  2. С другой стороны, если ошибок тысячи/сотнитысяч - то вероятность, что что-то не так в проверке, на это обратить внимание. В любом случае - такая проверка бесполезна при включении по-умолчанию, т.к. столько ошибок никто в здравом уме править не будет по умолчанию.

@DoublesunRUS
Copy link
Contributor Author

Да, давай тогда этот кейс сделай параметром "Разрешить порционные циклы" - по умолчанию "истина", тогда кому не надо такое - тот отключит. Но в этом случае надо как-то очень точно анализировать что запрос именно "порционный", чтобы не было ложных срабатываний. Эти кейсы надо бы в документации четко описать - чтобы не было вопросов у разработчика, почему тут ошибка не нашлась.

Поэтому мне и кажется что надо разрешать бесконечные циклы "Пока Истина Цикл". Так как в них явно предусмотрен выход по условию.

@DoublesunRUS
Copy link
Contributor Author

Было бы очень неплохо - в реквесте публиковать сколько в типовых конфигурациях (ЗУП, БП, ERP итд) нашлось таких ошибок.

  1. С одной стороны: сразу понятна полезность проверки
  2. С другой стороны, если ошибок тысячи/сотнитысяч - то вероятность, что что-то не так в проверке, на это обратить внимание. В любом случае - такая проверка бесполезна при включении по-умолчанию, т.к. столько ошибок никто в здравом уме править не будет по умолчанию.

С одной стороны 1С:Специалист по платформе не получить при одном запросе в цикле. А с другой выключим проверку по умолчанию, чтобы разработчики типовых не сильно от этого расстраивались. :)

@marmyshev
Copy link
Collaborator

Да, давай тогда этот кейс сделай параметром "Разрешить порционные циклы" - по умолчанию "истина", тогда кому не надо такое - тот отключит. Но в этом случае надо как-то очень точно анализировать что запрос именно "порционный", чтобы не было ложных срабатываний. Эти кейсы надо бы в документации четко описать - чтобы не было вопросов у разработчика, почему тут ошибка не нашлась.

Поэтому мне и кажется что надо разрешать бесконечные циклы "Пока Истина Цикл". Так как в них явно предусмотрен выход по условию.

Просто "бесконечный цикл с запросом" - это ошибка. А "порционный бесконечный цикл" - это что-то другое, но как разделять - я об этом как раз писал.

Если ты предлагаешь не проверять просто "бесконечные циклы с запросом" (если порционность слишком сложно или дорого определять) - то это быстро сделать - можно пойти супер-простым путем - параметр называть "Проверять запросы в бесконечных циклах" (по умолчанию "ложь").

@DoublesunRUS
Copy link
Contributor Author

Типовой ЗУП суммарно содержит 20 тыс. ошибок и предупреждений. Будет теперь не 20 тыс., а 21 тыс. Я предлагаю для разработчиков типовых ничего не отключать. Пусть используют новый гибкий механизм подавления ошибок чтобы подавлять проверки в тех объектах, над которыми они не работают.

А вот тем кто только начинает новые проекты в EDT, надо включить все проверки. Чтобы они сразу разрабатывали правильно.

@marmyshev
Copy link
Collaborator

Пусть используют новый гибкий механизм подавления ошибок чтобы

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

@DoublesunRUS
Copy link
Contributor Author

Пусть используют новый гибкий механизм подавления ошибок чтобы

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

Я говорю о том, что сейчас в типовой конфигурации больше 20 тыс. ошибок. И если разработчики типовой не собираются их править, то они их уже как-то выключили. Ровно таким же образом им нужно будет выключать и новые проверки. Я считаю неправильным поставлять именно эту проверку выключенной по умолчанию, так как не вижу чем она отличается от десятка других.

@marmyshev
Copy link
Collaborator

Пусть используют новый гибкий механизм подавления ошибок чтобы

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

Я говорю о том, что сейчас в типовой конфигурации больше 20 тыс. ошибок. И если разработчики типовой не собираются их править, то они их уже как-то выключили. Ровно таким же образом им нужно будет выключать и новые проверки. Я считаю неправильным поставлять именно эту проверку выключенной по умолчанию, так как не вижу чем она отличается от десятка других.

Понял мысль, но кажется есть недопонимание.

  1. Я не предлагаю поставлять эту проверку выключенной совсем
  2. Я предлагаю ее параметризировать - чтобы была разного рода гибкость в настройке
  3. Я предлагаю значения параметров этой проверки - по умолчанию делать таким образом - чтобы массовые срабатывания (которые могут быть ложно-положительными) были по умолчанию отключены в параметрах, чтобы проверка в своем базовом варианте находила только самые достоверные ошибки.

количество текущих легаси-ошибок в типовых - в этих вопросах (имхо) не должно быть аргументом за то чтобы НЕ делать параметры в этой проверке.

@marmyshev
Copy link
Collaborator

Еще, тут прокомментирую, но наверное можно делать в рамках отдельной задачи.

бывают какие-то типовые процедуры которые выполняют запросы, лежащие в общем модуле: например ОбщегоНазначения.ЗначениеРеквизитаОбъекта() и прочие.

Поэтому было бы неплохо, указывать в параметрах вызовы каких-то общих процедур специфичных для проекта - список процедур через запятую, что считать вызовом запроса.

@marmyshev
Copy link
Collaborator

@DoublesunRUS По скольку проверка тяжелая и потенциально опасная (ввиду тысяч/сотентысяч срабатываний) предлагаю пока временно ее поставлять отключенной. Почему:

  1. Сможем ее залить сейчас и протестировать на типовых конфигурациях ее производительность, может придумаем как оптимизировать
  2. Проекты типа ERP смогут сами осознанно сейчас ее включать, если хотят. Т.к. щас нет возможности легко отфильтровать легаси-код и код библиотек - это будет полезно.
  3. В будущем, можем ее включить по умолчанию, если всё будет хорошо.

@marmyshev marmyshev merged commit 4b9bf95 into 1C-Company:master Aug 11, 2021
@DoublesunRUS DoublesunRUS deleted the feature/issue-17-query-in-loop branch August 11, 2021 13:37
@marmyshev marmyshev added this to the 0.1 для EDT 2021.2 milestone Oct 1, 2021
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