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

Conversation

artbear
Copy link
Contributor

@artbear artbear commented Jun 19, 2022

Описание

  • реализация правила
  • типизированный метод Trees::getRootNode
  • Метод Trees::findContextContainsPosition
  • Метод Trees::findNodeSuchThat
  • ускорен метод ReferenceIndex::getReferencesTo(SourceDefinedSymbol symbol)
    .
  • проверено на БСП 3.1, 3.0, КА 2.5

Учтены кейсы из исходной задачи

  • 0
  • 1
  • 3 (частично?)
  • 4
    Есть FP, смотреть комментарии

Связанные задачи

Closes #1088

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

  • проверено на БСП 3.1, 3.0, КА 2.5
  • на внутреннем Сонаре Инфостарт правило работает уже в течение нескольких месяцев

@artbear
Copy link
Contributor Author

artbear commented Jun 19, 2022

Упал шаг javadoc на обращении к https://javadoc.io/ - остальные шаги отработали верно.

}

private List<VarData> getVarData(MethodSymbol methodSymbol) {
final var variables = methodSymbol.getChildren().stream()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

здесь можно пропустить переменные, лежащие в областях

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

здесь можно пропустить переменные, лежащие в областях

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

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

98.5% 98.5% Coverage
0.0% 0.0% Duplication

@artbear
Copy link
Contributor Author

artbear commented Jun 23, 2022

В шапку добавил

  • типизированный метод Trees::getRootNode
  • Метод Trees::findNodeSuchThat
  • ускорен метод ReferenceIndex::getReferencesTo(SourceDefinedSymbol symbol)

artbear added 7 commits June 24, 2022 00:40
уже без использования
- внесен падающий тест для дальнейшей реализации
# Conflicts:
#	src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java
#	src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java
для лучшего понимания
- ловится переустановка итератора во вложенном цикле
- кейс с переустановкой значения
для исключения ФП
рефакторинг
@artbear artbear changed the title Правило LostVariable - Затираемая\скрываемая переменная DRAFT Правило LostVariable - Затираемая\скрываемая переменная Nov 30, 2022
@artbear
Copy link
Contributor Author

artbear commented Nov 30, 2022

FP 1

<Для Каждого ТабличнаяЧасть Из СтрокаТаблицы.Метаданные.ТабличныеЧасти Цикл
    Для Каждого Реквизит Из ТабличнаяЧасть.Реквизиты Цикл
        Если Реквизит.Тип.СодержитТип(ТипЗнч(Ссылка)) Тогда
            СтрокаТабличнойЧасти = Параметры.Объект[ТабличнаяЧасть.Имя].Найти(Ссылка, Реквизит.Имя);
            Пока СтрокаТабличнойЧасти <> Неопределено Цикл
                СтрокаТабличнойЧасти[Реквизит.Имя] = ПравильныйЭлемент;
// выдается ошибка ниже
                СтрокаТабличнойЧасти = Параметры.Объект[ТабличнаяЧасть.Имя].Найти(Ссылка, Реквизит.Имя);
            КонецЦикла;
        КонецЕсли;
    КонецЦикла;
КонецЦикла;

image

@artbear
Copy link
Contributor Author

artbear commented Nov 30, 2022

Еще FP 2

  • на атрибуты объектов
  • на реквизиты форм

к сожалению, пока эту проблему простым способом не порешать

@artbear
Copy link
Contributor Author

artbear commented Nov 30, 2022

Еще FP 2

  • на атрибуты объектов
  • на реквизиты форм

к сожалению, пока эту проблему простым способом не порешать

варианты решения

  • или проверять только те модули, в которых нет обращения к подобным атрибутам\реквизитам
  • или сделать спец.флаг для того, чтобы пользователи могли выбирать - проверять все модули, но с ФП на этих объектах, или проверять только модули без атрибутов\реквизитов
  • или проверять атрибуты\реквизиты в правиле, т.к. они доступны через мдклассы

@artbear
Copy link
Contributor Author

artbear commented Nov 30, 2022

FP 3

Пока ВыборкаДетальныеЗаписи.Следующий() Цикл
    Если ПервуюПропускаем Тогда
        ПервуюПропускаем = Ложь; // тут FP
        Продолжить;
    КонецЕсли;
    Если ВыборкаДетальныеЗаписи.НеобходимоОбновить Тогда
        МассивБазКонфигурации.Добавить(ВыборкаДетальныеЗаписи.Ссылка);
    КонецЕсли;
КонецЦикла;

image

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 16 Code Smells

97.9% 97.9% Coverage
0.0% 0.0% Duplication

.findFirst()
.orElse(MODULE_SCOPE_NAME);
return Optional.ofNullable(methodContextsByMethodName.get(methodName))
.filter(Objects::nonNull)

Check warning

Code scanning / QDJVMC

Constant values

Method reference result is always 'true'
@artbear
Copy link
Contributor Author

artbear commented Jan 14, 2023

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

#Если НЕ МобильныйАвтономныйСервер Тогда
Процедура ОбработкаПолученияПолейПредставления(Поля, СтандартнаяОбработка)
  // код
КонецПроцедуры
#Иначе
Процедура ОбработкаПолученияПолейПредставления(Поля, СтандартнаяОбработка)
  // код
КонецПроцедуры
#КонецЕсли

ошибка

File: file:///.../src/cf/Catalogs/Номенклатура/Ext/ManagerModule.bsl
Diagnostic: Either [
  left = LostVariable
  right = null
]
java.lang.IllegalStateException: Duplicate key ОбработкаПолученияПолейПредставления (attempted merging values [428 223] and [428 223])
	at java.base/java.util.stream.Collectors.duplicateKeyException(Unknown Source)
	at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Unknown Source)
	at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(Unknown Source)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(Unknown Source)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(Unknown Source)
	at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
	at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
	at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
	at com.github._1c_syntax.bsl.languageserver.diagnostics.LostVariableDiagnostic.lambda$getMethodContextsByMethodName$1(LostVariableDiagnostic.java:117)
	at java.base/java.util.Optional.map(Unknown Source)
	at com.github._1c_syntax.bsl.languageserver.diagnostics.LostVariableDiagnostic.getMethodContextsByMethodName(LostVariableDiagnostic.java:115)
	at com.github._1c_syntax.bsl.languageserver.diagnostics.LostVariableDiagnostic.check(LostVariableDiagnostic.java:97)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NEW] Затираемая\скрываемая переменная - shadow variable
2 participants