bsl-language-server icon indicating copy to clipboard operation
bsl-language-server copied to clipboard

[NEW] Неверное изменение переменной\реквизита внутри цикла - имеет значение только последняя итерация цикла

Open artbear opened this issue 4 years ago • 6 comments

Описание проблемы, ошибки, которую надо диагностировать

Разработчики иногда ошибаются, выполняя внутри цикла установку одного и того же значения\реквизита. В таком случае фактически последняя запись перезапишет все остальные, т.е. предыдущие итерации не имеют смысла. Часто подобные ошибки возникают из копи-паста

Неверно

Для Каждого ЭлементЦеныПоставки Из ЦеныПоставок.НайтиСтроки(КлючПоискаЦенПоставок) Цикл
  ОписаниеПоставки.Цена= ЭлементЦеныПоставки.Цена;
  ОписаниеПоставки.Валюта = XMLСтрока(ЭлементЦеныПоставки.ВалютаСсылка);
КонецЦикла;

или

Для Каждого Элемент Из Коллекция Цикл
  Значение = Элемент.Цена;
КонецЦикла;

Нужно учесть

  • обычную переменную - Значение =
  • реквизита объекта - Объект.Реквизит =
    • с разным уровнем вложенности - Объект.Реквизит...ВложенныйРеквизит =

Ссылка на источник, подтверждающее нарушение либо обоснование наличия проблемы

Параметры диагностики

Тип Статья на русском

  • [ ] :ant: Ошибка
  • [ ] :cop: Уязвимость
  • [ ] :guardsman: Потенциальная уязвимость
  • [x] :poop: Качество кода
  • [ ] :trollface: Другое

Важность Статья на русском

  • [ ] :broken_heart: Блокирующая / Blocker
  • [ ] :heart: Критическая / Critical
  • [x] :yellow_heart: Важная / Major
  • [ ] :blue_heart: Незначительная / Minor
  • [ ] :green_heart: Информационная / Info
  • [ ] :revolving_hearts: Другое

Тэги Статья на русском

  • [ ] STANDARD - "Нарушение стандартов 1С"
  • [ ] LOCKINOS - "Не будет работать в другой ОС"
  • [ ] SQL - "Проблема с запросом"
  • [ ] PERFORMANCE - "Проблема производительности"
  • [ ] BRAINOVERLOAD - "Непонятный код"
  • [x] BADPRACTICE - "Плохая практика программирования"
  • [ ] CLUMSY - "Излишние действия"
  • [ ] DESIGN - "Ошибка в проектировании"
  • [x] SUSPICIOUS - "Подозрительный код"
  • [x] UNPREDICTABLE - "Непредсказуемо работающий код"
  • [ ] DEPRECATED - "Устаревшая функциональность"
  • [ ] ERROR - "Ошибочная конструкция"
  • [ ] LOCALIZE - "Проблемы локализации"

Время на исправление (минут)

3 минуты

Дополнительная информация

artbear avatar Jan 17 '21 16:01 artbear

Фактически это отдельная диагностика из разряда затираемых\скрываемых переменных из #1088

artbear avatar Jan 17 '21 16:01 artbear

Я бы не стал утверждать, что это однозначно ошибка. Частый кейс - поиск последнего значения в цикле по коллекции с какими-то условиями/отборами. Возможно можно считать, что если узел присваивания в переменной находится в такой точке графа потока управления, через которую гарантировано проходят все итерации (например, напрямую в теле цикла, без вложенных условий, или в условиях, которые всегда выполняются, но тут без DFA никак), то это подозрительное место, но надо еще подумать.

nixel2007 avatar Jan 17 '21 19:01 nixel2007

Да, если есть условие внутри, тут правило нужно аккуратно проверить, возможно, исключить для циклов с условиями.

но я несколько раз нарывался, что такая ошибка в цикле без условий встречается.

пример кода из реального коммита (

кажется, я что-то подобное в правилах из открытых плагинов Сонара встречал.

artbear avatar Jan 19 '21 10:01 artbear

Точно уже не помню, возможно, я такое правило реализовывал в Пульском плагине.

artbear avatar Jan 20 '21 16:01 artbear

Кажется будет много FP. Я бы не стал делать. Я не считаю это подозрительным кодом.

Может быть много случаев выхода из цикла до или после присваивания.

zeegin avatar Mar 18 '21 01:03 zeegin

Кажется будет много FP. Я бы не стал делать. Я не считаю это подозрительным кодом.

Может быть много случаев выхода из цикла до или после присваивания.

В этом правиле я предлагаю

  • 1 учитывать только циклы, в которых нет прерывания выполнения
    • ни Возврат, ни Продолжить, ни Прервать
  • 2 возможно, стоит пропускать циклы, в которых есть условия Если или есть вложенные циклы

т.е. анализировать простые циклы, в которых чаще всего и происходит описанная ошибка.

artbear avatar Nov 28 '22 13:11 artbear