Значительные исправления ПрочитатьJSON (fix #1373 и прочее) #1374

Mr-Rm · 2023-10-30T12:56:33Z

Summary by CodeRabbit

Новые возможности
- Улучшена обработка ошибок при разборе JSON.
Улучшения
- Оптимизированы методы для эффективной обработки вложенных массивов и объектов в JSON.
Тесты
- Добавлены тесты для проверки обработки неопределённых значений в JSON.

Mr-Rm · 2023-10-31T20:58:15Z

Глобальный ПрочитатьJSON переделан практически полностью.
Обрабатываемые ситуации см. в тестах. Предыдущая версия падала на половине из них.
Изменения существенные, прошу review и тестирования.

EvilBeaver

Посмотрел в меру разумения

EvilBeaver · 2023-11-01T11:47:54Z

src/ScriptEngine.HostedScript/Library/Json/GlobalJsonFunctions.cs

                {
-                    return ReadJSONInStruct(Reader, true);
+                    throw new RuntimeException();


Наверное, стоило бы добавить какой-то текст здесь, и если такое не должно случаться никогда - сделать исключение InvalidOperationException или NotSupportedException

Не должно случаться никогда. Это бы при компиляции следовало проверять, но C# не позволяет.

Все равно лучше выкинуть более конкретное исключение, или сделать ассерт

EvilBeaver · 2023-11-01T11:49:25Z

src/ScriptEngine.HostedScript/Library/Json/GlobalJsonFunctions.cs

                }
-                return NestedArray;
+
+                throw InvalidJsonException();


Потеряли исходное исключение. Вероятно, лучше добавить cause, чтобы не терять стек и описание причины.

Сделано намеренно, в исключении из Newtonsoft.JSON кроме позиции будет более подробное описание причины, но английское. Можно восстановить и вставить в сообщение.

А чем InnerException не подходит?

EvilBeaver · 2023-11-01T11:54:57Z

src/ScriptEngine.HostedScript/Library/Json/GlobalJsonFunctions.cs

+                    case JsonToken.StartObject:
+                        var jsonObject = Create();
+
+                        while (ReadJsonToken() == JsonToken.PropertyName)


Если встретим комментарий, то ReadJsonToken вернет None и цикл чтения прервется. Например,

{ "Свойство1": 1, // пояснение для свойства 2 "Свойство2": "Значение" }

После считывания 1 мы найдем комментарий, вернем Nonе и цикл закончится, не дойдя до чтения "Свойство2", нет?

Нет, Nonе возвращается только когда _reader.Read() вернуло false. Комментарии не выходят за пределы ReadJsonToken()

Mr-Rm · 2023-11-01T19:04:23Z

Снова возник вопрос, поднятый в #1253 (comment) - про чтение нестандартного литерала undefined.
Предпочтительней было бы убрать.

Mr-Rm · 2023-11-01T19:43:10Z

Тесты на пустые значения в коммите 595c6bd дают примеры ошибочных json, которые будут успешно читаться. Используемая библиотека не разделяет "пустое" значение и литерал undefined перед запятой, а наоборот - преобразует первое во второе.

EvilBeaver · 2024-04-11T12:03:03Z

@Mr-Rm давай добьем может? Что тут подвисло у нас?

Mr-Rm · 2024-04-11T13:32:48Z

Сейчас остановлено на таком:

Комментарии и undefined считать расширениями (как уже считаются одинарные кавычки: они вне стандарта).
Обработку пустых значений - пока игнорировать, иначе надо влезать внутрь библиотеки.
После 15.04.

coderabbitai · 2024-04-17T13:13:48Z

Обзор изменений

Обновление включает в себя улучшение работы с JSON в скриптовом движке: введение нового класса для чтения JSON, улучшение обработки структур и карт, а также усиленная обработка ошибок при разборе JSON.

Изменения

Файлы	Описание изменений
GlobalJsonFunctions.cs, JSONReader.cs	Введение `JsonReaderInternal`, улучшение методов чтения JSON, усиленная обработка ошибок.
tests/global-json.os, test-json_reader.os	Добавление тестов для проверки новой функциональности и обработки ошибок.
tests/json/json-mock.json	Удаление некорректной пары ключ-значение.

Возможно связанные проблемы

Метод ПрочитатьJSON добавляет в возвращаемых данных (в массиве) закоментированные строки #1373: Изменения в методах чтения JSON могут помочь в исключении комментариев из возвращаемых массивов JSON, что соответствует одной из целей этой проблемы.

Recent Review Details

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 1ddd39a and 0a6db86.

Files selected for processing (5)

src/ScriptEngine.HostedScript/Library/Json/GlobalJsonFunctions.cs (4 hunks)
src/ScriptEngine.HostedScript/Library/Json/JSONReader.cs (12 hunks)
tests/global-json.os (1 hunks)
tests/json/json-mock.json (2 hunks)
tests/json/test-json_reader.os (3 hunks)

Files not reviewed due to errors (1)

GlobalJsonFunctions.cs (no review received)

Additional comments not posted (7)

tests/json/json-mock.json (1)

1-1: Удаление пары ключ-значение "Неопределено": undefined соответствует целям PR по обработке undefined как ошибки.

tests/json/test-json_reader.os (3)

20-20: Добавление нового теста Тест_ДолженВызватьОшибку_ДляUndefined для проверки обработки неопределенных значений соответствует целям PR.

62-62: Обновление значения хеш-суммы в тесте Тест_Должен_СверитьХешСуммуРезультатаПарсингаJSON соответствует изменениям в данных JSON, используемых для тестирования.

136-146: Процедура Тест_ДолженВызватьОшибку_ДляUndefined корректно реализована для проверки сценария с неопределенным значением, что соответствует целям PR.

src/ScriptEngine.HostedScript/Library/Json/JSONReader.cs (2)

15-28: Реализация нового внутреннего класса JsonReaderInternal, производного от JsonTextReader, корректно обрабатывает неожиданные символы, что улучшает обработку JSON.

182-238: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [39-343]

Изменения в классе JSONReader, включая использование JsonReaderInternal и добавление методов валидации, соответствуют целям PR по улучшению обработки JSON.

tests/global-json.os (1)

1-362: Добавление новых тестов и обновление существующих в файле global-json.os корректно реализованы для проверки улучшенной функциональности чтения JSON.

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

… объектами +тесты

Mr-Rm · 2024-04-17T13:56:35Z

Итак, пустые значения - считать ошибкой.
Литерал undefined - также считать ошибкой, поскольку

1С так же считает это ошибкой
undefined не входит в стандарт JSON
библиотека Newtonsoft.Json не позволяет отделить undefined от пустых значений

…чений

Mr-Rm · 2024-04-19T11:32:22Z

Неправильно работает ЧтениеJSON.Пропустить(). Но это, наверное, в отдельный PR.

EvilBeaver · 2024-04-20T05:35:13Z

@coderabbitai review

EvilBeaver · 2024-04-20T13:24:39Z

Неправильно работает ЧтениеJSON.Пропустить(). Но это, наверное, в отдельный PR.

Давай тогда issue на это сформулируем, чтобы не забыть?

Mr-Rm changed the title ~~[WIP] Исправления ПрочитатьJSON (fix #1373)~~ Значительные исправления ПрочитатьJSON (fix #1373 и прочее) Oct 31, 2023

EvilBeaver reviewed Nov 1, 2023

View reviewed changes

Mr-Rm marked this pull request as draft November 1, 2023 18:48

Mr-Rm added 5 commits April 17, 2024 17:21

Тесты для ПрочитатьJSON к EvilBeaver#1373

6bc8225

Еще тесты для ПрочитатьJSON

34877a8

Полностью переделан ПрочитатьJSON по issue EvilBeaver#1373 и работе с…

f0743df

… объектами +тесты

Уточнение исключений; ограничение типов

a9195f7

Тесты на пустые значения в json

4165865

Mr-Rm force-pushed the fix/readjson branch from 81ea9f1 to 4165865 Compare April 17, 2024 13:47

Считать ошибкой пустые значения и undefined в обход Newtonsoft.Json

2ce5f3a

Mr-Rm added 3 commits April 18, 2024 13:23

проверка, что json открыт - в отдельной функции; добавлен класс исклю…

fccaa14

…чений

Уточнены сообщения о неверных данных в json

8289191

Чтение JsonToken.Undefined сразу выбрасывает исключение

0a6db86

Mr-Rm marked this pull request as ready for review April 19, 2024 11:32

EvilBeaver merged commit 17c6a61 into EvilBeaver:latest Apr 20, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Значительные исправления ПрочитатьJSON (fix #1373 и прочее) #1374

Значительные исправления ПрочитатьJSON (fix #1373 и прочее) #1374

Mr-Rm commented Oct 30, 2023 •

edited by coderabbitai bot

Loading

Mr-Rm commented Oct 31, 2023

EvilBeaver left a comment

EvilBeaver Nov 1, 2023

Mr-Rm Nov 1, 2023

EvilBeaver Nov 1, 2023

EvilBeaver Nov 1, 2023

Mr-Rm Nov 1, 2023

EvilBeaver Nov 1, 2023

EvilBeaver Nov 1, 2023

Mr-Rm Nov 1, 2023

Mr-Rm commented Nov 1, 2023

Mr-Rm commented Nov 1, 2023

EvilBeaver commented Apr 11, 2024

Mr-Rm commented Apr 11, 2024

coderabbitai bot commented Apr 17, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

Mr-Rm commented Apr 17, 2024

Mr-Rm commented Apr 19, 2024

EvilBeaver commented Apr 20, 2024

EvilBeaver commented Apr 20, 2024

Значительные исправления ПрочитатьJSON (fix #1373 и прочее) #1374

Значительные исправления ПрочитатьJSON (fix #1373 и прочее) #1374

Conversation

Mr-Rm commented Oct 30, 2023 • edited by coderabbitai bot Loading

Summary by CodeRabbit

Mr-Rm commented Oct 31, 2023

EvilBeaver left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mr-Rm commented Nov 1, 2023

Mr-Rm commented Nov 1, 2023

EvilBeaver commented Apr 11, 2024

Mr-Rm commented Apr 11, 2024

coderabbitai bot commented Apr 17, 2024 • edited Loading

Обзор изменений

Изменения

Возможно связанные проблемы

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

Mr-Rm commented Apr 17, 2024

Mr-Rm commented Apr 19, 2024

EvilBeaver commented Apr 20, 2024

EvilBeaver commented Apr 20, 2024

Mr-Rm commented Oct 30, 2023 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 17, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)