Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughRefactors event-handler and parameter-source APIs to object/string forms, adds an isolated multi-environment launcher and console reporter, converts parameter fields to accessor methods, renames core APIs (Класс→Бенчмарк), changes DTO/serialization shapes, adds FS/runtime locator utilities, and updates tests, fixtures and docs. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as КомандаЗапуска
participant Runner as ЗапускательБенчмарков
participant Isolated as ИзолированныйЗапускательБенчмарков
participant Proc as ProcessManager
participant Env as TargetEnv
participant Serializer as СериализаторРезультатов
participant Reporter as КонсольныйРепортер
User->>CLI: Запустить команду
CLI->>Runner: ЗапуститьБенчмарки()
alt single env
Runner->>Runner: Выполнить локально
Runner->>Reporter: ВывестиРезультатВыполнения(Результат)
else multi env
Runner->>Isolated: Запустить()
Isolated->>Isolated: Подготовить конфигурации/скрипты
loop для каждой среды
Isolated->>Proc: ПодготовитьПроцесс(конфиг)
Proc->>Env: Запустить(процесс)
Env-->>Proc: stdout/stderr, exit
Proc->>Isolated: ОжидатьЗавершения/СобратьАртефакты
Isolated->>Serializer: ПрочитатьРезультатЗапускаИзJson(артефакт)
Serializer-->>Isolated: РезультатЗапускаБенчмарков
end
Isolated-->>Runner: РезультатЗапуска
Runner->>Reporter: ВывестиРезультатВыполнения(Результат)
end
Reporter->>User: Форматированный вывод
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/BenchmarkOneScript/core/Модули/КомпараторОбъектов.os (1)
51-66:⚠️ Potential issue | 🟡 MinorAdd key existence check before accessing Соответствие2.
When both Соответствие objects have the same count but different keys, accessing a missing key returns
Неопределеноby default, causing false positives. For example,{"A": 1, "B": Неопределено}vs{"A": 1, "C": 2}would incorrectly returnИстинаbecause the missing key "B" in Соответствие2 returnsНеопределено, matching the value in Соответствие1.Use the
СодержитКлюч()method to verify key existence before comparing values, similar to howСтруктурыРавныusesСвойство(). Also, line 59 should consistently useКлючinstead of mixing it withКлючЗначение.Ключ.Proposed fix
Функция СоответствияРавны(Соответствие1, Соответствие2) Если Соответствие1.Количество() <> Соответствие2.Количество() Тогда Возврат Ложь; КонецЕсли; Для Каждого КлючЗначение Из Соответствие1 Цикл Ключ = КлючЗначение.Ключ; + Если Не Соответствие2.СодержитКлюч(Ключ) Тогда + Возврат Ложь; + КонецЕсли; - Если Не Равны(Соответствие1[Ключ], Соответствие2[КлючЗначение.Ключ]) Тогда + Если Не Равны(КлючЗначение.Значение, Соответствие2[Ключ]) Тогда Возврат Ложь; КонецЕсли; КонецЦикла; Возврат Истина; КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Модули/КомпараторОбъектов.os` around lines 51 - 66, The function СоответствияРавны currently assumes keys from Соответствие1 exist in Соответствие2 and mixes Ключ and КлючЗначение.Ключ; update СоответствияРавны to first check Соответствие2.СодержитКлюч(Ключ) for each Ключ extracted from the iteration (use Ключ consistently), and only then call Равны(Соответствие1[Ключ], Соответствие2[Ключ]); if the key is missing return Ложь; keep the existing count check and final return Истина.src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os (1)
162-169:⚠️ Potential issue | 🔴 CriticalRemove the unused
ЕстьЭталоны()function.This function is not called anywhere in the codebase. It should be removed unless it's intended as a public API for external consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os` around lines 162 - 169, The function ЕстьЭталоны() is unused and should be removed: delete the entire function definition (including its body that references ЕстьЭталоннаяВерсия, ЕстьЭталонныеБенчмарки, calls to _Конфигурация.ЭталоннаяВерсияИсполняющейСреды() and _ДескрипторыБенчмарков.ПолучитьЭталонные().Количество()) from the class to clean up dead code; if this was intended as a public API, instead add a comment or export and write tests, otherwise remove the function and any related unused local symbols.
🧹 Nitpick comments (4)
tests/fixtures/handlers/Модули/ОбработчикиСобытийБенчмарков.os (1)
28-47: LGTM!The
УстановкаМокСредыОкруженияprocedure sets up a complete mock environment for testing. The processor info matches the expected output inverified-report.md.Minor: Line 37 has extra whitespace before the frequency value.
🧹 Optional cleanup
- Контекст.СредаОкружения.ИнформацияОПроцессоре.НоминальнаяЧастота = 3100000000; + Контекст.СредаОкружения.ИнформацияОПроцессоре.НоминальнаяЧастота = 3100000000;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/fixtures/handlers/Модули/ОбработчикиСобытийБенчмарков.os` around lines 28 - 47, Remove the stray extra whitespace before the numeric literal in the УстановкаМокСредыОкружения procedure: locate the assignment to Контекст.СредаОкружения.ИнформацияОПроцессоре.НоминальнаяЧастота and change the spacing so it reads with a single space around the equals sign (no double space before 3100000000).tests/ТестыИсточниковПараметров.os (1)
143-148: Добавьте явную проверку количества параметров в запуске.Сейчас используется индексный доступ к
Параметры[0]. Лучше отдельно зафиксировать ожидаемую длину, чтобы тест ловил лишние/неожиданные параметры.Предложение
Ожидаем.Что(Результат.Запуски).ИмеетДлину(1); + Ожидаем.Что(Результат.Запуски[0].Параметры).ИмеетДлину(1); Значение = Результат.Запуски[0].Параметры[0].Значение(); Ожидаем.Что(Значение).ИмеетТип("Структура");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсточниковПараметров.os` around lines 143 - 148, Тест не проверяет количество параметров в запуске — добавьте явную проверку длины массива Параметры перед доступом по индексу: убедитесь, что Результат.Запуски имеет длину 1 (уже проверяется) и что Результат.Запуски[0].Параметры.ИмеетДлину(1) (или нужное ожидаемое число), затем доставайте Параметры[0].Значение() и проверяйте его поля (Значение.Парам1, Значение.Парам2); используйте те же идентификаторы Результат, Запуски и Параметры из текущего теста.src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os (1)
51-55: Проверьте существование каталога перед кешированием результатаovm which.Сейчас достаточно только
ЗначениеЗаполнено(...). Если выводovm whichнеожиданного формата, в кеш попадет несуществующий путь и последующие поиски будут тихо возвращатьНеопределено.♻️ Proposed refactor
- Если ЗначениеЗаполнено(КаталогУстановки) Тогда + Если ЗначениеЗаполнено(КаталогУстановки) И ФС.КаталогСуществует(КаталогУстановки) Тогда _КаталогУстановки = КаталогУстановки; Возврат КаталогУстановки; КонецЕсли;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os` around lines 51 - 55, Проверьте реальное существование пути перед кешированием результата вызова ОпределитьКаталогУстановкиЧерезWhich(): вместо только ЗначениеЗаполнено(КаталогУстановки) выполните проверку, что каталог физически существует (например через функцию проверки существования каталога), и только после подтверждения присваивайте _КаталогУстановки = КаталогУстановки и выполняйте Возврат КаталогУстановки; если путь не существует — не кешировать результат и продолжить поиск или вернуть Неопределено.tests/ТестыКонфигурации.os (1)
820-828: Consider adding error handling for missing log file.The
ПрочитатьСобытияИзЛогаfunction assumes the file exists. If a test fails before writing the log, this could throw a confusing error.Proposed improvement
Функция ПрочитатьСобытияИзЛога(ПутьКФайлу) + Если Не (Новый Файл(ПутьКФайлу)).Существует() Тогда + Возврат Новый Массив(); + КонецЕсли; + ЧтениеТекста = Новый ЧтениеТекста(ПутьКФайлу); ТекстЛога = ЧтениеТекста.Прочитать(); ЧтениеТекста.Закрыть();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыКонфигурации.os` around lines 820 - 828, Функция ПрочитатьСобытияИзЛога предполагает наличие файла и не обрабатывает ошибку при его отсутствии — обнови функцию: перед созданием ЧтениеТекста проверить наличие файла (или обернуть создание/Прочитать/Закрыть в обработчик исключений), при отсутствии файла или ошибке чтения вернуть пустой массив (или понятное значение ошибки) вместо проброса исключения; ссылайся на символы ПрочитатьСобытияИзЛога, ЧтениеТекста, Прочитать, Закрыть и СтрРазделить при внесении правок.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ОбработчикиСобытий.md`:
- Around line 70-74: Update the two inline comments above the examples that
register handlers to use the grammatically correct form "расположенного": change
the comment before Конфигурация.ДобавитьОбработчикСобытия(Новый
ОбработчикСобытияБенчмарка("ВыполнитьПослеВсех", СобытияБенчмарков.ПослеВсех));
and the comment before Конфигурация.ДобавитьОбработчикСобытия(Новый
ОбработчикСобытияБенчмарка("МодульОбработчиков.ВыполнитьПослеВсех",
СобытияБенчмарков.ПослеВсех)); so they read "Регистрация обработчика,
расположенного в классе бенчмарка" and "Регистрация обработчика, расположенного
в другом модуле/классе" respectively.
In `@samples/api/УстановкаИсточникаПараметровПример.os`:
- Line 14: Fix the typo "существуюие" → "существующие" in the inline comment
following the .ОчиститьИсточникиПараметров() call; update that comment text and
search for other occurrences of the same misspelling in the file (and replace
them) so all comments read "Очищаем существующие источники параметров".
- Line 8: The inline comment next to the .ОчиститьИсточникиПараметров() call
contains a typo ("существуюие"); update the comment text to the correct spelling
"существующие" so it reads "// Очищаем существующие источники параметров"
preserving the existing comment style and placement adjacent to the
.ОчиститьИсточникиПараметров() invocation.
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 254-256: В коде в классе/файле
ИзолированныйЗапускательБенчмарков.os переменная объявлена и используется с
опечаткой: "ИсходноеЗначеие" — нужно переименовать её в "ИсходноеЗначение" в
месте присвоения (ИсходноеЗначеие = Настройки[Ключ]) и во всех последующих
ссылках (в проверке ЗначениеЗаполнено(...) и при формировании НовоеЗначение =
ИсходноеЗначеие + Разделитель + Значение), чтобы имена совпадали и не было
ошибки из‑за неправильного идентификатора.
- Around line 167-181: В процедуре ОжидатьЗавершениеПроцесса неверно рассчитана
единица времени: ТекущаяУниверсальнаяДата() возвращает время в секундах, а код
делит разницу на МиллисекундВМинуте=60000, из‑за чего таймаут не срабатывает;
исправьте вычисление прошедших минут в теле процедуры (например, оставить
ВремяНачала/ТекущаяУниверсальнаяДата() в секундах и использовать
СекундВМинуте=60 или явно преобразовать в миллисекунды:
(ТекущаяУниверсальнаяДата() - ВремяНачала)*1000 / 60000), обновив
соответствующие переменные (МиллисекундВМинуте или ввести СекундВМинуте) и
сохранив логику проверки с ТаймаутМинут и вызовом Процесс.Завершить() и
ВызватьИсключение; затронутые символы: ОжидатьЗавершениеПроцесса,
ТекущаяУниверсальнаяДата, МиллисекундВМинуте, ТаймаутМинут,
ПроверитьЗавершениеПроцесса.
- Around line 404-429: В функции НайтиПоследнююОшибку защитить доступ к
_ЖурналСообщенийПроцесса от выхода за границы: перед циклом вычислить реальное
число итераций как минимум(КоличествоПроверяемыхСообщений, КоличествоСообщений)
или внутри цикла проверять что (КоличествоСообщений - ИндексСообщения) >= 1 и
прерывать цикл, если сообщений меньше; примените это к использованию
ИндексСообщения и доступа _ЖурналСообщенийПроцесса[КоличествоСообщений -
ИндексСообщения], чтобы избежать обращения к несуществующим элементам.
- Around line 388-401: ПрочитатьРезультатЗапускаБенчмарковИзJSON может вернуть
Неопределено; добавьте проверку результата в функцию ЗапуститьВИсполняющейСреде
перед доступом к свойствам .СредаОкружения и .Запуски: если результат ===
Неопределено — либо верните безопасное значение/ошибку/лог с понятным
сообщением, либо инициируйте альтернативный путь выполнения (например, пропуск
запуска), чтобы избежать разыменования Неопределено; используйте явную проверку
результата от ПрочитатьРезультатЗапускаБенчмарковИзJSON и скорректируйте
поведение вызывающего кода (тот, что читает .СредаОкружения и .Запуски)
соответственно.
In `@src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os`:
- Around line 74-80: The ПолучитьМодуль function currently calls
Вычислить(ИмяМодуля) without sanitizing user input; add explicit validation of
ИмяМодуля (used from ОбработчикСобытияБенчмарка._КвалифицированноеИмяМетода
populated via ПрочитатьИзJson) to ensure it matches a safe qualified identifier
pattern (e.g., module[.submodule]*:identifier or module.method format as your
project expects) before invoking Вычислить; if validation fails, return
Неопределено (or log and return) instead of calling Вычислить so arbitrary
expressions cannot be executed.
In `@src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os`:
- Around line 60-62: Проверка кэша возвращает _Делегат, даже если он ещё не
инициализирован, из‑за чего при первом вызове Делегат() с Объект = Неопределено
могут вернуть Неопределено; исправьте условие в методе (где используются
переменные _Объект и _Делегат, например в классе ОбработчикСобытияБенчмарка) на
возврат только когда _Объект = Объект И _Делегат <> Неопределено; в противном
случае вызовите/инициализируйте Делегат() и присвойте _Делегат и _Объект перед
возвратом, чтобы гарантировать, что возвращаемое значение всегда
инициализировано.
In `@src/BenchmarkOneScript/core/Классы/ПостроительОтчетаБенчмарков.os`:
- Around line 417-419: При добавлении сортировки сейчас не фильтруются
непроимитивные типы параметра, из‑за чего ДобавитьСортировку(Сортировка,
Колонка.Имя) получает значения вроде ТаблицаЗначений/ЧтениеXML/объект и вызывает
ошибку; исправь это, добавив проверку примитивности перед вызовом: вместо только
Проверки ЗначениеЗаполнено(ИмяПараметра) проверяй ТипЗнч(ИмяПараметра) и
разрешай добавление сортировки только для примитивных типов (Число, Строка,
Дата, Булево и т.д.), либо вынеси проверку в вспомогательную функцию (например,
IsPrimitiveValue) и используй её при принятии решения о вызове
ДобавитьСортировку.
In `@tests/helpers/Модули/ТестированиеБенчмарков.os`:
- Around line 47-50: В цикле разбор строки Версии использует жесткий разделитель
", " и потому при входе вида "dev,stable" отдаёт одну запись; замените вызов
СтрРазделить(Версии, ", ") на разбор по запятой с учётом возможных пробелов —
например СтрРазделить(Версии, ",") и затем для каждого элемента выполнить
Обрезать/Trim (переменная Версия) или использовать регулярный сплит по запятой и
пробелам, чтобы перед использованием в
ЛокаторИсполняющейСреды.НайтиИсполняемыйФайл(Версия) и присвоении ИмяФайла
убрать ведущие/концевые пробелы и пустые элементы.
---
Outside diff comments:
In `@src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os`:
- Around line 162-169: The function ЕстьЭталоны() is unused and should be
removed: delete the entire function definition (including its body that
references ЕстьЭталоннаяВерсия, ЕстьЭталонныеБенчмарки, calls to
_Конфигурация.ЭталоннаяВерсияИсполняющейСреды() and
_ДескрипторыБенчмарков.ПолучитьЭталонные().Количество()) from the class to clean
up dead code; if this was intended as a public API, instead add a comment or
export and write tests, otherwise remove the function and any related unused
local symbols.
In `@src/BenchmarkOneScript/core/Модули/КомпараторОбъектов.os`:
- Around line 51-66: The function СоответствияРавны currently assumes keys from
Соответствие1 exist in Соответствие2 and mixes Ключ and КлючЗначение.Ключ;
update СоответствияРавны to first check Соответствие2.СодержитКлюч(Ключ) for
each Ключ extracted from the iteration (use Ключ consistently), and only then
call Равны(Соответствие1[Ключ], Соответствие2[Ключ]); if the key is missing
return Ложь; keep the existing count check and final return Истина.
---
Nitpick comments:
In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os`:
- Around line 51-55: Проверьте реальное существование пути перед кешированием
результата вызова ОпределитьКаталогУстановкиЧерезWhich(): вместо только
ЗначениеЗаполнено(КаталогУстановки) выполните проверку, что каталог физически
существует (например через функцию проверки существования каталога), и только
после подтверждения присваивайте _КаталогУстановки = КаталогУстановки и
выполняйте Возврат КаталогУстановки; если путь не существует — не кешировать
результат и продолжить поиск или вернуть Неопределено.
In `@tests/fixtures/handlers/Модули/ОбработчикиСобытийБенчмарков.os`:
- Around line 28-47: Remove the stray extra whitespace before the numeric
literal in the УстановкаМокСредыОкружения procedure: locate the assignment to
Контекст.СредаОкружения.ИнформацияОПроцессоре.НоминальнаяЧастота and change the
spacing so it reads with a single space around the equals sign (no double space
before 3100000000).
In `@tests/ТестыИсточниковПараметров.os`:
- Around line 143-148: Тест не проверяет количество параметров в запуске —
добавьте явную проверку длины массива Параметры перед доступом по индексу:
убедитесь, что Результат.Запуски имеет длину 1 (уже проверяется) и что
Результат.Запуски[0].Параметры.ИмеетДлину(1) (или нужное ожидаемое число), затем
доставайте Параметры[0].Значение() и проверяйте его поля (Значение.Парам1,
Значение.Парам2); используйте те же идентификаторы Результат, Запуски и
Параметры из текущего теста.
In `@tests/ТестыКонфигурации.os`:
- Around line 820-828: Функция ПрочитатьСобытияИзЛога предполагает наличие файла
и не обрабатывает ошибку при его отсутствии — обнови функцию: перед созданием
ЧтениеТекста проверить наличие файла (или обернуть создание/Прочитать/Закрыть в
обработчик исключений), при отсутствии файла или ошибке чтения вернуть пустой
массив (или понятное значение ошибки) вместо проброса исключения; ссылайся на
символы ПрочитатьСобытияИзЛога, ЧтениеТекста, Прочитать, Закрыть и СтрРазделить
при внесении правок.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (81)
.bsl-language-server.jsondocs/ОбработчикиСобытий.mddocs/Параметризация.mdpackagedefsamples/api/ДобавлениеОбработчиковСобытийПример.ossamples/api/УстановкаИсточникаПараметровПример.ossamples/benchmarks/Классы/БенчмаркИсточникПараметров.ossamples/paramsSources/Классы/КоллекцияМножителей.ossamples/paramsSources/Классы/ПараметрыБенчмаркаФакториала.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияИсточникПараметров.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияПараметры.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияЭталон.ossrc/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковПараметр.ossrc/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЗапускательБенчмарковИзКаталога.ossrc/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/КонсольныйРепортерБенчмарков.ossrc/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.ossrc/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.ossrc/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ПараметрБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ПараметрыМетодаБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ПостроительОтчетаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СредаОкруженияБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СтатистикаБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЭкспортерРезультатовБенчмарковВHtml.ossrc/BenchmarkOneScript/core/Классы/ЭкспортерРезультатовБенчмарковВJson.ossrc/BenchmarkOneScript/core/Классы/ЭкспортерРезультатовБенчмарковВMarkdown.ossrc/BenchmarkOneScript/core/Модули/Бенчмаркинг.ossrc/BenchmarkOneScript/core/Модули/ИзвлекательКонфигурацииБенчмарков.ossrc/BenchmarkOneScript/core/Модули/ИнформацияСредыБенчмарков.ossrc/BenchmarkOneScript/core/Модули/КомбинаторПараметровБенчмарка.ossrc/BenchmarkOneScript/core/Модули/КомпараторОбъектов.ossrc/BenchmarkOneScript/core/Модули/КонсольныйВыводБенчмаркера.ossrc/BenchmarkOneScript/core/Модули/РаботаСПутямиФайловойСистемы.ossrc/BenchmarkOneScript/dto/Классы/РезультатЗапускаБенчмаркаДто.ostasks/oscript.cfgtests/fixtures/benchmarks/Классы/БенчмаркБезБенчмарков.ostests/fixtures/benchmarks/Классы/БенчмаркБуферДвоичныхДанных.ostests/fixtures/benchmarks/Классы/БенчмаркСАннотациямиКонфигурации.ostests/fixtures/benchmarks/Классы/БенчмаркСИсточникамиПараметров.ostests/fixtures/benchmarks/Классы/БенчмаркСОбработчикамиСобытий.ostests/fixtures/benchmarks/Классы/БенчмаркСПараметрамиМетода.ostests/fixtures/benchmarks/Классы/БенчмаркСПараметрамиМетодаИПоля.ostests/fixtures/benchmarks/Классы/БенчмаркСПараметрамиПоля.ostests/fixtures/benchmarks/Классы/БенчмаркиДляЭкспорта.ostests/fixtures/benchmarks/Классы/БенчмаркиПоКатегориям.ostests/fixtures/benchmarks/Классы/БенчмаркиСЗадержкой.ostests/fixtures/benchmarks/Классы/БенчмаркиСКолонками.ostests/fixtures/benchmarks/Классы/БенчмаркиССортировкой.ostests/fixtures/benchmarks/Классы/БенчмаркиСЭталоном.ostests/fixtures/benchmarks/Классы/БенчмаркиСЭталономИПараметрами.ostests/fixtures/benchmarks/Классы/ПустойБенчмарк.ostests/fixtures/handlers/Модули/ОбработчикиСобытийБенчмарков.ostests/fixtures/paramsSources/Классы/КлассСИсточникамиПараметров.ostests/fixtures/paramsSources/Модули/МодульСИсточникамиПараметров.ostests/fixtures/verified-report.htmltests/fixtures/verified-report.jsontests/fixtures/verified-report.mdtests/fixtures/БенчмаркиСОшибкой/БенчмаркБезОшибки.ostests/fixtures/БенчмаркиСОшибкой/БенчмаркСОшибкой.ostests/fixtures/БенчмаркиЧерезКаталог/БенчмаркИзКаталога1.ostests/fixtures/БенчмаркиЧерезКаталог/БенчмаркИзКаталога2.ostests/fixtures/БенчмаркиЧерезКаталог/Вложенный каталог/БенчмаркИзВложенногоКаталога.ostests/fixtures/Классы/БенчмаркСПараметрамиМетодаИПоля.ostests/helpers/Модули/ТестированиеБенчмарков.ostests/ТестыБенчмарков.ostests/ТестыЗамеров.ostests/ТестыИсточниковПараметров.ostests/ТестыКонфигурации.ostests/ТестыСериализаторНастроекБенчмарков.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыЭкспортеров.ostests/ТестыЭталонов.os
💤 Files with no reviewable changes (5)
- src/BenchmarkOneScript/core/Модули/КонсольныйВыводБенчмаркера.os
- tests/fixtures/benchmarks/Классы/БенчмаркиСЭталоном.os
- tests/fixtures/Классы/БенчмаркСПараметрамиМетодаИПоля.os
- src/BenchmarkOneScript/core/Модули/ИнформацияСредыБенчмарков.os
- tests/fixtures/benchmarks/Классы/БенчмаркиПоКатегориям.os
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os
Show resolved
Hide resolved
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os
Outdated
Show resolved
Hide resolved
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os
Show resolved
Hide resolved
Comment on lines
74
to
80
| Функция ПолучитьМодуль(ИмяМодуля) | ||
|
|
||
| Попытка | ||
| Модуль = Вычислить(ИмяМодуля); | ||
| Исключение | ||
| Возврат Неопределено | ||
| КонецПопытки; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, let's check if the file exists and examine its context
echo "=== Examining ЛенивыйДелегат.os file ==="
if [ -f "src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os" ]; then
wc -l "src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os"
echo
echo "=== Lines 74-80 context (with surrounding lines) ==="
sed -n '65,90p' "src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os"
else
echo "File not found at expected path, searching..."
find . -name "ЛенивыйДелегат.os" -o -name "*Ленивый*"
fiRepository: Stivo182/BenchmarkOneScript
Length of output: 622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Where is ЛенивыйДелегат instantiated ==="
rg -n 'Новый\s+ЛенивыйДелегат' -A 3 -B 1
echo
echo "=== 2) Sources of handler/source strings ==="
rg -n 'Сериализуемое|ИсточникПараметров|ДобавитьОбработчикСобытия|ПрочитатьИзJson|ПрочитатьИз' -B 2 -A 2 | head -100
echo
echo "=== 3) Validation before Вычислить ==="
rg -n 'Вычислить\(' -B 5 -A 2
echo
echo "=== 4) Check for input validation patterns ==="
rg -n 'Регулярное\s*выражение|КорректноеИмя|ПроверитьФормат|ОчиститьСтроку|ПроверитьИдентификатор'Repository: Stivo182/BenchmarkOneScript
Length of output: 7665
🏁 Script executed:
#!/bin/bash
echo "=== Full ЛенивыйДелегат.os file ==="
cat -n "src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os"Repository: Stivo182/BenchmarkOneScript
Length of output: 3052
🏁 Script executed:
#!/bin/bash
echo "=== ЧитательПараметровБенчмарков constructor and _Источник source ==="
rg -n 'ЧитательПараметровБенчмарков|_Источник|_ОбъектБенчмарков' "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os" -A 3 -B 1 | head -120
echo
echo "=== ОбработчикСобытияБенчмарка constructor and _КвалифицированноеИмяМетода source ==="
rg -n 'ОбработчикСобытияБенчмарка|_КвалифицированноеИмяМетода' "src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os" -A 3 -B 1 | head -120Repository: Stivo182/BenchmarkOneScript
Length of output: 1248
🏁 Script executed:
#!/bin/bash
echo "=== Where is ОбработчикСобытияБенчмарка deserialized/instantiated ==="
rg -n 'ОбработчикСобытияБенчмарка' -B 2 -A 2
echo
echo "=== Check if _КвалифицированноеИмяМетода gets validated before ЛенивыйДелегат ==="
rg -n 'ДобавитьОбработчикСобытия|Обработчик.*Событ' -B 5 -A 5 | head -150Repository: Stivo182/BenchmarkOneScript
Length of output: 22126
🏁 Script executed:
#!/bin/bash
echo "=== Check if _Источник in ЧитательПараметровБенчмарков is validated ==="
rg -n '_Источник|Источник' "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os" -B 2 -A 2
echo
echo "=== Check for any validation utilities for qualified names ==="
rg -n 'Валид|Проверь|Format|Regexp' --type-list
fd -e os | xargs rg -l 'Валид|ПроверитьИмя|ПроверитьФормат'Repository: Stivo182/BenchmarkOneScript
Length of output: 1464
🏁 Script executed:
#!/bin/bash
echo "=== КонфигурацияБенчмарков serialization annotations ==="
sed -n '60,70p' "src/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.os"
echo
echo "=== Confirm ОбработчикСобытияБенчмарка serialization ==="
sed -n '1,10p' "src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os"
echo
echo "=== Check if JSON deserialization is actually used ==="
rg -n 'ПрочитатьИзJson|ДесериализовЗнач|ЗагрузитьКонфиг' | head -20Repository: Stivo182/BenchmarkOneScript
Length of output: 1610
Validate qualified method names before passing to Вычислить().
The ПолучитьМодуль() function passes ИмяМодуля directly to Вычислить() without validation. Since ОбработчикСобытияБенчмарка._КвалифицированноеИмяМетода is marked @Сериализуемое("Handler") and configuration is loaded from JSON via ПрочитатьИзJson(), user-provided strings can reach this function unevaluated. Arbitrary expressions would execute before the type check at line 82.
Add validation to ensure the input is a valid identifier (module/method format) before calling Вычислить().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os` around lines 74 - 80,
The ПолучитьМодуль function currently calls Вычислить(ИмяМодуля) without
sanitizing user input; add explicit validation of ИмяМодуля (used from
ОбработчикСобытияБенчмарка._КвалифицированноеИмяМетода populated via
ПрочитатьИзJson) to ensure it matches a safe qualified identifier pattern (e.g.,
module[.submodule]*:identifier or module.method format as your project expects)
before invoking Вычислить; if validation fails, return Неопределено (or log and
return) instead of calling Вычислить so arbitrary expressions cannot be
executed.
src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/ТестыКонфигурации.os (1)
821-829: Рассмотрите защиту от утечки ресурса при исключении.Если между открытием файла (строка 823) и закрытием (строка 825) произойдёт исключение, файловый дескриптор останется открытым. Для тестового кода это не критично, поскольку
МенеджерВременныхФайлов.Удалить()очистит файлы.♻️ Альтернативный вариант с обработкой исключений
Функция ПрочитатьСобытияИзЛога(ПутьКФайлу) ЧтениеТекста = Новый ЧтениеТекста(ПутьКФайлу); - ТекстЛога = ЧтениеТекста.Прочитать(); - ЧтениеТекста.Закрыть(); + Попытка + ТекстЛога = ЧтениеТекста.Прочитать(); + Исключение + ЧтениеТекста.Закрыть(); + ВызватьИсключение; + КонецПопытки; + ЧтениеТекста.Закрыть(); Возврат СтрРазделить(ТекстЛога, Символы.ПС, Ложь); КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыКонфигурации.os` around lines 821 - 829, В функции ПрочитатьСобытияИзЛога защитите ресурс ЧтениеТекста от утечки: после создания объекта ЧтениеТекста используйте блок try...finally (или эквивалентный конструкцию "Использовать" если доступна) чтобы гарантировать вызов ЧтениеТекста.Закрыть() в finally независимо от исключений, затем вернуть разбитый текст; ссылайтесь на создание ЧтениеТекста(ПутьКФайлу), вызов .Прочитать() и .Закрыть() для внесения правки.samples/api/УстановкаИсточникаПараметровПример.os (1)
13-15: Добавьте проверку результатаНайтиПоИмениперед вызовом цепочки.Если
"БенчмаркФакториала"не найден, следующий вызов методов упадет наНеопределено.Предложение
-ДескрипторБенчмарка = ДескрипторыБенчмарков.НайтиПоИмени("БенчмаркФакториала") - .ОчиститьИсточникиПараметров() // Очищаем существующие источники параметров - .ДобавитьИсточникПараметров("ПараметрыБенчмаркаФакториала.Получить"); +ДескрипторБенчмарка = ДескрипторыБенчмарков.НайтиПоИмени("БенчмаркФакториала"); +Если ДескрипторБенчмарка = Неопределено Тогда + ВызватьИсключение "Не найден дескриптор бенчмарка: БенчмаркФакториала"; +КонецЕсли; + +ДескрипторБенчмарка + .ОчиститьИсточникиПараметров() // Очищаем существующие источники параметров + .ДобавитьИсточникПараметров("ПараметрыБенчмаркаФакториала.Получить");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/api/УстановкаИсточникаПараметровПример.os` around lines 13 - 15, Проверьте результат вызова ДескрипторыБенчмарков.НайтиПоИмени("БенчмаркФакториала") и не вызывайте методы ОчиститьИсточникиПараметров/ДобавитьИсточникПараметров на Неопределено: присвойте результат в переменную (например, ДескрипторБенчмарка), выполните условную проверку на Неопределено и только в случае успеха вызывайте ДескрипторБенчмарка.ОчиститьИсточникиПараметров().ДобавитьИсточникПараметров(...); в ветке ошибки логгируйте или выбрасывайте понятное исключение/возвращайте ошибку, чтобы избежать падения на вызове методов у Неопределено.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 110-132: В функции ЗапуститьВИсполняющейСреде сбрасывайте буфер
логов процесса перед подготовкой/запуском новой исполняющей среды: очистите или
переинициализируйте поле _ЖурналСообщенийПроцесса (например присвоить новый
пустой список/строку/объект в зависимости от типа) сразу перед вызовом
ПодготовитьПроцесс(Конфигурация, ФайлИлиВерсия) или перед Процесс.Запустить(),
чтобы гарантировать, что сообщения от предыдущих запусков не попадут в
диагностику (НайтиПоследнююОшибку) текущего запуска.
- Around line 41-104: Оберните основную логику внутри функции Запустить() в один
верхнеуровневый блок Попытка...Исключение: внутри Исключение сразу вызывайте
УдалитьАртефакты(); ВызватьИсключение; чтобы очистка выполнялась при ошибках на
любой стадии (включая ОпределитьЭталоны() и Построитель.Сформировать()), а после
завершения блока Попытка (в нормальном пути) оставьте существующий вызов
УдалитьАртефакты() перед Возврат РезультатЗапуска; это гарантирует вызов
УдалитьАртефакты() и при исключениях, и при успешном завершении.
- Around line 150-153: СтрокаКоманды строится без кавычек вокруг
ПутьИсполняемогоФайла, что ломает CreateProcess на Windows для путей со
пробелами; оберните ПутьИсполняемогоФайла в вызов ОбернутьВКавычки перед
добавлением в массив СтрокаКоманды (т.е. использовать
ОбернутьВКавычки(ПутьИсполняемогоФайла)), затем объедините элементы как сейчас и
передайте результат в СоздатьПроцесс/функцию запуска, чтобы CreateProcess
получил корректно экранированный путь.
In `@src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os`:
- Line 3: Comment has a typo: change "Дейтсвие" to "Действие" in the comment for
the variable _Действие; locate the declaration of the variable named _Действие
and update its inline comment to read "Действие, Неопределено" (or the correct
Russian spelling) so the comment accurately reflects the variable name.
In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os`:
- Around line 70-93: В документации функции НайтиИсполняемыйФайлВКаталоге и в её
конце нужно явно указать, что функция может вернуть Неопределено и добавить
явный возврат Неопределено при отсутствии найденного файла; обновите блок
"Возвращаемое значение" чтобы перечислить "Строка | Неопределено" (или аналог) и
в теле функции после цикла добавить "Возврат Неопределено" для явного поведения,
проверяя существующие похожие функции (например, НайтиИсполняемыйФайл) на
предмет формулировки.
In `@src/BenchmarkOneScript/core/Классы/ПараметрБенчмарка.os`:
- Around line 54-56: The Представление() Экспорт currently returns only
_Представление which can be a non-string when object comes from old data; update
Представление() to check for a TextValue property (e.g., this.TextValue or
TextValue) and if present return it as a string, otherwise fall back to
returning _Представление coerced to a string (or "" if null/undefined) so the
method always fulfills its Строка contract; locate and modify the Представление
function and references to _Представление/TextValue to implement this safe
fallback.
---
Nitpick comments:
In `@samples/api/УстановкаИсточникаПараметровПример.os`:
- Around line 13-15: Проверьте результат вызова
ДескрипторыБенчмарков.НайтиПоИмени("БенчмаркФакториала") и не вызывайте методы
ОчиститьИсточникиПараметров/ДобавитьИсточникПараметров на Неопределено:
присвойте результат в переменную (например, ДескрипторБенчмарка), выполните
условную проверку на Неопределено и только в случае успеха вызывайте
ДескрипторБенчмарка.ОчиститьИсточникиПараметров().ДобавитьИсточникПараметров(...);
в ветке ошибки логгируйте или выбрасывайте понятное исключение/возвращайте
ошибку, чтобы избежать падения на вызове методов у Неопределено.
In `@tests/ТестыКонфигурации.os`:
- Around line 821-829: В функции ПрочитатьСобытияИзЛога защитите ресурс
ЧтениеТекста от утечки: после создания объекта ЧтениеТекста используйте блок
try...finally (или эквивалентный конструкцию "Использовать" если доступна) чтобы
гарантировать вызов ЧтениеТекста.Закрыть() в finally независимо от исключений,
затем вернуть разбитый текст; ссылайтесь на создание ЧтениеТекста(ПутьКФайлу),
вызов .Прочитать() и .Закрыть() для внесения правки.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/ОбработчикиСобытий.mdsamples/api/УстановкаИсточникаПараметровПример.ossrc/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.ossrc/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.ossrc/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ПараметрБенчмарка.ostests/ТестыИсточниковПараметров.ostests/ТестыКонфигурации.ostests/ТестыСериализаторНастроекБенчмарков.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыЭкспортеров.os
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/ТестыСериализаторРезультатовБенчмарков.os
- src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os
Show resolved
Hide resolved
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os
Show resolved
Hide resolved
Comment on lines
70
to
93
| // Находит исполняемый файл в каталоге. | ||
| // | ||
| // Параметры: | ||
| // Каталог - Строка - Каталог поиска | ||
| // | ||
| // Возвращаемое значение: | ||
| // Строка | ||
| Функция КаталогБиблиотек() Экспорт | ||
| Возврат ПолучитьЗначениеСистемнойНастройки("lib.system"); | ||
| КонецФункции | ||
| Функция НайтиИсполняемыйФайлВКаталоге(Каталог) Экспорт | ||
|
|
||
| ВариантыИмени = Новый Массив(); | ||
| ВариантыИмени.Добавить("oscript.exe"); | ||
| ВариантыИмени.Добавить("oscript"); | ||
|
|
||
| // Определяет, является ли текущая ОС Windows | ||
| // | ||
| // Возвращаемое значение: | ||
| // Булево | ||
| Функция ЭтоWindows() Экспорт | ||
| Для Каждого ИмяФайла Из ВариантыИмени Цикл | ||
|
|
||
| СистемнаяИнформация = Новый СистемнаяИнформация(); | ||
| ТекущаяПлатформа = СистемнаяИнформация.ТипПлатформы; | ||
| ПутьКФайлу = ОбъединитьПути(Каталог, ИмяФайла); | ||
|
|
||
| Возврат ТекущаяПлатформа = ТипПлатформы.Windows_x86_64 | ||
| Или ТекущаяПлатформа = ТипПлатформы.Windows_x86; | ||
| Если ФС.ФайлСуществует(ПутьКФайлу) Тогда | ||
| Возврат ПутьКФайлу; | ||
| КонецЕсли; | ||
|
|
||
| КонецЦикла; | ||
|
|
||
| КонецФункции |
There was a problem hiding this comment.
Documentation inconsistency: missing Неопределено return type.
The function returns Неопределено when no executable is found (implicit fall-through at line 93), but the documentation only lists Строка as the return type. For consistency with other functions in this file (e.g., НайтиИсполняемыйФайл at lines 14-15), update the documentation and add an explicit return.
📝 Proposed fix
// Находит исполняемый файл в каталоге.
//
// Параметры:
// Каталог - Строка - Каталог поиска
//
// Возвращаемое значение:
-// Строка
+// Строка - Полный путь к исполняемому файлу, если найден
+// Неопределено - Если исполняемый файл не найден в каталоге
Функция НайтиИсполняемыйФайлВКаталоге(Каталог) Экспорт
ВариантыИмени = Новый Массив();
ВариантыИмени.Добавить("oscript.exe");
ВариантыИмени.Добавить("oscript");
Для Каждого ИмяФайла Из ВариантыИмени Цикл
ПутьКФайлу = ОбъединитьПути(Каталог, ИмяФайла);
Если ФС.ФайлСуществует(ПутьКФайлу) Тогда
Возврат ПутьКФайлу;
КонецЕсли;
КонецЦикла;
+ Возврат Неопределено;
+
КонецФункции📝 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.
Suggested change
| // Находит исполняемый файл в каталоге. | |
| // | |
| // Параметры: | |
| // Каталог - Строка - Каталог поиска | |
| // | |
| // Возвращаемое значение: | |
| // Строка | |
| Функция КаталогБиблиотек() Экспорт | |
| Возврат ПолучитьЗначениеСистемнойНастройки("lib.system"); | |
| КонецФункции | |
| Функция НайтиИсполняемыйФайлВКаталоге(Каталог) Экспорт | |
| ВариантыИмени = Новый Массив(); | |
| ВариантыИмени.Добавить("oscript.exe"); | |
| ВариантыИмени.Добавить("oscript"); | |
| // Определяет, является ли текущая ОС Windows | |
| // | |
| // Возвращаемое значение: | |
| // Булево | |
| Функция ЭтоWindows() Экспорт | |
| Для Каждого ИмяФайла Из ВариантыИмени Цикл | |
| СистемнаяИнформация = Новый СистемнаяИнформация(); | |
| ТекущаяПлатформа = СистемнаяИнформация.ТипПлатформы; | |
| ПутьКФайлу = ОбъединитьПути(Каталог, ИмяФайла); | |
| Возврат ТекущаяПлатформа = ТипПлатформы.Windows_x86_64 | |
| Или ТекущаяПлатформа = ТипПлатформы.Windows_x86; | |
| Если ФС.ФайлСуществует(ПутьКФайлу) Тогда | |
| Возврат ПутьКФайлу; | |
| КонецЕсли; | |
| КонецЦикла; | |
| КонецФункции | |
| // Находит исполняемый файл в каталоге. | |
| // | |
| // Параметры: | |
| // Каталог - Строка - Каталог поиска | |
| // | |
| // Возвращаемое значение: | |
| // Строка - Полный путь к исполняемому файлу, если найден | |
| // Неопределено - Если исполняемый файл не найден в каталоге | |
| Функция НайтиИсполняемыйФайлВКаталоге(Каталог) Экспорт | |
| ВариантыИмени = Новый Массив(); | |
| ВариантыИмени.Добавить("oscript.exe"); | |
| ВариантыИмени.Добавить("oscript"); | |
| Для Каждого ИмяФайла Из ВариантыИмени Цикл | |
| ПутьКФайлу = ОбъединитьПути(Каталог, ИмяФайла); | |
| Если ФС.ФайлСуществует(ПутьКФайлу) Тогда | |
| Возврат ПутьКФайлу; | |
| КонецЕсли; | |
| КонецЦикла; | |
| Возврат Неопределено; | |
| КонецФункции |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os` around lines
70 - 93, В документации функции НайтиИсполняемыйФайлВКаталоге и в её конце нужно
явно указать, что функция может вернуть Неопределено и добавить явный возврат
Неопределено при отсутствии найденного файла; обновите блок "Возвращаемое
значение" чтобы перечислить "Строка | Неопределено" (или аналог) и в теле
функции после цикла добавить "Возврат Неопределено" для явного поведения,
проверяя существующие похожие функции (например, НайтиИсполняемыйФайл) на
предмет формулировки.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os (1)
63-65:⚠️ Potential issue | 🟠 MajorПроверьте существование каталога перед кэшированием результата
which.На Line 63-65 сейчас достаточно только
ЗначениеЗаполнено, поэтому в_КаталогУстановкиможет закэшироваться невалидный путь, и дальнейшие вызовы будут стабильно промахиваться.💡 Proposed fix
- Если ЗначениеЗаполнено(КаталогУстановки) Тогда + Если ЗначениеЗаполнено(КаталогУстановки) И ФС.КаталогСуществует(КаталогУстановки) Тогда _КаталогУстановки = КаталогУстановки; Возврат КаталогУстановки; КонецЕсли;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os` around lines 63 - 65, Проверьте существование каталога перед кэшированием: вместо лишь проверки ЗначениеЗаполнено(КаталогУстановки) добавьте проверку реального существования каталога (например, используя функцию КаталогСуществует или эквивалент ФС API) и только если каталог существует присвойте _КаталогУстановки = КаталогУстановки и верните значение; иначе не кэшируйте и продолжайте поиск/верните пустое значение. Убедитесь, что изменения касаются блока, где используются переменные КаталогУстановки и _КаталогУстановки.
♻️ Duplicate comments (3)
src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os (1)
75-94:⚠️ Potential issue | 🟡 MinorНужен явный
Возврат Неопределенов конце функции для консистентного контракта.Комментарий уже поднимался ранее и остаётся актуальным: блок "Возвращаемое значение" (Line 76-77) заявляет
Неопределено, но в конце функции после цикла его лучше вернуть явно.📝 Proposed fix
КонецЦикла; + + Возврат Неопределено; КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os` around lines 75 - 94, Функция НайтиИсполняемыйФайлВКаталоге сейчас возвращает путь внутри цикла, но неявно возвращает значение при отсутствии совпадений; добавьте в конец функции явный `Возврат Неопределено` чтобы соблюсти контракт, указаный в блоке "Возвращаемое значение" (после использования ВариантыИмени, ПутьКФайлу и проверки ФС.ФайлСуществует) — просто вставьте один возвращающий оператор перед `КонецФункции`.src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os (2)
112-116:⚠️ Potential issue | 🟡 MinorСбрасывайте
_ЖурналСообщенийПроцессаперед запуском каждой среды.Иначе сообщения предыдущей среды могут попадать в диагностику текущего падения и ухудшать точность
НайтиПоследнююОшибку().💡 Предлагаемая правка
Функция ЗапуститьВИсполняющейСреде(Конфигурация, ИсполняющаяСреда) КаталогАртефактов = Конфигурация.КаталогАртефактов(); ФС.ОбеспечитьПустойКаталог(КаталогАртефактов); + _ЖурналСообщенийПроцесса = Новый Массив();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 112 - 116, В функции ЗапуститьВИсполняющейСреде перед подготовкой артефактов явно сбрасывайте/очищайте переменную _ЖурналСообщенийПроцесса (например присвоением нового пустого журнала или вызовом метода очистки) чтобы удалить сообщения предыдущей среды и не допустить их попадания в результаты диагностики/работу НайтиПоследнююОшибку(); добавьте это очистное действие сразу в начале ЗапуститьВИсполняющейСреде (до обращения к КаталогАртефактов и ФС.ОбеспечитьПустойКаталог).
41-104:⚠️ Potential issue | 🟠 MajorГарантируйте удаление временных артефактов при любом исключении в
Запустить().Сейчас cleanup не покрывает исключения после цикла сред (например, на этапе формирования отчета), из-за чего временные файлы могут остаться.
💡 Предлагаемая правка
Функция Запустить() Экспорт _ЖурналСообщенийПроцесса.Очистить(); - - ПередаваемаяКонфигурация = _Конфигурация + Попытка + ПередаваемаяКонфигурация = _Конфигурация .Скопировать() .УстановитьКаталогАртефактов(_МенеджерВременныхФайлов.НовоеИмяФайла()) .УдалитьЭкспортеры() .ДобавитьЭкспортер(ЭкспортерыРезультатовБенчмарков.Json); @@ - УдалитьАртефакты(); - - Возврат РезультатЗапуска; + УдалитьАртефакты(); + Возврат РезультатЗапуска; + Исключение + УдалитьАртефакты(); + ВызватьИсключение; + КонецПопытки; КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 41 - 104, The Запустить() function currently calls УдалитьАртефакты() only on exceptions inside the environment loop, leaving temp artifacts if an exception occurs later (e.g. during Построитель.Сформировать()); wrap the main body of Запустить() in a try...finally (or ensure a finally-equivalent) and call УдалитьАртефакты() in the finally block so artifacts are removed on any exit path; keep existing calls inside the loop as-is or remove duplicates so УдалитьАртефакты() is guaranteed to run after the loop and after Построитель.Сформировать() regardless of exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os`:
- Around line 53-56: The code instantiates types and evaluates expressions using
an unchecked string originating from _КвалифицированноеИмяМетода (used in
Получить(), Новый(Имя) and Вычислить()), which is a trust boundary issue; fix by
validating _КвалифицированноеИмяМетода at the start of Получить() against a
strict OneScript identifier regex (for example: allow letters, digits,
underscores and optional dotted qualifiers like
^[А-Яа-яA-Za-z_][А-Яа-яA-Za-z0-9_]*(\.[А-Яа-яA-Za-z_][А-Яа-яA-Za-z0-9_]*)?$),
reject or throw on mismatch, and only then use the value in Новый(...) and
Вычислить() so both code paths are secured.
---
Outside diff comments:
In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os`:
- Around line 63-65: Проверьте существование каталога перед кэшированием: вместо
лишь проверки ЗначениеЗаполнено(КаталогУстановки) добавьте проверку реального
существования каталога (например, используя функцию КаталогСуществует или
эквивалент ФС API) и только если каталог существует присвойте _КаталогУстановки
= КаталогУстановки и верните значение; иначе не кэшируйте и продолжайте
поиск/верните пустое значение. Убедитесь, что изменения касаются блока, где
используются переменные КаталогУстановки и _КаталогУстановки.
---
Duplicate comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 112-116: В функции ЗапуститьВИсполняющейСреде перед подготовкой
артефактов явно сбрасывайте/очищайте переменную _ЖурналСообщенийПроцесса
(например присвоением нового пустого журнала или вызовом метода очистки) чтобы
удалить сообщения предыдущей среды и не допустить их попадания в результаты
диагностики/работу НайтиПоследнююОшибку(); добавьте это очистное действие сразу
в начале ЗапуститьВИсполняющейСреде (до обращения к КаталогАртефактов и
ФС.ОбеспечитьПустойКаталог).
- Around line 41-104: The Запустить() function currently calls
УдалитьАртефакты() only on exceptions inside the environment loop, leaving temp
artifacts if an exception occurs later (e.g. during Построитель.Сформировать());
wrap the main body of Запустить() in a try...finally (or ensure a
finally-equivalent) and call УдалитьАртефакты() in the finally block so
artifacts are removed on any exit path; keep existing calls inside the loop
as-is or remove duplicates so УдалитьАртефакты() is guaranteed to run after the
loop and after Построитель.Сформировать() regardless of exceptions.
In `@src/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os`:
- Around line 75-94: Функция НайтиИсполняемыйФайлВКаталоге сейчас возвращает
путь внутри цикла, но неявно возвращает значение при отсутствии совпадений;
добавьте в конец функции явный `Возврат Неопределено` чтобы соблюсти контракт,
указаный в блоке "Возвращаемое значение" (после использования ВариантыИмени,
ПутьКФайлу и проверки ФС.ФайлСуществует) — просто вставьте один возвращающий
оператор перед `КонецФункции`.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.ossrc/BenchmarkOneScript/core/Классы/ЛокаторИсполняющейСреды.os
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os (1)
92-102:⚠️ Potential issue | 🟠 MajorГарантируйте cleanup артефактов при любом исключении после цикла.
Код на строках 96-100 (
ОпределитьЭталоны()иПостроитель.Сформировать()) не обёрнут вПопытка...Исключение. Если там возникнет исключение,УдалитьАртефакты()не будет вызван, и временные файлы останутся.🛠️ Предлагаемая правка
КонецЦикла; + Попытка Если ВерсииИсполняющейСреды.Количество() > 1 Тогда РезультатЗапуска.СредаОкружения.ВерсияИсполняющейСреды = ""; КонецЕсли; РезультатЗапуска.ОпределитьЭталоны(); Построитель = Новый ПостроительОтчетаБенчмарков(_ДескрипторыБенчмарков, _Конфигурация, РезультатЗапуска.Запуски); РезультатЗапуска.Отчет = Построитель.Сформировать(); + Исключение + УдалитьАртефакты(); + ВызватьИсключение; + КонецПопытки; УдалитьАртефакты(); Возврат РезультатЗапуска;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 92 - 102, Обёрните вызовы РезультатЗапуска.ОпределитьЭталоны() и Построитель.Сформировать() в блок Попытка...Наконец (или Попытка...Исключение...Наконец) так, чтобы УдалитьАртефакты() выполнялся в секции Наконец независимо от исключений; при этом пробросьте исключение дальше после удаления артефактов, чтобы не менять существующее поведение, и сохраните использование тех же переменных (РезультатЗапуска, Построитель) для локализации изменений.
🧹 Nitpick comments (1)
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os (1)
364-374: Рассмотрите экранирование путей в генерируемом сценарии.Пути встраиваются в текст сценария без экранирования. Если путь содержит двойные кавычки (хотя это редкость), сценарий будет синтаксически некорректным.
💡 Возможное улучшение
Добавьте функцию экранирования кавычек в строках путей:
Функция ЭкранироватьПутьДляСценария(Путь) Возврат СтрЗаменить(Путь, """", """"""); КонецФункцииИ используйте её при формировании текста сценария.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 364 - 374, Текст сценария формируется с подстановкой путей (ПутьКБиблиотеке, ПутьКФайлуБенчмарка, ПутьФайлаКонфигурации) без экранирования, что сломает синтаксис при наличии двойных кавычек; добавьте функцию ЭкранироватьПутьДляСценария(Путь) которая заменяет все " на "" (или другой корректный экранирующий механизм OneScript) и примените её при вставке всех путей в ТекстСценария (в выражениях, где используются ПутьКБиблиотеке, ПутьКФайлаБенчмарка и ПутьФайлаКонфигурации) чтобы гарантировать корректность сгенерированного скрипта.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 152-155: Замените вызов ОбернутьВКавычкиДляWindows на
универсальный ОбернутьВКавычки при формировании массива СтрокаКоманды для
ПутьИсполняемогоФайла (и аналогично в других местах, указанных в комментарии),
чтобы путь исполняемого файла всегда оборачивался в кавычки на всех платформах;
после правки удалите или рефакторьте функцию ОбернутьВКавычкиДляWindows, если
она больше не используется, и проверьте места использования СтрокаКоманды,
СтрСоединить и ОбернутьВКавычки на предмет совместимости с Windows/Unix.
---
Duplicate comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 92-102: Обёрните вызовы РезультатЗапуска.ОпределитьЭталоны() и
Построитель.Сформировать() в блок Попытка...Наконец (или
Попытка...Исключение...Наконец) так, чтобы УдалитьАртефакты() выполнялся в
секции Наконец независимо от исключений; при этом пробросьте исключение дальше
после удаления артефактов, чтобы не менять существующее поведение, и сохраните
использование тех же переменных (РезультатЗапуска, Построитель) для локализации
изменений.
---
Nitpick comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 364-374: Текст сценария формируется с подстановкой путей
(ПутьКБиблиотеке, ПутьКФайлуБенчмарка, ПутьФайлаКонфигурации) без экранирования,
что сломает синтаксис при наличии двойных кавычек; добавьте функцию
ЭкранироватьПутьДляСценария(Путь) которая заменяет все " на "" (или другой
корректный экранирующий механизм OneScript) и примените её при вставке всех
путей в ТекстСценария (в выражениях, где используются ПутьКБиблиотеке,
ПутьКФайлаБенчмарка и ПутьФайлаКонфигурации) чтобы гарантировать корректность
сгенерированного скрипта.
Comment on lines
+152
to
+155
| СтрокаКоманды = Новый Массив(); | ||
| СтрокаКоманды.Добавить(ОбернутьВКавычкиДляWindows(ПутьИсполняемогоФайла)); | ||
| СтрокаКоманды.Добавить(ОбернутьВКавычки(ПутьСтартовогоСценария)); | ||
| СтрокаКоманды = СтрСоединить(СтрокаКоманды, " "); |
There was a problem hiding this comment.
Путь к исполняемому файлу не экранируется на Unix.
ОбернутьВКавычкиДляWindows добавляет кавычки только на Windows. На Unix-системах пути с пробелами также требуют экранирования, но сейчас они передаются без кавычек, что может привести к ошибке запуска процесса.
💡 Предлагаемая правка
Используйте ОбернутьВКавычки для всех платформ:
СтрокаКоманды = Новый Массив();
- СтрокаКоманды.Добавить(ОбернутьВКавычкиДляWindows(ПутьИсполняемогоФайла));
+ СтрокаКоманды.Добавить(ОбернутьВКавычки(ПутьИсполняемогоФайла));
СтрокаКоманды.Добавить(ОбернутьВКавычки(ПутьСтартовогоСценария));После чего функцию ОбернутьВКавычкиДляWindows можно удалить, если она больше не используется.
Also applies to: 496-502
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`
around lines 152 - 155, Замените вызов ОбернутьВКавычкиДляWindows на
универсальный ОбернутьВКавычки при формировании массива СтрокаКоманды для
ПутьИсполняемогоФайла (и аналогично в других местах, указанных в комментарии),
чтобы путь исполняемого файла всегда оборачивался в кавычки на всех платформах;
после правки удалите или рефакторьте функцию ОбернутьВКавычкиДляWindows, если
она больше не используется, и проверьте места использования СтрокаКоманды,
СтрСоединить и ОбернутьВКавычки на предмет совместимости с Windows/Unix.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/ТестыСериализаторРезультатовБенчмарков.os (1)
24-31: Consider path separator for cross-platform compatibility.The hardcoded path
.\BenchmarkArtifactsuses a Windows-specific backslash separator. If cross-platform support is desired, consider usingОбъединитьПути()or similar path utilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыСериализаторРезультатовБенчмарков.os` around lines 24 - 31, The hardcoded Windows-style path in ПослеЗапускаТеста (КаталогАртефактов = ".\BenchmarkArtifacts") breaks cross-platform runs; change КаталогАртефактов to construct the path with a path utility (e.g., ОбъединитьПути or аналог) and then call ФС.УдалитьФайлы(КаталогАртефактов) so the separator is correct on all OSes.src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os (1)
42-48: Небольшая неточность в документации.Комментарий на Line 45 указывает "см. СобытияБенчмарков", но метод
ИмяМетода()возвращает квалифицированное имя метода-обработчика, а не событие.✏️ Предлагаемое исправление
// Возвращает квалифицированное имя метода. // // Возвращаемое значение: -// Строка - см. СобытияБенчмарков +// Строка - Квалифицированное имя метода-обработчика Функция ИмяМетода() Экспорт🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os` around lines 42 - 48, Дописать или поправить комментарий к функции ИмяМетода(), т.к. текущая документация («см. СобытияБенчмарков») неверно подразумевает возврат события; указать, что функция возвращает квалифицированное имя обработчика метода (значение поля _КвалифицированноеИмяМетода) — исправьте описание возвращаемого значения и при необходимости примечание, чтобы ясно отличать возвращаемое квалифицированное имя метода от объектов/идентификаторов событий.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/api/ДобавлениеОбработчиковСобытийПример.os`:
- Around line 23-29: В цепочке вызовов на объекте Конфигурация (созданном через
Новый КонфигурацияБенчмарков) присутствует лишняя закрывающая скобка перед
точкой с запятой: удалите из выражения дополнительный символ ")" в строке с
цепочкой .ДобавитьОбработчикСобытия("ПослеЗапуска",
СобытияБенчмарков.ПослеКаждого) так чтобы вызовы завершались корректно точкой с
запятой и синтаксис стал валидным.
---
Nitpick comments:
In `@src/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.os`:
- Around line 42-48: Дописать или поправить комментарий к функции ИмяМетода(),
т.к. текущая документация («см. СобытияБенчмарков») неверно подразумевает
возврат события; указать, что функция возвращает квалифицированное имя
обработчика метода (значение поля _КвалифицированноеИмяМетода) — исправьте
описание возвращаемого значения и при необходимости примечание, чтобы ясно
отличать возвращаемое квалифицированное имя метода от объектов/идентификаторов
событий.
In `@tests/ТестыСериализаторРезультатовБенчмарков.os`:
- Around line 24-31: The hardcoded Windows-style path in ПослеЗапускаТеста
(КаталогАртефактов = ".\BenchmarkArtifacts") breaks cross-platform runs; change
КаталогАртефактов to construct the path with a path utility (e.g.,
ОбъединитьПути or аналог) and then call ФС.УдалитьФайлы(КаталогАртефактов) so
the separator is correct on all OSes.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/ОбработчикиСобытий.mdsamples/api/ДобавлениеОбработчиковСобытийПример.ossrc/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ОбработчикСобытияБенчмарка.ossrc/BenchmarkOneScript/core/Модули/ИзвлекательКонфигурацииБенчмарков.ostests/ТестыБенчмарков.ostests/ТестыКонфигурации.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыЭкспортеров.os
🚧 Files skipped from review as they are similar to previous changes (1)
- src/BenchmarkOneScript/core/Модули/ИзвлекательКонфигурацииБенчмарков.os
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
1 participant
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.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores