Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ошибка при рефлексии свойств у загруженного с контекстом сценария #1424

Open
sfaqer opened this issue Jul 16, 2024 · 21 comments

Comments

@sfaqer
Copy link
Contributor

sfaqer commented Jul 16, 2024

Опишите ошибку
В случае если в сценарий, который был загружен через ЗагрузитьСценарий() с передачей во втором параметре контекст, попробую себе отрефлексировать таблицу свойств через ЭтотОбъект, будет получена ошибка:

Внешнее исключение (System.ArgumentOutOfRangeException): Индекс за пределами диапазона. Индекс должен быть положительным числом, а его размер не должен превышать размер коллекции.
Имя параметра: index}

Воспроизведение ошибки

// test.os

ЗагрузитьСценарий("./Загружаемый.os"); // Ок

Контекст = Новый Структура(
	"АргументыКоманднойСтроки",
	Новый Массив
); 

ЗагрузитьСценарий("./Загружаемый.os", Контекст); // Ошибка
// Загружаемый.os

Рефлектор = Новый Рефлектор();
Рефлектор.ПолучитьТаблицуСвойств(
	ЭтотОбъект
);

Сообщить("Ок");
oscript test.os

image

Ожидаемое поведение
В ошибку не падает

Окружение

  • ОС: Win11 23H2
  • Версия: 1.9.1.80

Дополнительная информация
OsReflectorBug.zip

@sfaqer
Copy link
Contributor Author

sfaqer commented Jul 16, 2024

/CC @EvilBeaver @Mr-Rm

@EvilBeaver
Copy link
Owner

Кажется, таких извращений рефлектор не предусматривает by design. Ну т.е. примерно понятно где ошибка, такое просто не предусмотрели.

@sfaqer
Copy link
Contributor Author

sfaqer commented Jul 16, 2024

Кажется, таких извращений рефлектор не предусматривает by design. Ну т.е. примерно понятно где ошибка, такое просто не предусмотрели.

Дык в 1.8 работало, в 1.9 сломалось

@EvilBeaver
Copy link
Owner

Даже так? Спасибо, посмотрю (но не знаю когда)

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 16, 2024

Падает на

var variable = userScript.Module.Variables[i - 1];

Сломалось в 1.8.1

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 18, 2024

К @Absolemus по PR #1242
Верно ли, что Рефлектор.ПолучитьТаблицуСвойств(ЭтотОбъект,Истина); должен получать то же, что и ПолучитьТаблицуСвойств(ЭтотОбъект, Ложь), включая переданный контекст, плюс глобальные неэкспортные переменные модуля? А неглобальные?

@nixel2007
Copy link
Collaborator

Что-то я не помню, чтобы переданный контекст когда-то отдавался рефлектором (и не думаю, что должен). Это же просто символы, доступные для обращения, как имя модуля.

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 18, 2024

По описанию метода "ЗагрузитьСценарий", да и фактически, передаваемый контекст - это полноценный набор глобальных переменных, экспортируемых в вызывающий модуль:

Контекст = Новый Структура("Арг", 1);
ЗагрузитьСценарийИзСтроки("Арг=2;", Контекст);
Сообщить("Арг="+Контекст.Арг);

Арг=2

И это несмотря на возникающую ошибку:
{Модуль <string 8A9886C3> / Ошибка в строке: 1 / Свойство недоступно для записи}
Баг?

@EvilBeaver
Copy link
Owner

К @Absolemus по PR #1242 Верно ли, что Рефлектор.ПолучитьТаблицуСвойств(ЭтотОбъект,Истина); должен получать то же, что и ПолучитьТаблицуСвойств(ЭтотОбъект, Ложь), включая переданный контекст, плюс глобальные неэкспортные переменные модуля? А неглобальные?

По задумке - должен. Контекст это внешние видимые части объекта, как реквизиты в модуле документа, например.

@sfaqer
Copy link
Contributor Author

sfaqer commented Jul 19, 2024

По задумке - должен. Контекст это внешние видимые части объекта, как реквизиты в модуле документа, например.

А это разве не "глобальный контекст" типа как "АргументыКоманднойСтроки" ?

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 19, 2024

Передаваемый контекст перекрывает глобальный:

Контекст = Новый Структура("АргументыКоманднойСтроки", "Перекрыто!");
ЗагрузитьСценарийИзСтроки("Cообщить(АргументыКоманднойСтроки)", Контекст);

Перекрыто!

То же для , например Консоль. Но:

