Merge pull request #1725 from EvilBeaver/feature/cfg

ControlFlowGraph и диагностика с примером
1c-syntax · Aug 29, 2021 · ff1ac97 · ff1ac97
2 parents 626dfc1 + 1a15ad1
commit ff1ac97
Show file tree

Hide file tree

Showing 35 changed files with 2,801 additions and 4 deletions.
diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -91,6 +91,9 @@ dependencies {
     implementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310")
     implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-xml")
 
+    // graphs
+    implementation("org.jgrapht", "jgrapht-core", "1.5.1")
+
     // SARIF serialization
     implementation("com.contrastsecurity", "java-sarif", "2.0")
 

diff --git a/docs/diagnostics/AllFunctionPathMustHaveReturn.md b/docs/diagnostics/AllFunctionPathMustHaveReturn.md
@@ -0,0 +1,93 @@
+# Все возможные пути выполнения функции должны содержать оператор Возврат (AllFunctionPathMustHaveReturn)
+
+|      Тип      |    Поддерживаются<br>языки    | Важность |    Включена<br>по умолчанию    |    Время на<br>исправление (мин)    |                             Теги                             |
+|:-------------:|:-----------------------------:|:--------:|:------------------------------:|:-----------------------------------:|:------------------------------------------------------------:|
+| `Дефект кода` |         `BSL`<br>`OS`         | `Важный` |              `Да`              |                 `1`                 |       `unpredictable`<br>`badpractice`<br>`suspicious`       |
+
+## Параметры 
+
+
+|            Имя             |   Тип    |                                                                        Описание                                                                        |    Значение<br>по умолчанию    |
+|:--------------------------:|:--------:|:------------------------------------------------------------------------------------------------------------------------------------------------------:|:------------------------------:|
+| `loopsExecutedAtLeastOnce` | `Булево` |                                                `Считать, что циклы выполняются, как минимум, один раз.`                                                |             `true`             |
+| `ignoreMissingElseOnExit`  | `Булево` | `Игнорировать блоки Если-ИначеЕсли, которые не имеют ветки Иначе. Отключите, если нужно детектировать выход из метода по ложному условию в ИначеЕсли.` |            `false`             |
+<!-- Блоки выше заполняются автоматически, не трогать -->
+## Описание диагностики
+Каждая функция в языке 1С имеет в самом конце неявный оператор "Возврат Неопределено". Если управление доходит до конца функции, то функция возвращает неопределено.
+
+Как правило, это не является штатным функционированием, программист должен явно описать все возвращаемые значения функции. Однако, довольно легко пропустить ситуацию, при которой управление дойдет до строки КонецФункции и вернется непредусмотренное значение Неопределено.
+
+Данная диагностика проверяет, что все возможные пути выполнения функции имеют явный оператор Возврат и функция не возвращает непредвиденных значений.
+
+## Примеры
+
+### Неправильно
+
+```bsl
+// если ставка заполнена, но не НДС10 и не НДС10 - вернется Неопределено
+// это может быть, как запланированное поведение, 
+// так и ошибка проверки прочих вариантов.
+Функция ОпределитьСтавкуНДС(Знач Ставка)
+    Если Ставка = Перечисления.СтавкиНДС.НДС20 Тогда
+        Возврат 20;
+    ИначеЕсли Ставка = Перечисления.СтавкиНДС.НДС10 Тогда
+        Возврат 10;
+    ИначеЕсли Не ЗначениеЗаполнено(Ставка) Тогда
+        Возврат Константы.СтавкаНДСПоУмолчанию.Получить();
+    КонецЕсли;
+    
+    // здесь будет неявный возврат Неопределено
+КонецФункции
+```
+
+### Правильно
+
+```
+// явно указать намерение вернуть результат в конце функции.
+Функция ОпределитьСтавкуНДС(Знач Ставка)
+    Если Ставка = Перечисления.СтавкиНДС.НДС20 Тогда
+        Возврат 20;
+    ИначеЕсли Ставка = Перечисления.СтавкиНДС.НДС10 Тогда
+        Возврат 10;
+    ИначеЕсли Не ЗначениеЗаполнено(Ставка) Тогда
+        Возврат Константы.СтавкаНДСПоУмолчанию.Получить();
+    КонецЕсли;
+    
+    // Явно декларируем намерение вернуть Неопределено
+    Возврат Неопределено;
+КонецФункции
+```
+
+### Еще пример ошибочного кода:
+
+```bsl
+Функция СуммаСкидки(Знач КорзинаЗаказа)
+    Если КорзинаЗаказа.Строки.Количество() > 10 Тогда
+        Возврат Скидки.СкидкаНаКрупнуюКорзину(КорзинаЗаказа);
+    ИначеЕсли КорзинаЗаказа.ЕстьКартаЛояльности Тогда
+        // функция возвращает непредусмотренное значение Неопределено
+        Скидки.СкидкаПоКартеЛояльности(КорзинаЗаказа);
+    Иначе 
+        Возврат 0;
+    КонецЕсли;
+КонецФункции
+```
+
+## Сниппеты
+
+<!-- Блоки ниже заполняются автоматически, не трогать -->
+### Экранирование кода
+
+```bsl
+// BSLLS:AllFunctionPathMustHaveReturn-off
+// BSLLS:AllFunctionPathMustHaveReturn-on
+```
+
+### Параметр конфигурационного файла
+
+```json
+"AllFunctionPathMustHaveReturn": {
+    "loopsExecutedAtLeastOnce": true,
+    "ignoreMissingElseOnExit": false
+}
+```
diff --git a/docs/diagnostics/index.md b/docs/diagnostics/index.md
@@ -8,16 +8,17 @@
 
 ## Список реализованных диагностик
 
-Общее количество: **143**
+Общее количество: **144**
 
 * Потенциальная уязвимость: **4**
 * Уязвимость: **4**
 * Ошибка: **44**
-* Дефект кода: **91**
+* Дефект кода: **92**
 
 
 | Ключ | Название | Включена по умолчанию | Важность | Тип | Тэги |
 | --- | --- | :-: | --- | --- | --- |
+ [AllFunctionPathMustHaveReturn](AllFunctionPathMustHaveReturn.md) | Все возможные пути выполнения функции должны содержать оператор Возврат | Да | Важный | Дефект кода | `unpredictable`<br>`badpractice`<br>`suspicious` 
  [AssignAliasFieldsInQuery](AssignAliasFieldsInQuery.md) | Назначение псевдонимов выбранным полям в запросе | Да | Важный | Дефект кода | `standard`<br>`sql`<br>`badpractice` 
  [BeginTransactionBeforeTryCatch](BeginTransactionBeforeTryCatch.md) | Нарушение правил работы с транзакциями для метода 'НачатьТранзакцию' | Да | Важный | Ошибка | `standard` 
  [CachedPublic](CachedPublic.md) | Кеширование программного интерфейса | Да | Важный | Дефект кода | `standard`<br>`design` 

diff --git a/docs/en/diagnostics/AllFunctionPathMustHaveReturn.md b/docs/en/diagnostics/AllFunctionPathMustHaveReturn.md
@@ -0,0 +1,50 @@
+# All execution paths of a function must have a Return statement (AllFunctionPathMustHaveReturn)
+
+|     Type     |        Scope        | Severity |    Activated<br>by default    |    Minutes<br>to fix    |                             Tags                             |
+|:------------:|:-------------------:|:--------:|:-----------------------------:|:-----------------------:|:------------------------------------------------------------:|
+| `Code smell` |    `BSL`<br>`OS`    | `Major`  |             `Yes`             |           `1`           |       `unpredictable`<br>`badpractice`<br>`suspicious`       |
+
+## Parameters 
+
+
+|            Name            |   Type    |                                                    Description                                                     | Default value |
+|:--------------------------:|:---------:|:------------------------------------------------------------------------------------------------------------------:|:-------------:|
+| `loopsExecutedAtLeastOnce` | `Boolean` |                                     `Assume loops are executed at least once`                                      |    `true`     |
+| `ignoreMissingElseOnExit`  | `Boolean` | `Ignore ElIf clauses which has no Else branch. Disable to detect exits from ElIf condition which results to False` |    `false`    |
+<!-- Блоки выше заполняются автоматически, не трогать -->
+## Description
+Functions should not have an implicit return. All returned values must be shown excplicitly.
+
+## Examples
+```bsl
+Function CalculadeDiscount(ShoppingCart)
+    If ShoppingCart.Rows.Count() > 10 Then
+        Return Discounts.DiscountForABigCart(ShoppingCart);
+    ElIf ShoppingCart.HasLoyaltyCard Then
+        // Return is missed unintentionally
+        // causing unexpected return of Undefined
+        Discounts.DiscountByLoyaltyCard(ShoppingCart);
+    Else 
+        Return 0;
+    EndIf;
+EndFunction
+```
+
+## Snippets
+
+<!-- Блоки ниже заполняются автоматически, не трогать -->
+### Diagnostic ignorance in code
+
+```bsl
+// BSLLS:AllFunctionPathMustHaveReturn-off
+// BSLLS:AllFunctionPathMustHaveReturn-on
+```
+
+### Parameter for config
+
+```json
+"AllFunctionPathMustHaveReturn": {
+    "loopsExecutedAtLeastOnce": true,
+    "ignoreMissingElseOnExit": false
+}
+```
diff --git a/docs/en/diagnostics/index.md b/docs/en/diagnostics/index.md
@@ -8,16 +8,17 @@ To escape individual sections of code or files from triggering diagnostics, you
 
 ## Implemented diagnostics
 
-Total: **143**
+Total: **144**
 
 * Security Hotspot: **4**
 * Vulnerability: **4**
 * Error: **44**
-* Code smell: **91**
+* Code smell: **92**
 
 
 | Key | Name| Enabled by default | Severity | Type | Tags |
 | --- | --- | :-: | --- | --- | --- |
+ [AllFunctionPathMustHaveReturn](AllFunctionPathMustHaveReturn.md) | All execution paths of a function must have a Return statement | Yes | Major | Code smell | `unpredictable`<br>`badpractice`<br>`suspicious` 
  [AssignAliasFieldsInQuery](AssignAliasFieldsInQuery.md) | Assigning aliases to selected fields in a query | Yes | Major | Code smell | `standard`<br>`sql`<br>`badpractice` 
  [BeginTransactionBeforeTryCatch](BeginTransactionBeforeTryCatch.md) | Violating transaction rules for the 'BeginTransaction' method | Yes | Major | Error | `standard` 
  [CachedPublic](CachedPublic.md) | Cached public methods | Yes | Major | Code smell | `standard`<br>`design`