feat: реализована поддержка классов типов атрибутов в MDO #515
Conversation
Implement comprehensive attribute type system for metadata objects: - Add AttributeType interface with TypeDescription, TypeCategory, TypeQualifier - Support primitive, reference, and composite types with qualifiers - Add StringQualifier and NumberQualifier for type constraints - Extend Attribute interface with getType() method - Update all Attribute implementations (ObjectAttribute, Resource, Dimension, etc.) - Add AttributeTypeConverter for Designer XML and EDT MDO parsing - Register type aliases in DesignerReader and EDTReader - Support v8:Type, v8:StringQualifiers, v8:NumberQualifiers parsing Addresses GitHub issue #277 for metadata attribute type information. BREAKING CHANGE: Attribute interface now requires getType() method implementation
Add thorough test coverage for new attribute type functionality: - Add AttributeTypeTest with tests for all type categories: * Primitive types (String, Number) with qualifiers * Reference types with metadata references * Composite types with multiple type descriptions * Empty type handling - Extend existing MDO tests: * CatalogTest: verify type information available on all attributes * AccountingRegisterTest: add _edt postfix for EDT fixtures * AccumulationRegisterTest: add _edt postfix for EDT fixtures * TasksTest: add _edt postfix for SSL 3.1 tests - Add new EDT test fixtures with complete type information: * AccountingRegisters.РегистрБухгалтерии1_edt.json * AccumulationRegisters.РегистрНакопления1_edt.json * Tasks.Задача1_edt.json All tests validate that attribute type information is properly parsed from both Designer (XML) and EDT (MDO) formats.
Add comprehensive documentation for new attribute type functionality:
- docs/ru/features.md:
* New section 'Типы данных реквизитов' describing attribute type support
* Explains primitive, reference, and composite type categories
* Documents type qualifiers (string length, number precision)
- docs/ru/examples.md:
* New section 'Работа с типами реквизитов' with code examples
* Demonstrates AttributeType usage patterns:
- Accessing type information from attributes
- Working with primitive types and their qualifiers
- Handling reference types with metadata links
- Processing composite types with multiple descriptions
* Complete Java code examples for all type scenarios
Documentation provides clear guidance for developers using the new
attribute type API in their applications.
…ements Systematic correction of corrupted Russian text and enhancement of test fixtures: RUSSIAN LANGUAGE CORRECTIONS: - Fix extensive 'й' → '_' corruption across SSL 3.1 fixtures (100+ cases) - Correct grammar cases: 'ПредметСтроки' → 'ПредметСтрокой' (instrumental case) - Fix complex names: 'СостоянияПредметовВзаимоде_ствий' → 'СостоянияПредметовВзаимодействий' - Restore proper declensions in system configuration names - Preserve correct '_' in procedure names: 'Показывать_УстановитьПометки' FIXTURE ENHANCEMENTS: - Add complete type information to all metadata fixtures - Update mdclasses fixtures with accurate type descriptions and qualifiers - Add new _edt fixtures for AccountingRegister, AccumulationRegister, Tasks - Regenerate SSL 3.1 fixtures with new attribute type functionality PATH CORRECTIONS: - Fix non-existent file paths discovered through expert analysis - Correct 'БыстрыеНастро_киОтчета' → 'БыстрыеНастройкиОтчета' (actual filesystem paths) - Validate all 192 SSL 3.1 paths point to existing files DATA QUALITY IMPROVEMENTS: - Eliminate Russian letter corruption throughout configuration data - Ensure fixture accuracy matches real 1C source data - Maintain test compatibility while improving data quality Test coverage: 229/229 tests pass (100% success rate) Quality improvement: Enterprise-grade Russian language data throughout fixtures
WalkthroughДобавлена поддержка типов реквизитов: модель типов и квалификаторов, фабрика для создания типов, парсер/конвертеры чтения (Designer/EDT) с базовым конвертером; в MDO‑дети добавлено поле Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Клиент
participant Reader as Designer/EDT Reader
participant XStream as XStream
participant Conv as AttributeTypeConverter
participant Base as AttributeTypeConverterBase
participant Factory as AttributeTypeFactory
participant Model as AttributeTypeImpl/TypeDescription
Client->>Reader: загрузка метаданных
Reader->>XStream: xstream.fromXML(...)
XStream->>Conv: unmarshal(node)
Conv->>Base: createAttributeType(typeNames, stringQ, numberQ)
Base->>Factory: нормализация и создание описаний
Factory->>Model: сборка TypeDescription и квалификаторов
Model-->>Conv: AttributeTypeImpl
Conv-->>XStream: вернуть AttributeType
XStream-->>Reader: MDO с заполненным Attribute.type
Reader-->>Client: вернуть объекты (Attribute.getType())
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
johnnyshut
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ru/examples.md (1)
135-147: Обновить примеры: заменить getDescription() на getDisplayName()В файле
docs/ru/examples.md(строки 136, 146 и 437) заменитьattribute.getType().getDescription()на
attribute.getType().getDisplayName()- attribute.getType().getDescription() + attribute.getType().getDisplayName()
🧹 Nitpick comments (40)
docs/ru/features.md (2)
36-36: Опечатка в формулировке: согласовать падежСейчас: «планы счетов и видов характеристик». Корректнее: «планы счетов и виды характеристик».
-Типы поддерживаются для всех объектов, содержащих реквизиты: справочники, документы, регистры, планы счетов и видов характеристик, задачи и др. +Типы поддерживаются для всех объектов, содержащих реквизиты: справочники, документы, регистры, планы счетов и виды характеристик, задачи и др.
28-35: Добавить перекрёстную ссылку на раздел с примерамиПолезно сразу направить читателя к примерам использования новых API.
### Типы данных реквизитов Библиотека поддерживает получение информации о типах данных реквизитов метаданных. Для любого реквизита можно получить: - тип данных (примитивный, ссылочный, определяемый, составной) - квалификаторы типа (длина строки, точность числа и т.д.) - полную информацию о составных типах + +См. раздел «Работа с типами реквизитов» в документации с примерами использования: docs/ru/examples.md.src/main/java/com/github/_1c_syntax/bsl/mdo/Attribute.java (1)
47-51: Уточнить контракт метода getType(): non-null и «пустой» тип
Явно зафиксировать в JavaDoc, что getType() никогда не возвращает null (при отсутствии явного типа возвращается AttributeTypeImpl.EMPTY)./** - * Тип данных реквизита + * Тип данных реквизита. Не возвращает {@code null}. При отсутствии явного типа возвращается {@link AttributeTypeImpl#EMPTY}. */ AttributeType getType();При критичной бинарной совместимости можно добавить в интерфейс default-реализацию getType(), возвращающую EMPTY.
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java (1)
48-53: Хорошая проверка доступности типов реквизитовАссерты на notNull и непустое имя типа уместны и отражают новый контракт.
Можно расширить проверку до категории типа (например, что getCategory() не null) — по желанию.
for (var attribute : catalog.getAllAttributes()) { assertThat(attribute.getType()).isNotNull(); assertThat(attribute.getType().getDisplayName()).isNotEmpty(); + // assertThat(attribute.getType().getCategory()).isNotNull(); }src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java (1)
40-41: Alias "type" на весь XStream — лучше сузить до aliasField для конкретных владельцев"type" — слишком общий тег; глобальный alias класса может перехватывать несвязанные узлы и создать редкие, но болезненные конфликты при (де)сериализации. Предлагаю вместо alias класса задать alias поля для конкретных классов, где поле действительно есть. Это убирает риск побочных эффектов без потери функциональности (конвертер останется задействован по типу поля).
Примените патч:
@@ -import com.github._1c_syntax.bsl.mdo.support.AttributeType; @@ - xStream.alias("type", AttributeType.class); + // Узел <type> встречается в разных сущностях; задаем alias для поля, + // чтобы не плодить глобальных коллизий с одноименными узлами. + xStream.aliasField("type", ObjectAttribute.class, "type"); + xStream.aliasField("type", TaskAddressingAttribute.class, "type"); + xStream.aliasField("type", ExtDimensionAccountingFlag.class, "type");Проверьте, что чтение типов из EDT/Designer по-прежнему проходит:
- gradlew clean test
- выборочно открыть любые .mdo/.xml фикстуры и убедиться, что поля читаются без регрессий.
Also applies to: 248-249
src/test/java/com/github/_1c_syntax/bsl/mdo/AccumulationRegisterTest.java (1)
35-37: CsvSource с разной арностью — для читаемости лучше выровнять количество колонокJUnit с ArgumentsAccessor переварит строки разной длины, но единая арность упрощает сопровождение и исключает ловушки при будущих рефакторингах.
- "false, mdclasses, AccumulationRegisters.РегистрНакопления1" + "false, mdclasses, AccumulationRegisters.РегистрНакопления1, "Убедитесь, что MDTestUtils.getMDWithSimpleTest корректно обрабатывает пустой 4-й аргумент (fallback на формат по умолчанию).
src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectAttribute.java (1)
27-29: Поле type: зафиксировать не-null контракт на уровне билдераЧтобы не пропустить null при сборке через Lombok Builder, добавьте @nonnull. Значение по умолчанию (EMPTY) останется как прежде.
@@ -import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; +import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; +import lombok.NonNull; @@ - @Default - AttributeType type = AttributeTypeImpl.EMPTY; + @NonNull + @Default + AttributeType type = AttributeTypeImpl.EMPTY;Also applies to: 119-124
src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExtDimensionAccountingFlag.java (1)
27-29: Гарантировать не-null для типа реквизитаАналогично ObjectAttribute, стоит защититься от передачи null через билдер.
@@ -import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; +import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; +import lombok.NonNull; @@ - @Default - AttributeType type = AttributeTypeImpl.EMPTY; + @NonNull + @Default + AttributeType type = AttributeTypeImpl.EMPTY;Also applies to: 75-80
src/main/java/com/github/_1c_syntax/bsl/mdo/children/TaskAddressingAttribute.java (1)
27-29: Не-null контракт для type в задачных адресных реквизитахСимметрично остальным сущностям — фиксация @nonnull упростит инварианты.
@@ -import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; +import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; +import lombok.NonNull; @@ - @Default - AttributeType type = AttributeTypeImpl.EMPTY; + @NonNull + @Default + AttributeType type = AttributeTypeImpl.EMPTY;Also applies to: 75-80
src/main/java/com/github/_1c_syntax/bsl/mdo/children/AccountingFlag.java (2)
27-28: Избежать прямой зависимости от реализации AttributeTypeImplЛучше не импортировать реализацию в публичной модели MDO. Предпочтительнее интерфейсная константа (например, AttributeType.EMPTY) или фабрика (AttributeTypeFactory.empty()) — так снизим связность.
75-80: Поле type: зафиксировать non-null контракт и скрыть реализацию в значении по умолчаниюРекомендую пометить поле как @nonnull и использовать интерфейсную константу вместо AttributeTypeImpl.EMPTY.
Пример правки:
- @Default - AttributeType type = AttributeTypeImpl.EMPTY; + @Default + @NonNull + AttributeType type = AttributeType.EMPTY;Дополнительно вне текущего диапазона: добавить импорт
import lombok.NonNull;src/main/java/com/github/_1c_syntax/bsl/mdo/children/DocumentJournalColumn.java (2)
26-28: Сократить связность: не импортировать AttributeTypeImpl в модельАналогично другим классам MDO, избегайте прямого импорта реализации; используйте интерфейсную константу/фабрику.
71-76: Поле type: явно обозначить non-null и интерфейсное значение по умолчаниюДобавьте @nonnull и используйте интерфейсную константу (или фабрику), чтобы не тянуть зависимость от реализации.
- @Default - AttributeType type = AttributeTypeImpl.EMPTY; + @Default + @NonNull + AttributeType type = AttributeType.EMPTY;Дополнительно вне диапазона:
import lombok.NonNull;src/main/java/com/github/_1c_syntax/bsl/mdo/support/QualifierType.java (1)
1-47: Enum QualifierType ок; подумайте о безопасном парсинге и будущей совместимости
- Неплохо бы иметь утилиту безопасного парсинга из строки (например, QualifierType.fromStringOrNull) для конвертеров.
- Опционально: предусмотреть запасной сценарий для неизвестных значений (обработка на уровне конвертера), чтобы не падать при встрече новых квалификаторов.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java (1)
251-251: Alias "Type": конвертер зарегистрирован, дубликатов alias нет
Добавьте или обновите XML-фикстуры в тестах с тегом<Type>, чтобы покрыть alias("Type", AttributeType).src/main/java/com/github/_1c_syntax/bsl/mdo/support/TypeCategory.java (1)
1-47: Согласовать терминологию: composite/union vs CHARACTERISTICВ PR заявлена поддержка составных типов; в enum отсутствует явная категория COMPOSITE/UNION. Если составность моделируется иначе (например, в AttributeType/TypeDescription), ок. Иначе стоит либо добавить отдельную категорию, либо уточнить JavaDoc, как классифицируются составные типы.
src/main/java/com/github/_1c_syntax/bsl/mdo/support/TypeQualifier.java (1)
27-32: Контракт ок; добавьте мелкие метаданные.Нит: имеет смысл указать в Javadoc @SInCE и явно оговорить требования к реализациям (иммутабельность/сериализуемость). Если в проекте используются аннотации nullability — пометьте возвращаемое значение как NonNull.
src/test/java/com/github/_1c_syntax/bsl/test_utils/MDTestUtils.java (1)
28-39: Импорты новых MDO: ок, но, возможно, избыточны.ClassGraph уже сканирует пакет mdo; импорты не влияют на сканирование. Если они добавлены «на всякий случай», их можно опустить; иначе — оставить. Убедитесь, что тестовые фикстуры учитывают новые поля type.
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeType.java (2)
31-36: Уточнить контракт метода и формулировку Javadoc.Сейчас речь про “список примитивных типов”, хотя список содержит любые описания (в т.ч. ссылочные, составные). Предлагаю в Javadoc снять подтипизацию и закрепить immutability/non-null контракт для списка.
- /** - * Возвращает список примитивных типов, составляющих данный тип - * Для примитивных типов - один элемент - * Для составных типов - несколько элементов - */ + /** + * Возвращает список описаний типов, составляющих данный тип. + * Для простых типов — один элемент, для составных — несколько. + * Должен быть неизменяемым и не null (может быть пустым для отсутствующего типа). + */ List<TypeDescription> getTypeDescriptions();
43-47: Добавить алиас для обратной совместимости: getDescription().В документации и примерах встречается getDescription(); чтобы не ломать пользователей, дайте дефолт-метод-алиас.
/** * Возвращает строковое представление типа */ String getDisplayName(); + + /** + * Устаревший алиас. Эквивалент {@link #getDisplayName()}. + */ + default String getDescription() { + return getDisplayName(); + } }docs/ru/examples.md (1)
291-301: Импорт класса Attribute в примере.Пример использует List, но импорт отсутствует внутри блока.
-import com.github._1c_syntax.bsl.mdo.support.TypeCategory; +import com.github._1c_syntax.bsl.mdo.support.TypeCategory; +import com.github._1c_syntax.bsl.mdo.Attribute; // Группировка реквизитов по категориям типов Map<TypeCategory, List<Attribute>> attributesByCategory =src/main/java/com/github/_1c_syntax/bsl/mdo/support/StringQualifier.java (2)
24-29: Лишние импорты Lombok.@value уже генерирует toString/equals/hashCode — эти импорты не нужны.
import lombok.Builder; import lombok.Builder.Default; -import lombok.EqualsAndHashCode; -import lombok.ToString; import lombok.Value;
33-37: Упростить аннотации Lombok.Уберите дублирование — @value покрывает @ToString/@EqualsAndHashCode.
@Value @Builder -@ToString -@EqualsAndHashCode public class StringQualifier implements TypeQualifier {src/main/java/com/github/_1c_syntax/bsl/mdo/support/NumberQualifier.java (2)
24-29: Лишние импорты Lombok.Аналогично StringQualifier — избыточные импорты.
import lombok.Builder; import lombok.Builder.Default; -import lombok.EqualsAndHashCode; -import lombok.ToString; import lombok.Value;
33-37: Упростить аннотации Lombok.@value покрывает toString/equals/hashCode.
@Value @Builder -@ToString -@EqualsAndHashCode public class NumberQualifier implements TypeQualifier {src/main/java/com/github/_1c_syntax/bsl/mdo/support/TypeDescription.java (3)
24-29: Лишние импорты Lombok.@value генерирует equals/hashCode/toString — импорты можно убрать.
import lombok.Builder; import lombok.Builder.Default; -import lombok.EqualsAndHashCode; -import lombok.ToString; import lombok.Value;
35-39: Упростить аннотации Lombok.Уберите дублирующие аннотации.
@Value @Builder -@ToString -@EqualsAndHashCode public class TypeDescription {
59-72: Удобный аксессор для наличия квалификатора.Часто в примерах проверяется qualifier.isPresent(). Добавьте шорткат.
@Default Optional<TypeQualifier> qualifier = Optional.empty(); + /** + * Есть ли квалификатор у типа + */ + public boolean hasQualifier() { + return qualifier.isPresent(); + } + /** * Возвращает true, если тип является примитивным */ public boolean isPrimitive() {src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java (5)
55-57: Нормализуйте значение типа (trim и пропуск пустых).Во избежание пустых/пробельных элементов.
- if ("types".equals(nodeName)) { - typeNames.add(reader.getValue()); - } else if ("stringQualifiers".equals(nodeName)) { + if ("types".equals(nodeName)) { + String tn = reader.getValue() == null ? "" : reader.getValue().trim(); + if (!tn.isEmpty()) { + typeNames.add(tn); + } + } else if ("stringQualifiers".equals(nodeName)) {
96-119: Мёртвый код: parseTypeWithQualifiers не используется. Удалите или используйте.Сейчас метод вводит в заблуждение и поддерживать его не будут.
- private TypeDescription parseTypeWithQualifiers(HierarchicalStreamReader reader, String typeName) { - TypeDescription.TypeDescriptionBuilder builder = TypeDescription.builder() - .typeName(typeName) - .category(determineTypeCategory(typeName)); - - // Ищем квалификаторы на том же уровне что и types - HierarchicalStreamReader parent = reader; - while (parent.hasMoreChildren()) { - parent.moveDown(); - String qualifierName = parent.getNodeName(); - - if ("stringQualifiers".equals(qualifierName)) { - StringQualifier qualifier = parseStringQualifiers(parent); - builder.qualifier(Optional.of(qualifier)); - } else if ("numberQualifiers".equals(qualifierName)) { - NumberQualifier qualifier = parseNumberQualifiers(parent); - builder.qualifier(Optional.of(qualifier)); - } - - parent.moveUp(); - } - - return builder.build(); - }
170-178: Единая точка классификации типов.Логику определения категории лучше вынести/синхронизировать с фабрикой (AttributeTypeFactory), чтобы исключить расхождение при расширении списка типов.
Могу подготовить утилиту TypeCategories.withCategoryOf(String typeName) и заменить вызовы здесь и в фабрике.
44-89: Покрыть конвертер тестами на составные типы с обоими квалификаторами.Сценарий: types=[String, Number], заданы stringQualifiers и numberQualifiers. Ожидаем, что квалификаторы применены адресно (см. правку выше).
Могу добавить параметризованные тесты к EDT-конвертеру с минимальными XML-фикстурами.
134-136: Используйте equalsIgnoreCase при сравнении EDT-значений
Заменить"Fixed".equals(value)и"Nonnegative".equals(value)наequalsIgnoreCaseв кейсахallowedLengthиallowedSign(аналогично на строках 160–161) для корректной работы при разных регистрах.src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeImpl.java (1)
59-72: Минор: getDisplayName может опираться на порядок.Если источник — Set/Map, порядок может «плавать». Не критично, но при необходимости стабильности добавьте сортировку по имени.
- return typeDescriptions.stream() - .map(TypeDescription::getTypeName) - .collect(Collectors.joining(", ")); + return typeDescriptions.stream() + .map(TypeDescription::getTypeName) + .sorted() + .collect(Collectors.joining(", "));src/test/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeTest.java (2)
57-64: Добавьте проверку категорий в составном типе.Небольшая доп. гарантия корректной классификации.
assertThat(type.getTypeDescriptions().get(0).getCategory()).isEqualTo(TypeCategory.PRIMITIVE); assertThat(type.getTypeDescriptions().get(1).getCategory()).isEqualTo(TypeCategory.PRIMITIVE); assertThat(type.getTypeDescriptions().get(2).getCategory()).isEqualTo(TypeCategory.REFERENCE);
66-78: Нет теста на составной тип с обоими квалификаторами (String+Number).Такой тест поймает ошибку распределения квалификаторов в конвертере/фабрике.
Пример теста:
@Test void testCompositeStringAndNumberWithOwnQualifiers() { // Имитация сборки вручную через билдеры StringQualifier sq = StringQualifier.builder().length(5).allowedLength(StringQualifier.AllowedLength.FIXED).build(); NumberQualifier nq = NumberQualifier.builder().precision(8).fractionDigits(2).allowedSign(NumberQualifier.AllowedSign.NONNEGATIVE).build(); TypeDescription tdStr = TypeDescription.builder() .typeName("String") .category(TypeCategory.PRIMITIVE) .qualifier(Optional.of(sq)) .build(); TypeDescription tdNum = TypeDescription.builder() .typeName("Number") .category(TypeCategory.PRIMITIVE) .qualifier(Optional.of(nq)) .build(); AttributeType type = AttributeTypeImpl.builder() .typeDescription(tdStr) .typeDescription(tdNum) .build(); assertThat(type.isComposite()).isTrue(); assertThat(type.getTypeDescriptions().get(0).getQualifier()).containsInstanceOf(StringQualifier.class); assertThat(type.getTypeDescriptions().get(1).getQualifier()).containsInstanceOf(NumberQualifier.class); }Also applies to: 80-93
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java (2)
90-113: Заменить неиспользуемый parseTypeWithQualifiers на helper, который используется
parseTypeWithQualifiers(...)не вызывается. Предлагаю заменить его наbuildTypeDescription(...), который использует собранные квалификаторы для конкретногоtypeName(см. правку в unmarshal).- private TypeDescription parseTypeWithQualifiers(HierarchicalStreamReader reader, String typeName) { - TypeDescription.TypeDescriptionBuilder builder = TypeDescription.builder() - .typeName(normalizeTypeName(typeName)) - .category(determineTypeCategory(typeName)); - - // Ищем квалификаторы на том же уровне что и v8:Type - HierarchicalStreamReader parent = reader; - while (parent.hasMoreChildren()) { - parent.moveDown(); - String qualifierName = parent.getNodeName(); - - if ("v8:StringQualifiers".equals(qualifierName)) { - StringQualifier qualifier = parseStringQualifiers(parent); - builder.qualifier(Optional.of(qualifier)); - } else if ("v8:NumberQualifiers".equals(qualifierName)) { - NumberQualifier qualifier = parseNumberQualifiers(parent); - builder.qualifier(Optional.of(qualifier)); - } - - parent.moveUp(); - } - - return builder.build(); - } + private TypeDescription buildTypeDescription( + String typeName, + Optional<StringQualifier> stringQualifier, + Optional<NumberQualifier> numberQualifier + ) { + TypeDescription.TypeDescriptionBuilder builder = TypeDescription.builder() + .typeName(normalizeTypeName(typeName)) + .category(determineTypeCategory(typeName)); + + stringQualifier.ifPresent(q -> builder.qualifier(Optional.of(q))); + numberQualifier.ifPresent(q -> builder.qualifier(Optional.of(q))); + return builder.build(); + }
164-172: Нормализация: trim и регистронезависимые префиксы xs:/cfg:
reader.getValue()может содержать пробелы/переводы строк; также возможен регистрXS:/Cfg:. Это устраняет лишние расхождения и снижает риск NFE в парсерах.- private String normalizeTypeName(String typeName) { - if (typeName.startsWith("xs:")) { - return typeName.substring(3); - } - if (typeName.startsWith("cfg:")) { - return typeName.substring(4); - } - return typeName; - } + private String normalizeTypeName(String typeName) { + if (typeName == null) { + return null; + } + String s = typeName.trim(); + String lower = s.toLowerCase(); + if (lower.startsWith("xs:")) { + return s.substring(3); + } + if (lower.startsWith("cfg:")) { + return s.substring(4); + } + return s; + }src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java (2)
81-103: Свести дублируемую логiku классификации типов к общему месту
determineTypeCategory/isPrimitiveType/isReferenceTypeдублируют подход из Designer-конвертера (и расходятся с ним). Рекомендую вынести классификацию и нормализацию в общий util, чтобы избежать дрейфа логики.Готов предложить общий
TypeNameClassifierи заменить вызовы в обоих местах — дайте знать.
59-64: Minor: обработка пустых строк в createType(String)Имеет смысл трактовать пустую/пробельную строку как отсутствие типа.
- public static AttributeType createType(String typeName) { - if (typeName == null) { + public static AttributeType createType(String typeName) { + if (typeName == null || typeName.trim().isEmpty()) { return AttributeTypeImpl.EMPTY; } return createType(List.of(typeName)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (78)
src/test/resources/fixtures/external/ТестоваяВнешняяОбработка.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/external/ТестоваяВнешняяОбработка_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/external/ТестовыйВнешнийОтчет.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/external/ТестовыйВнешнийОтчет_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/AccountingRegisters.РегистрБухгалтерии1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/AccountingRegisters.РегистрБухгалтерии1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/AccumulationRegisters.РегистрНакопления1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/AccumulationRegisters.РегистрНакопления1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CalculationRegisters.РегистрРасчета1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CalculationRegisters.РегистрРасчета1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Catalogs.Справочник1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Catalogs.Справочник1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ГлобальныйОбщийМодуль.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ГлобальныйОбщийМодуль_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульВызовСервера.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульВызовСервера_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспВызов.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспВызов_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспСеанс.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспСеанс_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПолныйеПрава.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПолныйеПрава_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ПростойОбщийМодуль.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CommonModules.ПростойОбщийМодуль_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/DocumentJournals.ЖурналДокументов1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/DocumentJournals.ЖурналДокументов1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Documents.Документ1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Documents.Документ1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ExternalDataSources.ТекущаяСУБД.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ExternalDataSources.ТекущаяСУБД_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/FilterCriteria.КритерийОтбора1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/FilterCriteria.КритерийОтбора1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений2.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений2_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Reports.Отчет1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Reports.Отчет1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Sequences.Последовательность1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Sequences.Последовательность1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Tasks.Задача1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Tasks.Задача1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_18/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_18/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_24/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_24/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_5_1/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_ext/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_ext/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/BusinessProcesses.Задание.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/BusinessProcesses.Задание_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Catalogs.Заметки.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Catalogs.Заметки_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/CommonAttributes.ОбластьДанныхВспомогательныеДанные.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/DataProcessors.ЗагрузкаКурсовВалют.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/DataProcessors.ЗагрузкаКурсовВалют_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/DocumentJournals.Взаимодействия.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/DocumentJournals.Взаимодействия_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Documents.Анкета.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Documents.Анкета_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ExchangePlans.ОбновлениеИнформационнойБазы.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ExchangePlans.ОбновлениеИнформационнойБазы_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/InformationRegisters.ЭлектронныеПодписи.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/InformationRegisters.ЭлектронныеПодписи_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/SettingsStorages.ХранилищеВариантовОтчетов.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/SettingsStorages.ХранилищеВариантовОтчетов_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Tasks.ЗадачаИсполнителя.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Tasks.ЗадачаИсполнителя_edt.jsonis excluded by!**/*.json
📒 Files selected for processing (30)
docs/ru/examples.md(3 hunks)docs/ru/features.md(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Attribute.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/AccountingFlag.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/Dimension.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/DocumentJournalColumn.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExtDimensionAccountingFlag.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceTableField.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectAttribute.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/Resource.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/TaskAddressingAttribute.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeType.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeImpl.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/NumberQualifier.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/QualifierType.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/StringQualifier.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/TypeCategory.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/TypeDescription.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/TypeQualifier.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/AccountingRegisterTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/AccumulationRegisterTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/TasksTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/test_utils/MDTestUtils.java(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/ru/examples.md
[uncategorized] ~252-~252: Неправильная форма предлога. Исправление: «со ссылочными».
Context: ...h()); } } } ### Работа с ссылочными типами java // Определение ссылочны...
(S_SO)
⏰ 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). (9)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (8)
src/test/java/com/github/_1c_syntax/bsl/mdo/TasksTest.java (1)
33-37: Уточнить проверку CSV-кейсовТекущий скрипт (
rg -nP 'Tasks\.' -C2 | rg -n '_edt|CsvSource') фильтрует только строки с_edtи аннотацию, поэтому не гарантирует наличие кейсов без_edt. Проверьте, пожалуйста, что для каждогоTasks.*есть соответствующий кейс без суффикса_edt.src/main/java/com/github/_1c_syntax/bsl/mdo/Attribute.java (1)
25-25: Импорт нового типа выглядит корректноИмпорт AttributeType добавлен верно и соответствует новому аксессору.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java (1)
40-40: Импорт AttributeType — корректноИмпорт используется только для alias; претензий нет.
src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceTableField.java (1)
77-82: Поле типа реквизита добавлено корректно; проверьте путь данных.Дефолт через @default и EMPTY — ок. Подтвердите, что конвертеры Designer/EDT заполняют type для ExternalDataSourceTableField и что соответствующие фикстуры/тесты обновлены. Поле type по-прежнему не участвует в equals/hashCode — если это осознанное решение, всё хорошо.
src/main/java/com/github/_1c_syntax/bsl/mdo/children/Dimension.java (1)
95-100: Поле типа реквизита: ок; убедитесь в сквозной поддержке.Дефолт и EMPTY — корректно. Просьба подтвердить заполнение при чтении из Designer/EDT и наличие проверок в интеграционных тестах/фикстурах.
src/main/java/com/github/_1c_syntax/bsl/mdo/children/Resource.java (1)
79-84: Поле типа реквизита — всё хорошо; проверьте чтение/фикстуры.Добавление согласовано с остальными MDO. Подтвердите сквозную сериализацию/десериализацию и обновление тестовых фикстур.
src/test/java/com/github/_1c_syntax/bsl/test_utils/MDTestUtils.java (1)
114-114: Косметика — ок.Пустая строка после блока — без замечаний.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java (1)
115-136: Требуется ручная валидация XML-фикстур: в проекте не обнаружены.xml-файлы для автоматической проверки допустимых значений<v8:AllowedLength>и<v8:AllowedSign>. Пожалуйста, вручную убедитесь, что к значениям применяетсяtrim(), числовые поля парсятся безопасно (с пропуском пустых/нечисловых строк) и строковые константы сравниваются черезequalsIgnoreCase.
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/mdo/support/StringQualifier.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java (1)
44-67: Составные типы больше не теряются — окСписок
typeNamesрешает проблему потери последних типов при множественных<v8:Type>.
🧹 Nitpick comments (4)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java (1)
78-83: Дедупликация типов при сохранении порядкаЕсли в источнике есть дубли, мы соберём лишние
TypeDescription. Уберите дубликаты, сохраняя порядок.Примените:
@@ -import java.util.ArrayList; +import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; @@ - for (String typeName : typeNames) { + for (String typeName : new LinkedHashSet<>(typeNames)) { if (typeName != null && !typeName.trim().isEmpty()) { typeDescriptions.add(createTypeDescription(typeName, stringQualifier, numberQualifier)); } }Also applies to: 31-34
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java (1)
92-93: Сделать сравнения значения квалификаторов регистронезависимымиДля устойчивости к вариациям регистра.
- builder.allowedLength("Fixed".equals(value) ? StringQualifier.AllowedLength.FIXED : StringQualifier.AllowedLength.VARIABLE); + builder.allowedLength("Fixed".equalsIgnoreCase(value) ? StringQualifier.AllowedLength.FIXED : StringQualifier.AllowedLength.VARIABLE); @@ - builder.allowedSign("Nonnegative".equals(value) ? NumberQualifier.AllowedSign.NONNEGATIVE : NumberQualifier.AllowedSign.ANY); + builder.allowedSign("Nonnegative".equalsIgnoreCase(value) ? NumberQualifier.AllowedSign.NONNEGATIVE : NumberQualifier.AllowedSign.ANY);Also applies to: 129-130
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java (2)
55-57: Подчистить пробелы у значений типов для предсказуемостиТримминг на стороне конвертера уменьшит шум и дубликаты.
- if ("types".equals(nodeName)) { - typeNames.add(reader.getValue()); + if ("types".equals(nodeName)) { + String v = reader.getValue(); + typeNames.add(v != null ? v.trim() : null); } else if ("stringQualifiers".equals(nodeName)) {
92-93: Регистронезависимые сравнения значений квалификаторовАналогично дизайнерскому конвертеру.
- builder.allowedLength("Fixed".equals(value) ? StringQualifier.AllowedLength.FIXED : StringQualifier.AllowedLength.VARIABLE); + builder.allowedLength("Fixed".equalsIgnoreCase(value) ? StringQualifier.AllowedLength.FIXED : StringQualifier.AllowedLength.VARIABLE); @@ - builder.allowedSign("Nonnegative".equals(value) ? NumberQualifier.AllowedSign.NONNEGATIVE : NumberQualifier.AllowedSign.ANY); + builder.allowedSign("Nonnegative".equalsIgnoreCase(value) ? NumberQualifier.AllowedSign.NONNEGATIVE : NumberQualifier.AllowedSign.ANY);Also applies to: 129-130
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
src/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Documents.Анкета_edt.jsonis excluded by!**/*.json
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java(1 hunks)
⏰ 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). (9)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
🔇 Additional comments (3)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java (1)
56-63: LGTM: корректное адресное применение квалификаторов и сборка составных типовКвалификаторы применяются только к соответствующим примитивам, составные типы собираются без побочных эффектов.
Also applies to: 71-92
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java (1)
55-61: Проверить, что квалификаторы в Designer-грамматике относятся ко всей группе типовЕсли в реальных данных есть несколько секций квалификаторов, привязанных к конкретным
v8:Type, текущая модель (глобальные опциональные квалификаторы) их не различит.Можете приложить минимальный XML-пример с несколькими
v8:Typeи разнымиv8:*Qualifiers? При необходимости помогу адаптировать парсер под «пер-типовую» привязку.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java (1)
44-67: LGTM: корректный парсинг EDT-структуры и делегирование в базовый конвертерЧтение
typesи опциональных квалификаторов выглядит корректно.
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java (1)
66-67: Применение квалификаторов адресно — ок.Возврат через
createAttributeType(...)из базового класса решает прежнюю проблему глобального применения квалификаторов к составным типам.
🧹 Nitpick comments (4)
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java (4)
70-72: Сигнатура canConvert: избегайте raw types.Замените
Class typeнаClass<?> typeдля устранения raw‑warnings.- public boolean canConvert(Class type) { + public boolean canConvert(Class<?> type) { return AttributeType.class.isAssignableFrom(type); }
80-108: Строковые квалификаторы: trim значений, регистронезависимые сравнения, фильтр отрицательных.Устойчивее парсить с
trim()иequalsIgnoreCase, игнорировать отрицательные длины.- String nodeName = reader.getNodeName(); - String value = reader.getValue(); + String nodeName = reader.getNodeName(); + String raw = reader.getValue(); + String value = raw != null ? raw.trim() : ""; @@ - case "length": - try { - builder.length(Integer.parseInt(value)); + case "length": + try { + int len = Integer.parseInt(value); + if (len >= 0) { + builder.length(len); + } } catch (NumberFormatException e) { // Игнорируем некорректные значения длины } break; case "allowedLength": - builder.allowedLength("Fixed".equals(value) ? StringQualifier.AllowedLength.FIXED : StringQualifier.AllowedLength.VARIABLE); + builder.allowedLength("Fixed".equalsIgnoreCase(value) + ? StringQualifier.AllowedLength.FIXED + : StringQualifier.AllowedLength.VARIABLE); break;
115-150: Числовые квалификаторы: trim, регистронезависимый allowedSign, фильтр отрицательных значений.Parsing с обрезкой пробелов и защитой от отрицательных значений повысит устойчивость. Также сделайте
allowedSignрегистронезависимым. ОграничениеfractionDigits <= precisionоставляю на ваше усмотрение — проверьте требования EDT и тестами.- String nodeName = reader.getNodeName(); - String value = reader.getValue(); + String nodeName = reader.getNodeName(); + String raw = reader.getValue(); + String value = raw != null ? raw.trim() : ""; @@ - case "precision": - try { - builder.precision(Integer.parseInt(value)); + case "precision": + try { + int p = Integer.parseInt(value); + if (p >= 0) { + builder.precision(p); + } } catch (NumberFormatException e) { // Игнорируем некорректные значения точности } break; case "fractionDigits": - try { - builder.fractionDigits(Integer.parseInt(value)); + try { + int fd = Integer.parseInt(value); + if (fd >= 0) { + builder.fractionDigits(fd); + } } catch (NumberFormatException e) { // Игнорируем некорректные значения дробных разрядов } break; case "allowedSign": - builder.allowedSign("Nonnegative".equals(value) ? NumberQualifier.AllowedSign.NONNEGATIVE : NumberQualifier.AllowedSign.ANY); + builder.allowedSign("Nonnegative".equalsIgnoreCase(value) + ? NumberQualifier.AllowedSign.NONNEGATIVE + : NumberQualifier.AllowedSign.ANY); break;Предложение к тестам: проверить парсинг с ведущими/замыкающими пробелами и смешанным регистром значений.
24-31: Чистка импортов.
AttributeTypeImpl,TypeDescription,TypeCategoryздесь не используются.import com.github._1c_syntax.bsl.mdo.support.AttributeType; -import com.github._1c_syntax.bsl.mdo.support.AttributeTypeImpl; import com.github._1c_syntax.bsl.mdo.support.NumberQualifier; import com.github._1c_syntax.bsl.mdo.support.StringQualifier; -import com.github._1c_syntax.bsl.mdo.support.TypeDescription; -import com.github._1c_syntax.bsl.mdo.support.TypeCategory; import com.github._1c_syntax.bsl.reader.common.converter.AttributeTypeConverterBase;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java
- src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java (1)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java (1)
AttributeTypeConverterBase(38-118)
⏰ 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). (9)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
| if ("types".equals(nodeName)) { | ||
| typeNames.add(reader.getValue()); | ||
| } else if ("stringQualifiers".equals(nodeName)) { | ||
| stringQualifier = Optional.of(parseStringQualifiers(reader)); | ||
| } else if ("numberQualifiers".equals(nodeName)) { | ||
| numberQualifier = Optional.of(parseNumberQualifiers(reader)); | ||
| } | ||
|
|
||
| reader.moveUp(); | ||
| } | ||
|
|
||
| return createAttributeType(typeNames, stringQualifier, numberQualifier); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean canConvert(Class type) { | ||
| return AttributeType.class.isAssignableFrom(type); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Парсит строковые квалификаторы из EDT | ||
| * @param reader EDT reader | ||
| * @return объект StringQualifier | ||
| */ | ||
| private StringQualifier parseStringQualifiers(HierarchicalStreamReader reader) { | ||
| StringQualifier.StringQualifierBuilder builder = StringQualifier.builder(); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Разбор узла типов: поддержать контейнер с дочерними и одиночные значения; тримминг.
Сейчас getValue() берётся прямо у узла types. Если в EDT это контейнер с дочерними type, список типов не будет разобран. Предлагаю универсальную обработку обоих вариантов и обрезку пробелов.
- if ("types".equals(nodeName)) {
- typeNames.add(reader.getValue());
- } else if ("stringQualifiers".equals(nodeName)) {
+ if ("types".equals(nodeName)) {
+ if (reader.hasMoreChildren()) {
+ while (reader.hasMoreChildren()) {
+ reader.moveDown();
+ if ("type".equals(reader.getNodeName())) {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
+ } else {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
+ }
+ reader.moveUp();
+ }
+ } else {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
+ }
+ } else if ("type".equals(nodeName)) {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
+ } else if ("stringQualifiers".equals(nodeName)) {
stringQualifier = Optional.of(parseStringQualifiers(reader));
} else if ("numberQualifiers".equals(nodeName)) {
numberQualifier = Optional.of(parseNumberQualifiers(reader));
}P.S. Добавьте тесты на оба варианта структуры: контейнер <types><type>String</type><type>Number</type></types> и повторяющиеся одноуровневые теги.
🏁 Script executed:
#!/bin/bash
# Поиск и вывод первых 200 строк файла AttributeTypeConverter.java
file=$(fd -e java AttributeTypeConverter.java)
sed -n '1,200p' "$file"Length of output: 660
🏁 Script executed:
#!/bin/bash
# Определение файла EDT-конвертера и вывод строк 1–200 для проверки
file=$(fd -e java AttributeTypeConverter.java | grep "/edt/")
if [ -z "$file" ]; then
echo "EDT-конвертер не найден"
exit 1
fi
sed -n '1,200p' "$file"Length of output: 5437
Обработать вложенные и одиночные теги <type> в <types> и обрезать пробелы
Нынешняя реализация берёт reader.getValue() прямо у узла <types>, из-за чего вложенные <type> не разбираются, а возвращаемые строки могут содержать лишние пробелы. Нужно добавить ветку для обработки дочерних <type> и обрезку значений.
if ("types".equals(nodeName)) {
- typeNames.add(reader.getValue());
+ if (reader.hasMoreChildren()) {
+ while (reader.hasMoreChildren()) {
+ reader.moveDown();
+ if ("type".equals(reader.getNodeName())) {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
+ }
+ reader.moveUp();
+ }
+ } else {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
+ }
+ } else if ("type".equals(nodeName)) {
+ String val = reader.getValue();
+ if (val != null && !val.isBlank()) {
+ typeNames.add(val.trim());
+ }
} else if ("stringQualifiers".equals(nodeName)) {
stringQualifier = Optional.of(parseStringQualifiers(reader));
} else if ("numberQualifiers".equals(nodeName)) {P.S. Добавьте тесты на оба варианта структуры:
- контейнер
<types><type>Foo</type><type>Bar</type></types> - повторяющиеся одноуровневые
<types>Foo</types><types>Bar</types>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ("types".equals(nodeName)) { | |
| typeNames.add(reader.getValue()); | |
| } else if ("stringQualifiers".equals(nodeName)) { | |
| stringQualifier = Optional.of(parseStringQualifiers(reader)); | |
| } else if ("numberQualifiers".equals(nodeName)) { | |
| numberQualifier = Optional.of(parseNumberQualifiers(reader)); | |
| } | |
| reader.moveUp(); | |
| } | |
| return createAttributeType(typeNames, stringQualifier, numberQualifier); | |
| } | |
| @Override | |
| public boolean canConvert(Class type) { | |
| return AttributeType.class.isAssignableFrom(type); | |
| } | |
| /** | |
| * Парсит строковые квалификаторы из EDT | |
| * @param reader EDT reader | |
| * @return объект StringQualifier | |
| */ | |
| private StringQualifier parseStringQualifiers(HierarchicalStreamReader reader) { | |
| StringQualifier.StringQualifierBuilder builder = StringQualifier.builder(); | |
| if ("types".equals(nodeName)) { | |
| if (reader.hasMoreChildren()) { | |
| while (reader.hasMoreChildren()) { | |
| reader.moveDown(); | |
| if ("type".equals(reader.getNodeName())) { | |
| String val = reader.getValue(); | |
| if (val != null && !val.isBlank()) { | |
| typeNames.add(val.trim()); | |
| } | |
| } | |
| reader.moveUp(); | |
| } | |
| } else { | |
| String val = reader.getValue(); | |
| if (val != null && !val.isBlank()) { | |
| typeNames.add(val.trim()); | |
| } | |
| } | |
| } else if ("type".equals(nodeName)) { | |
| String val = reader.getValue(); | |
| if (val != null && !val.isBlank()) { | |
| typeNames.add(val.trim()); | |
| } | |
| } else if ("stringQualifiers".equals(nodeName)) { | |
| stringQualifier = Optional.of(parseStringQualifiers(reader)); | |
| } else if ("numberQualifiers".equals(nodeName)) { | |
| numberQualifier = Optional.of(parseNumberQualifiers(reader)); | |
| } | |
| reader.moveUp(); |
🤖 Prompt for AI Agents
In
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java
around lines 55 to 82, the current branch that handles "types" calls
reader.getValue() on the <types> node which fails to process nested <type> child
elements and does not trim whitespace. Update the "types" branch to detect
whether the <types> node has child <type> elements: if so iterate its children,
read each child's value, trim() it, and add non-empty names to typeNames;
otherwise fall back to trimming reader.getValue() and add if non-empty. Ensure
reader position is preserved (moveDown/moveUp around child iteration) and add
unit tests for both nested container form
(<types><type>Foo</type><type>Bar</type></types>) and repeated flat form
(<types>Foo</types><types>Bar</types>) verifying names are trimmed and
collected.
…класса конвертера - Добавлен новый класс AttributeTypeConverterBase для упрощения конвертации типов атрибутов. - Реализована нормализация имен типов, удаляющая префиксы пространств имен. - Добавлена валидация параметров для методов создания типов атрибутов (строковых и числовых). - Обновлены существующие конвертеры для использования нового базового класса и улучшенной логики обработки типов. - Обновлены тестовые фикстуры с полными данными о типах атрибутов.
johnnyshut
force-pushed
the
feature/277
branch
from
ef81e70 to
d3ebcb5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java (2)
126-134: Убрать мёртвую веткуxs:вisPrimitiveTypeПосле нормализации
xs:/cfg:не попадут в классификатор. ПроверкаstartsWith("xs:")недостижима.private static boolean isPrimitiveType(String typeName) { return typeName.equals("String") || typeName.equals("Number") || typeName.equals("Boolean") || typeName.equals("Date") || typeName.equals("Binary") - || typeName.equals("UUID") - || typeName.startsWith("xs:"); + || typeName.equals("UUID"); }
139-145: Гевристика ссылочных типов может давать ложные срабатывания
contains("List.")/contains("Selection.")и т.п. потенциально совпадут с произвольными именами. Рассмотрите шаблон с якорями по известным подсистемам (Catalog|Document|Enum|...) и ролям (Ref|Object|Manager|List|Selection), желательно как precompiledPattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
src/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Documents.Анкета_edt.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AttributeTypeConverterBase.java
- src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/AttributeTypeConverter.java
- src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/AttributeTypeConverter.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). (9)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
🔇 Additional comments (4)
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java (4)
99-121: Проверьте корректность дефолта категории для неизвестных типовПо умолчанию всё нераспознанное помечается как
PRIMITIVE. Это может скрыть ошибки в исходных данных. Уточните, не безопаснее ли возвращатьDEFINEDили явно сигнализировать о неизвестном типе.
150-173: LGTM: валидация и сборка строкового типаПроверки параметров и дефолт
AllowedLength.VARIABLEвыглядят корректно.
178-205: LGTM: валидация и сборка числового типаПроверки
precisionиfractionDigitsкорректны.
186-188: Уточнить дефолт знака числа
AllowedSign.ANY— верный дефолт для доменной модели? В 1С по умолчанию часто используется «любой» или «с положительным и отрицательным», но лучше подтвердить требование.
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/mdo/support/AttributeTypeFactory.java
Show resolved
Hide resolved
|
@theshadowco Посмотри, пожалуйста |
|
Привет. Спасибо за проделанную работу, но к сожалению, архитектура решения не укладывается в целевую. Если нужно что-то добавить для решения твоих задач - пиши в нем, оперативно докину. Еще, у тебя здесь есть дока, было бы супер круто, если бы ты ее адаптировал и реквестнул. |
|
Хорошо |
2 participants
Описание
Реализована полная поддержка типов атрибутов в MDO (Metadata Objects) с расширенными возможностями работы с типами данных реквизитов.
Добавлена система типизации для всех объектов метаданных, включающая примитивные, ссылочные, определяемые и составные типы.
Создана фабрика для создания типов атрибутов с поддержкой квалификаторов для строковых и числовых типов.
Добавлены комплексные тесты для проверки корректности работы с различными типами данных.
Обновлена документация с примерами использования новой функциональности.
Улучшены тестовые фикстуры с корректными русскими названиями и качественными данными для всех типов объектов метаданных.
Связанные задачи
Closes #277
Чеклист
Общие
gradlew precommit)Дополнительно
Добавлены новые классы:
AttributeType- базовый интерфейс для типов реквизитовAttributeTypeFactory- фабрика для создания типов атрибутовAttributeTypeImpl- реализация интерфейса AttributeTypeTypeDescription- описание типа с квалификаторамиTypeCategory- категории типов (примитивные, ссылочные, определяемые, составные)StringQualifierиNumberQualifier- квалификаторы для строковых и числовых типовОбновлены существующие классы:
Attribute- добавлен методgetType()для получения типа атрибутаObjectAttribute- добавлено полеtypeс типомAttributeTypeCommonAttribute- добавлена поддержка типов атрибутовTaskAddressingAttribute- добавлена поддержка типов атрибутовДобавлены конвертеры:
AttributeTypeConverter- конвертер для преобразования типов атрибутов из EDT форматаОбновлены тестовые фикстуры с корректными русскими названиями и полной информацией о типах атрибутов для всех объектов метаданных.