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 13 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,54 @@
# Код расположен после асинхронного вызова

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

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

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

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

```bsl
ПоказатьПредупреждение(,ТекстПредупреждения);
Отказ = Истина;
```

## Правильно

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

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

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

```bsl
Отказ = Истина;
ПоказатьПредупреждение(,ТекстПредупреждения);
```

## См.

- [Синхронные и асинхронные методы работы](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,34 @@
/*******************************************************************************
* 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;

import java.util.Collection;
import java.util.Map;

import com._1c.g5.v8.dt.platform.version.Version;

/**
* Platform context asynchronic methods provider
*
* @author Artem Iliukhin
*/
public interface IAsyncInvocationProvider
{

// Global context methods
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
Collection<String> getAsyncInvocationNames(Version version);

// Methods with a list of types in which they are used
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
Map<String, Collection<String>> getAsyncTypeMethodNames(Version version);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
/*******************************************************************************
* 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 java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.xtext.EcoreUtil2;
import org.eclipse.xtext.naming.IQualifiedNameConverter;

import com._1c.g5.v8.dt.bsl.common.IBslPreferences;
import com._1c.g5.v8.dt.bsl.model.BslPackage;
import com._1c.g5.v8.dt.bsl.model.Conditional;
import com._1c.g5.v8.dt.bsl.model.DynamicFeatureAccess;
import com._1c.g5.v8.dt.bsl.model.EmptyStatement;
import com._1c.g5.v8.dt.bsl.model.Expression;
import com._1c.g5.v8.dt.bsl.model.FeatureAccess;
import com._1c.g5.v8.dt.bsl.model.IfStatement;
import com._1c.g5.v8.dt.bsl.model.Invocation;
import com._1c.g5.v8.dt.bsl.model.LoopStatement;
import com._1c.g5.v8.dt.bsl.model.PreprocessorConditional;
import com._1c.g5.v8.dt.bsl.model.PreprocessorItemStatements;
import com._1c.g5.v8.dt.bsl.model.ReturnStatement;
import com._1c.g5.v8.dt.bsl.model.SimpleStatement;
import com._1c.g5.v8.dt.bsl.model.Statement;
import com._1c.g5.v8.dt.bsl.model.StaticFeatureAccess;
import com._1c.g5.v8.dt.bsl.model.TryExceptStatement;
import com._1c.g5.v8.dt.core.platform.IResourceLookup;
import com._1c.g5.v8.dt.platform.version.IRuntimeVersionSupport;
import com._1c.g5.v8.dt.platform.version.Version;
import com._1c.g5.wiring.ServiceAccess;
import com.e1c.g5.v8.dt.check.CheckComplexity;
import com.e1c.g5.v8.dt.check.ICheckParameters;
import com.e1c.g5.v8.dt.check.components.BasicCheck;
import com.e1c.g5.v8.dt.check.settings.IssueSeverity;
import com.e1c.g5.v8.dt.check.settings.IssueType;
import com.e1c.v8codestyle.bsl.IAsyncInvocationProvider;
import com.e1c.v8codestyle.check.CommonSenseCheckExtension;
import com.e1c.v8codestyle.internal.bsl.BslPlugin;
import com.google.common.collect.Lists;
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 BasicCheck
{

private static final String STATEMENT_NAME_RU = "Ждать"; //$NON-NLS-1$
private static final String STATEMENT_NAME = "Await"; //$NON-NLS-1$
private static final String CHECK_ID = "code-after-async-call"; //$NON-NLS-1$
private final IResourceLookup resourceLookup;
private final IAsyncInvocationProvider asyncInvocationProvider;

@Inject
public CodeAfterAsyncCallCheck(IResourceLookup resourceLookup, IBslPreferences bslPreferences,
IQualifiedNameConverter qualifiedNameConverter, IAsyncInvocationProvider asyncInvocationProvider)
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
{
super();
this.resourceLookup = resourceLookup;
this.asyncInvocationProvider = asyncInvocationProvider;
}

@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)
.extension(new CommonSenseCheckExtension(getCheckId(), BslPlugin.PLUGIN_ID))
.module()
.checkedObjectType(INVOCATION);
}

@Override
protected void check(Object object, ResultAcceptor resultAceptor, ICheckParameters parameters,
IProgressMonitor monitor)
{

IProject project = resourceLookup.getProject((EObject)object);

IRuntimeVersionSupport runtimeVersionSupport = ServiceAccess.get(IRuntimeVersionSupport.class);
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
Version version = runtimeVersionSupport.getRuntimeVersion(project);

Invocation inv = (Invocation)object;
FeatureAccess featureAccess = inv.getMethodAccess();
if (featureAccess instanceof StaticFeatureAccess)
{
Collection<String> asyncMethodsNames = asyncInvocationProvider.getAsyncInvocationNames(version);
if (asyncMethodsNames.contains(featureAccess.getName()))
{
addIssue(resultAceptor, inv);
}
}
else if (featureAccess instanceof DynamicFeatureAccess)
{
Map<String, Collection<String>> names = asyncInvocationProvider.getAsyncTypeMethodNames(version);
if (names.containsKey(featureAccess.getName()))
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
{
addIssue(resultAceptor, inv);
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

private void addIssue(ResultAcceptor resultAceptor, Invocation inv)
{
Statement statement = getStatementFromInvoc(inv);
if (statement != null && isPreviousStatementAwait(statement))
{
statement = getNextStatement(statement);
if (statement != null && !(statement instanceof ReturnStatement)
&& !(statement instanceof EmptyStatement))
{
resultAceptor.addIssue(Messages.CodeAfterAsyncCallCheck_Issue, statement);
}
}
}

private Statement getStatementFromInvoc(Invocation invocation)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Можно вместо этого метода использовать метод EcoreUtil2.getContainerOfType(EObject, Class<T>).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, только в данном случае получается что неизвестно какой там тип будет

{
EObject container = invocation.eContainer();
while (!(container instanceof Statement))
{
container = container.eContainer();
}
return container instanceof Statement ? (Statement)container : null;
}

private boolean isPreviousStatementAwait(Statement statement)
{
Iterator<EObject> it = EcoreUtil2.getAllContainers(statement).iterator();
iArtemv marked this conversation as resolved.
Show resolved Hide resolved
while (it.hasNext())
{
EObject container = it.next();
if (container instanceof PreprocessorConditional)
{
continue;
}
List<Statement> st = getContainer(container);
if (st != null)
{
int index = st.indexOf(statement);
if (index > 0 && index - 1 < st.size())
{
Statement awaitStatement = st.get(index - 1);
if (awaitStatement instanceof SimpleStatement)
{
Expression left = ((SimpleStatement)awaitStatement).getLeft();
if (left instanceof StaticFeatureAccess)
{
String name = ((StaticFeatureAccess)left).getName();
if (STATEMENT_NAME.equalsIgnoreCase(name) || STATEMENT_NAME_RU.equalsIgnoreCase(name))
{
return true;
}
}
}

return awaitStatement != null;
}
}
}
return false;
}

private Statement getNextStatement(Statement statement)
{
Iterator<EObject> it = EcoreUtil2.getAllContainers(statement).iterator();
while (it.hasNext())
{
EObject container = it.next();
if (container instanceof PreprocessorConditional)
{
continue;
}
List<Statement> st = getContainer(container);
if (st != null)
{
int index = st.indexOf(statement);
if (index != -1 && index + 1 < st.size())
{
return st.get(index + 1);
}
}
}
return null;
}

private List<Statement> getContainer(EObject container)
{
List<Statement> st = null;
if (container instanceof LoopStatement)
{
st = ((LoopStatement)container).getStatements();
}
else if (container instanceof Conditional)
{
st = ((Conditional)container).getStatements();
}
else if (container instanceof IfStatement)
{
st = ((IfStatement)container).getElseStatements();
}
else if (container instanceof TryExceptStatement)
{
st = getStatementsFromContainer((TryExceptStatement)container);
}
else if (container instanceof PreprocessorItemStatements)
{
st = ((PreprocessorItemStatements)container).getStatements();
}
else
{
st = getStatementsFromContainer(container);
}
return st;
}

private List<Statement> getStatementsFromContainer(TryExceptStatement container)
{
List<Statement> res = Lists.newArrayList();
res.addAll(container.getTryStatements());
res.addAll(container.getExceptStatements());
return res;
}

@SuppressWarnings("unchecked")
private List<Statement> getStatementsFromContainer(EObject container)
{
Object obj = container.eGet(BslPackage.Literals.BLOCK__STATEMENTS);
return obj instanceof List ? (List<Statement>)obj : null;
}
}
Loading