Всплывающая подсказка и переход к определениям для аннотаций в OneScript #3364
Conversation
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}
| .map(DocumentContext::getSymbolTree) | ||
| .flatMap(symbolTree -> symbolTree.getMethodSymbol("ПриСозданииОбъекта").stream()) | ||
| .filter(methodSymbol -> methodSymbol.getAnnotations().stream().anyMatch(annotation -> annotation.getName().equals("Аннотация"))) | ||
| .map(methodSymbol -> Pair.of(methodSymbol, methodSymbol.getAnnotations().stream().filter(annotation -> annotation.getName().equals("Аннотация")).findFirst().get())) |
Check warning
Code scanning / QDJVM
Optional.get() is called without isPresent() check Warning
|
![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 выполнено корректно
| @@ -1,8 +1,10 @@ | |||
| annotation=Annotation | |||
| callOptions=Сall options | |||
There was a problem hiding this comment.
Исправьте кириллический символ в английском тексте
В строке callOptions используется кириллическая буква 'С' вместо латинской 'C'.
Примените следующее исправление:
-callOptions=Сall options
+callOptions=Call options📝 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.
| callOptions=Сall options | |
| callOptions=Call options |
| .map(DocumentContext::getSymbolTree) | ||
| .flatMap(symbolTree -> symbolTree.getMethodSymbol("ПриСозданииОбъекта").stream()) | ||
| .filter(methodSymbol -> methodSymbol.getAnnotations().stream().anyMatch(annotation -> annotation.getName().equals("Аннотация"))) | ||
| .map(methodSymbol -> Pair.of(methodSymbol, methodSymbol.getAnnotations().stream().filter(annotation -> annotation.getName().equals("Аннотация")).findFirst().get())) |
There was a problem hiding this comment.
Проверка наличия значения перед вызовом get()
В строке 61 вызывается метод get() на объекте Optional без предварительной проверки наличия значения, что может привести к NoSuchElementException, если аннотация не будет найдена. Несмотря на предварительный фильтр, рекомендуется явно обработать Optional для повышения надежности кода.
Предлагаю изменить код следующим образом:
- .map(methodSymbol -> Pair.of(methodSymbol, methodSymbol.getAnnotations().stream().filter(annotation -> annotation.getName().equals("Аннотация")).findFirst().get()))
+ .flatMap(methodSymbol -> methodSymbol.getAnnotations().stream()
+ .filter(annotation -> annotation.getName().equals("Аннотация"))
+ .findFirst()
+ .map(annotation -> Pair.of(methodSymbol, annotation))
+ )📝 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.
| .map(methodSymbol -> Pair.of(methodSymbol, methodSymbol.getAnnotations().stream().filter(annotation -> annotation.getName().equals("Аннотация")).findFirst().get())) | |
| .flatMap(methodSymbol -> methodSymbol.getAnnotations().stream() | |
| .filter(annotation -> annotation.getName().equals("Аннотация")) | |
| .findFirst() | |
| .map(annotation -> Pair.of(methodSymbol, annotation)) | |
| ) |
🧰 Tools
🪛 GitHub Check: QDJVM
[warning] 61-61: Optional.get() is called without isPresent() check
'Optional.get()' without 'isPresent()' check
| 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.
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно