Skip to content

Conversation

@theshadowco
Copy link
Member

@theshadowco theshadowco commented Jan 8, 2026

Описание

1, добавлена обработка чтения квалификатора двоичных данных в измененном формате edt
2. обновлены зависимости

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

Closes: #565
Closes: #566
Closes: #567

Чеклист

Общие

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

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

Summary by CodeRabbit

  • New Features

    • Improved qualifier parsing with support for FIXED flag to enhance length qualification interpretation.
    • Expanded compatibility with additional qualifier node naming conventions for broader data format support.
  • Chores

    • Updated core dependencies to latest versions for improved stability and compatibility.

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

@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

The pull request updates two dependencies to their latest versions and extends the value type qualifier converter to recognize the FIXED flag and BinaryQualifiers node, addressing parsing errors for previously unknown qualifiers.

Changes

Cohort / File(s) Summary
Dependency Updates
build.gradle.kts
Updated io.github.1c-syntax:bsl-common-library to 0.9.2 and io.github.1c-syntax:utils to 0.6.8
Qualifier Parsing Logic
src/main/java/.../ValueTypeQualifierConverter.java
Added support for FIXED flag mapping to AllowedLength enum and introduced handling for BinaryQualifiers node alongside BinaryDataQualifiers; added private constants for the new qualifier paths

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰✨ With qualifiers now complete,
FIXED flags and BinaryQualifiers meet!
No more parsing errors in sight,
Dependencies updated, everything's right! 🎉

🚥 Pre-merge checks | ✅ 1 | ❌ 4
❌ Failed checks (3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses issue #565 (binaryQualifiers support) but shows no code changes to ValueTypeConverter for issues #566 and #567 regarding BinaryData and EnumManager types. Verify that ValueTypeConverter includes changes for BinaryData and EnumManager type support as required by issues #566 and #567.
Out of Scope Changes check ⚠️ Warning Dependency updates in build.gradle.kts appear unrelated to the core objectives of supporting new qualifiers and value types from the linked issues. Clarify whether dependency updates are necessary for the bug fixes or separate changes; consider splitting into separate PRs if unrelated.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'Исправления ошибок' (Bug fixes) is generic and vague, not clearly summarizing the main changes (dependency updates and binary qualifier support). Provide a more specific title that describes the primary changes, such as 'Add binaryQualifiers support and update dependencies' or similar.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54237f6 and 50654f2.

⛔ Files ignored due to path filters (2)
  • src/test/resources/ext/edt/mdclasses/configuration/src/Documents/Документ1/Документ1.mdo is excluded by !src/test/resources/**
  • src/test/resources/fixtures/mdclasses/Documents.Документ1_edt.json is excluded by !src/test/resources/**
📒 Files selected for processing (2)
  • build.gradle.kts
  • src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/ValueTypeQualifierConverter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: build (25, macOS-latest)
  • GitHub Check: build (17, ubuntu-latest)
  • GitHub Check: build (21, macOS-latest)
  • GitHub Check: build (21, windows-latest)
  • GitHub Check: build (25, ubuntu-latest)
  • GitHub Check: build (21, ubuntu-latest)
  • GitHub Check: build (17, windows-latest)
  • GitHub Check: build (25, windows-latest)
  • GitHub Check: build (17, macOS-latest)
  • GitHub Check: QA
  • GitHub Check: build (25, macOS-latest)
  • GitHub Check: build (17, macOS-latest)
🔇 Additional comments (5)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/ValueTypeQualifierConverter.java (4)

46-46: LGTM! New constants added for enhanced qualifier support.

The new constants BINARY_QUALIFIERS_NODE_NAME and FIXED_NODE_NAME are properly named and positioned, supporting the parsing of the modified EDT format mentioned in the PR objectives.

Also applies to: 49-49


76-78: LGTM! FIXED flag correctly mapped to AllowedLength enum.

The implementation correctly parses the boolean fixed flag and maps it to the appropriate AllowedLength enum value. Note that this logic modifies the same allowedLength variable as the ALLOWED_LENGTH_NODE_NAME branch (line 75), which means if both nodes appear in the same qualifier, the one parsed later will take precedence. This appears intentional for supporting different EDT format variations.


101-102: LGTM! BinaryQualifiers node support successfully addresses issue #565.

The addition of BINARY_QUALIFIERS_NODE_NAME to the condition correctly handles the previously unknown BinaryQualifiers node by treating it identically to BinaryDataQualifiers. This directly resolves the "Unknown qualifiers binaryQualifiers" error reported in issue #565.


40-114: Dependency update addresses issues #566 and #567; code changes handle issue #565.

The changes in this file correctly address issue #565 (BinaryQualifiers support). Issues #566 (BinaryData type handling) and #567 (EnumManager type handling) are not addressed by code changes in this repository. These issues are resolved by the dependency update to bsl-common-library:0.9.2, which adds support for unknown value types like BinaryData and EnumManager through the ValueTypes.getOrCompute() method in ValueTypeConverter. The converter gracefully handles these types even when unknown, logging a warning and creating appropriate CustomValueType instances.

build.gradle.kts (1)

63-64: Code changes are present and directly related to the dependency updates.

The file ValueTypeQualifierConverter.java was newly created in this commit with 114 lines of implementation code that specifically handles BinaryDataQualifiers and binary data qualifier processing in the EDT format—directly aligned with handling the BinaryData type. Both dependency versions exist in Maven Central and are marked as latest/release versions:

  • bsl-common-library@0.9.2 exists and is the latest release
  • utils@0.6.8 exists and is the latest release

The commit message explicitly mentions adding binary data qualifier handling alongside the dependency updates, so the corresponding code changes are visible and substantial. However, the commit does not reference issues #566 or #567 explicitly, so the specific mapping of these dependency updates to those issue fixes cannot be directly confirmed from the repository history.

Likely an incorrect or invalid review comment.


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.

❤️ Share

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2026

Test Results

  603 files  ±0    603 suites  ±0   9m 5s ⏱️ +47s
  284 tests ±0    284 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 646 runs  ±0  2 646 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 50654f2. ± Comparison against base commit 54237f6.

♻️ This comment has been updated with latest results.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 8, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
76.9% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@theshadowco theshadowco merged commit 4247395 into develop Jan 8, 2026
32 of 46 checks passed
@theshadowco theshadowco deleted the feature/fixes260108 branch January 8, 2026 11:48
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.

Parsing error due to unknown value type EnumManager Parsing error due to unknown value type BinaryData. [BUG] Unknown qualifiers binaryQualifiers

2 participants