Merge branch 'sebastienros:main' into main

Directory.Build.props

@@ -0,0 +1,7 @@
+<Project>
+
+  <PropertyGroup>
+    <UseArtifactsOutput>true</UseArtifactsOutput>
+  </PropertyGroup>
+
+</Project>

Jint.Tests/Runtime/InteropTests.cs

@@ -3333,5 +3333,93 @@ public void CanDestructureInteropTargetMethod()
             var result = engine.Evaluate("const { getStrings } = test; getStrings().Count;");
             Assert.Equal(3, result);
         }
+
+        private class MetadataWrapper : IDictionary<string, object>
+        {
+            public IEnumerator<KeyValuePair<string, object>> GetEnumerator() => throw new NotImplementedException();
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+            public void Add(KeyValuePair<string, object> item) => throw new NotImplementedException();
+            public void Clear() => throw new NotImplementedException();
+            public bool Contains(KeyValuePair<string, object> item) => throw new NotImplementedException();
+            public void CopyTo(KeyValuePair<string, object>[] array, int arrayIndex) => throw new NotImplementedException();
+            public bool Remove(KeyValuePair<string, object> item) => throw new NotImplementedException();
+            public int Count { get; set; }
+            public bool IsReadOnly { get; set; }
+            public bool ContainsKey(string key) => throw new NotImplementedException();
+            public void Add(string key, object value) => throw new NotImplementedException();
+            public bool Remove(string key) => throw new NotImplementedException();
+            public bool TryGetValue(string key, out object value)
+            {
+                value = "from-wrapper";
+                return true;
+            }
+
+            public object this[string key]
+            {
+                get => "from-wrapper";
+                set
+                {
+                }
+            }
+
+            public ICollection<string> Keys { get; set; }
+            public ICollection<object> Values { get; set; }
+        }
+
+        private class ShadowedGetter : IReadOnlyDictionary<string, object>
+        {
+            private Dictionary<string, object> _dictionary = new();
+
+            public void SetInitial(object value, string key)
+            {
+                _dictionary[key] = value;
+            }
+
+            public IEnumerator<KeyValuePair<string, object>> GetEnumerator() => throw new NotImplementedException();
+
+            IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+
+            public int Count { get; }
+            public bool ContainsKey(string key) => _dictionary.ContainsKey(key);
+
+            public bool TryGetValue(string key, out object value) => _dictionary.TryGetValue(key, out value);
+
+            public object this[string key]
+            {
+                get
+                {
+                    _dictionary.TryGetValue(key, out var value);
+                    return value;
+                }
+            }
+
+            public IEnumerable<string> Keys { get; set; }
+            public IEnumerable<object> Values { get; set; }
+        }
+
+        private class ShadowingSetter : ShadowedGetter
+        {
+            public Dictionary<string, int> Metadata
+            {
+                set
+                {
+                    SetInitial(new MetadataWrapper(), "metadata");
+                }
+            }
+        }
+
+        [Fact]
+        public void CanSelectShadowedPropertiesBasedOnReadableAndWritable()
+        {
+            var engine = new Engine();
+            engine.SetValue("test", new ShadowingSetter
+            {
+                Metadata = null
+            });
+
+            engine.Evaluate("test.metadata['abc'] = 123");
+            var result = engine.Evaluate("test.metadata['abc']");
+            Assert.Equal("from-wrapper", result);
+        }
     }
 }

Jint.sln

@@ -22,6 +22,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 		README.md = README.md
 		.editorconfig = .editorconfig
 		Directory.Packages.props = Directory.Packages.props
+		Directory.Build.props = Directory.Build.props
 	EndProjectSection
 EndProject
 Global

Jint/Collections/DictionarySlim.cs

