Is it possible to define a binding?

[ShaneHudson]

I think I've found an issue where the bindings are not being used correctly. In the following examples I'm using genCel copied from the tests.

genCel("foo == true", {
    foo: true,
    bar: false,
  });
  // Returns true
  
  
genCel("foo_abc == true", {
    foo_abc true,
    bar: false,
  });
  // Errors. Should return true.


genCel("af1e1521-4046-4860-be4e-962f1f4e0d64 == true", {
    "af1e1521-4046-4860-be4e-962f1f4e0d64": true,
    abc: false,
  });
// Errors. Should return true. Providing directly without being a string throws an error too.


genCel("123 == true", {
    "123": true,
    abc: false,
  });
// Errors. Should return true BUT will need a way to say it is a binding not a number

These also apply when attempting to nest them (ie. answers["123"] does not work).

I think we would need to simply define a syntax, that matches CEL-spec, by using {af1e1521-4046-} == true.

Is this something you've already looked into? I can see the project is not actively maintained but it would be really useful to have a implementation of CEL that is usable in JS/TS 👍

So my ideal approach would be:

genCel("{af1e1521-4046-4860-be4e-962f1f4e0d64} == true", {
    "af1e1521-4046-4860-be4e-962f1f4e0d64": true,
    abc: false,
  });
// Return true

genCel("{answers['af1e1521-4046-4860-be4e-962f1f4e0d64']} == true", {
    answers: {
        "af1e1521-4046-4860-be4e-962f1f4e0d64": true,
        abc: false,
    }
  });
// Return true

[Fleker]

When I first added a CEL implementation I only did a few cases primarily as a client-side filtering language. As such, some of the cases you provided were not considered. In particular, defining a variable in-between quotes is interpreted as a literal string. I also never added support for nested objects.

I've considered it, but never had a reason to implement the rest of the spec.

If reasons open up then it might be worth spending a bit more time improving the grammar.

[ShaneHudson]

You do provide some support for bindings but not everything works (ie abc works but abc_def doesn’t). Do you currently have any way to just say it should look in the bindings? On 24 Jan 2023 at 5:28 PM +0000, Nick @.***>, wrote:

When I first added a CEL implementation I only did a few cases primarily as a client-side filtering language. As such, some of the cases you provided were not considered. In particular, defining a variable in-between quotes is interpreted as a literal string. I also never added support for nested objects. I've considered it, but never had a reason to implement the rest of the spec. If reasons open up then it might be worth spending a bit more time improving the grammar. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

[ShaneHudson]

Looking at the code, it looks like the problem I'm having could be solved by reordering the switch statement.

https://github.com/Fleker/cel-js/blob/master/src/formatters/TextFormatter.ts#L204

On like 204 you can see the variable check, but if the variable check came before any (without the error on undeclared) then any binding you provide would be taken before considering literals.

Would you consider a PR for this?

[Fleker]

I've tested everything in the tests. I can't say that anything else necessarily works.

I'd definitely welcome a pull request.

[ShaneHudson]

I haven't done this as a PR yet as it seems potentially specific to myself I'm not sure. But to solve my issue I essentially passed the bindings through the options into the Grammar so I can specify which variables/bindings are being expected. This does mean it either needs a stable object of bindings or to be instantiated each time.

In the CelSpecGrammar:

this.variable = options?.bindings
      ? m.choice(...Object.keys(options.bindings)).ast
      : m.seq(
          m.letterLower.oneOrMore,
          m.seq(m.letterUpper, m.letterLower.zeroOrMore).zeroOrMore,
          m.digits.zeroOrMore
        ).ast;

I've also got some tests which may make it make sense:

describe("bindings", () => {
  it("simple letter variable", () => {
    const expr = "123 <= x";
    const bindings = {
      x: 124,
    };
    const expected = {
      bool_value: true,
    };
    const cel = genCel(expr, bindings);
    expect(cel).toStrictEqual(expected);
  });

  it("letters with captialisation", () => {
    const expr = "123 <= xY";
    const bindings = {
      xY: 124,
    };
    const expected = {
      bool_value: true,
    };
    const cel = genCel(expr, bindings);
    expect(cel).toStrictEqual(expected);
  });

  it("letters with underscore", () => {
    const expr = "123 <= x_y";
    const bindings = {
      x_y: 124,
    };
    const expected = {
      bool_value: true,
    };
    const cel = genCel(expr, bindings);
    expect(cel).toStrictEqual(expected);
  });

  it("combination of letters and numbers", () => {
    const expr = "123 <= x123abc";
    const bindings = {
      x123abc: 124,
    };
    const expected = {
      bool_value: true,
    };
    const cel = genCel(expr, bindings);
    expect(cel).toStrictEqual(expected);
  });
});

[Fleker]

This is an interesting change and it seems a bit specific. There are already ways to define variables which follow a more standard variable name scheme.