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

DRAFT Правило LostVariable - Затираемая\скрываемая переменная #2814

Open
wants to merge 50 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
6aedb8a
заготовки правила
artbear Jun 9, 2022
4a558de
реализация + новые падающие тест-кейсы
artbear Jun 13, 2022
d28decb
реализация новых кейсов + новые падающие тест-кейсы
artbear Jun 14, 2022
f9bb389
временно исключил падающие тесты
artbear Jun 14, 2022
0c27eb9
Revert "временно исключил падающие тесты"
artbear Jun 14, 2022
5838d6a
уточнил текст замечания
artbear Jun 14, 2022
a1b9d88
убрал ненужные исключения
artbear Jun 14, 2022
42609c8
исключил дубли внутри препроцессора
artbear Jun 15, 2022
f0a5bff
Документировал правило
artbear Jun 15, 2022
0eb00cf
Уточнил описание
artbear Jun 15, 2022
66363ee
Еще тест-кейсы
artbear Jun 15, 2022
2b99b47
Добавил пояснение для исправления
artbear Jun 15, 2022
b8f9270
Дополнил описание
artbear Jun 19, 2022
d6e36cd
Метод findNodeSuchThat
artbear Jun 19, 2022
ab0e292
типизированный метод getRootNode
artbear Jun 19, 2022
04c5781
поддержка новых тест-кейсов
artbear Jun 19, 2022
4643e7a
уточнил сообщение
artbear Jun 19, 2022
df9e488
precommit
artbear Jun 19, 2022
021870c
убрал методы, пересенные в Trees
artbear Jun 19, 2022
6a1004b
убрал ненужную проверку в findNodeSuchThat на null
artbear Jun 19, 2022
067aad3
улучшил покрытие
artbear Jun 19, 2022
78cb724
удалил неиспользуемый код и интерфейс
artbear Jun 19, 2022
5f05309
исправил внесенную ошибку в тесте
artbear Jun 19, 2022
d92d8eb
Merge branch 'develop' into lost-vars-1088
artbear Jun 21, 2022
4adf46a
натуральный порядок сравнения
artbear Jun 21, 2022
5e028f6
добавил проверку end в Ranges.compare
artbear Jun 21, 2022
8829fb4
натуральный порядок сортировки
artbear Jun 21, 2022
cb13984
учел вложение переменных в области
artbear Jun 21, 2022
fbdbc32
учет тела модуля
artbear Jun 21, 2022
5722023
замечания Сонара
artbear Jun 21, 2022
dce636c
кейс локальной и глобальной переменной
artbear Jun 22, 2022
4004547
Merge branch 'develop' into lost-vars-1088
artbear Jun 22, 2022
3240ea1
небольшое ускорение расчета глобальных\модульных переменных
artbear Jun 22, 2022
b15ba7e
рефакторинг
artbear Jun 22, 2022
ac095c3
ускорен метод getReferencesTo
artbear Jun 23, 2022
9f1815b
кейс глобальная переменная в теле модуля
artbear Jun 23, 2022
e9eb5ad
кейс:переменные использованы и заново переписаны
artbear Jun 23, 2022
820fbac
Merge remote-tracking branch 'origin/develop' into lost-vars-1088
artbear Jul 24, 2022
bf542e2
исправление неточного слияния
artbear Jul 24, 2022
e15edd8
TODO
artbear Jul 24, 2022
978427d
Merge remote-tracking branch 'origin/develop' into lost-vars-1088
artbear Nov 26, 2022
0400d35
уточнил обработку циклов и доп.кейс
artbear Nov 26, 2022
6e571b0
ограничение на типы модулей
artbear Nov 28, 2022
3cf5f48
убрал ненужное исключение из доки
artbear Dec 1, 2022
79a702f
добавил полезный кейс в документацию
artbear Dec 1, 2022
649bfbf
имя ресурса уточнено
artbear Dec 17, 2022
215f118
заготовка исправлений для циклов
artbear Dec 17, 2022
832e9a2
Merge branch 'develop' into lost-vars-1088
artbear Dec 17, 2022
b37656a
убрал неверный импорт
artbear Dec 17, 2022
74ac2d1
исправлен неверный мерж
artbear Dec 17, 2022
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
149 changes: 149 additions & 0 deletions docs/diagnostics/LostVariable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
# Потерянная переменная (LostVariable)