Контекст = Новый Структура("ЭтотОбъект", "Перекрыто!");
ЗагрузитьСценарийИзСтроки("Cообщить(ЭтотОбъект)", Контекст);

{Модуль C:_lab\OneScript\OsReflectorBug\test.os / Ошибка в строке: 2 / Внешнее исключение (System.InvalidOperationException): Symbol already defined in the scope (ЭтотОбъект)}
ЗагрузитьСценарийИзСтроки("Cообщить(ЭтотОбъект)", Контекст);

Замеченный выше баг со "Свойство недоступно для записи" воспроизвести не удалось. Значит, переданные через контекст свойства - всё же строго r/o (так ли задумывалось?)

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 19, 2024

Странное с Рефлектором:

Ст = Новый Структура("Поле1, Поле2", -1, -2);
ТСв=Рефлектор.ПолучитьТаблицуСвойств(Ст);
Сообщить("Т0:"+ТСв.Количество());
ТСв=Рефлектор.ПолучитьТаблицуСвойств(Ст, Истина); // <--
Сообщить("Т1:"+ТСв.Количество());

Т0:2
Т1:0
// ??
GetPropertiesWithPrivate и GetPropertiesWithoutPrivate сделаны существенно по-разному

@EvilBeaver
Copy link
Owner

EvilBeaver commented Jul 21, 2024

Баг?

Нет. Передаваемый контекст доступен только для чтения, так и задумано.

С перекрытием то же самое, перекрывать должен, как перекрывает Перем в методе

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 22, 2024

Сейчас получается, что Рефлектор.ПолучитьТаблицуСвойств(..., Истина); возвращает только приватные свойства, а не все, включая приватные. Верно ли это? Изменение может затронуть библиотеки autumn и decorator, возможно и другие.
Исключение при попытке перекрытия ЭтотОбъект нужно более информативное.
В таблицу свойств не передаётся признак свойства о возможности записи.

Это для v1. В v2 всё реализовано и работает по-другому, в т. ч. "System.NotImplementedException" в ряде случаев

@EvilBeaver
Copy link
Owner

@nixel2007 @sfaqer вы основные клиенты рефлектора, подскажите, как правильно? Я полагал, что "все, включая приватные"

@EvilBeaver
Copy link
Owner

GetPropertiesWithPrivate и GetPropertiesWithoutPrivate сделаны существенно по-разному

@Mr-Rm там ужас что и давно бы пора навести там порядок со свойствами, полями, экспортными неэкспортными и тем, что выставляется в контекст.

@sfaqer
Copy link
Contributor Author

sfaqer commented Jul 24, 2024

@nixel2007 @sfaqer вы основные клиенты рефлектора, подскажите, как правильно? Я полагал, что "все, включая приватные"

Полагаю так же, "все включая приватные", ей богу я был уверен что оно так и работает.

@EvilBeaver
Copy link
Owner

Полагаю так же, "все включая приватные", ей богу я был уверен что оно так и работает.

Оно даже тестами вроде как покрыто....

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Jul 25, 2024

Тесты покрывают не все типы объектов, а сосредоточены, в основном, на Сценариях - там всё работает.
Можно, конечно, ограничить применение Рефлектора только Сценариями. Аннотаций у свойств/методов других типов не бывает.

@Mr-Rm
Copy link
Collaborator

Mr-Rm commented Aug 9, 2024

Отмечу здесь

Ст = Новый Структура("Поле1, Поле2", -1, -2);
ТСв=Рефлектор.ПолучитьТаблицуСвойств(Ст);
Сообщить("КСт="+ТСв.Количество());

В v1 КСт=2, v2 падает с NotImplemented

ФСт = Новый ФиксированнаяСтруктура("Поле1, Поле2", -1, -2);
ТСв=Рефлектор.ПолучитьТаблицуСвойств(ФСт);
Сообщить("КФСт="+ТСв.Количество());

Обе версии показывают КФСт=0, всегда. Причина - у ФиксированнаяСтруктура нет переопределения функции GetPropCount(), а из базового DynamicPropertiesAccessor приходит 0.
(Посмотреть в других объектах)

@nixel2007
Copy link
Collaborator

Я от флага "включая приватные" ожидаю, что будут возвращены все, включая приватные :)
В плане либ - практически все высокоуровневые либы используют Хоревский reflector, так что главное, чтобы он работал. Но выпустить фикс, конечно же, не проблема, главное понять, что там сломается. Тесты у либы тоже в избытке.

@EvilBeaver EvilBeaver added this to the Version 2.0.0 milestone Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants