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.

Sign up for GitHub

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

Jump to bottom

Правило "Передача параметров между клиентом и сервером" - ГОТОВО #2854

Merged
merged 14 commits into from
Mar 31, 2023
Merged

Правило "Передача параметров между клиентом и сервером" - ГОТОВО #2854

merged 14 commits into from
Mar 31, 2023

Conversation

artbear
Copy link
Contributor

@artbear artbear commented Jul 19, 2022 •
edited
Loading

Описание

  • реализация новой диагностики

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

Closes #2853

Чеклист

Общие

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

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

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

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

@artbear artbear changed the title Client server params Правило "Передача параметров между клиентом и сервером" Jul 19, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

95.2% 95.2% Coverage
0.0% 0.0% Duplication

@artbear artbear changed the title Правило "Передача параметров между клиентом и сервером" Правило "Передача параметров между клиентом и сервером" - ГОТОВО Sep 25, 2022
@artbear
Copy link
Contributor Author

artbear commented Sep 25, 2022

@theshadowco @nixel2007 правило готово. примите его, если нет возражений.

@theshadowco
Copy link
Member

@artbear
а чего столько тудушек осталось?

@otymko otymko self-requested a review December 21, 2022 06:52
otymko
otymko requested changes Dec 23, 2022
View reviewed changes
...guageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic_ru.properties Outdated
@@ -0,0 +1,2 @@
diagnosticMessage=Установите модификатор Знач для параметра %s метода %s
Copy link
Member

Choose a reason for hiding this comment

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

@artbear у Знач кавычки пропущены.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Исправил

...x/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java Outdated
DiagnosticTag.PERFORMANCE,
DiagnosticTag.STANDARD
}
// TODO учесть подходящие типы модулей - только клиентские, только серверные или все?
Copy link
Member

Choose a reason for hiding this comment

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

@artbear, todo, либо решаем, либо создаем задачу и здесь оставляем ее номер..

...x/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java Outdated
public class TransferringParametersBetweenClientAndServerDiagnostic extends AbstractDiagnostic {
private static final Set<CompilerDirectiveKind> SERVER_COMPILER_DIRECTIVE_KINDS = EnumSet.of(
CompilerDirectiveKind.AT_SERVER,
// CompilerDirectiveKind.AT_CLIENT_AT_SERVER_NO_CONTEXT,
Copy link
Member

Choose a reason for hiding this comment

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

@artbear, закомментированный код.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

исправил

...x/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java Outdated
getMethodParamsStream()
// сначала получаю вызовы из клиентских методов, а уже потом проверяю использование параметров внутри метода,
// чтобы исключить лишний анализ серверных методов, которые вызываются из серверных методов
.map(pair -> Triple.of(pair.getLeft(),
Copy link
Member

Choose a reason for hiding this comment

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

@artbear, не читабельно (

Copy link
Member

Choose a reason for hiding this comment

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

предлагаю функциональщину на каждом этапе, если это несколько statement вынести в методы с говорящими именами. Будет более читабельно и сразу можно будет понять, что происходит.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

да, переделал\упростил

...x/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java Outdated
pair.getRight(),
getRefCalls(pair.getLeft())))
.filter(triple -> !triple.getRight().isEmpty())
.map(triple -> Triple.of(triple.getLeft(),
Copy link
Member

Choose a reason for hiding this comment

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

@artbear, не читабельно (

...x/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java Outdated
getRefCalls(pair.getLeft())))
.filter(triple -> !triple.getRight().isEmpty())
.map(triple -> Triple.of(triple.getLeft(),
notAssignedParams(triple.getLeft(), triple.getMiddle()),
Copy link
Member

Choose a reason for hiding this comment

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

@artbear, кажется имеется в виде не измененные "not changed", а не присвоенные?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

я учитываю\пропускаю именно "не присвоенные" параметры

...l/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnosticTest.java Outdated
}

private String getMessage(String paramName, String methodName) {
return String.format("Установите модификатор Знач для параметра %s метода %s",
Copy link
Member

Choose a reason for hiding this comment

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

@artbear, кстати, а мы это из ресурсов получить не можем?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@artbear, кстати, а мы это из ресурсов получить не можем?

я специально явно укаываю строку-копию, чтобы не пропустить пустые ресурсы.
Предлагаю так и оставить.

src/test/resources/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.bsl Show resolved Hide resolved
src/test/resources/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.bsl Show resolved Hide resolved
src/test/resources/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.bsl Show resolved Hide resolved
@otymko
Copy link
Member

otymko commented Dec 23, 2022

@artbear, просьба, поправь, пожалуйста, оформление стримов, не читабельны через ПР(

@artbear artbear changed the title Правило "Передача параметров между клиентом и сервером" - ГОТОВО Правило "Передача параметров между клиентом и сервером" Dec 24, 2022
@artbear artbear mentioned this pull request Dec 24, 2022
Open
@artbear
Copy link
Contributor Author

artbear commented Dec 24, 2022 •
edited
Loading

@artbear а чего столько тудушек осталось?

@theshadowco @otymko
перенес все нерешенные неважные TODO в задачу #2972

@artbear
Copy link
Contributor Author

artbear commented Dec 24, 2022

@otymko почти все замечания исправлены. в паре мест, где я не совсем согласен, я оставил свой комментарий.

@artbear artbear changed the title Правило "Передача параметров между клиентом и сервером" Правило "Передача параметров между клиентом и сервером" - ГОТОВО Dec 24, 2022
@artbear
убрал TODO
353568f 
перенес в отдельную задачу
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

97.1% 97.1% Coverage
0.0% 0.0% Duplication

@artbear
Copy link
Contributor Author

artbear commented Mar 26, 2023

@theshadowco @otymko @nixel2007 Правило готово, все замечания исправлены.

Правило работает в Инфостарте уже много месяцев!!

Досмотрите уже ПР и принимайте его, пожалуйста!

@nixel2007 nixel2007 merged commit 1982cec into 1c-syntax:develop Mar 31, 2023
@artbear artbear deleted the client-server-params branch December 17, 2023 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@otymko otymko otymko requested changes

@theshadowco theshadowco theshadowco approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[NEW] Правило "Передача параметров между клиентом и сервером"
4 participants
@artbear @theshadowco @otymko @nixel2007