<!-- Блоки выше заполняются автоматически, не трогать -->
## Описание диагностики
<!-- Описание диагностики заполняется вручную. Необходимо понятным языком описать смысл и схему работу -->
Переменной присваивается значение, которое не используется. Далее переменной еще раз присваивается новое значение.
Подобный код приводит к снижению читабельности кода, также может приводить к снижению производительности.
Может возникать при опечатках, "копипасте" или рефакторинге кода, а также из-за проблем в алгоритме.

В текущей реализации возможны "ложно-положительные" срабатывания, т.е. могут выдаваться замечания к правильному коду, т.к. некоторые варианты кода сложно отличить от правильного. Подробнее в примерах, пункт "Примеры исключений".

## Примеры
<!-- В данном разделе приводятся примеры, на которые диагностика срабатывает, а также можно привести пример, как можно исправить ситуацию -->
Неверный код - безусловная повторная переустановка значения
```bsl
МояПеременная = СтрЗаменить(Текст, " ", ""); // замечание
МояПеременная = СтрЗаменить(Текст, ";", "");
```
Правильный код
```bsl
МояПеременная = СтрЗаменить(Текст, " ", "");
МояПеременная = СтрЗаменить(МояПеременная, ";", "");
```

Или другой подозрительный код - повторная переустановка значения при некотором условии и только потом переменная используется
```bsl
МояПеременная = СтрЗаменить(Текст, " ", ""); // замечание
Если Условие Тогда
МояПеременная = СтрЗаменить(Текст, ";", "");
Возврат МояПеременная;
КонецЕсли;
```

А вот такой слегка переделанный код уже не будет выявляться правилом
- т.к. переустановка выполняется не всегда, а только при выполнения условия.
```bsl
МояПеременная = СтрЗаменить(Текст, " ", "");
Если Условие Тогда
МояПеременная = СтрЗаменить(Текст, ";", "");
КонецЕсли;
Возврат МояПеременная;
```

Часто встречающаяся "ошибка последней строки"
```bsl
ТемаПисьма = ДанныеЗаполнения.Тема;// потерялось
ТекстШаблонаПисьмаHTML = ДанныеЗаполнения.ТекстHTML;
ТекстШаблонаПисьма = ДанныеЗаполнения.Текст;
ТемаПисьма = ДанныеЗаполнения.Тема; // а здесь переопредели
Наименование = ДанныеЗаполнения.Тема;
```

Еще подозрительный код - переменная `Файл` меняется несколько раз, и во втором случае ошибочно.
```bsl
Процедура ПолучитьФайлОбработки()
Файл = Новый Файл(КаталогФич); // нет замечания
Если Не Файл.Существует() Тогда
Возврат;
КонецЕсли;

Файл = Новый Файл(КаталогФич); // замечание
НачальныйКаталог = КаталогФич;

Файл = Новый Файл(НачальныйКаталог);
КонецПроцедуры
```

Пример подозрительного кода - Инициализация переменных в начале метода
```bsl
Функция ПолучитьПутьФайлаВРабочемКаталоге(ДанныеФайла)

ПутьДляВозврата = "";
ПолноеИмяФайла = ""; // замечание
ИмяКаталога = РабочийКаталогПользователя();

// Сперва пытаемся найти такую запись в регистре сведений.
ПолноеИмяФайла = ДанныеФайла.ПолноеИмяФайлаВРабочемКаталоге;
```
- начальная установка `КэшНастроек` не имеет смысла

**Примеры исключений**

