Feauture/query nested fields by dot #3470
Conversation
… таблиц, условий приобразования типов, запросов в виртуальных таблицах и соединений
…кими полями объекта в одной колонке запроса
… данных сообщения диагностики
…истрации проверок, сообщения проверки упрощено, исключено форматирование в связи с тем, что контекст, передаваемый в сообщение полностью аналогичен контексту диагностики
WalkthroughВ проект добавлена новая диагностика QueryNestedFieldsByDot, предназначенная для выявления разыменования ссылочных полей через точку в тексте запроса 1С. Реализованы диагностический класс, документация, локализации сообщений, тесты и тестовые данные, охватывающие различные сценарии использования точки для доступа к вложенным полям. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BSL_LanguageServer
participant QueryNestedFieldsByDotDiagnostic
User->>BSL_LanguageServer: Запуск анализа запроса 1С
BSL_LanguageServer->>QueryNestedFieldsByDotDiagnostic: Передача AST запроса
QueryNestedFieldsByDotDiagnostic->>QueryNestedFieldsByDotDiagnostic: Обход элементов запроса
alt Найдено разыменование через точку
QueryNestedFieldsByDotDiagnostic->>BSL_LanguageServer: Добавить диагностическое сообщение
end
BSL_LanguageServer->>User: Возврат списка диагностик
Assessment against linked issues
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
🧹 Nitpick comments (6)
docs/en/diagnostics/QueryNestedFieldsByDot.md (1)
5-7: Улучшить грамматику в описанииВ английском тексте есть грамматическая ошибка в использовании слова "dereference". В данном контексте лучше использовать "dereferencing", так как речь идет о процессе.
- Diagnostics allows you to control the dereference of reference fields through a dot in the 1C query language. + Diagnostics allows you to control the dereferencing of reference fields through a dot in the 1C query language.🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: The verb ‘dereference’ does not usually follow articles like ‘the’. Check that ‘dereference’ is spelled correctly; using ‘dereference’ as a noun may be non-standard.
Context: ...ption Diagnostics allows you to control the dereference of reference fields through a dot in th...(A_INFINITIVE)
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnosticTest.java (1)
36-55: Тесты охватывают все сценарии диагностикиТестовый метод проверяет 12 различных случаев срабатывания диагностики с указанием конкретных диапазонов в исходном коде. Комментарии "Ошибка №X" помогают соотнести тестовые случаи с примерами из документации.
Возможно, стоит добавить небольшое описание к тесту для лучшего понимания, какой именно случай разыменования проверяется в каждом утверждении.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java (4)
44-44: Рекомендуется изменить модификатор доступа.Флаг
isVirtualTableобъявлен как public, но поскольку это внутренняя деталь реализации, рекомендуется использовать модификатор private или protected.- public boolean isVirtualTable = false; + private boolean isVirtualTable = false;
53-56: Рекомендуется использовать enterVirtualTableParameter вместо exitVirtualTableParameter.Метод
exitVirtualTableParameterиспользуется для установки флагаisVirtualTable, но логичнее было бы установить флаг при входе в узел виртуальной таблицы, используя методenterVirtualTableParameter.- @Override - public void exitVirtualTableParameter(SDBLParser.VirtualTableParameterContext ctx) { - isVirtualTable = true; //Взводим флаг при начале обработки параметров виртуальной таблицы - super.exitVirtualTableParameter(ctx); - } + @Override + public void enterVirtualTableParameter(SDBLParser.VirtualTableParameterContext ctx) { + isVirtualTable = true; //Взводим флаг при начале обработки параметров виртуальной таблицы + super.enterVirtualTableParameter(ctx); + }
41-80: Рекомендуется добавить JavaDoc-комментарии.Диагностический класс и его методы не содержат JavaDoc-комментариев. Для улучшения поддержки кода рекомендуется добавить документацию, объясняющую назначение класса и логику работы каждого метода.
61-61: Рекомендуется вынести магическое значение в константу.Условие
ctx.columnNames.size() > 1используется в нескольких местах. Рекомендуется вынести это значение в именованную константу для лучшей читаемости кода.+ private static final int MIN_COLUMNS_FOR_DEREFERENCE = 1; + @Override public void enterFunctionCall(SDBLParser.FunctionCallContext ctx) { //Контролируем разыменование в функциях (ВЫРАЗИТЬ, ЕСТЬNULL и т.д.) - if(ctx.identifier != null && ctx.columnNames.size() > 1){ + if(ctx.identifier != null && ctx.columnNames != null && ctx.columnNames.size() > MIN_COLUMNS_FOR_DEREFERENCE){ diagnosticStorage.addDiagnostic(ctx); } super.enterFunctionCall(ctx); } @Override public void enterColumn(SDBLParser.ColumnContext ctx) { /*Если взведен флаг обработки виртуальной таблицы и определен контекст метаданных, то проверяем заполненность контекста имен колонок. В противном случае считаем что работаем со стандартным полем выборки или соединения и выводим ошибку когда список имен колонки содержит более одного идентификатора */ - if((isVirtualTable && ctx.mdoName != null && !ctx.columnNames.isEmpty()) || ctx.columnNames.size() > 1){ + if((isVirtualTable && ctx.mdoName != null && ctx.columnNames != null && !ctx.columnNames.isEmpty()) || + (ctx.columnNames != null && ctx.columnNames.size() > MIN_COLUMNS_FOR_DEREFERENCE)){Also applies to: 74-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.jsonis excluded by!**/*.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.jsonis excluded by!**/*.json
📒 Files selected for processing (7)
docs/diagnostics/QueryNestedFieldsByDot.md(1 hunks)docs/en/diagnostics/QueryNestedFieldsByDot.md(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic_en.properties(1 hunks)src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic_ru.properties(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnosticTest.java(1 hunks)src/test/resources/diagnostics/QueryNestedFieldsByDotDiagnostic.bsl(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/diagnostics/QueryNestedFieldsByDot.md
[grammar] ~5-~5: The verb ‘dereference’ does not usually follow articles like ‘the’. Check that ‘dereference’ is spelled correctly; using ‘dereference’ as a noun may be non-standard.
Context: ...ption Diagnostics allows you to control the dereference of reference fields through a dot in th...
(A_INFINITIVE)
[grammar] ~26-~26: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...ЕЖДУ &НачалоПериода И &КонецПериода` ## Sources Source: [Dereference of composite type referen...
(REPEATED_VERBS)
🔇 Additional comments (17)
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic_ru.properties (1)
1-2: Корректные локализованные сообщения для диагностикиСообщения для диагностики определены корректно и соответствуют назначению диагностики.
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic_en.properties (1)
1-2: Корректные локализованные сообщения на английскомАнглоязычная версия сообщений соответствует русскоязычной и адекватно отражает суть диагностики.
docs/en/diagnostics/QueryNestedFieldsByDot.md (2)
9-25: Примеры хорошо иллюстрируют различные случаи применения диагностикиПредставленные примеры наглядно демонстрируют различные контексты, в которых может возникать разыменование ссылочных полей через точку.
26-27: Ссылка на документацию корректнаяПриведенная ссылка на официальную документацию 1С является корректной и полезной для понимания темы.
🧰 Tools
🪛 LanguageTool
[grammar] ~26-~26: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...ЕЖДУ &НачалоПериода И &КонецПериода` ## Sources Source: [Dereference of composite type referen...(REPEATED_VERBS)
docs/diagnostics/QueryNestedFieldsByDot.md (3)
4-8: Корректное описание диагностикиОписание диагностики четко объясняет её назначение и преимущества использования.
10-26: Примеры наглядно демонстрируют все варианты срабатывания диагностикиПримеры охватывают все необходимые случаи разыменования ссылочных полей через точку в запросах и соответствуют английской версии документации.
30-31: Корректная ссылка на источникСсылка на официальную документацию 1С полезна и корректна.
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnosticTest.java (1)
31-35: Правильное наследование и конструкторКласс корректно наследуется от AbstractDiagnosticTest и передает информацию о тестируемом классе диагностики.
src/test/resources/diagnostics/QueryNestedFieldsByDotDiagnostic.bsl (7)
1-10: Хорошее описание тестовых кейсов.Подробное описание тестовых случаев с указанием ожидаемого поведения диагностики помогает понять назначение теста и критерии успешного выполнения.
22-25: Обратите внимание на разыменование ссылочных полей в выборке.Эти строки содержат примеры разыменования ссылочных полей через точку, которые должны быть обнаружены диагностикой.
30-30: Правильный пример разыменования в секции ГДЕ.Строка демонстрирует правильный пример использования разыменования в условии WHERE, который должен быть обнаружен диагностикой.
54-54: Пример разыменования в виртуальной таблице.Строка демонстрирует корректный пример для тестирования разыменования полей в параметрах виртуальной таблицы.
102-104: Примеры разыменования в соединениях таблиц.Эти строки демонстрируют корректные примеры для тестирования разыменования в условиях соединения таблиц.
115-116: Правильное использование конструкции ВЫРАЗИТЬ.Строка 115 показывает корректное использование ВЫРАЗИТЬ без разыменования, а строка 116 демонстрирует случай с разыменованием, который должен быть обнаружен диагностикой.
145-154: Полезное итоговое резюме по ожидаемым результатам.Таблица с ожидаемым количеством срабатываний диагностики по каждому тестовому кейсу обеспечивает четкие критерии для проверки работы диагностики.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java (2)
30-40: Корректное использование аннотаций метаданных диагностики.Аннотации метаданных диагностики правильно сконфигурированы, указывая, что это диагностика типа CODE_SMELL с уровнем серьезности INFO и соответствующими тегами SQL, PERFORMANCE и DESIGN.
47-50: Корректная инициализация флага при входе в запрос.Метод правильно сбрасывает флаг
isVirtualTableпри начале обработки нового запроса, что предотвращает ложные срабатывания при обработке последовательных запросов.
| public void enterFunctionCall(SDBLParser.FunctionCallContext ctx) { | ||
| //Контролируем разыменование в функциях (ВЫРАЗИТЬ, ЕСТЬNULL и т.д.) | ||
| if(ctx.identifier != null && ctx.columnNames.size() > 1){ | ||
| diagnosticStorage.addDiagnostic(ctx); | ||
| } | ||
| super.enterFunctionCall(ctx); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Отсутствует защита от NullPointerException.
В методе enterFunctionCall условие ctx.identifier != null && ctx.columnNames.size() > 1 может вызвать NullPointerException, если ctx.columnNames равно null. Рекомендуется добавить дополнительную проверку.
- if(ctx.identifier != null && ctx.columnNames.size() > 1){
+ if(ctx.identifier != null && ctx.columnNames != null && ctx.columnNames.size() > 1){📝 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.
| public void enterFunctionCall(SDBLParser.FunctionCallContext ctx) { | |
| //Контролируем разыменование в функциях (ВЫРАЗИТЬ, ЕСТЬNULL и т.д.) | |
| if(ctx.identifier != null && ctx.columnNames.size() > 1){ | |
| diagnosticStorage.addDiagnostic(ctx); | |
| } | |
| super.enterFunctionCall(ctx); | |
| } | |
| public void enterFunctionCall(SDBLParser.FunctionCallContext ctx) { | |
| //Контролируем разыменование в функциях (ВЫРАЗИТЬ, ЕСТЬNULL и т.д.) | |
| if (ctx.identifier != null && ctx.columnNames != null && ctx.columnNames.size() > 1) { | |
| diagnosticStorage.addDiagnostic(ctx); | |
| } | |
| super.enterFunctionCall(ctx); | |
| } |
| @Override | ||
| public void enterColumn(SDBLParser.ColumnContext ctx) { | ||
| /*Если взведен флаг обработки виртуальной таблицы | ||
| и определен контекст метаданных, то проверяем заполненность контекста имен колонок. | ||
| В противном случае считаем что работаем со стандартным полем выборки или соединения | ||
| и выводим ошибку когда список имен колонки содержит более одного идентификатора | ||
| */ | ||
| if((isVirtualTable && ctx.mdoName != null && !ctx.columnNames.isEmpty()) || ctx.columnNames.size() > 1){ | ||
|
|
||
| diagnosticStorage.addDiagnostic(ctx); | ||
| } | ||
| super.enterColumn(ctx); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Отсутствует защита от NullPointerException.
В методе enterColumn условие (isVirtualTable && ctx.mdoName != null && !ctx.columnNames.isEmpty()) || ctx.columnNames.size() > 1 также может вызвать NullPointerException, если ctx.columnNames равно null. Рекомендуется добавить дополнительную проверку.
- if((isVirtualTable && ctx.mdoName != null && !ctx.columnNames.isEmpty()) || ctx.columnNames.size() > 1){
+ if((isVirtualTable && ctx.mdoName != null && ctx.columnNames != null && !ctx.columnNames.isEmpty()) ||
+ (ctx.columnNames != null && ctx.columnNames.size() > 1)){📝 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.
| @Override | |
| public void enterColumn(SDBLParser.ColumnContext ctx) { | |
| /*Если взведен флаг обработки виртуальной таблицы | |
| и определен контекст метаданных, то проверяем заполненность контекста имен колонок. | |
| В противном случае считаем что работаем со стандартным полем выборки или соединения | |
| и выводим ошибку когда список имен колонки содержит более одного идентификатора | |
| */ | |
| if((isVirtualTable && ctx.mdoName != null && !ctx.columnNames.isEmpty()) || ctx.columnNames.size() > 1){ | |
| diagnosticStorage.addDiagnostic(ctx); | |
| } | |
| super.enterColumn(ctx); | |
| } | |
| @Override | |
| public void enterColumn(SDBLParser.ColumnContext ctx) { | |
| /*Если взведен флаг обработки виртуальной таблицы | |
| и определен контекст метаданных, то проверяем заполненность контекста имен колонок. | |
| В противном случае считаем что работаем со стандартным полем выборки или соединения | |
| и выводим ошибку когда список имен колонки содержит более одного идентификатора | |
| */ | |
| if ((isVirtualTable | |
| && ctx.mdoName != null | |
| && ctx.columnNames != null | |
| && !ctx.columnNames.isEmpty()) | |
| || (ctx.columnNames != null | |
| && ctx.columnNames.size() > 1)) { | |
| diagnosticStorage.addDiagnostic(ctx); | |
| } | |
| super.enterColumn(ctx); | |
| } |
Описание
Добавил диагностику, проверяющую наличие разыменований ссылочных полей через точку.
Диагностика ругается на разыменование:
Реализация:
Реализовал через "AbstractSDBLListenerDiagnostic", посчитал такой вариант более легковесным. В основном используется функция "enterColumn", так как в по контексту колонки через свойство "columnNames" очень удобно проверять разыменование (если количество элементов у списка более 1, то это "редфлаг" =)), исключением являются виртуальные таблицы, где даже 1 элемент в columnNames свидетельствует о наличии разыменования, в такой ситуации дополнительно контролируется заполненность свойства mdoName, также при посещении секции параметров виртуальной таблицы взводится флаг о начале обработки ВТ, дабы при посещении контекста колонки можно было легче понять где мы находимся. Для контроля функций по схожему принципу используется метод enterFunctionnCall.
Что то красивое через stream и подобное реализовать не вышло ибо не нашел этому применения. Возможно диагностика описана косноязычно, так что жду критики)
Связанные задачи
Основная задача:
Closes #1301
Задача, описывающая схожий функционал, но более частное его проявление:
Closes #2792
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно