Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions src/OneScript.Core/Contexts/ContextMethodAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ This Source Code Form is subject to the terms of the
at http://mozilla.org/MPL/2.0/.
----------------------------------------------------------*/

#nullable enable
using System;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using OneScript.Commons;

Expand All @@ -16,6 +19,10 @@ public class ContextMethodAttribute : Attribute, INameAndAliasProvider
{
private readonly string _name;
private readonly string _alias;
private bool _isDeprecated;
private bool _throwOnUse;
private bool _skipForDocumenter;
private Type? _converter;

public ContextMethodAttribute(string name, string alias)
{
Expand All @@ -35,16 +42,49 @@ public ContextMethodAttribute(string name, string _ = null,
{
}

public bool IsDeprecated { get; set; }
public bool IsDeprecated
{
get => _isDeprecated;
set => _isDeprecated = value;
}

public bool ThrowOnUse
{
get => _throwOnUse;
set => _throwOnUse = value;
}

public bool ThrowOnUse { get; set; }

/// <summary>
/// Данный метод не будет обработан генератором документации при обходе типов
/// </summary>
public bool SkipForDocumenter { get; set; }
public bool SkipForDocumenter
{
get => _skipForDocumenter;
set => _skipForDocumenter = value;
}

public string Name => _name;
public string Alias => _alias;

/// <summary>
/// Конвертер возвращаемого значения функции, необходим для неподдерживаемых маршаллингом типов
/// </summary>
public Type? Converter
{
get => _converter;
set
{
if (value != null)
{
if (!value.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IContextValueConverter<>)))
throw new Exception("Конвертер должен реализовывать интерфейс IContextValueConverter");

if (value.IsAbstract || value.IsInterface)
throw new Exception("Конвертер не может быть абстрактным типом или интерфейсом");
}

_converter = value;
}
}
}
}
22 changes: 22 additions & 0 deletions src/OneScript.Core/Contexts/ContextMethodInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public sealed class ContextMethodInfo : BslMethodInfo, IObjectWrapper
{
private readonly MethodInfo _realMethod;
private readonly ContextMethodAttribute _scriptMark;

public Type ConverterType { get; private set; }

public ContextMethodInfo(MethodInfo realMethod)
{
Expand All @@ -39,10 +41,18 @@ public ContextMethodInfo(MethodInfo realMethod)
public ContextMethodInfo(MethodInfo realMethod, ContextMethodAttribute binding)
{
_realMethod = realMethod;

_scriptMark = binding;

InjectsProcess = _realMethod.GetParameters().FirstOrDefault()?.ParameterType == typeof(IBslProcess);
}

private void InitConverter()
{
if (_scriptMark.Converter != null)
ConverterType = _scriptMark.Converter;
}

public override Type ReturnType => _realMethod.ReturnType;

public override ParameterInfo ReturnParameter => _realMethod.ReturnParameter;
Expand Down Expand Up @@ -85,6 +95,18 @@ public override MethodInfo GetBaseDefinition()
{
return _realMethod.GetBaseDefinition();
}

public bool TryGetConverter(out object converter)
{
if (_scriptMark.Converter == null)
{
converter = null;
return false;
}

converter = Activator.CreateInstance(_scriptMark.Converter);
return true;
}
Comment on lines +99 to +109

@coderabbitai coderabbitai Bot Aug 8, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

TryGetConverter can throw; enforce “Try” semantics and validate type viability*

Activator.CreateInstance will throw for abstract types, open generics, or when no public parameterless ctor exists. A Try* method should not throw. Guard and catch, returning false.

         public bool TryGetConverter(out object converter)
         {
             if (ConverterType == null)
             {
                 converter = null;
                 return false;
             }
-            
-            converter = Activator.CreateInstance(ConverterType);
-            return true;
+
+            try
+            {
+                if (ConverterType.IsAbstract || ConverterType.IsInterface || ConverterType.ContainsGenericParameters)
+                {
+                    converter = null;
+                    return false;
+                }
+
+                if (ConverterType.GetConstructor(Type.EmptyTypes) == null)
+                {
+                    converter = null;
+                    return false;
+                }
+
+                converter = Activator.CreateInstance(ConverterType);
+                return converter != null;
+            }
+            catch (Exception)
+            {
+                converter = null;
+                return false;
+            }
         }

Optional: if converters are stateless, consider caching a single instance per ContextMethodInfo to reduce allocations. Happy to propose a thread-safe cache if desired.

📝 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
public bool TryGetConverter(out object converter)
{
if (ConverterType == null)
{
converter = null;
return false;
}
converter = Activator.CreateInstance(ConverterType);
return true;
}
public bool TryGetConverter(out object converter)
{
if (ConverterType == null)
{
converter = null;
return false;
}
try
{
if (ConverterType.IsAbstract || ConverterType.IsInterface || ConverterType.ContainsGenericParameters)
{
converter = null;
return false;
}
if (ConverterType.GetConstructor(Type.EmptyTypes) == null)
{
converter = null;
return false;
}
converter = Activator.CreateInstance(ConverterType);
return converter != null;
}
catch (Exception)
{
converter = null;
return false;
}
}
🤖 Prompt for AI Agents
In src/OneScript.Core/Contexts/ContextMethodInfo.cs around lines 103 to 113, the
TryGetConverter method can throw exceptions when Activator.CreateInstance is
called on abstract types, open generics, or types without a public parameterless
constructor, violating the expected "Try*" method semantics. To fix this, add
checks to ensure ConverterType is not abstract, not an open generic, and has a
public parameterless constructor before calling CreateInstance. Wrap the
instantiation in a try-catch block to catch any exceptions and return false if
instantiation fails. Optionally, consider caching a single instance of the
converter if it is stateless to reduce allocations.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akpaevj может есть какой-то способ запретить указывать абстрактный тип в атрибуте? Ну или можно забить...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


public override ICustomAttributeProvider ReturnTypeCustomAttributes => _realMethod.ReturnTypeCustomAttributes;

Expand Down
23 changes: 23 additions & 0 deletions src/OneScript.Core/Contexts/ContextPropertyAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This Source Code Form is subject to the terms of the
----------------------------------------------------------*/

using System;
using System.Linq;
using OneScript.Commons;

namespace OneScript.Contexts
Expand All @@ -15,6 +16,7 @@ public class ContextPropertyAttribute : Attribute, INameAndAliasProvider
{
private readonly string _name;
private readonly string _alias;
private Type _converter;

public ContextPropertyAttribute(string name, string alias = "")
{
Expand All @@ -40,5 +42,26 @@ public ContextPropertyAttribute(string name, string alias = "")

public string Name => _name;
public string Alias => _alias;

/// <summary>
/// Конвертер значения свойства
/// </summary>
public Type Converter
{
get => _converter;
set
{
if (value != null)
{
if (!value.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IContextValueConverter<>)))
throw new Exception("Конвертер должен реализовывать интерфейс IContextValueConverter");

if (value.IsAbstract || value.IsInterface)
throw new Exception("Конвертер не может быть абстрактным типом или интерфейсом");
}

_converter = value;
}
}
}
}
29 changes: 28 additions & 1 deletion src/OneScript.Core/Contexts/ContextPropertyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This Source Code Form is subject to the terms of the
using System.Linq;
using System.Reflection;
using OneScript.Commons;
using ScriptEngine.Machine;

