Support kwargs in method calls

docs/getting-started/policies/application-types.rst

@@ -164,7 +164,7 @@ using the :ref:`operator-new` operator if the class has been **registered**:
         .. code-block:: polar
             :caption: :fa:`oso` policy.polar
 
-            ?= allow(new User{name: "alice", is_admin: true}, "foo", "bar");
+            ?= allow(new User(name: "alice", is_admin: true), "foo", "bar");
 
         Initialization arguments provided in this way are passed as keywords.
         We can also pass positional arguments to the class constructor:
@@ -188,7 +188,7 @@ using the :ref:`operator-new` operator if the class has been **registered**:
         .. code-block:: polar
             :caption: :fa:`oso` policy.polar
 
-            ?= allow(new User{name: "alice", is_admin: true}, "foo", "bar");
+            ?= allow(new User(name: "alice", is_admin: true), "foo", "bar");
 
         Initialization arguments provided in this way are passed as keywords.
         We can also pass positional arguments to the class constructor:

docs/more/dev-tools/tracing.rst

@@ -32,7 +32,7 @@ Example
   [debug]     ]
   [debug]     RULE:
   [debug]     f(x) if
-  [debug]       new Foo{x: x}.foo() = x
+  [debug]       new Foo(x: x).foo() = x
   [debug]       QUERY: new (Foo{x: _x_10}, _instance_2_11) and .(_instance_2_11, foo(), _value_1_12) and _value_1_12 = _x_10, BINDINGS: {"_x_10": "12"}
   [debug]         QUERY: new (Foo{x: _x_10}, _instance_2_11), BINDINGS: {"_x_10": "12"}
   [debug]         QUERY: .(_instance_2_11, foo(), _value_1_12) and _value_1_12 = _x_10, BINDINGS: {"_instance_2_11": "Foo{x: 12}", "_x_10": "12"}

docs/using/polar-syntax.rst

@@ -282,7 +282,7 @@ The dot ``.`` operator can be used to access the value associated with
 a key in a dictionary or class instance. For example, the rule::
 
   first_name(dict, x) if
-    dict = new Person{} and
+    dict = new Person() and
     x = dict.first_name;
 
 will access the value of the field named ``"first_name"`` in ``dict``,
@@ -498,11 +498,11 @@ the dictionary.  For example::
     not {x: 1, y: 3} matches {x:1, y: 4}
 
     # a type name matches if the value has the same type
-    new Person{} matches Person
+    new Person() matches Person
 
     # The fields are checked in the same manner as dictionaries, and the type is
     # checked like above.
-    new Person{x: 1, y: 2} matches Person{x: 1}
+    new Person(x: 1, y: 2) matches Person{x: 1}
 
 For type matching, subclasses are also considered.  So, a class that is a
 subclass of ``Person`` would match ``Person{x: 1}``.

languages/java/oso/src/main/java/com/osohq/oso/Query.java