@@ -12,7 +12,7 @@
 namespace Jint.Collections
 {
     /// <summary>
-    /// DictionarySlim<string, TValue> is similar to Dictionary<TKey, TValue> but optimized in three ways:
+    /// DictionarySlim&lt;string, TValue> is similar to Dictionary&lt;TKey, TValue> but optimized in three ways:
     /// 1) It allows access to the value by ref replacing the common TryGetValue and Add pattern.
     /// 2) It does not store the hash code (assumes it is cheap to equate values).
     /// 3) It does not accept an equality comparer (assumes Object.GetHashCode() and Object.Equals() or overridden implementation are cheap and sufficient).

Jint/Collections/StringDictionarySlim.cs

@@ -15,7 +15,7 @@
 namespace Jint.Collections
 {
     /// <summary>
-    /// DictionarySlim<string, TValue> is similar to Dictionary<TKey, TValue> but optimized in three ways:
+    /// DictionarySlim&lt;string, TValue> is similar to Dictionary&lt;TKey, TValue> but optimized in three ways:
     /// 1) It allows access to the value by ref replacing the common TryGetValue and Add pattern.
     /// 2) It does not store the hash code (assumes it is cheap to equate values).
     /// 3) It does not accept an equality comparer (assumes Object.GetHashCode() and Object.Equals() or overridden implementation are cheap and sufficient).

Jint/Jint.csproj

@@ -2,20 +2,33 @@
   <PropertyGroup>
     <NeutralLanguage>en-US</NeutralLanguage>
     <TargetFrameworks>net462;netstandard2.0;netstandard2.1;net6.0</TargetFrameworks>
+
     <AssemblyOriginatorKeyFile>Jint.snk</AssemblyOriginatorKeyFile>
     <SignAssembly>true</SignAssembly>
-    <LangVersion>latest</LangVersion>
     <IsPackable>true</IsPackable>
+
+    <LangVersion>latest</LangVersion>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+
     <AnalysisLevel>latest-Recommended</AnalysisLevel>
+
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    <PackageReadmeFile>README.md</PackageReadmeFile>
+
+    <NoWarn>$(NoWarn);1591</NoWarn>
+
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(TargetFramework)' == 'net6.0' or '$(TargetFramework)' == 'netstandard2.1' ">
     <DefineConstants>$(DefineConstants);SUPPORTS_SPAN_PARSE;SUPPORTS_WEAK_TABLE_ADD_OR_UPDATE;SUPPORTS_WEAK_TABLE_CLEAR</DefineConstants>
   </PropertyGroup>
 
+  <ItemGroup>
+    <None Include="../README.md" Pack="true" PackagePath="\"/>
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Esprima" />
     <PackageReference Include="Meziantou.Analyzer" PrivateAssets="all" />

Jint/Options.Extensions.cs

@@ -75,6 +75,7 @@ public static Options AddObjectConverter(this Options options, IObjectConverter
         /// <summary>
         /// Sets maximum allowed depth of recursion.
         /// </summary>
+        /// <param name="options">Options to modify</param>
         /// <param name="maxRecursionDepth">
         /// The allowed depth.
         /// a) In case max depth is zero no recursion is allowed.
@@ -140,6 +141,7 @@ public static Options SetTypeConverter(this Options options, Func<Engine, ITypeC
         /// to change what values are returned for specific CLR objects, or if any value
         /// is returned at all.
         /// </summary>
+        /// <param name="options">Options to modify</param>
         /// <param name="accessor">
         /// The delegate to invoke for each CLR member. If the delegate
         /// returns <c>null</c>, the standard evaluation is performed.
@@ -246,6 +248,7 @@ public static Options SetTypeResolver(this Options options, TypeResolver resolve
         /// Registers some custom logic to apply on an <see cref="Engine"/> instance when the options
         /// are loaded.
         /// </summary>
+        /// <param name="options">Options to modify</param>
         /// <param name="configuration">The action to register.</param>
         public static Options Configure(this Options options, Action<Engine> configuration)
         {

Jint/Options.cs

@@ -400,7 +400,7 @@ public class ConstraintOptions
         /// <remarks>
         /// Chrome and V8 based engines (ClearScript) that can handle 13955.
         /// When set to a different value except -1, it can reduce slight performance/stack trace readability drawback. (after hitting the engine's own limit),
-        /// When max stack size to be exceeded, Engine throws an exception <see cref="JavaScriptException">.
+        /// When max stack size to be exceeded, Engine throws an exception <see cref="JavaScriptException" />.
         /// </remarks>
         public int MaxExecutionStackCount { get; set; } = StackGuard.Disabled;
 

Jint/Runtime/Debugger/DebuggerStatementHandling.cs

@@ -16,7 +16,7 @@ public enum DebuggerStatementHandling
         Clr,
 
         /// <summary>
-        /// <c>debugger</c> statements will trigger a break in Jint's DebugHandler. See <see cref="Jint.Engine.Break"/>.
+        /// <c>debugger</c> statements will trigger a break in Jint's DebugHandler. See <see cref="DebugHandler.Break"/>.
         /// </summary>
         Script
     }

Jint/Runtime/Descriptors/Specialized/ReflectionDescriptor.cs

@@ -43,13 +43,14 @@ protected internal override JsValue? CustomValue
             set => DoSet(null, value);
         }
 
-        JsValue DoGet(JsValue? thisObj)
+        private JsValue DoGet(JsValue? thisObj)
         {
             var value = _reflectionAccessor.GetValue(_engine, _target);
             var type = _reflectionAccessor.MemberType;
             return JsValue.FromObjectWithType(_engine, value, type);
         }
-        void DoSet(JsValue? thisObj, JsValue? v)
+
+        private void DoSet(JsValue? thisObj, JsValue? v)
         {
             try
             {

Jint/Runtime/Interop/ObjectWrapper.cs

@@ -50,7 +50,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver)
                 if (_properties is null || !_properties.ContainsKey(member))
                 {
                     // can try utilize fast path
-                    var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member, forWrite: true);
+                    var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member, mustBeReadable: false, mustBeWritable: true);
 
                     if (ReferenceEquals(accessor, ConstantValueAccessor.NullAccessor))
                     {
@@ -116,7 +116,13 @@ public override JsValue Get(JsValue property, JsValue receiver)
                 return (uint) index < list.Count ? FromObject(_engine, list[index]) : Undefined;
             }
 
-            return base.Get(property, receiver);
+            var desc = GetOwnProperty(property, mustBeReadable: true, mustBeWritable: false);
+            if (desc != PropertyDescriptor.Undefined)
+            {
+                return UnwrapJsValue(desc, receiver);
+            }
+
+            return Prototype?.Get(property, receiver) ?? Undefined;
         }
 
         public override List<JsValue> GetOwnPropertyKeys(Types types = Types.None | Types.String | Types.Symbol)
@@ -182,6 +188,12 @@ private IEnumerable<JsValue> EnumerateOwnPropertyKeys(Types types)
         }
 
         public override PropertyDescriptor GetOwnProperty(JsValue property)
+        {
+            // we do not know if we need to read or write
+            return GetOwnProperty(property, mustBeReadable: false, mustBeWritable: false);
+        }
+
+        private PropertyDescriptor GetOwnProperty(JsValue property, bool mustBeReadable, bool mustBeWritable)
         {
             if (TryGetProperty(property, out var x))
             {
@@ -234,13 +246,17 @@ public override PropertyDescriptor GetOwnProperty(JsValue property)
                 return new PropertyDescriptor(result, PropertyFlag.OnlyEnumerable);
             }
 
-            var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member);
+            var accessor = _engine.Options.Interop.TypeResolver.GetAccessor(_engine, ClrType, member, mustBeReadable, mustBeWritable);
             var descriptor = accessor.CreatePropertyDescriptor(_engine, Target, enumerable: !isDictionary);
-            if (!isDictionary && !ReferenceEquals(descriptor, PropertyDescriptor.Undefined))
+            if (!isDictionary
+                && !ReferenceEquals(descriptor, PropertyDescriptor.Undefined)
+                && (!mustBeReadable || accessor.Readable)
+                && (!mustBeWritable || accessor.Writable))
             {
                 // cache the accessor for faster subsequent accesses
                 SetProperty(member, descriptor);
             }
+
             return descriptor;
         }
 
@@ -258,7 +274,9 @@ public static PropertyDescriptor GetPropertyDescriptor(Engine engine, object tar
                     _ => null
                 };
             }
