Показ ховера при наведении на параметры аннотации. #3423
Conversation
В качестве имени параметра аннотации без явного имени используется имя "Значение"
WalkthroughВ данном пулл-реквесте добавлены новые возможности для работы с аннотациями в BSL Language Server. В частности, введён новый класс Changes
Sequence Diagram(s)sequenceDiagram
participant C as Клиент
participant ARF as AnnotationReferenceFinder
participant AS as AnnotationSymbol
C->>ARF: findReference(URI, Position)
ARF->>ARF: Проверка наличия TerminalNode
alt TerminalNode найден
ARF->>ARF: getAnnotationSymbol(annotationNode)
ARF->>ARF: getReferenceToAnnotationParam(docContext, annotationParamContext)
ARF-->>C: Optional(Reference)
else Нет TerminalNode
ARF-->>C: Optional.empty()
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (1)src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/RunAllTestsCodeLensSupplier.java (3)⏰ Context from checks skipped due to timeout of 90000ms (19)
🔇 Additional comments (8)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (1)
141-173: Метод getReferenceToAnnotationParamЛогика разрешения имени параметра и формирования ссылки на символ параметра аннотации выглядит корректной. Однако цепочка методов map/orElse/flatMap достаточно длинная и может снижать читаемость.
Рассмотрите возможность выделить ключевые шаги в отдельные локальные переменные для упрощения понимания кода.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationParamSymbol.java (1)
39-43: Добавьте документацию классаРекомендуется добавить JavaDoc документацию для класса
AnnotationParamSymbol, описывающую его назначение и использование.src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.java (1)
47-72: Добавьте логированиеРекомендуется добавить логирование в следующих случаях:
- Когда
maybeMethodSymbolне содержитMethodSymbol- Когда не найдено определение параметра
@Override public MarkupContent getContent(AnnotationParamSymbol symbol) { var maybeMethodSymbol = symbol.getParent(); if (maybeMethodSymbol.filter(MethodSymbol.class::isInstance).isEmpty()) { + log.debug("Parent symbol is not a MethodSymbol for {}", symbol.getName()); return new MarkupContent(MarkupKind.MARKDOWN, ""); } var markupBuilder = new StringJoiner("\n"); var methodSymbol = (MethodSymbol) maybeMethodSymbol.get(); var maybeParameterDefinition = methodSymbol.getParameters().stream() .filter(parameter -> parameter.getName().equalsIgnoreCase(symbol.getName())) .findFirst(); if (maybeParameterDefinition.isEmpty()) { + log.debug("Parameter definition not found for {} in {}", + symbol.getName(), methodSymbol.getName()); return new MarkupContent(MarkupKind.MARKDOWN, ""); }src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java (1)
87-107: Проверка поведения значений параметров аннотаций!Тест хорошо покрывает поведение значений параметров, включая случай с параметром по умолчанию "Значение".
Предлагаю добавить дополнительные тест-кейсы для:
- Параметров с пустым значением
- Параметров с некорректным форматом значения
- Нескольких параметров в одной аннотации
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationParamSymbol.java(1 hunks)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/AnnotationParamSymbolMarkupContentBuilder.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/references/AnnotationReferenceFinder.java(3 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java(2 hunks)src/test/resources/references/AnnotationReferenceFinder.os(1 hunks)src/test/resources/references/annotations/ТестоваяАннотация.os(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: build
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: Analyse
- GitHub Check: Benchmark
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
🔇 Additional comments (18)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (9)
29-29: Добавление импорта для AnnotationParamSymbolНовый импорт выглядит корректным и необходим для поддержки логики работы с параметрами аннотаций.
38-38: Добавление импорта BSLParserRuleContextИмпорт BSLParserRuleContext обоснован и упрощает работу с контекстом парсера при поиске ссылок.
98-106: Проверка DocumentContext и формата файлаЛогика проверки на null и соответствующий формат файла (OS) выглядит корректной, предотвращает дальнейшие ошибки при неправильном типе документа.
108-110: Получение терминального узла и его родителяИзвлечение terminalNode и получение parent выглядят корректно и позволяют точно определить контекст для дальнейших проверок.
111-120: Обработка имени аннотации (AnnotationNameContext)Условие и получение AnnotationSymbol корректно формируют ссылку на аннотацию, что обеспечивает правильное разрешение ссылок в дальнейшем.
120-125: Обработка именованного параметра аннотации (AnnotationParamNameContext)Логика поиска и преобразования AnnotationParamContext выглядит правильно и предоставляет корректную привязку к параметру аннотации.
125-131: Обработка значения параметра аннотации (ConstValueContext)Ветвь для ConstValueContext корректно обрабатывает анонимный параметр, упрощая разрешение ссылок при отсутствии имени.
133-134: Возврат пустого результатаЗавершение метода пустым Optional при отсутствии подходящего контекста гарантирует корректное поведение поиска ссылок.
136-139: Метод getAnnotationSymbolПростая реализация, эффективно возвращающая сохранённый AnnotationSymbol на основе его имени. Логика выглядит надёжной.
src/test/resources/references/AnnotationReferenceFinder.os (1)
1-1: Добавление параметров в аннотациюДобавление именованного параметра ВторойПараметр и анонимного параметра обеспечивает тестовое покрытие сценариев реального использования аннотаций. Изменение выглядит логичным и согласуется с поддерживаемой функциональностью.
src/test/resources/references/annotations/ТестоваяАннотация.os (2)
1-6: Добавление комментариев о параметрах аннотацииОписание параметров аннотации повышает ясность и упрощает поддержку кода. Документация выглядит полезной и понятной.
8-8: Изменение сигнатуры процедуры ПриСозданииОбъектаРасширение параметров процедуры для учёта новых аргументов аннотации выглядит согласованным с обновлённой логикой.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationSymbol.java (1)
67-69: Хорошее изменение!Замена поля
symbolKindна методgetSymbolKind()с константным значением упрощает код и делает его более поддерживаемым.src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/AnnotationParamSymbol.java (1)
44-85: Реализация выглядит правильнойСтруктура класса хорошо продумана:
- Использование Lombok аннотаций для уменьшения шаблонного кода
- Корректная реализация интерфейсов
- Правильное использование Optional для nullable полей
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.java (1)
40-78: Реализация выглядит правильнойКласс корректно реализует построение разметки для параметров аннотаций:
- Правильная обработка Optional значений
- Корректное форматирование описания параметра
- Соответствующий символьный тип TypeParameter
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java (1)
90-92: Корректное изменение типа символаИзменение возвращаемого значения на
SymbolKind.Interfaceсогласуется с изменениями в классеAnnotationSymbolи лучше отражает семантику аннотаций.src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java (2)
44-63: Улучшение читаемости и расширение покрытия тестов!Переименование метода делает его назначение более понятным, а добавленные проверки
symbolKindиselectionRangeулучшают покрытие тестами.
65-85: Хорошее покрытие тестами для имён параметров аннотаций!Тест корректно проверяет поиск ссылок на имена параметров аннотаций, включая проверку типа символа и диапазонов выделения.
nixel2007
force-pushed
the
feature/hover-annotation-param
branch
from
756041d to
c8d25e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/test/resources/references/AnnotationReferenceFinder.os (3)
1-1: Проверьте порядок параметров аннотации.Рекомендуется сначала указывать позиционные параметры, а затем именованные. В текущей реализации именованный параметр
ВторойПараметруказан перед позиционным параметром.-&ТестоваяАннотация(ВторойПараметр = "ТестовоеЗначение", "ТестовоеЗначение2") +&ТестоваяАннотация("ТестовоеЗначение2", ВторойПараметр = "ТестовоеЗначение")
8-11: Рассмотрите возможность использования heredoc-строк.Для улучшения читаемости многострочных строк рекомендуется использовать heredoc-синтаксис вместо конкатенации с символом
|.
18-28: Улучшите форматирование многострочной аннотации.Для улучшения читаемости рекомендуется:
- Группировать параметры по типам
- Добавить комментарии для логического разделения групп параметров
&ТестоваяАннотация( + // Числовые значения 42, -42, + + // Строковые значения "Строка", "Многострочная |Строка", + '20200101', + + // Логические значения Истина, - '20200101', + + // Специальные значения Неопределено, NULL )src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java (2)
111-132: Улучшите организацию тестовых данных.Для улучшения поддерживаемости тестов рекомендуется:
- Вынести тестовые данные в отдельный ресурсный файл
- Добавить описательные комментарии к каждому набору тестовых данных
61-63: Используйте константы для магических чисел в проверках диапазонов.Рекомендуется вынести числовые значения диапазонов в именованные константы для улучшения читаемости и поддерживаемости тестов.
+ private static final int ANNOTATION_START = 0; + private static final int ANNOTATION_END = 76; + private static final int SOURCE_SELECTION_START = 7; + private static final int SOURCE_SELECTION_END = 28; + @Test void findReferenceOfAnnotation() { // ... assertThat(optionalReference) .hasValueSatisfying(reference -> assertThat(reference.getSymbol().getSymbolKind()).isEqualTo(SymbolKind.Interface)) - .hasValueSatisfying(reference -> assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(0, 0, 76))) - .hasValueSatisfying(reference -> assertThat(reference.getSourceDefinedSymbol().orElseThrow().getSelectionRange()).isEqualTo(Ranges.create(7, 10, 28))) + .hasValueSatisfying(reference -> assertThat(reference.getSelectionRange()).isEqualTo(Ranges.create(ANNOTATION_START, 0, ANNOTATION_END))) + .hasValueSatisfying(reference -> assertThat(reference.getSourceDefinedSymbol().orElseThrow().getSelectionRange()).isEqualTo(Ranges.create(SOURCE_SELECTION_START, 10, SOURCE_SELECTION_END)))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java(3 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java(3 hunks)src/test/resources/references/AnnotationReferenceFinder.os(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: Sentry
🔇 Additional comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinderTest.java (1)
133-151: Добавьте проверку имени параметра для каждого типа литерала.В текущей реализации проверяется только имя параметра "Значение". Рекомендуется добавить тесты для проверки корректности работы с именованными параметрами разных типов.
|
В качестве имени параметра аннотации без явного имени используется имя "Значение"
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно