Исправлена ошибка при чтении строк, содержащих дату в формате ISO 8601, при использовании объекта ЧтениеJSON и глобальной функции ПрочитатьJSON: строки с датой больше не преобразуются в Дату автоматически#1682
Open
leemuar wants to merge 7 commits into
Conversation
Исправлено поведение объекта ЧтениеJSON и глобальной функции ПрочитатьJSON, строки с датами не конвертируются автоматически в дату
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughJSONReader now sets SupportMultipleContent and disables Newtonsoft date parsing inside its internal reader. Tests and a fixture were added to confirm ISO-8601 timestamps are read and preserved as strings from string, stream, and file sources. ChangesISO-8601 Datetime String Handling
Sequence DiagramsequenceDiagram
participant JSONInput as JSON Input
participant JSONReader as JSONReader
participant JsonReaderInternal as JsonReaderInternal
participant TestConsumer as Test/Consumer
JSONInput->>JSONReader: provide JSON with ISO-8601 string
JSONReader->>JsonReaderInternal: construct internal reader (SupportMultipleContent=true, DateParseHandling=None)
JsonReaderInternal->>TestConsumer: emit token as String (not Date)
TestConsumer->>TestConsumer: assert value and type are String
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/json/test-json_reader.os (1)
265-333: ⚡ Quick winAdd ISO-string regression tests for
ОткрытьФайлandОткрытьПоток.These new tests cover
УстановитьСтроку, but this PR also changes initialization in file/stream entrypoints. Adding one focused case per path will lock the behavior across all three constructors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/json/test-json_reader.os` around lines 265 - 333, The tests only exercise УстановитьСтроку but the PR changed initialization paths for file/stream entrypoints (ОткрытьФайл, ОткрытьПоток), so add parallel regression tests that mirror Тест_Должен_ОпределятьСтрокуСДатойВФорматеISOКакСтроку, Тест_Должен_ЧитатьСтрокуСДатойВФорматеISOКакСтроку and Тест_Должен_ЧитатьДатуВФорматеISOКакСтрокуПриИспользованииПрочитатьJSON but using ЧтениеJSON created via ОткрытьФайл(...) and ОткрытьПоток(...) (and their corresponding close semantics) to assert the same sequence of ТипТекущегоЗначения and that ТекущееЗначение/ПрочитатьJSON preserve ISO timestamps as strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/json/test-json_reader.os`:
- Around line 265-333: The tests only exercise УстановитьСтроку but the PR
changed initialization paths for file/stream entrypoints (ОткрытьФайл,
ОткрытьПоток), so add parallel regression tests that mirror
Тест_Должен_ОпределятьСтрокуСДатойВФорматеISOКакСтроку,
Тест_Должен_ЧитатьСтрокуСДатойВФорматеISOКакСтроку and
Тест_Должен_ЧитатьДатуВФорматеISOКакСтрокуПриИспользованииПрочитатьJSON but
using ЧтениеJSON created via ОткрытьФайл(...) and ОткрытьПоток(...) (and their
corresponding close semantics) to assert the same sequence of
ТипТекущегоЗначения and that ТекущееЗначение/ПрочитатьJSON preserve ISO
timestamps as strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbb85473-a4a9-40d4-bb39-90c64570a5c5
📒 Files selected for processing (2)
src/OneScript.StandardLibrary/Json/JSONReader.cstests/json/test-json_reader.os
leemuar
changed the title
… ОткрытьФайл() - по замечаниям coderabbit
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/json/test-json_reader.os`:
- Around line 359-375: В тестовой процедуре
Тест_Должен_ОткрытьФайлЧитаетСтрокуСДатойВФорматеISOКакСтроку() используется
переменная Чтение для создания экземпляра JSON-ридера, но далее вызываются
методы у неинициализированной переменной ЧтениеJSON; исправьте это, заменив все
вызовы ЧтениеJSON.Прочитать(), ЧтениеJSON.ТекущееЗначение,
ЧтениеJSON.ТипТекущегоЗначения и ЧтениеJSON.Закрыть() на эквивалентные вызовы
через Чтение (т.е. Чтение.Прочитать(), Чтение.ТекущееЗначение,
Чтение.ТипТекущегоЗначения, Чтение.Закрыть()) либо инициализируйте ЧтениеJSON =
Новый ЧтениеJSON перед использованием, если вы намеренно хотите использовать
переменную ЧтениеJSON.
- Around line 267-292: Тест
Тест_Должен_ОпределятьСтрокуСДатойВФорматеISOКакСтроку собирает ожидаемые и
фактические последовательности в ЭталонЗначения и ФактЗначения, но не проверяет
их длину, поэтому лишние токены пройдут незамеченными; добавьте в процедуру до
цикла сравнения явную проверку размера массивов (например через Колво() или
ВГраница+1) с использованием юТест.ПроверитьРавенство, чтобы убедиться, что
ФактЗначения и ЭталонЗначения имеют одинаковую длину перед посекторным
сравнением, затем оставить существующий цикл сравнения по индексам.
- Around line 333-357: The test currently opens a stream on the reader instance
named Чтение but then performs reads and assertions on a different reader
ЧтениеJSON, so ОткрытьПоток is never actually verified; fix by using the same
reader instance (Чтение) for
Прочитать/ТекущееЗначение/ТипТекущегоЗначения/Закрыть or remove the unused
ЧтениеJSON setup and instead call УстановитьСтроку only on Чтение if needed;
ensure the assertions reference Чтение and its ТипТекущегоЗначения so the
ОткрытьПоток call is actually exercised (look for symbols:
Тест_Должен_ОткрытьПотокЧитаетСтрокуСДатойВФорматеISOКакСтроку, Чтение,
ЧтениеJSON, ОткрытьПоток, УстановитьСтроку, Прочитать).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04826260-11e9-47e7-9062-eacb1d66c62e
📒 Files selected for processing (2)
tests/json/json-mock_isodate.jsontests/json/test-json_reader.os
✅ Files skipped from review due to trivial changes (1)
- tests/json/json-mock_isodate.json
Mr-Rm
reviewed
May 16, 2026
Comment on lines
+302
to
+303
| SupportMultipleContent = true | ||
| ,DateParseHandling = DateParseHandling.None |
Collaborator
There was a problem hiding this comment.
Обе эти инициализации во всех трех местах стоит перенести в конструктор JsonReaderInternal
Author
There was a problem hiding this comment.
Исправлено. Ошибки в тестах тоже исправлены
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Внутренняя реализация чтения json использовала популярную библиотеку работы с json от newtonsoft.
Эта библиотека по-умолчанию преобразует в тип Дата строки, содержащие дату в формате ISO - "2026-05-13T22:51:46.230Z". Из-за этого поведение объекта ЧтениеJSON и функии ПрочитатьJSON() отличалось от платформы 1С:
Возврат поведения к идиоматическому платформенному позволит обрабатывать json свободно, писать свои алгоритмы преобразования дат при использовании последовательного чтения с помощью ЧтениеJSON и пр. Сейчас это невозможно, точное значение даты может теряться при автоматической конвертации рантаймом.
Важно! Это может быть breaking change если какой-то код заложился на этот баг - когда строка с датой автоматически преобразуется в тип Дата функцией ПрочитатьJSON(). Но имхо "типовое" поведение важнее иметь
Summary by CodeRabbit
Bug Fixes
Tests