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

#1224 Проверяет отсутствие кода после асинхронного вызова #1234

Merged
merged 28 commits into from
Apr 16, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
642868e
Проверяет отсутствие кода после асинхронного вызова
iArtemv Dec 9, 2022
24fcc00
Merge remote-tracking branch 'origin/master' into feature/issue-1224-…
iArtemv Jan 23, 2023
c453d38
Merge remote-tracking branch 'origin/master' into feature/issue-1224-…
iArtemv Jan 24, 2023
4b513d4
Исправление замечаний cr
iArtemv Jan 26, 2023
80e93f7
Исправление по замечаниям
iArtemv Mar 7, 2023
00ccad7
Исправление ошибок
iArtemv Mar 7, 2023
5b10563
Исправление потокобезопасности
iArtemv Mar 9, 2023
7b2ef3f
Исправление имени типа
iArtemv Mar 9, 2023
b0df18b
Исправление учтен оператор Ждать
iArtemv Mar 10, 2023
2967576
Добавлен тест
iArtemv Mar 10, 2023
7ca2399
Исправления
iArtemv Mar 10, 2023
0b53010
Исправление
iArtemv Mar 11, 2023
53b4341
Добавлен пример в документацию
iArtemv Mar 13, 2023
1c5d333
Merge remote-tracking branch 'origin/master' into
iArtemv Mar 13, 2023
ddda9f5
Исправление замечаний CR
iArtemv Mar 14, 2023
ecefabd
Добавлен параметр в настройку проверки
iArtemv Mar 14, 2023
968ddaa
Уточнен комментарий
iArtemv Mar 14, 2023
824d327
Исправление проверки типа
iArtemv Mar 14, 2023
3b67312
Merge remote-tracking branch 'origin/master' into
iArtemv Mar 14, 2023
b26b76e
Исправление форматирования
iArtemv Mar 14, 2023
d4beb5e
Исправление тестов
iArtemv Mar 15, 2023
f42d46a
Исправление дублей
iArtemv Mar 15, 2023
64ae81b
Исправление имени
iArtemv Mar 15, 2023
fbbb6ba
Исправление по cr
iArtemv Mar 20, 2023
d6c0932
Исправление по замечаниям cr
iArtemv Mar 22, 2023
24ad506
Исправление замечаний и добавление тестов
iArtemv Mar 22, 2023
89186b2
Merge remote-tracking branch 'origin/master' into
iArtemv Mar 22, 2023
600bc10
Исправление форматирования файла свойств
iArtemv Mar 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
- Проверка наличия префикса расширения в методе расширения.
- Устаревшая процедура (функция) расположена вне области "УстаревшиеПроцедурыИФункции"
- Использован обработчик событий, подключаемый из кода и не содержащий префикса "Подключаемый_"
- Проверка отсутствия кода после асинхронного вызова
iArtemv marked this conversation as resolved.
Show resolved Hide resolved

#### Запросы

Expand Down
24 changes: 24 additions & 0 deletions bundles/com.e1c.v8codestyle.bsl/markdown/code-after-async-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# The asynchronous method is not followed by lines of code

In the asynchronous approach the method is called as usual, but control returns to the caller before the asynchronous
method is completed. After that, execution of the caller continues.

## Noncompliant Code Example

```bsl
Text = "Warning text";
ShowMessageBox( , Text);
Message("Warning is closed");
```

## Compliant Solution

```bsl
Text = "Warning text";
Await DoMessageBoxAsync(Text);
Message("Warning is closed");
```

## See