namespace OneScript.Contexts
{
Expand All @@ -20,7 +21,7 @@ public class ContextPropertyInfo : BslPropertyInfo, IObjectWrapper
{
private readonly PropertyInfo _realProperty;
private readonly ContextPropertyAttribute _scriptMark;

public ContextPropertyInfo(PropertyInfo wrappedInfo)
{
_realProperty = wrappedInfo;
Expand Down Expand Up @@ -54,6 +55,32 @@ public override bool Equals(BslPropertyInfo other)

public override string Alias => _scriptMark.Alias;

public Type ConverterType => _scriptMark.Converter;

public bool TryGetConverter(out object converter)
{
if (ConverterType == null)
{
converter = null;
return false;
}

converter = Activator.CreateInstance(ConverterType);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не совсем понял, какой смысл в этом методе, что если я укажу в атрибуте в качестве конвертера typeof(Int32) и получу тут число? Может он должен быть private?

return true;
}

public bool TryGetStrictConverter<T>(out IContextValueConverter<T> converter)
{
var result = TryGetConverter(out var c);

if (result)
converter = c as IContextValueConverter<T>;
else
converter = null;

return result;
}

public override MethodInfo[] GetAccessors(bool nonPublic)
{
var getter = GetGetMethod(nonPublic);
Expand Down
23 changes: 23 additions & 0 deletions src/OneScript.Core/Contexts/IContextValueConverter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*----------------------------------------------------------
This Source Code Form is subject to the terms of the
Mozilla Public License, v.2.0. If a copy of the MPL
was not distributed with this file, You can obtain one
at http://mozilla.org/MPL/2.0/.
----------------------------------------------------------*/
using System;
using System.Linq;
using ScriptEngine.Machine;

namespace OneScript.Contexts
{
public interface IContextValueConverter<TClr>
{
IValue ToIValue(TClr obj);

TClr ToClr(IValue obj);

public static bool ImplementsIt(Type type)
=> type.GetInterfaces()
.Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IContextValueConverter<>));
}
}
46 changes: 26 additions & 20 deletions src/ScriptEngine/Machine/Contexts/ContextMethodMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,25 +152,35 @@ private static MethodSignature CreateMetadata(MethodInfo target, ContextMethodAt

}

var scriptMethInfo = new MethodSignature();
scriptMethInfo.IsFunction = isFunc;
scriptMethInfo.IsExport = true;
scriptMethInfo.IsDeprecated = binding.IsDeprecated;
scriptMethInfo.ThrowOnUseDeprecated = binding.ThrowOnUse;
scriptMethInfo.Name = binding.Name;
scriptMethInfo.Alias = binding.Alias;

scriptMethInfo.Params = paramDefs;
var scriptMethInfo = new MethodSignature
{
IsFunction = isFunc,
IsExport = true,
IsDeprecated = binding.IsDeprecated,
ThrowOnUseDeprecated = binding.ThrowOnUse,
Name = binding.Name,
Alias = binding.Alias,
Params = paramDefs
};

return scriptMethInfo;
}

private static ContextCallableDelegate<TInstance> CreateFunction(ContextMethodInfo target)
{
var methodCall = MethodCallExpression(target, out var instParam, out var argsParam, out var processParam);

var convertRetMethod = ContextValuesMarshaller.BslReturnValueGenericConverter.MakeGenericMethod(target.ReturnType);
var convertReturnCall = Expression.Call(convertRetMethod, methodCall);

var convertReturnCall = target.ConverterType switch
{
null => Expression.Call(
ContextValuesMarshaller.BslReturnValueGenericConverter.MakeGenericMethod(target.ReturnType),
methodCall),
_ => Expression.Call(
Expression.New(target.ConverterType),
target.ConverterType.GetMethod("ToIValue")!,
methodCall)
};

var body = convertReturnCall;

var l = Expression.Lambda<ContextCallableDelegate<TInstance>>(body, instParam, argsParam, processParam);
Expand Down Expand Up @@ -226,28 +236,24 @@ private static InvocationExpression MethodCallExpression(

var (clrIndexStart, argsLen) = contextMethod.InjectsProcess ? (1, parameters.Length - 1) : (0, parameters.Length);

var argsPass = new List<Expression>();
argsPass.Add(instParam);

var argsPass = new List<Expression> { instParam };

if (contextMethod.InjectsProcess)
argsPass.Add(processParam);

for (int bslIndex = 0,clrIndex = clrIndexStart; bslIndex < argsLen; bslIndex++, clrIndex++)
{
var targetType = parameters[clrIndex].ParameterType;
var convertMethod = ContextValuesMarshaller.BslGenericParameterConverter.MakeGenericMethod(targetType);

Expression defaultArg;
if (parameters[clrIndex].HasDefaultValue)
{
defaultArg = Expression.Constant(parameters[clrIndex].DefaultValue, targetType);
}
else
{
defaultArg = ContextValuesMarshaller.GetDefaultBslValueConstant(targetType);
}

var indexedArg = Expression.ArrayIndex(argsParam, Expression.Constant(bslIndex));

var convertMethod = ContextValuesMarshaller.BslGenericParameterConverter.MakeGenericMethod(targetType);
var conversionCall = Expression.Call(convertMethod,
indexedArg,
defaultArg,
Expand Down
45 changes: 23 additions & 22 deletions src/ScriptEngine/Machine/Contexts/ContextPropertyMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ namespace ScriptEngine.Machine.Contexts
{
public class PropertyTarget<TInstance>
{
private readonly BslPropertyInfo _propertyInfo;

private readonly ContextPropertyInfo _propertyInfo;
public PropertyTarget(PropertyInfo propInfo)
{
_propertyInfo = new ContextPropertyInfo(propInfo);
Expand All @@ -27,16 +27,6 @@ public PropertyTarget(PropertyInfo propInfo)
if (string.IsNullOrEmpty(Alias))
Alias = propInfo.Name;

IValue CantReadAction(TInstance inst)
{
throw PropertyAccessException.PropIsNotReadableException(Name);
}

void CantWriteAction(TInstance inst, IValue val)
{
throw PropertyAccessException.PropIsNotWritableException(Name);
}

if (_propertyInfo.CanRead)
{
var getMethodInfo = propInfo.GetGetMethod();
Expand Down Expand Up @@ -84,6 +74,18 @@ void CantWriteAction(TInstance inst, IValue val)
{
Setter = CantWriteAction;
}

return;

void CantWriteAction(TInstance inst, IValue val)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а чем помешали в качестве локальных методов?

{
throw PropertyAccessException.PropIsNotWritableException(Name);
}

IValue CantReadAction(TInstance inst)
{
throw PropertyAccessException.PropIsNotReadableException(Name);
}
}

public Func<TInstance, IValue> Getter { get; }
Expand All @@ -108,19 +110,18 @@ private Func<TInstance, IValue> CreateGetter<T>(MethodInfo methInfo)
private Action<TInstance, IValue> CreateSetter<T>(MethodInfo methInfo)
{
var method = (Action<TInstance, T>)Delegate.CreateDelegate(typeof(Action<TInstance, T>), methInfo);
return (inst, val) => method(inst, ConvertParam<T>(val));
return (inst, val) => method(inst, ConvertValue<T>(val));
}

private static T ConvertParam<T>(IValue value)
{
return ContextValuesMarshaller.ConvertValueStrict<T>(value);
}

private static IValue ConvertReturnValue<TRet>(TRet param)
{
return ContextValuesMarshaller.ConvertReturnValue(param);
}
private T ConvertValue<T>(IValue value)
=> _propertyInfo.TryGetStrictConverter<T>(out var converter) ?
converter.ToClr(value)
: ContextValuesMarshaller.ConvertValueStrict<T>(value);

private IValue ConvertReturnValue<TRet>(TRet param)
=> _propertyInfo.TryGetStrictConverter<TRet>(out var converter)
? converter!.ToIValue(param)
: ContextValuesMarshaller.ConvertReturnValue(param);
}

public class ContextPropertyMapper<TInstance>
Expand Down
Loading