Всплывающая подсказка и переход к определениям для аннотаций в OneScript #3364
Conversation
|
Warning Rate limit exceeded@nixel2007 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (18)
WalkthroughВ данном пулл-запросе добавлены несколько новых классов и методов, а также внесены изменения в существующие классы для улучшения управления символами и форматирования описаний в контексте BSL Language Server. Основные изменения включают создание классов Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
nixel2007
force-pushed
the
feature/annotation-hover
branch
from
589f2e1 to
fe53693
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Qodana for JVM77 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 104 dependenciesThird-party software listThis page lists the third-party software dependencies used in bsl-language-server
Contact Qodana teamContact us at [email protected]
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
...main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java
Fixed
Show fixed
Hide fixed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (5)
src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java (1)
40-59: Рекомендуется добавить дополнительные тестовые сценарииТекущая реализация тестового метода хорошо структурирована и следует паттерну AAA (Arrange-Act-Assert). Однако рекомендуется добавить следующие тестовые сценарии:
- Проверка поведения при некорректной позиции
- Проверка случая, когда аннотация не найдена
- Проверка граничных случаев позиции курсора
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java (1)
42-46: Правильное использование аннотаций LombokАннотации Lombok корректно настроены для оптимальной производительности:
@EqualsAndHashCode(onlyExplicitlyIncluded = true)предотвращает включение всех полей в equals/hashCode@ToString(exclude = {"children", "parent"})исключает коллекции из toString для предотвращения рекурсииРекомендуется добавить документацию (JavaDoc) к классу, описывающую его назначение и взаимодействие с другими компонентами системы.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java (2)
46-54: Рекомендуется дополнить документацию классаТекущая документация описывает только назначение класса. Предлагаю добавить:
- Описание взаимодействия с
DescriptionFormatter- Примеры использования класса
/** * Построитель контента для всплывающего окна для {@link AnnotationSymbol}. + * + * Использует {@link DescriptionFormatter} для форматирования различных секций документации: + * сигнатуры, описания, параметров и примеров использования метода. + * + * @see DescriptionFormatter + * @see MarkupContentBuilder */
55-99: Предложения по улучшению реализации метода getContent
- Рекомендуется сделать проверку родительского символа более явной
- Комментарии можно заменить на методы с говорящими названиями
- Желательно добавить проверку на null для methodSymbol
@Override public MarkupContent getContent(AnnotationSymbol symbol) { var maybeMethodSymbol = symbol.getParent(); - if (maybeMethodSymbol.filter(MethodSymbol.class::isInstance).isEmpty()) { + if (maybeMethodSymbol.isEmpty() || !maybeMethodSymbol.get() instanceof MethodSymbol) { return new MarkupContent(MarkupKind.MARKDOWN, ""); } var markupBuilder = new StringJoiner("\n"); var methodSymbol = (MethodSymbol) maybeMethodSymbol.get(); + + if (methodSymbol == null) { + return new MarkupContent(MarkupKind.MARKDOWN, ""); + } - // сигнатура - // местоположение метода - // описание метода - // параметры - // примеры - // варианты вызова + return buildMarkupContent(markupBuilder, symbol, methodSymbol); } + + private MarkupContent buildMarkupContent(StringJoiner markupBuilder, + AnnotationSymbol symbol, + MethodSymbol methodSymbol) { // ... rest of the implementation }src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java (1)
78-103: Рассмотрите возможность устранения дублирования кода при добавлении секцийПовторяющийся шаблон кода для добавления секций в
markupBuilderможет быть упрощен. Это улучшит читаемость и облегчит поддержку кода.Предлагаемый рефакторинг:
+ String[] sections = new String[] { + descriptionFormatter.getSignature(symbol), + descriptionFormatter.getLocation(symbol), + descriptionFormatter.getPurposeSection(symbol), + descriptionFormatter.getParametersSection(symbol), + descriptionFormatter.getReturnedValueSection(symbol), + descriptionFormatter.getExamplesSection(symbol), + descriptionFormatter.getCallOptionsSection(symbol) + }; + for (String section : sections) { + descriptionFormatter.addSectionIfNotEmpty(markupBuilder, section); + } - // Удаление повторяющихся блоков кода ниже - String signature = descriptionFormatter.getSignature(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, signature); - String methodLocation = descriptionFormatter.getLocation(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, methodLocation); - String purposeSection = descriptionFormatter.getPurposeSection(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, purposeSection); - String parametersSection = descriptionFormatter.getParametersSection(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, parametersSection); - String returnedValueSection = descriptionFormatter.getReturnedValueSection(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, returnedValueSection); - String examplesSection = descriptionFormatter.getExamplesSection(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, examplesSection); - String callOptionsSection = descriptionFormatter.getCallOptionsSection(symbol); - descriptionFormatter.addSectionIfNotEmpty(markupBuilder, callOptionsSection);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/SourceDefinedMethodCallInlayHintSupplier.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_en.properties(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_ru.properties(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_en.properties(0 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_ru.properties(0 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java(1 hunks)src/test/resources/references/AnnotationReferenceFinder.os(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_en.properties
- src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_ru.properties
✅ Files skipped from review due to trivial changes (2)
- src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_ru.properties
- src/test/resources/references/AnnotationReferenceFinder.os
🧰 Additional context used
🪛 GitHub Check: QDJVM
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java
[warning] 61-61: Optional.get() is called without isPresent() check
'Optional.get()' without 'isPresent()' check
🔇 Additional comments (20)
src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_en.properties (1)
1-1: Локализация выглядит корректно
Все ключи и значения соответствуют стандартам английской локализации. Заглавные буквы используются правильно, термины выбраны корректно.
Also applies to: 3-10
src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java (3)
1-22: Корректный заголовок лицензии!
Заголовок файла содержит всю необходимую информацию о лицензии и авторских правах.
34-39: Правильная настройка тестового класса!
Класс корректно настроен для интеграционного тестирования с использованием Spring Boot. Внедрение зависимостей реализовано правильно.
43-43: Проверить существование тестового файла
Рекомендуется убедиться в наличии тестового файла и корректности его содержимого.
✅ Verification successful
Тестовый файл существует и содержит необходимые данные
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test file existence and content
# Check if test file exists
if [ -f "./src/test/resources/references/AnnotationReferenceFinder.os" ]; then
echo "Test file exists"
# Show file content for verification
cat "./src/test/resources/references/AnnotationReferenceFinder.os"
else
echo "Test file not found!"
fi
Length of output: 307
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java (5)
1-22: Корректный заголовок лицензии и структура пакета
Заголовок файла содержит актуальную информацию о лицензии LGPL-3.0 и корректные данные об авторских правах.
24-41: Корректная организация импортов
Импорты правильно сгруппированы и все используются в коде.
75-85: Корректная реализация фабричного метода
Метод from правильно инициализирует все необходимые поля и устанавливает связь с родительским символом метода.
70-73: Пустая реализация метода accept
Метод accept(SymbolTreeVisitor visitor) помечен как no-op. Необходимо уточнить:
- Является ли это временным решением?
- Планируется ли реализация в будущем?
- Нужен ли комментарий, объясняющий причину пустой реализации?
#!/bin/bash
# Поиск других реализаций accept в символах
ast-grep --pattern 'class $_ implements SourceDefinedSymbol {
$$$
accept($_) {
$$$
}
$$$
}'60-64: Проверить безопасность изменения родительского элемента
Поле parent помечено как @Setter, что позволяет его изменение после создания объекта. Это может привести к нарушению целостности дерева символов.
✅ Verification successful
Перезапустить проверку безопасности изменения родительского элемента
Перезапустить проверку безопасности изменения родительского элемента
Изменяемость поля parent безопасна
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск мест, где вызывается setParent
rg -l "setParent.*AnnotationSymbol"
# Поиск использования parent в тестах
fd -e java -E "test" | xargs rg "parent.*AnnotationSymbol"
Length of output: 200
Script:
#!/bin/bash
# Поиск мест, где вызывается setParent
rg -l 'setParent.*AnnotationSymbol'
# Поиск использования parent в тестах
fd -e java -E 'test' | xargs rg 'parent.*AnnotationSymbol'
Length of output: 96
Script:
#!/bin/bash
# Поиск прямых присваиваний поля parent
rg 'parent\s*='
# Поиск любых методов, изменяющих поле parent
rg 'setParent\('
# Поиск использования parent без явных вызовов setParent
rg 'parent\.'
# Поиск всех мест, где используется parent
rg '\bparent\b'
Length of output: 46038
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java (2)
1-45: Корректное оформление заголовка файла и импортов!
Лицензионный заголовок и организация импортов соответствуют стандартам проекта.
101-104: Корректная реализация метода getSymbolKind!
Метод возвращает правильный тип символа для аннотаций.
src/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/SourceDefinedMethodCallInlayHintSupplier.java (3)
28-28: Улучшение архитектуры с использованием DescriptionFormatter
Хорошее архитектурное решение по выделению логики форматирования в отдельный компонент. Внедрение зависимости через конструктор соответствует лучшим практикам Spring Framework.
Also applies to: 67-67
169-173: Корректная реализация форматирования подсказок
Метод setTooltip правильно переработан для использования нового форматтера. Изменение с статического на инстанс-метод логично в контексте внедрения зависимости.
86-86: Использование современного API Java для коллекций
Замена collect(Collectors.toList()) на toList() улучшает читаемость кода. Убедитесь, что проект настроен на использование Java 16 или выше.
Also applies to: 134-134
✅ Verification successful
Замена collect(Collectors.toList()) на toList() подтверждена
Использование toList() безопасно, так как проект настроен на Java 17.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Проверка версии Java в проекте
grep -r "sourceCompatibility" .
Length of output: 99
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (5)
45-45: Правильное внедрение DescriptionFormatter для улучшения модульности
Включение поля descriptionFormatter позволяет централизовать логику форматирования и улучшить повторное использование кода.
56-57: Использование DescriptionFormatter для получения сигнатуры символа
Замена прямых вызовов на использование descriptionFormatter.getSignature(symbol) повышает единообразие и поддерживаемость кода.
60-61: Использование DescriptionFormatter для получения местоположения символа
Использование descriptionFormatter.getLocation(symbol) улучшает модульность и упрощает поддержку.
66-66: Унификация добавления разделов с помощью DescriptionFormatter
Применение descriptionFormatter.addSectionIfNotEmpty упрощает код и обеспечивает согласованность форматирования.
71-71: Единообразное форматирование дополнительного описания переменной
Использование descriptionFormatter для добавления trailingDescription повышает читаемость и согласованность кода.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java (1)
63-63: Добавление зависимости DescriptionFormatter выполнено корректно
.../resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_en.properties
Outdated
Show resolved
Hide resolved
...main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java
Outdated
Show resolved
Hide resolved
| public String getSignature(MethodSymbol methodSymbol) { | ||
| String signatureTemplate = "```bsl\n%s %s(%s)%s%s\n```"; | ||
|
|
||
| String methodKind; | ||
| if (methodSymbol.isFunction()) { | ||
| methodKind = getResourceString(FUNCTION_KEY); | ||
| } else { | ||
| methodKind = getResourceString(PROCEDURE_KEY); | ||
| } | ||
| String methodName = methodSymbol.getName(); | ||
|
|
||
| var parameters = getParametersDescriptionPart(methodSymbol); | ||
| var returnedValueType = getReturnedValueTypeDescriptionPart(methodSymbol); | ||
| String export = methodSymbol.isExport() ? (" " + getResourceString(EXPORT_KEY)) : ""; | ||
|
|
||
| return String.format( | ||
| signatureTemplate, | ||
| methodKind, | ||
| methodName, | ||
| parameters, | ||
| export, | ||
| returnedValueType | ||
| ); | ||
| } | ||
|
|
||
| public String getSignature(AnnotationSymbol symbol, MethodSymbol methodSymbol) { | ||
| String signatureTemplate = "```bsl\n%s &%s(%s)\n```"; | ||
|
|
||
| String annotationKind = getResourceString(ANNOTATION_KEY); | ||
| String annotationName = symbol.getName(); | ||
|
|
||
| var parameters = getParametersDescriptionPart(methodSymbol); | ||
|
|
||
| return String.format( | ||
| signatureTemplate, | ||
| annotationKind, | ||
| annotationName, | ||
| parameters | ||
| ); | ||
| } | ||
|
|
||
| public String getSignature(VariableSymbol symbol) { | ||
| String signatureTemplate = "```bsl\n%s %s%s\n```"; | ||
|
|
||
| String varKey = getResourceString(VARIABLE_KEY); | ||
| String name = symbol.getName(); | ||
| String export = symbol.isExport() ? (" " + getResourceString(EXPORT_KEY)) : ""; | ||
|
|
||
| return String.format( | ||
| signatureTemplate, | ||
| varKey, | ||
| name, | ||
| export | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Рассмотрите возможность устранения дублирования в методах getSignature
В методах getSignature(MethodSymbol), getSignature(AnnotationSymbol, MethodSymbol) и getSignature(VariableSymbol) присутствует схожая логика формирования сигнатуры. Рекомендуется выделить общую часть кода в отдельный метод или использовать полиморфизм для уменьшения дублирования и повышения поддерживаемости.
| var description = type.getDescription().replace("\n", "<br>" + " ".repeat(level + 1)); | ||
|
|
||
| if (!description.isBlank()) { | ||
| description = "- " + description; | ||
| } |
There was a problem hiding this comment.
Проверьте правильность экранирования HTML в описаниях типа
Метод type.getDescription() может возвращать строки, содержащие специальные HTML-символы. Рекомендуется экранировать эти символы для предотвращения возможных XSS-уязвимостей при отображении описаний.
Примените следующий дифф для исправления:
-var description = type.getDescription().replace("\n", "<br>" + " ".repeat(level + 1));
+var description = StringEscapeUtils.escapeHtml4(type.getDescription()).replace("\n", "<br>" + " ".repeat(level + 1));Committable suggestion skipped: line range outside the PR's diff.
…ver/DescriptionFormatter_en.properties Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Qodana for JVM76 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 104 dependenciesThird-party software listThis page lists the third-party software dependencies used in bsl-language-server
Contact Qodana teamContact us at [email protected]
|
| return Optional.empty(); | ||
| } | ||
|
|
||
| var registeredAnnotations = serverContext.getDocuments().values().stream() |
There was a problem hiding this comment.
@coderabbitai предложи структуру хранения кэша расчитанных данных с учётом подписки на событие DocumentContentChangedEvent
There was a problem hiding this comment.
@nixel2007 Ниже предлагаю пример структурного решения, как можно хранить кэш вычисленных данных и оперативно его обновлять при получении события DocumentContextContentChangedEvent.
-
Интерфейс кэша
Создаём интерфейс (или абстрактный класс) для кэша, который хранит вычисленные данные по ключу (например,URIили самDocumentContext). В нём должны быть методы для:- получения данных;
- добавления или обновления данных;
- удаления или полной очистки.
public interface CalculatedDataCache<K, V> { Optional<V> get(K key); void put(K key, V value); void invalidate(K key); void invalidateAll(); }
-
Реализация кэша
В качестве реализации можно использовать, например,ConcurrentHashMap<URI, SomeComputedData>(или другую подходящую структуру). Если нужны политики просрочки (TTL) или ограничения по объёму, можно взять сторонние решения (Guava Cache, Caffeine и др.). Пример базовой реализации:@Component public class CalculatedDataCacheImpl implements CalculatedDataCache<URI, SomeComputedData> { private final ConcurrentMap<URI, SomeComputedData> cache = new ConcurrentHashMap<>(); @Override public Optional<SomeComputedData> get(URI key) { return Optional.ofNullable(cache.get(key)); } @Override public void put(URI key, SomeComputedData value) { cache.put(key, value); } @Override public void invalidate(URI key) { cache.remove(key); } @Override public void invalidateAll() { cache.clear(); } }
Где
SomeComputedData— это класс, в котором хранится результат вашей логики. -
Подписка на событие
В классе, отвечающем за взаимодействие с кэшем (или непосредственно внутри реализации кэша), добавляем метод, помеченный@EventListenerнаDocumentContextContentChangedEvent. В нём при получении события нужно:- определить уникальный идентификатор (URI) документа;
- инвалидировать запись в кэше по этому ключу (или обновить, если это необходимо).
@Component public class CalculatedDataCacheInvalidationListener { private final CalculatedDataCache<URI, SomeComputedData> dataCache; public CalculatedDataCacheInvalidationListener( CalculatedDataCache<URI, SomeComputedData> dataCache ) { this.dataCache = dataCache; } @EventListener public void onDocumentChanged(DocumentContextContentChangedEvent event) { DocumentContext documentContext = (DocumentContext) event.getSource(); URI docUri = documentContext.getUri(); // В зависимости от логики: dataCache.invalidate(docUri); // Или dataCache.put(docUri, пересчитанноеЗначение); } }
-
Использование кэша
В местах, где требуется вычислить и (пере)использовать данные, сначала обращаемся к кэшу:- Пытаемся получить данные по ключу
docUri. - Если данных нет или они устарели — пересчитываем и кладём в кэш.
public SomeComputedData getOrCompute(URI docUri) { return dataCache.get(docUri) .orElseGet(() -> { SomeComputedData result = compute(docUri); dataCache.put(docUri, result); return result; }); }
- Пытаемся получить данные по ключу
Такое решение позволяет:
- Локально хранить промежуточные результаты, не пересчитывая их при каждом обращении.
- Своевременно сбрасывать или обновлять записи, когда приходит событие
DocumentContextContentChangedEvent. - При необходимости подключать расширенные политики кэширования (ограничение по памяти, время жизни и т. п.).
Если в дальнейшем встанет вопрос об оптимальной стратегии обновления (например, полное пересоздание данных или точечное обновление), это тоже можно встроить в логику слушателя и методы кэша.
🔗 Analysis chain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск в кодовой базе упоминаний о DocumentContentChangedEvent (или похожих событиях).
rg "DocumentContentChangedEvent"
rg "ContentChanged"
Length of output: 2157
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java (1)
78-88: Фабричный метод from(...).Метод корректно собирает
AnnotationSymbolна основеMethodSymbol. Рекомендуется проверить, что передаваемые параметры (особенноmethodSymbol.getDescription()) не вызывают потенциальных исключений при отсутствии описания. Возможно, стоит добавить небольшую проверку или обработкуnullдля большей надежности.src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (3)
50-55: Добавьте документацию для публичного APIРекомендуется добавить JavaDoc для публичного класса и его методов, описывающий:
- Назначение класса
- Контракт интерфейса ReferenceFinder
- Потокобезопасность реализации
- Описание зависимостей
@Component @RequiredArgsConstructor +/** + * Реализация {@link ReferenceFinder} для поиска ссылок на аннотации. + * <p> + * Класс является потокобезопасным благодаря использованию {@link ConcurrentHashMap}. + */ public class AnnotationReferenceFinder implements ReferenceFinder {
57-74: Добавьте обработку ошибок в обработчики событийРекомендуется добавить обработку исключений в методы
handleContextRefreshиhandleDocumentContextChangeдля обеспечения стабильности работы сервера даже при возникновении ошибок в процессе обработки событий.@EventListener public void handleContextRefresh(ServerContextPopulatedEvent event) { + try { registeredAnnotations.clear(); serverContext.getDocuments() .values() .forEach(this::findAndRegisterAnnotation); + } catch (Exception e) { + LOGGER.error("Ошибка при обновлении контекста аннотаций", e); + } }
96-110: Улучшите читаемость цепочки OptionalРекомендуется разбить сложную цепочку Optional на отдельные шаги для улучшения читаемости и упрощения отладки.
- return Trees.findTerminalNodeContainsPosition(document.getAst(), position) - .filter(node -> node.getParent().getRuleContext().getRuleIndex() == BSLParser.RULE_annotationName) - .flatMap((TerminalNode annotationNode) -> { - var annotationName = annotationNode.getText(); - var annotationSymbol = registeredAnnotations.get(annotationName); - if (annotationSymbol == null) { - return Optional.empty(); - } - return Optional.of(Reference.of( - document.getSymbolTree().getModule(), - annotationSymbol, - new Location(uri.toString(), Ranges.create(annotationNode.getParent().getParent())) - )); - }); + Optional<TerminalNode> terminalNode = Trees.findTerminalNodeContainsPosition(document.getAst(), position); + + return terminalNode + .filter(this::isAnnotationNameNode) + .flatMap(this::createReferenceFromNode);И добавьте новые приватные методы:
private boolean isAnnotationNameNode(TerminalNode node) { return node.getParent().getRuleContext().getRuleIndex() == BSLParser.RULE_annotationName; } private Optional<Reference> createReferenceFromNode(TerminalNode annotationNode) { var annotationName = annotationNode.getText(); var annotationSymbol = registeredAnnotations.get(annotationName); if (annotationSymbol == null) { return Optional.empty(); } return Optional.of(Reference.of( document.getSymbolTree().getModule(), annotationSymbol, new Location(uri.toString(), Ranges.create(annotationNode.getParent().getParent())) )); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Methods.java(2 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_en.properties(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/context/AbstractServerContextAwareTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractDiagnosticTest.java(2 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java(1 hunks)src/test/resources/references/AnnotationReferenceFinder.os(1 hunks)src/test/resources/references/annotations/ТестоваяАннотация.os(1 hunks)src/test/resources/references/annotations/ТестоваяАннотация2.os(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/resources/references/AnnotationReferenceFinder.os
- src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter_en.properties
- src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java
🔇 Additional comments (17)
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/AbstractServerContextAwareTest.java (6)
1-1: Изменения подтверждены
Пакетная структура выбрана корректно, соответствует другим пакетам проекта.
10-10: Проверить необходимость использования @SpringBootTest
Аннотация @SpringBootTest запускает контекст Spring Boot. Если в тестах не требуются все возможности приложения, возможно достаточно @ExtendWith(SpringExtension.class) или использования специализированных аннотаций. Убедитесь, что это не увеличивает время сборки.
13-13: Инициализация ServerContext через @Autowired
Похоже на подходящую точку внедрения, учитывая что контекст синхронизируется со Spring.
15-18: Очистка контекста
Вызов context.clear() в методе с @PostConstruct гарантирует, что данные контекста будут сброшены перед тестами. Убедитесь, что это не конфликтует с другими тестовыми методами, где данные контекста могут быть необходимы.
20-23: Метод initServerContext(String path)
Переадресация к initServerContext(Path configurationRoot) упрощает код и устраняет дублирование. Это хороший пример стратегии DRY (Don’t Repeat Yourself).
25-28: Вызов context.populateContext()
Метод populateContext() рекомендуется вызывать после установки корня конфигурации. При необходимости добавьте тестовые проверки, что контекст корректно инициализируется, если путь не существует или некорректен.
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractDiagnosticTest.java (2)
25-25: Подключение AbstractServerContextAwareTest вместо локального контекста
Расположение AbstractDiagnosticTest в иерархии, где он наследует AbstractServerContextAwareTest, разгружает класс от ручного управления контекстом. Это улучшает читаемость кода и снижает дублирование.
50-50: Перенос аннотации @SpringBootTest в абстрактный родительский класс
Так как класс теперь расширяет AbstractServerContextAwareTest, содержащий @SpringBootTest, лишняя аннотация в наследнике может перегружать конфигурацию. Убедитесь, что оба класса не создают конфликтов при инициализации контекста Spring.
src/test/resources/references/annotations/ТестоваяАннотация.os (2)
1-1: Добавление аннотации.
Отлично, что используется явная аннотация для тестового сценария. Это обеспечивает наглядность и упрощает поиск ссылок на эту аннотацию.
2-3: Переход к процедуре при создании объекта.
Код выглядит корректно для тестовых целей. Однако, если в будущем потребуется логика внутри процедуры, стоит добавить соответствующие тесты и документацию.
src/test/resources/references/annotations/ТестоваяАннотация2.os (2)
1-1: Добавление аннотации.
Аналогично предыдущему файлу, аннотация хорошо структурирована. Обратите внимание на единообразное именование аннотаций, чтобы упростить дальнейшую поддержку.
2-3: Переход к процедуре при создании объекта.
Определение процедуры выглядит нормально для тестового файла. При необходимости дополнительной логики убедитесь, что процедура сопровождается комментариями и тестами.
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Methods.java (2)
24-25: Явные импорты SymbolTree и MethodSymbol.
Использование данных классов в этом файле уместно и упрощает чтение кода, поскольку ясно, какие именно объекты задействованы.
87-90: Поиск конструктора класса Oscript.
Реализация лямбда-выражением выглядит лаконично. При этом стоит проверить, что getMethodSymbol("ПриСозданииОбъекта") и "OnObjectCreate" действительно возвращают искомые методы во всех нужных сценариях. Рекомендуется добавить соответствующие юнит-тесты для уверенности в корректной работе.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java (2)
1-21: Заголовочный комментарий и лицензия.
Все выглядит корректно и соответствует принятому стилю проекта.
43-47: Определение класса AnnotationSymbol.
Выбранная модель через Lombok упрощает код и делает его более читабельным. Полезно, что поля children и parent исключены из toString. Это поможет избежать потенциальных рекурсивных вызовов при выводе строки.
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (1)
54-55: Рассмотрите использование специализированного решения для кэширования
Текущая реализация с ConcurrentHashMap работоспособна, но специализированные решения для кэширования (например, Caffeine) предоставляют дополнительные возможности:
- Автоматическая инвалидация по времени
- Ограничение размера кэша
- Метрики производительности
Рекомендуется рассмотреть следующие варианты улучшения:
- Использование Caffeine Cache
- Добавление метрик для мониторинга производительности кэша
- Настройка параметров кэша через конфигурацию
| private static String getAnnotationName(Annotation annotation) { | ||
| return annotation.getParameters().get(0).getValue(); | ||
| } |
There was a problem hiding this comment.
Добавьте проверку наличия параметров аннотации
В методе getAnnotationName отсутствует проверка на пустой список параметров, что может привести к NullPointerException или IndexOutOfBoundsException.
private static String getAnnotationName(Annotation annotation) {
+ if (annotation.getParameters().isEmpty()) {
+ throw new IllegalArgumentException("Аннотация должна содержать хотя бы один параметр с именем");
+ }
return annotation.getParameters().get(0).getValue();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static String getAnnotationName(Annotation annotation) { | |
| return annotation.getParameters().get(0).getValue(); | |
| } | |
| private static String getAnnotationName(Annotation annotation) { | |
| if (annotation.getParameters().isEmpty()) { | |
| throw new IllegalArgumentException("Аннотация должна содержать хотя бы один параметр с именем"); | |
| } | |
| return annotation.getParameters().get(0).getValue(); | |
| } |
There was a problem hiding this comment.
проверь новую версию кода
There was a problem hiding this comment.
@nixel2007, судя по результатам поиска, проверяемый метод getAnnotationName не содержит дополнительной проверки на пустой список параметров аннотации. В выводе выше по-прежнему используется return annotation.getParameters().get(0).getValue(); без проверки annotation.getParameters().isEmpty(). Возможно, последний коммит с добавленной проверкой ещё не включён в ветку, или изменения не были закоммичены.
Уточните, пожалуйста, действительно ли вы вносили исправления в этот метод и где именно находятся эти изменения, чтобы мы могли перепроверить.
🔗 Analysis chain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Let's search for the updated implementation of the method getAnnotationName
rg "private static String getAnnotationName" -A 10 -B 2
Length of output: 774
There was a problem hiding this comment.
Этот метод вызывается в пайплайне, на предыдущем шаге которого осуществляется проверка на количество параметров. Посмотри в соседние методы.
There was a problem hiding this comment.
@nixel2007, действительно, в методе findAnnotation уже есть фильтрация по количеству параметров аннотации:
.filter(annotation -> annotation.getParameters().size() == 1)Это означает, что на этапе перед вызовом getAnnotationName(...) гарантируется, что у аннотации ровно один параметр. Следовательно, дополнительная проверка на пустой список внутри самого getAnnotationName в текущем пайплайне не требуется: код не вылетит по IndexOutOfBoundsException, потому что findAnnotation просто не вернёт Optional для аннотаций с нулём (или любым другим числом) параметров.
Если же вы захотите поддерживать ситуацию с несколькими параметрами или нулевым количеством параметров, то в будущем можно расширить логику фильтрации и добавить соответствующие проверки. Но в текущем варианте код выглядит корректным.
🔗 Analysis chain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Поиск упоминаний метода getAnnotationName и
# проверка соседнего кода на предмет проверок размера списка параметров аннотации
echo "Searching for references to getAnnotationName..."
rg -A 10 -B 10 'getAnnotationName'
echo "Now searching for pipeline methods or references to annotations parameter checks..."
rg -A 20 -B 5 'annotation.*parameters.*size|annotation.*parameters.*isEmpty'
Length of output: 5345
✏️ Learnings added
Learnt from: nixel2007
PR: 1c-syntax/bsl-language-server#3364
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java:125-127
Timestamp: 2024-12-27T17:03:11.662Z
Learning: В методе `findAnnotation` класса `AnnotationReferenceFinder` уже проверяется, что у аннотации ровно один параметр, поэтому в `getAnnotationName` не требуется отдельная проверка на пустой список.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java (1)
334-340:⚠️ Potential issueУбедитесь в необходимости экранирования HTML-символов.
Значениеtype.getDescription()может содержать потенциально опасные символы, которые не экранируются. Подобный комментарий уже рассматривался ранее.Рекомендуется использовать
StringEscapeUtils.escapeHtml4(...), например:- var description = type.getDescription().replace("\n", "<br>" + " ".repeat(level + 1)); + var description = StringEscapeUtils.escapeHtml4(type.getDescription()) + .replace("\n", "<br>" + " ".repeat(level + 1));
🧹 Nitpick comments (7)
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/AbstractServerContextAwareTest.java (2)
31-32: Добавьте документацию для абстрактного классаРекомендуется добавить JavaDoc с описанием назначения класса, его ответственности и примером использования. Это упростит понимание класса для других разработчиков.
36-44: Добавьте документацию для методов инициализацииРекомендуется добавить JavaDoc для методов
abstractServerContextAwareTestInitиinitServerContext, описывающий:
- Когда вызываются эти методы
- Какие предусловия должны быть выполнены
- Какой эффект оказывает их выполнение
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (4)
54-55: Рассмотрите использование кэша с политикой устареванияТекущая реализация с
ConcurrentHashMapне предусматривает механизма очистки устаревших записей. Рекомендуется рассмотреть использование Caffeine или Guava Cache с настраиваемой политикой устаревания записей.
77-81: Создайте issue для отслеживания проблемы двойной регистрацииКомментарий в коде указывает на известную проблему с двойной регистрацией аннотаций. Рекомендуется создать issue для отслеживания этой проблемы и её последующего решения.
Хотите, чтобы я помог создать GitHub issue для отслеживания этой задачи?
94-115: Добавьте документацию для метода findReferenceРекомендуется добавить подробную документацию для метода
findReference:
- Описание входных параметров
- Описание возвращаемого значения
- Примеры использования
- Возможные краевые случаи
117-123: Улучшите читаемость метода findAnnotationРекомендуется выделить предикаты фильтрации в отдельные именованные методы для улучшения читаемости:
private static Optional<Pair<MethodSymbol, Annotation>> findAnnotation(MethodSymbol methodSymbol) { return methodSymbol.getAnnotations().stream() - .filter(annotation -> annotation.getName().equalsIgnoreCase("Аннотация")) - .findFirst() - .filter(annotation -> annotation.getParameters().size() == 1) + .filter(AnnotationReferenceFinder::isAnnotationType) + .findFirst() + .filter(AnnotationReferenceFinder::hasExactlyOneParameter) .map(annotation -> Pair.of(methodSymbol, annotation)); } +private static boolean isAnnotationType(Annotation annotation) { + return annotation.getName().equalsIgnoreCase("Аннотация"); +} + +private static boolean hasExactlyOneParameter(Annotation annotation) { + return annotation.getParameters().size() == 1; +}src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java (1)
63-69: Обратите внимание на возможные лишние разделители.
Текущий метод добавляет пустую строку и "---" при любом непустом контенте. Если секций будет слишком много, в итоговом описании может получиться чересчур много разделителей. Рассмотрите логику объединения близких секций, если это уместно в рамках дизайна.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/SourceDefinedMethodCallInlayHintSupplier.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/context/AbstractServerContextAwareTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractDiagnosticTest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/SourceDefinedMethodCallInlayHintSupplier.java
🔇 Additional comments (15)
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractDiagnosticTest.java (1)
47-47: Корректное использование наследования
Перенос функциональности управления контекстом в родительский класс улучшает организацию кода и устраняет дублирование.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java (1)
1-21: Заголовок и лицензионное уведомление соответствуют требуемым условиям.
Файл содержит корректную лицензионную информацию и сведения об авторах.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (5)
43-43: Использование DescriptionFormatter упрощает логику формирования контента.
Переход к общему форматтеру улучшает читаемость и снижает дублирование кода.
54-55: Добавление секции с сигнатурой переменной выглядит корректно.
Эти строки наглядно отделяют сигнатуру от остального содержимого.
58-59: Добавление секции с локацией переменной.
Отдельная секция повышает удобочитаемость и помогает пользователю быстро находить исходник.
64-64: Лаконичная передача описания через лямбду.
Использование ifPresent совместно с descriptionFormatter.addSectionIfNotEmpty выглядит аккуратно и читаемо.
69-69: Обработка trailingDescription выполнена аналогичным образом.
Наличие возможности дополнять описание дополнительными данными говорит о расширяемости кода.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java (8)
50-50: Расширение класса за счёт внедрения DescriptionFormatter.
Заменой LanguageServerConfiguration на DescriptionFormatter повышается модульность кода и единообразие при форматировании.
65-66: Выделение сигнатуры метода через общий форматтер.
Обращение к descriptionFormatter.getSignature(symbol) создаёт единый подход к формированию текста.
69-70: Отдельный блок для локации метода.
Прозрачно информирует о месте определения и упрощает навигацию.
73-74: Добавление смыслового блока описания метода.
Ясное отображение цели метода упрощает понимание кода для пользователей.
77-78: Форматирование параметров с помощью descriptionFormatter.
Уменьшает дублирование и делает структуру выходного текста более цельной.
81-82: Отдельная секция для возвращаемого значения.
Даёт пользователям чёткое представление об итоговом результате метода.
85-86: Секция примеров повышает понятность.
Прямой вызов descriptionFormatter.getExamplesSection(symbol) делает код чище.
89-90: Учёт вариантов вызова (call options).
Логично отделённая часть документации, улучшающая справочные материалы.
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
|
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно