-
Notifications
You must be signed in to change notification settings - Fork 108
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
Исправление поиска начала метода при наличии аннотаций #3359
Conversation
WalkthroughИзменения в коде касаются добавления проверки аннотаций в методах Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java (3)
Обновление размеров списков методов соответствует добавленным методам с аннотациями. Использование Also applies to: 52-52, 54-54
Тест проверяет корректность извлечения описания метода при наличии аннотации перед описанием. Структура теста соответствует общему стилю тестового класса.
Тест проверяет корректность извлечения описания метода при наличии аннотации. Структура теста соответствует общему стилю тестового класса. 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 (
|
4933dc3
to
9faef4b
Compare
Qodana for JVM76 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 101 dependenciesThird-party software listThis page lists the third-party software dependencies used in bsl-language-server
Contact Qodana teamContact us at [email protected]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java (1)
126-129
: Дублирование логики обработки аннотацийЛогика обработки аннотаций идентична методу
visitFunction
. Рекомендуется вынести общую логику в отдельный приватный метод для улучшения поддерживаемости кода.Предлагаемый рефакторинг:
+ private TerminalNode getStartNodeConsideringAnnotations(TerminalNode startNode, List<? extends BSLParser.AnnotationContext> annotations) { + if (!annotations.isEmpty()) { + return annotations.get(0).AMPERSAND(); + } + return startNode; + } @Override public ParseTree visitFunction(BSLParser.FunctionContext ctx) { // ... - if (!declaration.annotation().isEmpty()) { - startNode = declaration.annotation().get(0).AMPERSAND(); - } + startNode = getStartNodeConsideringAnnotations(startNode, declaration.annotation()); // ... } @Override public ParseTree visitProcedure(BSLParser.ProcedureContext ctx) { // ... - if (!declaration.annotation().isEmpty()) { - startNode = declaration.annotation().get(0).AMPERSAND(); - } + startNode = getStartNodeConsideringAnnotations(startNode, declaration.annotation()); // ... }src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java (1)
59-63
: Добавлен тест для проверки метода с аннотациейТест проверяет корректность описания метода с аннотацией, что соответствует целям PR.
Предлагаю добавить дополнительные проверки для самой аннотации:
@Test void testMethodWithAnnotation() { var method = methodsWithDescription.get(13); assertThat(method.getDescription()).isEqualTo("// Описание процедуры"); + assertThat(method.getAnnotations()).isNotEmpty(); + assertThat(method.getAnnotations().get(0).getName()).isEqualTo("Аннотация"); + assertThat(method.getAnnotations().get(0).getParameters()).containsExactly("Параметр"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java
(2 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java
(1 hunks)src/test/resources/context/symbol/MethodDescription.bsl
(1 hunks)
🔇 Additional comments (5)
src/test/resources/context/symbol/MethodDescription.bsl (2)
145-145
: Подтверждаю изменение модификатора экспорта!
Добавление модификатора Экспорт
соответствует стилю документирования и структуре файла.
147-150
: Корректное добавление процедуры с аннотацией!
Новая процедура ПроцедураСАннотацией
:
- Правильно документирована
- Корректно использует аннотацию
- Соответствует общему стилю файла
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java (1)
92-95
: Корректная обработка аннотаций в методе visitFunction
Логика обновления начального узла метода при наличии аннотаций реализована корректно. Проверка на непустой список аннотаций обеспечивает безопасность доступа к первой аннотации.
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java (2)
47-47
: Корректное обновление утверждений для количества методов
Увеличение ожидаемого количества методов соответствует добавлению нового метода с аннотацией.
Also applies to: 55-55
53-53
: Использование современного API Java для работы с коллекциями
Замена collect(Collectors.toList())
на toList()
улучшает читаемость кода.
✅ Verification successful
Использование toList()
безопасно с текущей версией Java
Проект использует Java 17, в которой метод Stream.toList()
доступен (добавлен в Java 16). Замена collect(Collectors.toList())
на toList()
является корректной и безопасной.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Проверка минимальной версии Java в проекте
grep -r "sourceCompatibility" . || grep -r "java.version" . || grep -r "<java.version>" .
Length of output: 99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java (1)
59-69
: Рекомендация по улучшению тестовДля повышения качества тестирования рекомендуется добавить проверки:
- Корректности обработки самих аннотаций
- Правильности определения начальной позиции метода
Предлагаю дополнить тесты следующими проверками:
@Test void testMethodWithAnnotationBeforeDescription() { var method = methodsWithDescription.get(14); assertThat(method.getDescription()).isEqualTo("// Описание процедуры"); + assertThat(method.getAnnotations()).isNotEmpty(); + assertThat(method.getRange().getStart()) + .isEqualTo(method.getAnnotations().get(0).getRange().getStart()); } @Test void testMethodWithAnnotation() { var method = methodsWithDescription.get(13); assertThat(method.getDescription()).isEqualTo("// Описание процедуры"); + assertThat(method.getAnnotations()).isNotEmpty(); + assertThat(method.getRange().getStart()) + .isEqualTo(method.getAnnotations().get(0).getRange().getStart()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java
(2 hunks)src/test/resources/context/symbol/MethodDescription.bsl
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/resources/context/symbol/MethodDescription.bsl
🔇 Additional comments (5)
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java (5)
47-47
: Обновление количества методов соответствует изменениям
Увеличение ожидаемого количества методов с 14 до 16 корректно отражает добавление новых методов с аннотациями.
53-53
: Использование современного API Java
Замена collect(Collectors.toList())
на toList()
улучшает читаемость кода. Это хорошая практика использования современных возможностей Java.
55-55
: Корректное обновление проверки количества методов с описанием
Увеличение ожидаемого количества методов с описанием с 13 до 15 соответствует добавленным методам с аннотациями.
59-63
: Новый тест для метода с аннотацией перед описанием
Тест корректно проверяет наличие описания для метода с аннотацией. Структура теста соответствует общему стилю проекта.
65-69
: Новый тест для метода с аннотацией
Тест корректно проверяет наличие описания для метода с аннотацией. Структура теста соответствует общему стилю проекта.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java (1)
127-140
: Рассмотрите возможность рефакторинга дублирующейся логикиЛогика обработки аннотаций идентична в методах visitFunction и visitProcedure. Рекомендуется выделить общую логику в отдельный приватный метод.
+ private TerminalNode getMethodStartNode(List<? extends BSLParser.AnnotationContext> annotations, TerminalNode defaultNode) { + return !annotations.isEmpty() ? annotations.get(0).AMPERSAND() : defaultNode; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java
(6 hunks)
🔇 Additional comments (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java (4)
98-105
: Улучшение семантики создания метода
Добавление параметра startOfMethod и переименование метода в createAnnotations улучшает читаемость кода и точнее отражает семантику операций.
251-251
: Оптимизация создания списков
Использование метода toList() вместо collect(Collectors.toList()) является правильным решением, так как это более лаконичный и эффективный способ создания неизменяемых списков.
Also applies to: 328-328, 349-349
325-328
: Улучшение именования метода создания аннотаций
Переименование метода в createAnnotations лучше отражает его ответственность за создание новых объектов аннотаций, а не просто получение существующих.
92-94
: Корректная обработка начала метода при наличии аннотаций
Логика обновления начальной позиции метода при наличии аннотаций реализована верно. Теперь startNode указывает на символ & первой аннотации, что соответствует фактическому началу метода в исходном коде.
✅ Verification successful
Обработка начала метода с аннотациями реализована корректно
На основе анализа тестов подтверждается, что логика обработки начальной позиции метода при наличии аннотаций реализована верно:
- Тесты проверяют корректное определение позиции начала метода для различных комбинаций аннотаций и директив компилятора
- Тестовые данные включают методы с одиночными аннотациями, множественными аннотациями и их комбинациями с директивами
- Проверяется корректное определение позиции символа & для первой аннотации в различных сценариях
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Проверка корректности обработки аннотаций в тестах
rg -l "annotation.*AMPERSAND" src/test/
Length of output: 41
Script:
#!/bin/bash
# Поиск тестов, связанных с обработкой аннотаций методов
rg -l "annotation" src/test/
# Поиск использования AMPERSAND в тестах
rg -l "AMPERSAND" src/test/
# Поиск тестовых файлов для MethodSymbolComputer
fd "MethodSymbolComputer.*Test" src/test/
# Поиск тестовых данных с аннотациями
fd -e bsl src/test/resources/
Length of output: 9436
Script:
#!/bin/bash
# Проверим содержимое тестового файла MethodSymbolComputerTest
cat src/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputerTest.java
# Проверим содержимое тестовых BSL файлов
cat src/test/resources/context/computer/MethodSymbolComputerTest.bsl
cat src/test/resources/context/computer/MethodSymbolComputerTestParseError.bsl
Length of output: 16060
Quality Gate passedIssues Measures |
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit
)Для диагностик
Дополнительно