-            return engine.Options.Interop.TypeResolver.GetAccessor(engine, target.GetType(), member.Name, Factory).CreatePropertyDescriptor(engine, target);
+
+            var accessor = engine.Options.Interop.TypeResolver.GetAccessor(engine, target.GetType(), member.Name, mustBeReadable: false, mustBeWritable: false, Factory);
+            return accessor.CreatePropertyDescriptor(engine, target);
         }
 
         internal static Type GetClrType(object obj, Type? type)

Jint/Runtime/Interop/TypeResolver.cs

@@ -47,8 +47,9 @@ internal ReflectionAccessor GetAccessor(
             Engine engine,
             Type type,
             string member,
-            Func<ReflectionAccessor?>? accessorFactory = null,
-            bool forWrite = false)
+            bool mustBeReadable,
+            bool mustBeWritable,
+            Func<ReflectionAccessor?>? accessorFactory = null)
         {
             var key = new Engine.ClrPropertyDescriptorFactoriesKey(type, member);
 
@@ -58,7 +59,7 @@ internal ReflectionAccessor GetAccessor(
                 return accessor;
             }
 
-            accessor = accessorFactory?.Invoke() ?? ResolvePropertyDescriptorFactory(engine, type, member, forWrite);
+            accessor = accessorFactory?.Invoke() ?? ResolvePropertyDescriptorFactory(engine, type, member, mustBeReadable, mustBeWritable);
 
             // racy, we don't care, worst case we'll catch up later
             Interlocked.CompareExchange(ref engine._reflectionAccessors,
@@ -74,7 +75,8 @@ private ReflectionAccessor ResolvePropertyDescriptorFactory(
             Engine engine,
             Type type,
             string memberName,
-            bool forWrite)
+            bool mustBeReadable,
+            bool mustBeWritable)
         {
             var isInteger = long.TryParse(memberName, NumberStyles.Integer, CultureInfo.InvariantCulture, out _);
 
@@ -86,7 +88,8 @@ private ReflectionAccessor ResolvePropertyDescriptorFactory(
             // properties and fields cannot be numbers
             if (!isInteger
                 && TryFindMemberAccessor(engine, type, memberName, BindingFlags, indexer, out var temp)
-                && (!forWrite || temp.Writable))
+                && (!mustBeReadable || temp.Readable)
+                && (!mustBeWritable || temp.Writable))
             {
                 return temp;
             }