Между "НачатьТранзакцию()" и "Попытка" есть исполняемый код (#1063)

CHANGELOG.md

@@ -37,6 +37,9 @@
     2. Отсутствует вызов "НачатьТранзакцию()", хотя вызываются "ОтменитьТранзакцию()"
     3. Для вызова "НачатьТранзакцию()" отсутствует парный вызов "ЗафиксироватьТранзакцию()"
     4. Mежду "Исключение" и "ОтменитьТранзакцию()" есть исполняемый код, который может вызвать исключение
+- Проверка нарушения схемы работы с транзакциями связанной с началом транзакции:
+    1. Mежду "НачатьТранзакцию()" и "Попытка" есть исполняемый код, который может вызвать исключение
+    2. Не найден оператор "Попытка" после вызова "НачатьТранзакцию()"
 - Отсутствует удаление временного файла после использования.
 
 #### Запросы

bundles/com.e1c.v8codestyle.bsl/markdown/begin-transaction.md

@@ -0,0 +1,31 @@
+# There is no Try-Exception block after the start of the transaction
+
+There should be no executable code between begin transaction and try,
+the try operator was not found after calling begin transaction
+
+## Noncompliant Code Example
+
+```bsl
+    BeginTransaction();
+    CommitTransaction();
+```
+
+## Compliant Solution
+
+```bsl
+    BeginTransaction();
+    Try
+    // ...
+    CommitTransaction();
+    Except
+    // ...
+    RollbackTransaction();
+    // ...
+    Raise;
+    EndTry;
+```
+
+## See
+
+- [Catching exceptions in code](https://support.1ci.com/hc/en-us/articles/360011002440-Catching-exceptions-in-code)
+- [Transactions: rules of use](https://support.1ci.com/hc/en-us/articles/360011121239-Transactions-rules-of-use)

bundles/com.e1c.v8codestyle.bsl/markdown/ru/begin-transaction.md

@@ -0,0 +1,31 @@
+# После начала транзакции отсуствует блок Попытка-Исключение
+
+Mежду "НачатьТранзакцию()" и "Попытка" есть исполняемый код, который может вызвать исключение,
+после "НачатьТранзакцию()" не найден оператор "Попытка"
+
+## Неправильно
+
+```bsl
+    НачатьТранзакцию();
+    ЗафиксироватьТранзакцию();
+```
+
+## Правильно
+
+```bsl
+    НачатьТранзакцию();
+    Попытка
+    // ...
+    ЗафиксироватьТранзакцию();
+    Исключение
+    // ...
+    ОтменитьТранзакцию();
+    // ...
+    ВызватьИсключение;
+    КонецПопытки;
+```
+
+## См.
+
+- [Перехват исключений в коде](https://its.1c.ru/db/v8std#content:499:hdoc:3.6)
+- [Транзакции: правила использования](https://its.1c.ru/db/v8std#content:783:hdoc:1.3)

bundles/com.e1c.v8codestyle.bsl/plugin.xml

@@ -267,6 +267,10 @@
             category="com.e1c.v8codestyle.bsl"
             class="com.e1c.v8codestyle.bsl.check.RollbackTransactionCheck">
       </check>
+      <check
+            category="com.e1c.v8codestyle.bsl"
+            class="com.e1c.v8codestyle.bsl.check.BeginTransactionCheck">
+      </check>
    </extension>
    <extension
          point="org.eclipse.core.runtime.preferences">

bundles/com.e1c.v8codestyle.bsl/src/com/e1c/v8codestyle/bsl/check/BeginTransactionCheck.java

@@ -0,0 +1,96 @@
+/*******************************************************************************
+ * Copyright (C) 2022, 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 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.Statement;
+import com._1c.g5.v8.dt.bsl.model.StaticFeatureAccess;
+import com._1c.g5.v8.dt.bsl.model.TryExceptStatement;
+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;
+
+/**
+ * The try operator was not found after calling begin transaction,
+ * there should be no executable code between begin transaction and try,
+ *
+ * @author Artem Iliukhin
+ */
+public final class BeginTransactionCheck
+    extends AbstractTransactionCheck
+{
+
+    private static final String CHECK_ID = "begin-transaction"; //$NON-NLS-1$
+
+    @Override
+    public String getCheckId()
+    {
+        return CHECK_ID;
+    }
+
+    @Override
+    protected void configureCheck(CheckConfigurer builder)
+    {
+        builder.title(Messages.BeginTransactionCheck_Begin_transaction_is_incorrect)
+            .description(Messages.BeginTransactionCheck_Try_must_be_after_begin)
+            .complexity(CheckComplexity.NORMAL)
+            .severity(IssueSeverity.MINOR)
+            .issueType(IssueType.WARNING)
+            .disable()
+            .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;
+            }
+
+            String nameFeature = featureAccess.getName();
+            if (!(BEGIN_TRANSACTION_RU.equalsIgnoreCase(nameFeature)
+                || BEGIN_TRANSACTION.equalsIgnoreCase(nameFeature)))
+            {
+                return;
+            }
+
+            Statement statement = getStatementFromInvoc(inv);
+            if (statement != null)
+            {
+                statement = getNextStatement(statement);
+                if (statement == null)
+                {
+                    resultAceptor.addIssue(Messages.BeginTransactionCheck_Try_was_not_found_after_calling_begin, inv);
+                }
+                else if (!(statement instanceof TryExceptStatement))
+                {
+                    resultAceptor.addIssue(
+                        Messages.BeginTransactionCheck_Executable_code_between_begin_transaction_and_try, inv);
+                }
+            }
+        }
+    }
+}

bundles/com.e1c.v8codestyle.bsl/src/com/e1c/v8codestyle/bsl/check/Messages.java

@@ -60,12 +60,20 @@ final class Messages
 
     public static String RollbackTransactionCheck_Should_be_no_executable_code_between_exception_and_rollback;
 
+    public static String BeginTransactionCheck_Executable_code_between_begin_transaction_and_try;
+
     public static String CommitTransactionCheck_Transaction_contains_empty_except;
 
     public static String CommitTransactionCheck_Transactions_is_broken;
 
     public static String CommitTransactionCheck_Transactions_is_broken_des;
 
+    public static String BeginTransactionCheck_Begin_transaction_is_incorrect;
+
+    public static String BeginTransactionCheck_Try_must_be_after_begin;
+
+    public static String BeginTransactionCheck_Try_was_not_found_after_calling_begin;
+
     public static String CommonModuleNamedSelfReferenceCheck_description;
 
     public static String CommonModuleNamedSelfReferenceCheck_issue;

bundles/com.e1c.v8codestyle.bsl/src/com/e1c/v8codestyle/bsl/check/messages.properties

@@ -54,12 +54,20 @@ CommitTransactionCheck_Should_be_no_executable_code_between_commit_and_exception
 
 RollbackTransactionCheck_Should_be_no_executable_code_between_exception_and_rollback=There should be no executable code between exception and rollback transaction
 
+BeginTransactionCheck_Executable_code_between_begin_transaction_and_try=There should be no executable code between begin transaction and try
+
 CommitTransactionCheck_Transaction_contains_empty_except=The transaction contains an empty exception block
 
 CommitTransactionCheck_Transactions_is_broken=Commit transaction is incorrect
 
 CommitTransactionCheck_Transactions_is_broken_des=Commit transaction must be in a try-catch, there should be no executable code between commit transaction and exception, there is no begin transaction for commit transaction, there is no rollback transaction for begin transaction.
 
+BeginTransactionCheck_Begin_transaction_is_incorrect=Begin transaction is incorrect
+
+BeginTransactionCheck_Try_must_be_after_begin=Try-catch must be after begin transaction
+
+BeginTransactionCheck_Try_was_not_found_after_calling_begin=The try operator was not found after calling begin transaction
+
 CommonModuleNamedSelfReferenceCheck_description=Excessive named self reference
 
 CommonModuleNamedSelfReferenceCheck_issue=Excessive named self reference

bundles/com.e1c.v8codestyle.bsl/src/com/e1c/v8codestyle/bsl/check/messages_ru.properties

@@ -46,14 +46,20 @@ CommitTransactionCheck_No_begin_transaction_for_commit_transaction=Отсутс
 
 CommitTransactionCheck_No_begin_transaction_for_rollback_transaction=Отсутствует вызов "НачатьТранзакцию()", хотя вызываются "ОтменитьТранзакцию()"
 
-CommitTransactionCheck_No_commit_transaction_for_begin_transaction=Для вызова "НачатьТранзакцию()" отсутствует парный вызов "ЗафиксироватьТранзакцию()"
-
 CommitTransactionCheck_No_rollback_transaction_for_begin_transaction=Для вызова "НачатьТранзакцию()" отсутствует парный вызов "ОтменитьТранзакцию()"
 
 CommitTransactionCheck_Should_be_no_executable_code_between_commit_and_exception=Mежду "ЗафиксироватьТранзакцию()" и "Исключение" есть исполняемый код, который может вызвать исключение
 
 CommitTransactionCheck_Should_be_no_executable_code_between_exception_and_rollback=Mежду "Исключение" и "ОтменитьТранзакцию()" есть исполняемый код, который может вызвать исключение
 
+BeginTransactionCheck_Executable_code_between_begin_transaction_and_try=Между "НачатьТранзакцию()" и "Попытка" есть исполняемый код
+
+BeginTransactionCheck_Try_was_not_found_after_calling_begin=Не найден оператор "Попытка" после вызова "НачатьТранзакцию()"
+
+BeginTransactionCheck_Begin_transaction_is_incorrect=Нарушена схема работы с "НачатьТранзакцию()"
+
+BeginTransactionCheck_Try_must_be_after_begin=После начала транзакции не найден оператор "Попытка"
+
 CommitTransactionCheck_Transaction_contains_empty_except=Транзакция содержит пустой блок Исключение
 
 CommitTransactionCheck_Transactions_is_broken=Нарушена схема работы с "ЗафиксироватьТранзакцию()"

tests/com.e1c.v8codestyle.bsl.itests/resources/not-found-try-after-begin.bsl

@@ -0,0 +1,5 @@
+Процедура Тест()
+    
+    НачатьТранзакцию();
+    
+КонецПроцедуры

tests/com.e1c.v8codestyle.bsl.itests/resources/should-be-no-executable-code-between-begin-and-try.bsl

@@ -0,0 +1,17 @@
+Процедура Тест()
+    
+    НачатьТранзакцию();
+    Тест = 1;   
+    Попытка
+    // 2. Вся логика блокировки и обработки данных размещается в блоке Попытка-Исключение
+    // 3. В самом конце обработки данных выполняется попытка зафиксировать транзакцию
+    ЗафиксироватьТранзакцию();
+    Исключение
+    // 4. В случае любых проблем с СУБД, транзакция сначала отменяется...
+    ОтменитьТранзакцию();
+    // 5. ...затем проблема фиксируется в журнале регистрации...
+    // 6. ... после чего, проблема передается дальше вызывающему коду.
+    ВызватьИсключение;
+    КонецПопытки;
+    
+КонецПроцедуры

tests/com.e1c.v8codestyle.bsl.itests/src/com/e1c/v8codestyle/bsl/check/itests/BeginTransactionCheckTest.java

@@ -0,0 +1,63 @@
+/*******************************************************************************
+ * Copyright (C) 2022, 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 java.util.List;
+
+import org.junit.Test;
+
+import com._1c.g5.v8.dt.validation.marker.Marker;
+import com.e1c.v8codestyle.bsl.check.BeginTransactionCheck;
+
+/**
+ * Tests for {@link BeginTransactionCheck} check.
+ *
+ * @author Artem Iliukhin
+ */
+public class BeginTransactionCheckTest
+    extends AbstractSingleModuleTestBase
+{
+
+    /**
+     * Instantiates a new commit transaction check test.
+     */
+    public BeginTransactionCheckTest()
+    {
+        super(BeginTransactionCheck.class);
+    }
+
+
+    @Test
+    public void testNotFoundTryAfterBegin() throws Exception
+    {
+        updateModule(FOLDER_RESOURCE + "not-found-try-after-begin.bsl");
+
+        List<Marker> markers = getModuleMarkers();
+        assertEquals(1, markers.size());
+        Marker marker = markers.get(0);
+        assertEquals("The try operator was not found after calling begin transaction", marker.getMessage());
+    }
+
+    @Test
+    public void testShouldBeNoExecutableCodebetweenBeginAndTry() throws Exception
+    {
+        updateModule(FOLDER_RESOURCE + "should-be-no-executable-code-between-begin-and-try.bsl");
+
+        List<Marker> markers = getModuleMarkers();
+        assertEquals(1, markers.size());
+        Marker marker = markers.get(0);
+        assertEquals("There should be no executable code between begin transaction and try", marker.getMessage());
+    }
+}

tests/com.e1c.v8codestyle.bsl.itests/src/com/e1c/v8codestyle/bsl/check/itests/CommitTransactionCheckTest.java

@@ -82,4 +82,5 @@ public class CommitTransactionCheckTest
         assertEquals("There should be no executable code between commit transaction and exception",
             marker.getMessage());
     }
+
 }