🐛 fix contains on nil dicts (#2720)

Any dict entry can be `null`, which means that subsequent calls to its
fields can lead to unexpected behavior. Such is the case of `contains`.

This call makes a lot of sense on top of e.g. a string array:

```coffee
["a", "b"].contains("b")
```

But if the structure doesn't exist, then the call fails in unexpected
ways:

```
left side of operation is null
```

This approach still uses an old way of addressing the problem, which
looks quite a bit ugly in the implementation. We have to check for
non-nil and for non-zero length. Once v10 comes around, we can rely on
the `empty` keyword and all this is reduced to just the internal
non-empty check.

Signed-off-by: Dominik Richter <[email protected]>

.github/actions/spelling/expect.txt

@@ -42,6 +42,7 @@ mgroup
 Mpim
 nodepool
 nullgroup
+nullstring
 opcplc
 Pids
 postgre

mql/mql_test.go

@@ -219,3 +219,25 @@ func TestNullResources(t *testing.T) {
 		},
 	})
 }
+
+func TestNullString(t *testing.T) {
+	x := testutils.InitTester(testutils.LinuxMock())
+	x.TestSimple(t, []testutils.SimpleTest{
+		{
+			Code:        "muser.nullstring.contains('123')",
+			ResultIndex: 0,
+			Expectation: false,
+		},
+	})
+}
+
+func TestDictMethods(t *testing.T) {
+	x := testutils.InitTester(testutils.LinuxMock())
+	x.TestSimple(t, []testutils.SimpleTest{
+		{
+			Code:        "muser.dict.nonexisting.contains('abc')",
+			ResultIndex: 1,
+			Expectation: false,
+		},
+	})
+}

mqlc/builtin_map.go

@@ -137,19 +137,50 @@ func compileDictContains(c *compiler, typ types.Type, ref uint64, id string, cal
 			Binding: c.tailRef(),
 		},
 	})
+	lengthRef := c.tailRef()
 
-	// > 0
+	// FIXME: DEPRECATED, replace in v10.0 wit the use of != empty vv
+	// != 0
 	c.addChunk(&llx.Chunk{
 		Call: llx.Chunk_FUNCTION,
-		Id:   string(">" + types.Int),
+		Id:   string("!=" + types.Int),
 		Function: &llx.Function{
 			Type:    string(types.Bool),
-			Binding: c.tailRef(),
+			Binding: lengthRef,
 			Args: []*llx.Primitive{
 				llx.IntPrimitive(0),
 			},
 		},
 	})
+	neq0Ref := c.tailRef()
+
+	// != null
+	c.addChunk(&llx.Chunk{
+		Call: llx.Chunk_FUNCTION,
+		Id:   string("!=" + types.Nil),
+		Function: &llx.Function{
+			Type:    string(types.Bool),
+			Binding: ref,
+			Args: []*llx.Primitive{
+				// Note: we need this for backwards compatibility because labels
+				// require 1 argument on < v9.1
+				llx.NilPrimitive,
+			},
+		},
+	})
+	neqNullRef := c.tailRef()
+
+	c.addChunk(&llx.Chunk{
+		Call: llx.Chunk_FUNCTION,
+		Id:   string("&&" + types.Bool),
+		Function: &llx.Function{
+			Type:    string(types.Bool),
+			Binding: neqNullRef,
+			Args: []*llx.Primitive{
+				llx.RefPrimitiveV2(neq0Ref),
+			},
+		},
+	})
 
 	checksum := c.Result.CodeV2.Checksums[c.tailRef()]
 	c.Result.Labels.Labels[checksum] = "[].contains()"

providers-sdk/v1/testutils/mockprovider/resources/all.go

@@ -27,6 +27,11 @@ func (c *mqlMuser) nullgroup() (*mqlMgroup, error) {
 	return nil, nil
 }
 
+func (c *mqlMuser) nullstring() (string, error) {
+	c.Nullstring.State = plugin.StateIsSet | plugin.StateIsNull
+	return "", nil
+}
+
 func (c *mqlMuser) groups() ([]interface{}, error) {
 	one, err := CreateResource(c.MqlRuntime, "mgroup", map[string]*llx.RawData{
 		"name": llx.StringData("group one"),
@@ -43,3 +48,10 @@ func (c *mqlMuser) groups() ([]interface{}, error) {
 func (c *mqlMgroup) id() (string, error) {
 	return c.Name.Data, nil
 }
+
+func (c *mqlMuser) dict() (any, error) {
+	return map[string]any{
+		"a1": []any{int64(1), int64(2), int64(3)},
+		"s1": "hello world",
+	}, nil
+}

providers-sdk/v1/testutils/mockprovider/resources/mockprovider.lr

@@ -8,7 +8,9 @@ muser {
   name string
   group() mgroup
   nullgroup() mgroup
+  nullstring() string
   groups() []mgroup
+  dict() dict
 }
 
 mgroup {

providers/os/resources/mql_test.go

@@ -261,37 +261,37 @@ func TestDict_Methods_Contains(t *testing.T) {
 	x.TestSimple(t, []testutils.SimpleTest{
 		{
 			Code:        p + "params['hello'].contains('ll')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['hello'].contains('lloo')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 		{
 			Code:        p + "params['hello'].contains(['xx','he'])",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['hello'].contains(['xx'])",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 		{
 			Code:        p + "params['string-array'].contains('a')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['string-array'].containsOnly(['c', 'a', 'b'])",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['string-array'].containsOnly(['a', 'b'])",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 		// {
@@ -300,37 +300,37 @@ func TestDict_Methods_Contains(t *testing.T) {
 		// },
 		{
 			Code:        p + "params['string-array'].containsNone(['d','e'])",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['string-array'].containsNone(['a', 'e'])",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 		{
 			Code:        p + "params['string-array'].none('a')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 		{
 			Code:        p + "params['string-array'].contains(_ == 'a')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['string-array'].none(_ == /a/)",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 		{
 			Code:        p + "params['string-array'].contains(value == 'a')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: true,
 		},
 		{
 			Code:        p + "params['string-array'].none(value == 'a')",
-			ResultIndex: 1,
+			ResultIndex: 2,
 			Expectation: false,
 		},
 	})
@@ -346,6 +346,11 @@ func TestDict_Methods_Map(t *testing.T) {
 
 	x := testutils.InitTester(testutils.LinuxMock())
 	x.TestSimple(t, []testutils.SimpleTest{
+		{
+			Code:        p + "params.nonexistent.contains('sth')",
+			Expectation: false,
+			ResultIndex: 1,
+		},
 		{
 			Code:        p + "params['string-array'].where(_ == 'a')",
 			Expectation: []interface{}{"a"},