@@ -109,23 +109,21 @@ private HashMap<String, Object> nextResult() throws Exceptions.OsoException {
           if (host.hasInstance(id)) {
             throw new Exceptions.DuplicateInstanceRegistrationError(id);
           }
+
           JSONObject constructor = data.getJSONObject("constructor").getJSONObject("value");
-          JSONArray initargs;
-          if (constructor.has("InstanceLiteral")) {
-            // Keyword initargs are not supported in Java.
-            className = constructor.getJSONObject("InstanceLiteral").getString("tag");
-            throw new Exceptions.InstantiationError(className);
-          } else if (constructor.has("Call")) {
+          if (constructor.has("Call")) {
             className = constructor.getJSONObject("Call").getString("name");
-            initargs = constructor.getJSONObject("Call").getJSONArray("args");
+            JSONArray initargs = constructor.getJSONObject("Call").getJSONArray("args");
+
+            // kwargs should always be null in Java
             if (constructor.getJSONObject("Call").get("kwargs") != JSONObject.NULL) {
               throw new Exceptions.InstantiationError(className);
             }
+            host.makeInstance(className, host.polarListToJava(initargs), id);
+            break;
           } else {
             throw new Exceptions.InvalidConstructorError("Bad constructor");
           }
-          host.makeInstance(className, host.polarListToJava(initargs), id);
-          break;
         case "ExternalCall":
           instance = data.getJSONObject("instance");
           callId = data.getLong("call_id");
@@ -135,6 +133,9 @@ private HashMap<String, Object> nextResult() throws Exceptions.OsoException {
           if (!data.get("args").equals(null)) {
             jArgs = Optional.of(data.getJSONArray("args"));
           }
+          if (!data.get("kwargs").equals(null)) {
+            throw new Exceptions.InvalidCallError("Java does not support keyword arguments");
+          }
           handleCall(attrName, jArgs, instance, callId);
           break;
         case "ExternalIsa":

languages/java/oso/src/test/java/com/osohq/oso/PolarTest.java

@@ -280,8 +280,12 @@ public void testMakeInstanceFromPolar() throws Exception {
 
   @Test
   public void testNoKeywordArgs() throws Exception {
-    p.loadStr("f(x) if x = new MyClass(\"test\", id: 1);");
-    assertThrows(Exceptions.InstantiationError.class, () -> p.query("f(x)"));
+    p.registerConstant("MyClass", true);
+    assertThrows(
+        Exceptions.InstantiationError.class, () -> p.query("x = new MyClass(\"test\", id: 1)"));
+    assertThrows(
+        Exceptions.InvalidCallError.class,
+        () -> p.query("x = (new MyClass(\"test\", 1)).foo(\"test\", id: 1)"));
   }
 
   @Test

languages/js/src/Polar.test.ts

@@ -149,6 +149,14 @@ Application error: Foo { a: 'A' }.a is not a function at line 1, column 1`
     expect(await qvar(p, 'try(new X(), x)', 'x')).toStrictEqual([]);
   });
 
+  test('rejects keyword arguments in method calls', async () => {
+    const p = new Polar();
+    p.registerClass(A);
+    expect(query(p, 'x = (new A()).a(arg: 1)')).rejects.toThrow(
+      'JavaScript does not support keyword arguments'
+    );
+  });
+
   describe('animal tests', () => {
     const wolf =
       'new Animal({species: "canis lupus", genus: "canis", family: "canidae"})';
@@ -466,6 +474,14 @@ describe('#makeInstance', () => {
     const instance = p.__host().getInstance(1);
     expect(instance).toStrictEqual(new ConstructorArgs(1, 2));
   });
+
+  test('rejects keyword args', async () => {
+    const p = new Polar();
+    p.registerClass(ConstructorArgs);
+    expect(
+      query(p, 'x = new ConstructorArgs(first: 1, second: 2)')
+    ).rejects.toThrow('JavaScript does not support keyword arguments');
+  });
 });
 
 describe('#registerConstant', () => {

languages/js/src/errors.ts

@@ -62,12 +62,10 @@ export class InvalidQueryEventError extends PolarError {
   }
 }
 
-export class KwargsConstructorError extends PolarError {
-  constructor(tag: string) {
-    super(
-      `To construct a JavaScript instance, use the positional args constructor syntax: new ${tag}(...)`
-    );
-    Object.setPrototypeOf(this, KwargsConstructorError.prototype);
+export class KwargsError extends PolarError {
+  constructor() {
+    super('JavaScript does not support keyword arguments');
+    Object.setPrototypeOf(this, KwargsError.prototype);
   }
 }
 

languages/js/src/helpers.ts

@@ -1,11 +1,7 @@
 import { inspect } from 'util';
 import { readFile as _readFile } from 'fs';
 
-import {
-  InvalidQueryEventError,
-  KwargsConstructorError,
-  PolarError,
-} from './errors';
+import { InvalidQueryEventError, KwargsError, PolarError } from './errors';
 import { isPolarTerm, QueryEventKind } from './types';
 import type { obj, QueryEvent } from './types';
 
@@ -102,11 +98,10 @@ function parseResult({ bindings }: obj): QueryEvent {
  */
 function parseMakeExternal(d: obj): QueryEvent {
   const instanceId = d.instance_id;
-  const ctor = d['constructor']?.value;
-  if (ctor?.InstanceLiteral !== undefined)
-    throw new KwargsConstructorError(ctor?.InstanceLiteral?.tag);
-  const tag = ctor?.Call?.name;
-  const fields = ctor?.Call?.args;
+  const ctor = d['constructor']?.value?.Call;
+  const tag = ctor?.name;
+  const fields = ctor?.args;
+  if (ctor?.kwargs) throw new KwargsError();
   if (
     !Number.isSafeInteger(instanceId) ||
     typeof tag !== 'string' ||
@@ -128,10 +123,12 @@ function parseMakeExternal(d: obj): QueryEvent {
  */
 function parseExternalCall({
   args,
+  kwargs,
   attribute,
   call_id: callId,
   instance,
 }: obj): QueryEvent {
+  if (kwargs) throw new KwargsError();
   if (
     !Number.isSafeInteger(callId) ||
     !isPolarTerm(instance) ||

languages/python/oso/oso/test_oso.polar

@@ -3,18 +3,18 @@ allow(actor, action, resource) if
     actorInRole(actor, role, resource);
 
 allow(token, action, resource) if
-    jwt = new Jwt{token: token} and
+    jwt = new Jwt(token: token) and
     jwt.attributes() = attributes and
     allow(attributes, action, resource);
 
 allow(_: {sub: sub}, action, resource) if
-    allow(new Actor{name: sub}, action, resource);
+    allow(new Actor(name: sub), action, resource);
 
 allow("guest", action, resource) if
-    allow(new Actor{name: "guest"}, action, resource);
+    allow(new Actor(name: "guest"), action, resource);
 
 allow(_: {username: name}, action, resource) if
-    allow(new Actor{name: name}, action, resource);
+    allow(new Actor(name: name), action, resource);
 
 allow(_actor: Actor, "get", _resource: Widget);
 allow(actor: Actor, "create", resource: Company) if

languages/python/oso/polar/query.py

@@ -68,18 +68,11 @@ def run(self):
     def handle_make_external(self, data):
         id = data["instance_id"]
         constructor = data["constructor"]["value"]
-        if "InstanceLiteral" in constructor:
-            cls_name = constructor["InstanceLiteral"]["tag"]
-            fields = constructor["InstanceLiteral"]["fields"]["fields"]
-            args = []
-            kwargs = {k: self.host.to_python(v) for k, v in fields.items()}
-        elif "Call" in constructor:
+        if "Call" in constructor:
             cls_name = constructor["Call"]["name"]
             args = [self.host.to_python(arg) for arg in constructor["Call"]["args"]]
-            kwargs = constructor["Call"]["kwargs"]
-            kwargs = (
-                {k: self.host.to_python(v) for k, v in kwargs.items()} if kwargs else {}
-            )
+            kwargs = constructor["Call"]["kwargs"] or {}
+            kwargs = {k: self.host.to_python(v) for k, v in kwargs.items()}
         else:
             raise PolarApiException("Bad constructor")
         self.host.make_instance(cls_name, args, kwargs, id)
@@ -103,7 +96,9 @@ def handle_external_call(self, data):
                 callable(attr) and not data["args"] is None
             ):  # If it's a function, call it with the args.
                 args = [self.host.to_python(arg) for arg in data["args"]]
-                result = attr(*args)
+                kwargs = data["kwargs"] or {}
+                kwargs = {k: self.host.to_python(v) for k, v in kwargs.items()}
+                result = attr(*args, **kwargs)
             elif not data["args"] is None:
                 raise RuntimeError(
                     f"tried to call '{attribute}' but it is not callable"

languages/python/oso/tests/parity/policies/test_api.polar

@@ -7,19 +7,19 @@ actorInRole(actor, role, resource: Widget) if
 
 allow(actor, "get", _: Http{path: path}) if
     new PathMapper("/widget/{id}").map(path) = {id: id} and
-    allow(actor, "get", new Widget{id: id});
+    allow(actor, "get", new Widget(id: id));
 
 allow(actor, "post", _: Http{path: path}) if
-    new PathMapper{template: "/widget/"}.map(path) = {} and
-    allow(actor, "create", new Widget{});
+    new PathMapper(template: "/widget/").map(path) = {} and
+    allow(actor, "create", new Widget());
 
 allow(actor, "what", _: Http{path: path}) if
     new PathMapper("/widget/{id}").map(path) = {id: id} and
-    allow(actor, "unparameterised_get", new Widget{id: id});
+    allow(actor, "unparameterised_get", new Widget(id: id));
 
 allow(actor, "what", _: Http{path: path, query: {param: "foo"}}) if
     new PathMapper("/widget/{id}").map(path) = {id: id} and
-    allow(actor, "parameterised_get", new Widget{id: id});
+    allow(actor, "parameterised_get", new Widget(id: id));
 
 allow(actor, "get", resource: Widget) if resource.frob("Widget") = x;
 allow(actor, "get", resource: DooDad) if resource.frob("DooDad") = x;
@@ -51,7 +51,7 @@ allow_two(actor, action, resource) if checkResource(_x, resource);
 checkResource(1, resource: Widget); # two slightly different specs so need to check
 checkResource("1", resource: Widget); # which to prioritise
 
-?= allow_two(_actor, _action, new Widget{});
+?= allow_two(_actor, _action, new Widget());
 
 # for testing lists
 allow(actor: Actor, "invite", resource: Widget) if

languages/python/oso/tests/parity/test_api.py

@@ -189,7 +189,7 @@ def test_instance_round_trip(polar, query, qvar):
 def test_instance_initialization(polar, query, qvar):
     # test round trip through kb query
     user = Actor("sam")
-    env = query('new Actor{name:"sam"} = returned_user')[0]
+    env = query('new Actor(name:"sam") = returned_user')[0]
     assert polar.host.to_python(env["returned_user"]) == user
 
     env = query('new Actor(name:"sam") = returned_user')[0]

languages/python/oso/tests/test_polar.py

@@ -473,6 +473,7 @@ def __init__(self, a, b, bar, baz):
 
     polar.register_class(Foo)
 
+    # test positional args
     instance = qvar(
         "instance = new Foo(1,2,3,4)",
         "instance",
@@ -483,6 +484,7 @@ def __init__(self, a, b, bar, baz):
     assert instance.bar == 3
     assert instance.baz == 4
 
+    # test positional and kwargs
     instance = qvar(
         "instance = new Foo(1, 2, bar: 3, baz: 4)",
         "instance",
@@ -493,6 +495,7 @@ def __init__(self, a, b, bar, baz):
     assert instance.bar == 3
     assert instance.baz == 4
 
+    # test kwargs
     instance = qvar(
         "instance = new Foo(bar: 3, a: 1, baz: 4, b: 2)",
         "instance",
@@ -700,3 +703,39 @@ def map(self, f):
     polar.register_class(Foo)
     polar.load_str("f(x: Foo) if x.map(Foo.plus_one) = [2, 3, 4];")
     assert next(polar.query_rule("f", Foo([1, 2, 3])))
+
+
+def test_method_with_kwargs(polar, qvar):
+    class Test:
+        def kwarg_method(self, x=1, y=2):
+            self.x = x
+            self.y = y
+            return True
+
+    polar.register_class(Test)
+    rules = """
+    defaults(result) if
+        test = new Test() and
+        test.kwarg_method() and
+        result = [test.x, test.y];
+
+    kwargs(result) if
+        test = new Test() and
+        test.kwarg_method(y: 4, x: 3) and
+        result = [test.x, test.y];
+
+    args(result) if
+        test = new Test() and
+        test.kwarg_method(5, 6) and
+        result = [test.x, test.y];
+
+    mixed(result) if
+        test = new Test() and
+        test.kwarg_method(7, y: 8) and
+        result = [test.x, test.y];
+    """
+    polar.load_str(rules)
+    qvar("defaults(result)", "result") == [1, 2]
+    qvar("kwargs(result)", "result") == [3, 4]
+    qvar("args(result)", "result") == [5, 6]
+    qvar("mixed(result)", "result") == [7, 8]

languages/ruby/lib/oso/polar/host.rb

@@ -121,6 +121,7 @@ def cache_instance(instance, id: nil)
       # @raise [PolarRuntimeError] if instance construction fails.
       def make_instance(cls_name, args:, kwargs:, id:) # rubocop:disable Metrics/MethodLength
         constructor = get_constructor(cls_name)
+        # The kwargs.empty? checks are for Ruby < 2.7.
         instance = if constructor == :new
                      if kwargs.empty?
                        get_class(cls_name).__send__(:new, *args)