- [Synchronous and asynchronous operations](https://kb.1ci.com/1C_Enterprise_Platform/Guides/Developer_Guides/1C_Enterprise_8.3.19_Developer_Guide/Chapter_4._1C_Enterprise_language/4.7._Queries/4.7.9._Synchronous_and_asynchronous_operations/)
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Код расположен после асинхронного вызова

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

Для правильного решения нужно вынести весь код, который должен быть выполнен после выполнения асинхронного действия,
в экспортный метод и указать его имя в обработке оповещения, которая будет вызвана после завершения асинхронного действия.
Или использовать асинхронность через обещания, например, Ждать ПредупреждениеАсинх(Текст).

## Неправильно

```bsl
Текст = "Текст предупреждения";
ПоказатьПредупреждение( , Текст);
Сообщить("Закрыли предупреждение");
```

## Правильно

```bsl
Текст = "Текст предупреждения";
Ждать ПредупреждениеАсинх(Текст);
Сообщить("Закрыли предупреждение");
```

```bsl
&НаКлиенте
Процедура Команда1(Команда)
Оповещение = Новый ОписаниеОповещения("ПредупреждениеЗавершение", ЭтотОбъект);
Текст = "Текст предупреждения";
ПоказатьПредупреждение(Оповещение, Текст);
КонецПроцедуры

&НаКлиенте
Процедура ПредупреждениеЗавершение(ДополнительныеПараметры) Экспорт
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
Сообщить("Закрыли предупреждение");
КонецПроцедуры;
```

## См.

- [Синхронные и асинхронные методы работы](https://its.1c.ru/db/v8319doc#bookmark:dev:TI000001505)
4 changes: 4 additions & 0 deletions bundles/com.e1c.v8codestyle.bsl/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@
category="com.e1c.v8codestyle.bsl"
class="com.e1c.v8codestyle.internal.bsl.ExecutableExtensionFactory:com.e1c.v8codestyle.bsl.check.DeprecatedProcedureOutsideDeprecatedRegionCheck">
</check>
<check
category="com.e1c.v8codestyle.bsl"
class="com.e1c.v8codestyle.internal.bsl.ExecutableExtensionFactory:com.e1c.v8codestyle.bsl.check.CodeAfterAsyncCallCheck">
</check>
</extension>
<extension
point="org.eclipse.core.runtime.preferences">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*******************************************************************************
* Copyright (C) 2023, 1C-Soft LLC and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* 1C-Soft LLC - initial API and implementation
*******************************************************************************/
package com.e1c.v8codestyle.bsl.check;

import static com._1c.g5.v8.dt.bsl.model.BslPackage.Literals.INVOCATION;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.xtext.naming.IQualifiedNameConverter;

import com._1c.g5.v8.dt.bsl.common.IBslPreferences;
import com._1c.g5.v8.dt.bsl.model.EmptyStatement;
import com._1c.g5.v8.dt.bsl.model.FeatureAccess;
import com._1c.g5.v8.dt.bsl.model.Invocation;
import com._1c.g5.v8.dt.bsl.model.ReturnStatement;
import com._1c.g5.v8.dt.bsl.model.Statement;
import com._1c.g5.v8.dt.bsl.model.StaticFeatureAccess;
import com._1c.g5.v8.dt.core.platform.IResourceLookup;
import com.e1c.g5.v8.dt.check.CheckComplexity;
import com.e1c.g5.v8.dt.check.ICheckParameters;
import com.e1c.g5.v8.dt.check.settings.IssueSeverity;
import com.e1c.g5.v8.dt.check.settings.IssueType;
import com.e1c.v8codestyle.check.CommonSenseCheckExtension;
import com.e1c.v8codestyle.internal.bsl.BslPlugin;
import com.google.inject.Inject;

/**
* Checks that the asynchronous method is not followed by lines of code,
* since in this case the specified lines of code are executed immediately,
* without waiting for the asynchronous method to execute.
*
* @author Artem Iliukhin
*/
public final class CodeAfterAsyncCallCheck
extends AbstractTransactionCheck
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
{

private static final String CHECK_ID = "code-after-async-call"; //$NON-NLS-1$

// @formatter:off
private static final String[] ASYNCHRONOUS_METHODS = {
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
"ShowMessageBox", "ПоказатьПредупреждение", //$NON-NLS-1$ //$NON-NLS-2$
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

Можно по контексту платформы собрать все методы у которых есть ОписаниеОповещения, или которые заканчиваются на Async - вынести это в отдельный Кэш-провайдер который по-требованию будут собирать из контекста по версии проверка и кэшировать на время вызова.

Не следует так же забывать про существование асинхронных методов у объектов платформы.

interface AsyncInvocationProvider
{

   // Методы глобального контекста
   Collection<String> getAsyncInvocationNames(Version version);

   // Методы со списком типов в которых они используются
   Map<String, Collection<String>> getAsyncTypeMethodNames(Version version);

}

Согласен с @AlmazNasibullin что асинхронные методы появляются в каждой новой версии платформы.

"ShowInputDate", "ПоказатьВводДаты", //$NON-NLS-1$ //$NON-NLS-2$
"ShowInputValue", "ПоказатьВводЗначения", //$NON-NLS-1$ //$NON-NLS-2$
"ShowInputString", "ПоказатьВводСтроки", //$NON-NLS-1$ //$NON-NLS-2$
"ShowInputNumber", "ПоказатьВводЧисла", //$NON-NLS-1$ //$NON-NLS-2$
"ShowQueryBox", "ПоказатьВопрос", //$NON-NLS-1$ //$NON-NLS-2$
"BeginRequestingUserPermission", "НачатьЗапросРазрешенияПользователя", //$NON-NLS-1$ //$NON-NLS-2$
"BeginRunningApplication", "НачатьЗапускПриложения", //$NON-NLS-1$ //$NON-NLS-2$
"BeginGettingTempFilesDir", "НачатьПолучениеКаталогаВременныхФайлов", //$NON-NLS-1$ //$NON-NLS-2$
"BeginGettingDocumentsDir", "НачатьПолучениеКаталогаДокументов", //$NON-NLS-1$ //$NON-NLS-2$
"BeginCopyingFile", "НачатьКопированиеФайла", //$NON-NLS-1$ //$NON-NLS-2$
"BeginFindingFiles", "НачатьПоискФайлов", //$NON-NLS-1$ //$NON-NLS-2$
"ShowValue", "ПоказатьЗначение", //$NON-NLS-1$ //$NON-NLS-2$
"OpenForm", "ОткрытьФорму", //$NON-NLS-1$ //$NON-NLS-2$
"BeginMovingFile", "НачатьПеремещениеФайла", //$NON-NLS-1$ //$NON-NLS-2$
"BeginAttachingCryptoExtension", "НачатьПодключениеРасширенияРаботыСКриптографией", //$NON-NLS-1$ //$NON-NLS-2$
"BeginAttachingFileSystemExtension", "НачатьПодключениеРасширенияРаботыСФайлами", //$NON-NLS-1$ //$NON-NLS-2$
"BeginGetFilesFromServer", "НачатьПолучениеФайловССервера", //$NON-NLS-1$ //$NON-NLS-2$
"BeginPutFileToServer", "НачатьПомещениеФайлаНаСервер", //$NON-NLS-1$ //$NON-NLS-2$
"BeginPutFilesToServer", "НачатьПомещениеФайловНаСервер", //$NON-NLS-1$ //$NON-NLS-2$
"BeginGettingUserDataWorkDir", "НачатьПолучениеРабочегоКаталогаДанныхПользователя", //$NON-NLS-1$ //$NON-NLS-2$
"BeginCreatingDirectory", "НачатьСозданиеКаталога", //$NON-NLS-1$ //$NON-NLS-2$
"BeginInstallAddIn", "НачатьУстановкуВнешнейКомпоненты", //$NON-NLS-1$ //$NON-NLS-2$
"BeginInstallCryptoExtension", "НачатьУстановкуРасширенияРаботыСКриптографией", //$NON-NLS-1$ //$NON-NLS-2$
"BeginInstallFileSystemExtension", "НачатьУстановкуРасширенияРаботыСФайлами"}; //$NON-NLS-1$ //$NON-NLS-2$
// @formatter:on

@Inject
public CodeAfterAsyncCallCheck(IResourceLookup resourceLookup, IBslPreferences bslPreferences,
IQualifiedNameConverter qualifiedNameConverter)
{
super();
}

@Override
public String getCheckId()
{
return CHECK_ID;
}

@Override
protected void configureCheck(CheckConfigurer builder)
{
builder.title(Messages.CodeAfterAsyncCallCheck_Title)
.description(Messages.CodeAfterAsyncCallCheck_Description)
.complexity(CheckComplexity.NORMAL)
.severity(IssueSeverity.MAJOR)
.issueType(IssueType.WARNING)
.disable()
.extension(new CommonSenseCheckExtension(getCheckId(), BslPlugin.PLUGIN_ID))
.module()
.checkedObjectType(INVOCATION);
}

@Override
protected void check(Object object, ResultAcceptor resultAceptor, ICheckParameters parameters,
IProgressMonitor monitor)
{
Invocation inv = (Invocation)object;
FeatureAccess featureAccess = inv.getMethodAccess();
if (featureAccess instanceof StaticFeatureAccess)
{
if (monitor.isCanceled())
{
return;
}
iArtemv marked this conversation as resolved.
Show resolved Hide resolved

String featureName = featureAccess.getName();
for (int i = 0; i < ASYNCHRONOUS_METHODS.length; i++)
{
if (ASYNCHRONOUS_METHODS[i].equalsIgnoreCase(featureName))
{
Statement statement = getStatementFromInvoc(inv);
if (statement != null)
{
statement = getNextStatement(statement);
if (statement != null && !(statement instanceof ReturnStatement)
&& !(statement instanceof EmptyStatement))
{
resultAceptor.addIssue(Messages.CodeAfterAsyncCallCheck_Issue, statement);
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ final class Messages
public static String ChangeAndValidateInsteadOfAroundCheck_Use_ChangeAndValidate_instead_of_Around;
public static String ChangeAndValidateInsteadOfAroundCheck_title;

public static String CodeAfterAsyncCallCheck_Description;

public static String CodeAfterAsyncCallCheck_Issue;

public static String CodeAfterAsyncCallCheck_Title;

public static String CommitTransactionCheck_Commit_transaction_must_be_in_try_catch;

public static String CommitTransactionCheck_No_begin_transaction_for_commit_transaction;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ ChangeAndValidateInsteadOfAroundCheck_description = Checks that pragma &ChangeAn

ChangeAndValidateInsteadOfAroundCheck_title = Use pragma &ChangeAndValidate instead of &Around

CodeAfterAsyncCallCheck_Description=Checks that the asynchronous method is not followed by lines of code, since in this case the specified lines of code are executed immediately, without waiting for the asynchronous method to execute

CodeAfterAsyncCallCheck_Issue=The asynchronous method is followed by lines of code

CodeAfterAsyncCallCheck_Title=The code should not follow an asynchronous call

CommitTransactionCheck_Commit_transaction_must_be_in_try_catch=Commit transaction must be in a try-catch

CommitTransactionCheck_No_begin_transaction_for_commit_transaction=There is no begin transaction for commit transaction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ ChangeAndValidateInsteadOfAroundCheck_description = Проверяет, что

ChangeAndValidateInsteadOfAroundCheck_title = Используется аннотация &ИзменениеИКонтроль вместо &Вместо

CodeAfterAsyncCallCheck_Description=Проверяет, что за асинхронным методом не следуют строки кода, поскольку в этом случае указанные строки кода выполняются немедленно, не дожидаясь выполнения асинхронного метода

CodeAfterAsyncCallCheck_Issue=За асинхронным методом следуют строки кода

CodeAfterAsyncCallCheck_Title=Код не должен следовать за асинхронным вызовом

CommitTransactionCheck_Commit_transaction_must_be_in_try_catch=Вызов "ЗафиксироватьТранзакцию()" находится вне конструкции "Попытка... Исключение"

CommitTransactionCheck_No_begin_transaction_for_commit_transaction=Отсутствует вызов "НачатьТранзакцию()", хотя вызываются "ЗафиксироватьТранзакцию()"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

&НаКлиенте
Процедура Тест()

Оповещение = Новый ОписаниеОповещения("ПредупреждениеЗавершение", ЭтотОбъект);
Текст = "Текст предупреждения";
ПоказатьПредупреждение(Оповещение, Текст);

КонецПроцедуры

&НаКлиенте
Процедура ПредупреждениеЗавершение(ДополнительныеПараметры) Экспорт
iArtemv marked this conversation as resolved.
Show resolved Hide resolved

Сообщить("Закрыли предупреждение");

КонецПроцедуры;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

&НаКлиенте
Процедура Тест()

Текст = "Текст предупреждения";
Ждать ПредупреждениеАсинх(Текст);
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
Сообщить("Закрыли предупреждение");

КонецПроцедуры
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

&НаКлиенте
Процедура Тест()

Текст = "Текст предупреждения";
ПоказатьПредупреждение(, Текст);
Сообщить("Закрыли предупреждение");

КонецПроцедуры
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*******************************************************************************
* Copyright (C) 2023, 1C-Soft LLC and others.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* 1C-Soft LLC - initial API and implementation
*******************************************************************************/
package com.e1c.v8codestyle.bsl.check.itests;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;

import org.junit.Test;

import com._1c.g5.v8.dt.validation.marker.IExtraInfoKeys;
import com._1c.g5.v8.dt.validation.marker.Marker;
import com.e1c.v8codestyle.bsl.check.CodeAfterAsyncCallCheck;

/**
* Tests for {@link CodeAfterAsyncCallCheck} check.
*
* @author Artem Iliukhin
*/
public class CodeAfterAsyncCallCheckTest
extends AbstractSingleModuleTestBase
{
private static final String CHECK_ID = "code-after-async-call"; //$NON-NLS-1$

/**
* Instantiates a new code after async call check test.
*/
public CodeAfterAsyncCallCheckTest()
{
super(CodeAfterAsyncCallCheck.class);
}

@Test
public void testCodeAfterExistence() throws Exception
{
updateModule(FOLDER_RESOURCE + "code-after-async-call-existence.bsl");

Marker marker = getFirstMarker(CHECK_ID, getModuleId(), getProject());
assertNotNull(marker);
assertEquals("7", marker.getExtraInfo().get(IExtraInfoKeys.TEXT_EXTRA_INFO_LINE_KEY));
}

@Test
public void testCallBackDescriptionCompliant() throws Exception
{
updateModule(FOLDER_RESOURCE + "async-call-back-descr.bsl");

Marker marker = getFirstMarker(CHECK_ID, getModuleId(), getProject());
assertNull(marker);
}

@Test
public void testPromiseCompliant() throws Exception
{
updateModule(FOLDER_RESOURCE + "async-call-promise.bsl");

Marker marker = getFirstMarker(CHECK_ID, getModuleId(), getProject());
assertNull(marker);
}
}