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

[NEW] Подозрительная проверка результата поиска в Найти\СтрНайти

Open artbear opened this issue 4 years ago • 5 comments

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

Разработчики иногда ошибаются в условиях на проверку результат поиска глобальных функций поиска в строках, неточно используя методы СтрНайти \ Найти

Указанные методы возвращают 0, если значение не найдено.

Поэтому условие Найти(ГдеИщем, ЧтоИщем) >= 0 не имеет смысла, т.к. всегда истинно. Или СтрНайти(ГдеИщем, ЧтоИщем)< 0 всегда ложно и также бессмысленно.

Правильные варианты

Если Найти(ГдеИщем, ЧтоИщем) > 0 Тогда
Если СтрНайти(ГдеИщем, ЧтоИщем) = 0 Тогда
  • Нужно проверять выражения с использованием глобальных функций Найти\СтрНайти при сравнении с нулем и операторами
    • >=
    • <
    • вариант Найти(ГдеИщем, ЧтоИщем) <= 0 можно не учитывать, т.к. это не явная ошибка, а неточность.
      • а можно также учесть, чтобы разработчик проверил, не ошибся ли он.
  • Сделать опцию для поиска Найти\СтрНайти(ГдеИщем, ЧтоИщем) без сравнения с числом
    • т.к. чаще всего это не ошибка. но может быть и ошибку
    • опция даст возможность командам управлять поведением

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

Похожие ишузы

  • https://github.com/1c-syntax/bsl-language-server/issues/1205
  • https://github.com/1c-syntax/bsl-language-server/issues/1549

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

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

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

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

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

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

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

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

1

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

artbear avatar Feb 18 '21 19:02 artbear

Найти - депрекейтед.

zeegin avatar Mar 18 '21 00:03 zeegin

Периодически встречаю

Если СтрНайти(ГдеИщем, ЧтоИщем) Тогда

И не могу сказать что мне это не нравится. Такой питонский стиль. Хочется к теме добавить обсужденияюе что делать в этом случае.

Так же надо понимать что эта же конструкция может быть не только в Если-Тогда ветках, проверять только в них?

zeegin avatar Mar 18 '21 01:03 zeegin

1 Да, вполне можно заменять на Если СтрНайти(ГдеИщем, ЧтоИщем) Тогда

2 проверять в разных выражениях, конечно, не только в Если

@zeegin

artbear avatar Mar 18 '21 07:03 artbear

По проверке не только в Если-Тогда надо уметь вычленить СтрНайти из сложной булевой функции, как ее часть. Например

СтрНайти > 0 И Флаг1 СтрНайти < 0 ИЛИ Флаг2 (Флаг1 ИЛИ Флаг2) И (СтрНайти = 0 ИЛИ СтрНайти < 0)

zeegin avatar Mar 18 '21 07:03 zeegin

Конечно, выделение подвыражения подразумевается

artbear avatar Mar 18 '21 07:03 artbear