Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 9, 2025

Implementation Plan for Cached Values on Client

Task Summary

Add configuration parameter to TransferringParametersBetweenClientAndServer diagnostic to allow ignoring specific parameter names (e.g., КэшированныеЗначения) if a corresponding module-level variable exists.

Implementation Checklist

  • Add configuration parameter cachedValueNames to the diagnostic class
  • Implement logic to check if parameter name matches cached value names
  • Add method to check if module-level variable with matching name exists
  • Verify the variable has @Client compiler directive
  • Update resource bundles (Russian and English) with parameter description
  • Update documentation (Russian and English)
  • Add comprehensive test cases
  • Run tests and verify implementation
  • Optimize parameter matching using Set with uppercase keys (performance improvement)
  • Fix Russian terminology: use "директива компиляции" instead of "компиляторная директива"

Recent Changes

  • Updated Russian terminology in properties file and documentation
  • Changed "компиляторной директивой" to "директивой компиляции" for correct Russian language usage
Original prompt

This section details on the original issue you should resolve

<issue_title>[MOD] Кэшируемые значения на клиенте.</issue_title>
<issue_description>## Диагностика
TransferringParametersBetweenClientAndServer

Описание предлагаемых изменений

Предлагаю в диагностику добавить возможность указывать список имен параметров методов, которые можно игнорировать, например КэшированныеЗначения. Дополнительно надо проверять, что в модуле формы есть соответствующая переменная.

&НаКлиенте
Перем КэшированныеЗначения; // используется механизмом обработки изменения реквизитов ТЧ

Если переменная отсутствует, то диагностировать ошибку, по текущему алгоритму диагностики.

Результат

Снизится количество срабатываний диагностики на тех методах, где действительно надо получать данные переменной с сервера на клиент. Не надо будет экранировать методы с общепринятыми переменными, такой как КэшированныеЗначения (используется в типовых конфигурациях).

Сейчас, чтобы не экранировать такой метод, необходимо его размещать в клиент-серверном общем модуле, с дальнейшим вызовом серверного общего модуля.

Дополнительная информация

Image

Comments on the Issue (you are @copilot in this section)

@nixel2007 Интересное предложение, спасибо! @nixel2007 @copilot попробуй реализовать. Для поиска переменной текущего модуля используй symbolTree

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Summary by CodeRabbit

  • New Features

    • Added configurable parameter cachedValueNames to customize diagnostic behavior, allowing users to exclude specific parameters from warnings when matching client-side variables with appropriate directives are present.
  • Documentation

    • Expanded documentation with parameter descriptions, JSON configuration examples, and usage scenarios in both English and Russian.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This pull request adds a caching mechanism to the TransferringParametersBetweenClientAndServer diagnostic. It introduces a configurable parameter cachedValueNames that allows certain parameters to be ignored if a corresponding client-side module variable with the &AtClient directive exists, reducing false positives for parameters like CachedValues in typical configurations.

Changes

Cohort / File(s) Summary
Documentation
docs/diagnostics/TransferringParametersBetweenClientAndServer.md, docs/en/diagnostics/TransferringParametersBetweenClientAndServer.md
Added "Параметры" / "Parameters" sections describing the new cachedValueNames configuration parameter, including type, default value, behavior description, JSON examples, and code demonstrations.
Resource Properties
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic_*.properties
Added cachedValueNames property key in both English and Russian localization files with descriptions of the ignored parameter behavior.
Core Diagnostic Logic
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java
Introduced caching mechanism: new field cachedValueNames (Set), configure() method to parse comma-separated parameters, modified calcNotAssignedParams() to filter cached values, and added helper methods (isCachedValueParameter(), hasClientModuleVariable(), hasVariableWithName(), hasClientCompilerDirective()) for AST-based client variable detection.
Diagnostic Tests
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnosticTest.java
Added three test methods: testWithCachedValues, testWithCachedValuesButNoVariable, and testWithCachedValuesButNotClientVariable to verify correct handling of cached parameters with and without corresponding client-side variables.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • AST traversal correctness: Verify the logic in hasClientModuleVariable() and related traversal methods properly navigates the parse tree and correctly identifies client-side variables with &AtClient directives.
  • Configuration parsing edge cases: Review the configure() method's handling of null/empty maps, malformed input, and case-insensitivity of parameter names (upper-case normalization).
  • Test coverage completeness: Ensure the three new test methods adequately cover scenarios where cached values exist with/without matching client variables and with/without the &AtClient directive.
  • Null safety: Check for potential null pointer exceptions in parameter filtering and variable lookup operations.

Suggested labels

hacktoberfest-accepted

Suggested reviewers

  • nixel2007

Poem

🐰 A caching fix hops down the lane,
Where CachedValues now find their reign,
Client directives align just right,
Parameters dancing in diagnostic light! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a cachedValueNames parameter to the TransferringParametersBetweenClientAndServer diagnostic.
Linked Issues check ✅ Passed The implementation fully addresses issue #3577: adds configurable cachedValueNames parameter, checks for module-level variables with @client directive, and updates documentation and tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the cachedValueNames feature: diagnostic code, configuration, documentation, tests, and resource bundles. No out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands and usage tips.

@nixel2007 nixel2007 marked this pull request as ready for review November 9, 2025 05:19
@nixel2007 nixel2007 marked this pull request as draft November 9, 2025 05:19
Copilot AI and others added 2 commits November 9, 2025 05:34
…ntAndServer diagnostic

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copilot AI changed the title [WIP] Add parameter ignoring for client-side caching Add cachedValueNames parameter to TransferringParametersBetweenClientAndServer diagnostic Nov 9, 2025
Copilot AI requested a review from nixel2007 November 9, 2025 05:45
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
… директива"

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
@theshadowco
Copy link
Member

@nixel2007
вроде похоже на правду

@theshadowco
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 9, 2025

@nixel2007 nixel2007 marked this pull request as ready for review December 9, 2025 14:09
Copilot AI review requested due to automatic review settings December 9, 2025 14:09
@nixel2007 nixel2007 merged commit 89370e0 into develop Dec 9, 2025
38 checks passed
@nixel2007 nixel2007 deleted the copilot/add-cache-parameter-ignoring branch December 9, 2025 14:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a configurable parameter cachedValueNames to the TransferringParametersBetweenClientAndServer diagnostic, allowing developers to suppress warnings for specific parameter names when a corresponding client-side module variable exists. This addresses a common use case in 1C configurations where cached values are intentionally transferred from server to client.

Key Changes

  • Added cachedValueNames configuration parameter to allow ignoring specific parameter names
  • Implemented AST-based validation to verify matching module-level variables with &AtClient directive exist
  • Optimized parameter matching using uppercase Set for case-insensitive comparison

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TransferringParametersBetweenClientAndServerDiagnostic.java Added cachedValueNames parameter with configure() method, implemented AST traversal to validate client module variables
TransferringParametersBetweenClientAndServerDiagnosticTest.java Added comprehensive test coverage for three scenarios: valid cached variable, missing variable, and non-client variable
TransferringParametersBetweenClientAndServerDiagnostic_ru.properties Added Russian description for the new parameter using correct terminology "директива компиляции"
TransferringParametersBetweenClientAndServerDiagnostic_en.properties Added English description for the new parameter with compiler directive reference
docs/diagnostics/TransferringParametersBetweenClientAndServer.md Added comprehensive Russian documentation with parameter description and code examples
docs/en/diagnostics/TransferringParametersBetweenClientAndServer.md Added comprehensive English documentation with parameter description and code examples

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.

[MOD] Кэшируемые значения на клиенте.

3 participants