1C-Company / v8-code-style

Расширение для 1C:EDT, которое помогает разрабатывать конфигурации/приложения по стандартам 1С для платформы "1С:Предприятие 8".
Eclipse Public License 2.0
175 stars 53 forks source link

Ложное срабатывание проверки: redundant-export-method (Избыточное ключевое слово Экспорт) #1232

Open RedMammoth opened 1 year ago

RedMammoth commented 1 year ago

Название/код проверки

Код проверки: redundant-export-method (Избыточное ключевое слово Экспорт)

Версия плагина: 0.4.0.131

Пример кода/метаданных содержащих ошибку

МойМодуль1:

#Область СлужебныйПрограммныйИнтерфейс

Процедура Метод3() Экспорт
    Возврат;
КонецПроцедуры

Процедура Метод4() Экспорт
    Возврат;
КонецПроцедуры

#КонецОбласти

МойМодуль2:

#Область СлужебныйПрограммныйИнтерфейс

Процедура Метод3() Экспорт
    Возврат;
КонецПроцедуры

Процедура Метод4() Экспорт
    Возврат;
КонецПроцедуры

#КонецОбласти

Общиймодуль1:

// @strict-types

#Область ПрограммныйИнтерфейс

Процедура Тест() Экспорт
    Тест2(Истина).Метод3();
    Тест2(Ложь).Метод4();
КонецПроцедуры

Функция Тест2(Параметр) Экспорт
    Если Параметр Тогда
        Возврат МойМодуль1;
    Иначе
        Возврат МойМодуль2;
    КонецЕсли;
КонецФункции

#КонецОбласти

Почему это НЕ ошибка

Вызовы есть.

Возвращающий метод может быть сложным, и статическим анализом сложно проверить что: МойМодуль1.Метод3() и МойМодуль2.Метод4() используются, а МойМодуль1.Метод4() и МойМодуль2.Метод3() не используются

Однако проверить, что Тест2() может вернуть МойМодуль1 и МойМодуль2 и у этих модулей может быть вызван Метод3 и Метод4 мне кажется реальным.

Если принять за нулевую гипотезу - метод экспортный. То ошибка первого рода "опаснее", ошибки второго рода

Т.е. если проверка на используемом методе сказала что можно убрать экспорт и мы уберем - получим ошибку в рантайме, если проверка на неиспользуемом методе не зафиксировала избыточность слова экспорт - то у нас просто будет лишний экспортный метод.

Поэтому я считаю, что проверка должна срабатывать только при максимальной уверенности

adminimusRU commented 1 year ago

В типовых конфигурациях и библиотеках очень часто используется код вида

ОбщийМодуль = ОбщегоНазначения.ОбщийМодуль("ИмяМодуля");
ОбщийМодуль.ИмяМетода();

И еще бывают вызовы через Вычислить, например библиотека МДЛП этим часто грешит. Если всё это не учитывается при проверке, то будет очень много ложных срабатываний.

marmyshev commented 1 year ago

В типовых конфигурациях и библиотеках очень часто используется код вида

ОбщийМодуль = ОбщегоНазначения.ОбщийМодуль("ИмяМодуля");
ОбщийМодуль.ИмяМетода();

Этот вариант корректно будет учитываться (или должен!) при наличии плагина 1C:SSL-support который поддерживает типизацию возврата функции ОбщегоНазначения.ОбщийМодуль("ИмяМодуля"); там так же должен работать переход к определению по F3 на "ИмяМетода()" - что означает наличие ссылки на метод (и при поиске должна находиться)

И еще бывают вызовы через Вычислить, например библиотека МДЛП этим часто грешит. Если всё это не учитывается при проверке, то будет очень много ложных срабатываний.

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

adminimusRU commented 1 year ago

при наличии плагина 1C:SSL-support включен в поставку начиная с 2021.3, значит с обращениями через ОбщегоНазначения.ОбщийМодуль проблем быть не должно. А за вычислить конечно нужно бить по рукам.

marmyshev commented 1 year ago

@iArtemv Посмотри задачу.

ЕДТ показывает такой вызов (из МойМодуль1.Метод3()) как опциональный вызов. Проверка не учитывает поиск опциональных ссылок на методы. Кажется что должна учитывать - или можно сделать это параметром проверки.

iArtemv commented 9 months ago

Предлагаю учесть при реализации задачи #314 , проверка redundant-export-method сейчас не всегда показывает корректный результат, так как индексы при выполнении проверкок постоянно пересчитываются, следовательно результат поиска не всегда будет корректный. Пока что проверку предлагается исключить из плагина.