При добавлении завязи из лямбды мета аннотации теперь корректно разворачиваются #110
Conversation
sfaqer
commented
Dec 29, 2024
•
sfaqer
commented
- При добавлении завязи из лямбды мета аннотации теперь корректно разворачиваются
- В разворачиватель аннотаций при создании завязи из лямбды передаётся объект владелец аннотаций (объект с лямбдой)
- Имена параметров при создании определения актуализированы что бы устранить неоднозначность
- Подняты зависимости от lambdas (добавлено строковое представление объекта лямбды) и annotations (документированно использование сценария как владельца аннотированных свойств)
|
""" WalkthroughИзменения затрагивают три файла в проекте, связанные с обработкой аннотаций и метасвязей. В файле Changes
Sequence DiagramsequenceDiagram
participant Factory as ФабрикаЖелудей
participant Annotation as РазворачивательАннотаций
participant TestClass as АннотацияМетаПрозвище
Factory->>Annotation: РазвернутьАннотацииСвойств()
Annotation->>TestClass: ПриРазворачиванииАннотации()
TestClass-->>Annotation: Обработка метаданных
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (20)
🔇 Additional comments (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/ТестКлассы/Классы/АннотацияМетаПрозвище.os (2)
1-2: Убедитесь, что деактивация предупреждений BSLLS оправдана
Отключение проверок может скрыть потенциально полезные подсказки и ограничить помощь статического анализа. Рекомендуется по возможности оставить эти проверки включёнными.
12-16: Учтите возможные многопоточность и повторное использование переменнойИмя
Глобальная переменнаяИмяможет стать источником проблем и непредвиденных эффектов, если код будет использоваться в сценариях с параллельной обработкой. Подумайте о том, чтобы заменить глобальную переменную более локальным хранением состояния (например, хранить значение в объекте или структуре).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/internal/Классы/ФабрикаЖелудей.os(1 hunks)tests/Поделка.os(1 hunks)tests/ТестКлассы/Классы/АннотацияМетаПрозвище.os(1 hunks)
🧰 Additional context used
📓 Learnings (1)
src/internal/Классы/ФабрикаЖелудей.os (1)
Learnt from: nixel2007
PR: autumn-library/autumn#87
File: :0-0
Timestamp: 2024-11-12T14:20:38.625Z
Learning: The method `ДобавитьЗавязьИзЛямбды` is located in `ФабрикаЖелудей` and should not be included in the change log.
🔇 Additional comments (3)
src/internal/Классы/ФабрикаЖелудей.os (1)
161-163: Уточните логику обработки методов перед развёртыванием аннотаций
Добавлен вызов РазвернутьАннотацииСвойств(Методы, "ЛямбдаЗавязь"). Убедитесь, что объекты в Методы корректно проинициализированы и не пусты. При необходимости добавьте сообщения об ошибках или проверку, чтобы избежать случаев, когда Методы отсутствуют.
tests/Поделка.os (2)
1091-1109: Тестовая процедура проверяет логику мета-завязи
Метод ДобавитьМетаЗавязь корректно проверяет создание желудя со значением 42 и подтверждает, что мета-аннотация работает. Тест выглядит логичным и охватывает основные сценарии. Вероятно, дополнительная проверка на наличие аннотаций могла бы ещё больше повысить надёжность теста.
1111-1131: Расширенный тест с мета-прозвищем
Тест ДобавитьМетаЗавязьСМетаПрозвищем успешно демонстрирует, что и исходное имя желудя, и прозвище ведут к одному и тому же экземпляру (значению). Тест выглядит убедительно и дополняет предыдущий сценарий.
Pull Request Test Coverage Report for Build 12803832047Details
💛 - Coveralls |
This comment has been minimized.
This comment has been minimized.
|
Идеальный пулл реквест, божечки |
| @@ -158,6 +158,9 @@ | |||
| Объект = Выражение.ВОбъект(); | |||
|
|
|||
| Методы = Рефлектор.ПолучитьТаблицуМетодов(Объект); | |||
|
|
|||
| РазворачивательАннотаций.РазвернутьАннотацииСвойств(Методы, "ЛямбдаЗавязь"); | |||
There was a problem hiding this comment.
Судя по апи, второй параметр - это тип. Как оно работает тогда?
There was a problem hiding this comment.
Оно там нужно только что бы в случае ошибки в сообщении написать что "Ошибка тута", я подумал лучше так чем "Сценарий"
There was a problem hiding this comment.
Но если у тебя есть предложения получше, я открыт.)
There was a problem hiding this comment.
Оно ещё передаётся в объект аннотации для проверки и в саму аннотацию для разворота. Так что там определённо должен быть тип, а не строка :(
There was a problem hiding this comment.
Оно ещё передаётся в объект аннотации для проверки
Ну как бы да, но
И даже тут оно везде нужно только чтоб в логе это написать
и в саму аннотацию для разворота
Это да, только что ты считаешь будет правильным в данном случае? Ну т.е лямбда это анонимный класс, тип у него "Сценарий" который не отрефлектить и в целом довольно бесполезный для конечного потребителя.
Можно передавать Тип("Сценарий") но единственное что мы получим это менее внятную ошибку в данном коде:
Я в целом понимаю твоё негодование, но у меня просто хорошего решения нет
There was a problem hiding this comment.
Ну как бы да, но
Так, а вот это возможно косяк передачи. Пускай ожидается там имя типа, но и разворачиватель тогда должен имя типа передавать, а не сам тип :(
There was a problem hiding this comment.
Я в целом понимаю твоё негодование, но у меня просто хорошего решения нет
Я не негодую, просто хочу соблюсти API и не добавлять UB ровном месте. Проблему с логом понимаю, думаю.
There was a problem hiding this comment.
Можно передавать Тип("Сценарий") но единственное что мы получим это менее внятную ошибку в данном коде:
Благо, это Отладка, а не ошибка. И за исключением типа есть информация о том, что искать - есть имя аннотации и имя свойства. При локальной установке зависимостей этого должно быть достаточно.
Согласен, не очень красиво, но как есть.
По поводу Тип("Сценарий") - у тебя вроде был какой-то код в лямбдасе, который позволяет лямбду зарегистрировать в системе типов. Тут это не поможет? Вроде и тогда рефлектором можно будет основной метод вытащить и аннотации с него посомтреть.
There was a problem hiding this comment.
Это у тебя в декораторе есть код который может вместо ЗагрузкиСценария сразу в рантайм зарегать его в системе типов, в лямбдас это не пробрасывалось, в целом наверное можно, только я не очень уверен в целесообразности, из-за того что система типов движка будет мусорными типами наполнятся, да и рефлектить тип лямбды это тоже такая история, там служебных декораторных полей\методов вагон и меленькая тележлка.
…ередаётся объект владелец аннотаций (объект с лямбдой) 2. Имена параметров при создании определения актуализированы что бы устранить неоднозначность 3. Подняты зависимости от lambdas (добавлено строковое представление объекта лямбды) и annotations (документированно использование сценария как владельца аннотированных свойств)
sfaqer
force-pushed
the
bugfix/lambdaOvaryAnnotation
branch
from
a1a623a to
10e2b52
Compare
Analysis Details0 Issues
Coverage and Duplications
Project ID: autumn |
