WIP: Неиспользуемые переменные на ссылках #1611
Conversation
|
Во еще и оптимизировать нужно |
...n/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java
Outdated
Show resolved
Hide resolved
...java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java
Outdated
Show resolved
Hide resolved
...java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java
Show resolved
Hide resolved
...java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedLocalVariableDiagnostic.java
Outdated
Show resolved
Hide resolved
| * Ссылка на символ. | ||
| */ | ||
| @Value | ||
| @Builder |
There was a problem hiding this comment.
если честно, в одном классе странно выглядит и factory-метод of и билдер :)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndex.java
Outdated
Show resolved
Hide resolved
qtLex
force-pushed
the
feature/unused-local-variable-ref-index
branch
from
2820d68 to
55ba9ef
Compare
qtLex
force-pushed
the
feature/unused-local-variable-ref-index
branch
from
55ba9ef to
77bda86
Compare
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/Reference.java
Outdated
Show resolved
Hide resolved
| * были вызваны из списка Location (URI + Range). | ||
| */ | ||
| private final Map<MultiKey<String>, MultiValuedMap<String, Location>> referencesTo = new HashMap<>(); | ||
| private final Map<ReferenceDTO, List <Location>> referencesTo = new HashMap<>(); |
There was a problem hiding this comment.
Возможно здесь стоит использовать concurrent map.
| String symbolName; | ||
|
|
||
| //@EqualsAndHashCode.Exclude | ||
| boolean isWrite; |
There was a problem hiding this comment.
Интересная идея. Заготовка на будущее чтобы не плодить дтохи на одно и тоже обращение в разных местах?
There was a problem hiding this comment.
Да, это была попытка избавиться от поиска по двум ключам без создания второго DTO. Но идея не прошла из-за getReferencesFrom. Reference формируется на основании ключа Map, а если при поиске не учитывается одно из полей, ссылка формируется не правильно. Для двух других коллекций такой проблемы не так как формирование ссылки проходит по значению Map.
There was a problem hiding this comment.
Пока временно остановился на варианте, что для коллекции referencesTo isWrite всегда false.
| * были вызваны из списка Location (URI + Range). | ||
| */ | ||
| private final Map<MultiKey<String>, MultiValuedMap<String, Location>> referencesTo = new HashMap<>(); | ||
| private final Map<ReferenceDTO, List <Location>> referencesTo = new HashMap<>(); |
There was a problem hiding this comment.
А здесь, случаем, не будет просадки производительности за счёт усложнения операции поиска по symbol name? Хотя без бенчей, наверное, и не понять...
There was a problem hiding this comment.
Думаю, нужно профайлить смотреть. без этого не понять где будет проседать
There was a problem hiding this comment.
Да вот бенч-то с ума и сходит. при средних 82-83 секундах на пулл реквесте сейчас 283 - https://github.com/1c-syntax/bsl-language-server/pull/1611/checks?check_run_id=2374076050#step:10:54
.../_1c_syntax/bsl/languageserver/references/SourceDefinedSymbolDeclarationReferenceFinder.java
Outdated
Show resolved
Hide resolved
| referenceKey.getScopeName(), | ||
| referenceKey.getSymbolKind(), | ||
| referenceKey.getSymbolName(), | ||
| false |
There was a problem hiding this comment.
ой как опасно. это ж copy/prototype constructor получается, а тут неожиданная мутация
There was a problem hiding this comment.
хотя бы метод чуть по-другому назвать
There was a problem hiding this comment.
Это эксперимент был. Переделывать буду.
| public void addReference(URI uri, Range range, ReferenceDTO referenceKey) { | ||
| Location location = new Location(uri.toString(), range); | ||
| referencesTo.computeIfAbsent(referenceKey, k -> new ArrayList<>()).add(location); | ||
| referencesTo.computeIfAbsent(ReferenceDTO.of(referenceKey), k -> new ArrayList<>()).add(location); |
There was a problem hiding this comment.
надо бы здесь конструкторы листов/мап заменить на метод-референсы. а то heap pollution дикий
There was a problem hiding this comment.
Я кажется в терминах запутался ((.
k -> new ArrayList<>() заменить на какой-то метод? Что бы абстрактными классами не засорять?
There was a problem hiding this comment.
на ArrayList::new
лямбда срабатывает на каждый вызов computeIfAbsent и на каждое срабатывание создаст пустой эррэйлист, а метод-референс (с двумя двоеточиями) сработает только если собственно absent.
Добавлен дерево символов включены переменные обявленные в теле процедур через оператор присваивания и параметры методов.
Вычистил использование метода of оставив только билдер
…о значением isWrite всегда в ложь
| } | ||
|
|
||
| public Optional<Reference> getReference(URI uri, Range range) { | ||
| var t = referencesRanges.getOrDefault(uri, Collections.emptyMap()).get(range); |
qtLex
force-pushed
the
feature/unused-local-variable-ref-index
branch
from
20cf882 to
80000ad
Compare
|
@qtLex @nixel2007 а что с этим зависшим ПР? правило очень важное и полезное, хочется быстрее в продуктив запустить |
|
По нему ведутся работы параллельно в нескольких ветках. Reference index - сложный механизм, и дорабатывать его нужно аккуратно. |
|
@artbear Работы ведуться. |
|
Новый pr #1772 |
|
#1772 смержен. тогда этот ПР уже невалиден, да? |
|
эм. нет, #1772 не смержен, а просто почему-то закрыт. |
|
наверное потому что я удалил бранч, в который был нацелен PR. @qtLex перетаргетишь на develop тогда? |
Да. Это PR уже не актуален.
Ага. Я создам новый PR на develop. Текущее состояниеПосле оптимизаций немного подразвалились тесты. Так как вся конструкция стала слишком чувствительной к правильности разбора модуля. Например будет падать на Перем А;
Перем А;Я вроде закончил адаптацию, тесты гоняю. Как закончу создам новый PR/ |
3 participants
Описание
Диагностика на неиспользуемые локальные переменные
В дерево символов добавлены переменные создаваемые внутри метода и параметрами методов
Добавлено хранение ссылок на использование переменных.
Работа еще ведется. Есть ряд мест которые хочется порефакторить. В основном они касаются добавления и использования ссылок. В частности ключи для методов и переменных получились разные. Думаю нужно от этого избавиться через использование информации о родителе переменной. Если не получится разделять на несколько мапов.
Выкладываю для оценки результата. Может где-то делаю вредное или пошел не туда. Или может еще какие идеи возникнут.
Связанные задачи
Closes: #15
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно