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

G5V8DT-24086 Ложные срабатывания проверки The asynchronous method is followed by lines of code #1408

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

MaksimDzyuba
Copy link
Collaborator

Проверка "code-after-async-call" выключена по умолчанию. Сделал это по следующим причинам:

  1. в стандартах есть одобренные примеры кода, которые приводит к возникновению данной ошибки - см. https://its.1c.ru/db/v8std#content:703:hdoc. И такого кода реально много, в одной БСП более 30 вхождений.
  2. кажется, что эти ошибки полезны только тем, кто переходит от синхронной работы к асинхронной, но на сколько мне известно, такие работы уже выполнены в большинстве конфигураций, просьба поправить, если не прав.

Поэтому предлагаю данную ошибку не включать по умолчанию.

@MaksimDzyuba MaksimDzyuba changed the base branch from master to edt-2023-3 February 6, 2024 15:21
@marmyshev
Copy link
Collaborator

@MaksimDzyuba Я тут не соглашусь что проверка полезна только тем кто переходит на асинхрон:

  1. когда код пишешь можно просто забыть и оставить код после вызова асинхронного метода.
  2. Так же платформа не дает четко понять какая ветка кода будет исполняться первая: синхронная (после вызова метода) или асинхронная - ответ для разработчиков может быть неожиданный, т.к. Клиент однопоточный. 😁

Т.е. практика оставлять код после асинх-метода - порочная и невозможно отделить всякий мусор который не захотели перенести на 2 строчки выше (типа СтандартнаяОбработка=Ложь;), от реально забытого кода.

Тут полезнее было бы сделать 2 квикфикса:

  1. Удалить код после асинх-метода
  2. Перенести код выше асинх-метода

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.

Нужно ещё в историю изменений запись добавить об отключении по умолчанию

@@ -101,6 +101,7 @@ protected void configureCheck(CheckConfigurer builder)
.complexity(CheckComplexity.NORMAL)
.severity(IssueSeverity.MAJOR)
.issueType(IssueType.WARNING)
.disable()
.extension(new CommonSenseCheckExtension(getCheckId(), BslPlugin.PLUGIN_ID))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Нужно убрать проверку из профиля общего смысла, потому что enablement переопределяется этим профилем.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

сделал

Copy link
Collaborator

@tretyakevich tretyakevich left a comment

Choose a reason for hiding this comment

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

Да, присоединяюсь к @marmyshev по поводу настроек профилей проверок (доп. страничка визарда создания проекта). В остальном вопросов нет.

@MaksimDzyuba
Copy link
Collaborator Author

Нужно ещё в историю изменений запись добавить об отключении по умолчанию

Сделал

@MaksimDzyuba
Copy link
Collaborator Author

@MaksimDzyuba Я тут не соглашусь что проверка полезна только тем кто переходит на асинхрон:

  1. когда код пишешь можно просто забыть и оставить код после вызова асинхронного метода.
  2. Так же платформа не дает четко понять какая ветка кода будет исполняться первая: синхронная (после вызова метода) или асинхронная - ответ для разработчиков может быть неожиданный, т.к. Клиент однопоточный. 😁

Т.е. практика оставлять код после асинх-метода - порочная и невозможно отделить всякий мусор который не захотели перенести на 2 строчки выше (типа СтандартнаяОбработка=Ложь;), от реально забытого кода.
да, согласен, но как это проверять, если ниже просто код, который логикой не связан с тем, что делается в отложенном вызове, всегда его переносить вверх, чтобы просто не показывалась ошибка, как-то тоже странно, то есть даже если человек себе отдает отчет, что он делает, он должен писать код, как ему велит проверка, это не много странно все равно

Тут полезнее было бы сделать 2 квикфикса:

  1. Удалить код после асинх-метода
  2. Перенести код выше асинх-метода
    С этим согласен, отдельно можно сделать.

@AlmazNasibullin AlmazNasibullin merged commit a6bfff7 into edt-2023-3 Feb 15, 2024
2 of 4 checks passed
@AlmazNasibullin AlmazNasibullin deleted the G5V8DT-24086 branch February 15, 2024 06:41
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.

None yet

4 participants