- 1 в коллекцию добавляются разные значения без явного переиспользования
```bsl
ВидПрава = ВидыПрав.Добавить(); // будет выдано замечание, хотя код валиден
ВидПрава = ВидыПрав.Добавить();

НовыйПереход = ТаблицаПереходовНоваяСтрока("Начало"); // будет выдано замечание, хотя код валиден
НовыйПереход = ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки");
```
Код выше вполне валиден, но его можно улучшить и упростить, явно выразив свои намерения, избавившись от присваивания переменным и дублирования имен переменных
Исправленный код
```bsl
ВидыПрав.Добавить();
ВидыПрав.Добавить();

ТаблицаПереходовНоваяСтрока("Начало");
ТаблицаПереходовНоваяСтрока("НастройкаВыгрузки");
```

Пример полезного замечания для подобных дублей добавления в коллекцию - пример из типовой конфигурации
```bsl
ВидПрава = ВидыПрав.Добавить(); // вот тут ошибка!
ВидПрава = "Просмотр"; // т.к. ниже опечатка, автор кода забыл добавить .Имя
ВидПрава.Интерактивное = Истина;

ВидПрава = ВидыПрав.Добавить();
ВидПрава.Имя = "Редактирование";
```

- 2 обработка переменных в блоках Попытка и Исключение
```bsl
Процедура УстановитьБлокировку()
Блокировка = Новый БлокировкаДанных;

НачатьТранзакцию();
Попытка
ЭтоОшибкаБлокировки = Истина; // будет выдано замечание, хотя код валиден
Блокировка.Заблокировать(); // здесь возможно исключение, и поэтому до следующей строки выполнение может не дойти

ЭтоОшибкаБлокировки = Ложь;
ЗафиксироватьТранзакцию();
Исключение
ОтменитьТранзакцию();
Если ЭтоОшибкаБлокировки Тогда
ОбработкаОшибкиБлокировки();
КонецЕсли;
КонецПопытки;
КонецПроцедуры
```

- 3 одно и то же имя счетчика в соседних циклах, причем счетчик цикла в цикле не используется
```bsl
Для НомерЯчейки = 1 По 2 Цикл // будет выдано замечание, хотя код валиден
ТабличныйДокумент.Присоединить(ПустаяЯчейка);
КонецЦикла;

Для НомерЯчейки = 1 По КоличествоЯчеекДляЗаполнения - 2 Цикл
ПечатьСтроки(ЗначениеЗаполнения);
КонецЦикла;
```
В этом коде счетчик `НомерЯчейки` фактически используется в обоих циклах.
Такой код лучше переписать, используя уникальные имена счетчиков в обоих циклах, что повысит читаемость и упростит сопровождение.

## Источники
<!-- Необходимо указывать ссылки на все источники, из которых почерпнута информация для создания диагностики -->
* [MITRE, CWE-563 - Assignment to Variable without Use](https://cwe.mitre.org/data/definitions/563.html)
* [MITRE, CWE-1109 - Use of Same Variable for Multiple Purposes](https://cwe.mitre.org/data/definitions/1109.html)
* [PVS-Studio.V763. Parameter is always rewritten in function body before being used.](https://pvs-studio.com/ru/docs/warnings/v763)
16 changes: 16 additions & 0 deletions docs/en/diagnostics/LostVariable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Lost variable (LostVariable)

<!-- Блоки выше заполняются автоматически, не трогать -->
## Description
<!-- Описание диагностики заполняется вручную. Необходимо понятным языком описать смысл и схему работу -->

## Examples
<!-- В данном разделе приводятся примеры, на которые диагностика срабатывает, а также можно привести пример, как можно исправить ситуацию -->

## Sources
<!-- Необходимо указывать ссылки на все источники, из которых почерпнута информация для создания диагностики -->
<!-- Примеры источников

* Источник: [Стандарт: Тексты модулей](https://its.1c.ru/db/v8std#content:456:hdoc)
* Полезная информация: [Отказ от использования модальных окон](https://its.1c.ru/db/metod8dev#content:5272:hdoc)
* Источник: [Cognitive complexity, ver. 1.4](https://www.sonarsource.com/docs/CognitiveComplexity.pdf) -->
Loading