Skip to content
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

Feature/1294 canonical keywords in query #1878

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

IT-Medved
Copy link

Описание

Реализация диагностики для проверки ключевых слов в запросах и квикфикс

Связанные задачи

Closes #1294

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

@IT-Medved
Copy link
Author

IT-Medved commented Oct 14, 2021

Хочу еще тест написать для остальных слов ключевых.
Не могу раскурить один момент, гладл не может запустить все тесты через idea или gradlew test с ошибкой

TestEngine with ID 'junit-jupiter' failed to discover tests
JUnitException: ClassSelector [className = 'com.github._1c_syntax.bsl.languageserver.diagnostics.reporter.GenericCoverageTest'] resolution failed
NoClassDefFoundError: com/github/_1c_syntax/bsl/languageserver/diagnostics/reporter/AbstractDiagnosticReporter
ClassNotFoundException: com.github._1c_syntax.bsl.languageserver.diagnostics.reporter.AbstractDiagnosticReporter

Но при этом по 1 тесту запустить могу через gradlew test --test "some"
есть мысли куда копнуть ?

ctx.getTokens().parallelStream().filter((Token t) ->
canonicalKeywords.get(t.getType()) != null && !canonicalKeywords.get(t.getType()).contains(t.getText()))
.forEach(token ->
diagnosticStorage.addDiagnostic(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

чтобы не делать getText(range) на строке 182 (https://github.com/1c-syntax/bsl-language-server/pull/1878/files#diff-aa81d1707520d13712c6b3b10e761b98bc91f363435eb914c5d3acfbdc6b1012R182) можно в полученную диагностику добавить данные (через setData на результат вызова addDiagnostic), куда сразу положить правильное написание ключевого слова. тогда в квик-фиксе останется только достать его из диагностики через getData и положить внутрь TextEdit

public class SDBLKeywords {


public static final String AUTOORDER_RU = "АВТОУПОРЯДОЧИВАНИЕ";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

было бы здорово (наверное) добавить тест на этот класс, чтобы проверять все ключевые слова из SDBLLexer на наличие в этом классе (хотя бы в одном из вариантов языков).
@theshadowco что думаешь?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хорошая тема, я с подобным столкнулся и в сонарплагине

severity = DiagnosticSeverity.INFO,
minutesToFix = 1,
tags = {
DiagnosticTag.STANDARD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не все теги из ишузы указал

@nixel2007
Copy link
Member

com.github._1c_syntax.bsl.languageserver.diagnostics.reporter.AbstractDiagnosticReporter

судя по названию класса, где-то в кэше затесалась очень старая версия бсл лс. попробуйте сделать ./gradlew clean

Поправил определение Истина и Ложь для ключевых слов
@IT-Medved
Copy link
Author

@nixel2007 @theshadowco , Спасибо за обратную связь! Послушав на стриме про sdbl парсер и погоняв на реальных конфигурациях диагностику пришел к выводу, что задача не решается просто перебирая кейворды, нужен еще анализ окружения где эти слова встречаются.

@IT-Medved IT-Medved marked this pull request as draft October 14, 2021 16:05
@nixel2007
Copy link
Member

Ммм... Про какой контекст речь? Мне кажется, что для этой диагностики достаточно данных с лексера. Если он распознал слово как ключевое, то его можно проверять, разве нет?

@theshadowco
Copy link
Member

Нет, просто так проверять нельзя, т.к. например в конструкции "Поле ССЫЛКА Справочник.МойСправочник" ссылка - кейворд, а "Выбрать Поле Ссылка" (КАК опущен) - это уже алиас, но так как слова КАК нет, то mode не отработает и токен будет определен не как идентификатор, а как ССЫЛКА

@theshadowco
Copy link
Member

Ну и тем более в конструкция вида "СГРУППИРОВАТЬ ПО Ссылка"

@nixel2007
Copy link
Member

Хм... А если обходить дерево разбора, подписавшись на терминальные ноды? Там тип уже будет правильный?

@IT-Medved
Copy link
Author

Ммм... Про какой контекст речь? Мне кажется, что для этой диагностики достаточно данных с лексера. Если он распознал слово как ключевое, то его можно проверять, разве нет?

Тоже так думал, на первый взгляд, А потом нашел как посмотреть результаты через репортер и вот. Это скорее Алиасы, ключевое слово это или нет, зависит от контекста.
image

image

image

Были мысли или узел проверить, т.е. для токена сделать предикаты и исполнять их при встрече или предложение от Валеры "Кейворды засунуть в филды и потом легко их доставать".

В общем(и целом) я пока углублю понимание в стороне sdbl парсера, и поправлю реализацию.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NEW] [Запросы] Каноническое написание ключевых слов
3 participants