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

Доработка BadWords - опция поиска по комментариям + показ запрещенного слова в замечаниях #3190

Merged
merged 7 commits into from
Nov 27, 2023

Conversation

artbear
Copy link
Contributor

@artbear artbear commented Nov 25, 2023

Описание

  • опция поиска по комментариям
    для возможности отключения поиска в комментариях
    часто выдаются срабатывания, которые не влияют на поведение системы, и не нужно их обрабатывать
    по умолчанию опция выключена, т.е. поведение остается, как в исходной реализации правила

  • показ запрещенного слова в замечаниях
    в случае хитрых шаблонов поиска бывает непонятно, на какое запрещенное слово или выражение выдано замечание.

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

Closes #3189

Чеклист

Общие

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

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

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

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

@artbear artbear changed the title Доработка BadWords - опция поиска по комментариям + показ запрещенного слова в комментариях Доработка BadWords - опция поиска по комментариям + показ запрещенного слова в замечаниях Nov 25, 2023
@ghost
Copy link

ghost commented Nov 25, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@@ -44,17 +48,25 @@
public class BadWordsDiagnostic extends AbstractDiagnostic {

private static final String BAD_WORDS_DEFAULT = "";
private static final boolean FIND_IN_COMMENTS = true;
Copy link
Member

Choose a reason for hiding this comment

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

_DEFAULT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

исправлено

@nixel2007
Copy link
Member

Не запускал перфоманс тест? Насколько процентов замедляется диагностика от этой доработки?

while (matcher.find()) {
diagnosticStorage.addDiagnostic(i, matcher.start(), i, matcher.end());
diagnosticStorage.addDiagnostic(Ranges.create(i, matcher.start(), i, matcher.end()),
info.getMessage(matcher.group()));
Copy link
Member

Choose a reason for hiding this comment

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

Супер

Matcher matcher = badWords.matcher(moduleLines[i]);
var moduleLines = getContentList();
for (var i = 0; i < moduleLines.length; i++) {
final var moduleLine = moduleLines[i];
Copy link
Member

Choose a reason for hiding this comment

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

Многое мусорных final

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Многое мусорных final

не согласен с этим,
мое мнение - final позволяют зафиксировать константность переменной, это полезно для понимания кода

но убрал все final у обычных переменных для принятия ПР!

@artbear
Copy link
Contributor Author

artbear commented Nov 26, 2023

Не запускал перфоманс тест? Насколько процентов замедляется диагностика от этой доработки?

Так это тест в CI проводится.
раз не упал, значит, потерь нет или они менее 10%

artbear and others added 3 commits November 26, 2023 09:43
…tics/BadWordsDiagnostic.java

Co-authored-by: Nikita Fedkin <nixel2007@gmail.com>
…tics/BadWordsDiagnostic.java

Co-authored-by: Nikita Fedkin <nixel2007@gmail.com>
@nixel2007
Copy link
Member

В тесте проверяется общая производительность в параллели,а меня интересует производительность конкретной диагностики

@artbear
Copy link
Contributor Author

artbear commented Nov 26, 2023

В тесте проверяется общая производительность в параллели,а меня интересует производительность конкретной диагностики

Не проверял, но не должно быть потерь )

  • вариант без опции работает как и раньше
  • вариант с новой опцией только добавил довольно быстрый расчет номеров строк,
  • последовательный одноразовый проход по токенам с проверкой только номеров строк
  • проверка номера строки в Set также быстра

@nixel2007
Copy link
Member

Перебор токенов - это всегда медленно.

@artbear
Copy link
Contributor Author

artbear commented Nov 26, 2023

Перебор токенов - это всегда медленно.

1 Тем не менее, в нескольких диагностиках вполне себе юзается, и даже без параллельности

  • см. скрин

2 можно попробовать слегка ускорить через спец.стрим, как в DocumentContext::getMetrics - возможно, он работает параллельно

    int[] nclocData = getTokensFromDefaultChannel().stream()
      .mapToInt(Token::getLine)
      .distinct().toArray();

image

@nixel2007
Copy link
Member

Я же не говорю, что не используется) я говорю, что медленно)

Кстати, эту же информацию можно прям из метрики дёрнуть же, да? Она там предрассчитана и закэширована

@artbear
Copy link
Contributor Author

artbear commented Nov 26, 2023

Кстати, эту же информацию можно прям из метрики дёрнуть же, да? Она там предрассчитана и закэширована

Да, нужный массив предрассчитан в метрике.
Воспользуюсь, если есть доступ.

Нормально для диагностики брать данные из метрики ?
не запрещено?

@nixel2007
Copy link
Member

Не, не запрещено. Мало того, оно все равно дернется в какой-то момент - цикломатикой/когнитивкой, запросом метрик в сонаре или явно при перфоманс тесте

использую предрассчитанное значение из метрик
@artbear
Copy link
Contributor Author

artbear commented Nov 26, 2023

Кстати, эту же информацию можно прям из метрики дёрнуть же, да? Она там предрассчитана и закэширована

использовал предрассчитанное значение, запушил

@nixel2007 nixel2007 merged commit 782ef5e into 1c-syntax:develop Nov 27, 2023
19 checks passed
@nixel2007
Copy link
Member

Спасибо!

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