Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions changelogs/unreleased/639-allow-iteration-over-dict-keys.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
description: Add support in the Inmanta DSL to iterate over dictionary keys in for loops.
issue-nr: 639
change-type: minor
destination-branches: [master, iso8]
sections:
minor-improvement: "{{description}}"
2 changes: 1 addition & 1 deletion src/inmanta/ast/statements/assign.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def execute_direct(self, requires: abc.Mapping[str, object]) -> object:

def _resolve(self, requires: dict[object, object], resolver: Resolver, queue: QueueScheduler) -> object:
"""
Create this list
Create this dict
"""
qlist = {}

Expand Down
4 changes: 2 additions & 2 deletions src/inmanta/ast/statements/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,8 @@ def execute(self, requires: dict[object, object], resolver: Resolver, queue: Que
super().execute(requires, resolver, queue)
var = self.base.execute(requires, resolver, queue)

if not isinstance(var, (list, Unknown)):
msg = "A for loop can only be applied to lists and relations."
if not isinstance(var, (list, Unknown, dict)):
msg = "A for loop can only be applied to lists, dictionaries and relations."
if isinstance(self.base, Reference):
msg += " Hint: '%s' resolves to '%s'." % (self.base, str(var))
else:
Expand Down
15 changes: 12 additions & 3 deletions src/inmanta/execute/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"""

from abc import abstractmethod
from collections.abc import Hashable, Sequence, Set
from collections.abc import Hashable, Mapping, Sequence, Set
from typing import TYPE_CHECKING, Deque, Generic, List, Literal, NewType, Optional, TypeVar, Union, cast

import inmanta.ast
Expand Down Expand Up @@ -81,15 +81,24 @@ def receive_result(self, value: T_contra, location: Location) -> bool:
"""
raise NotImplementedError()

def receive_result_flatten(self, value: Union[T_contra, Sequence[T_contra]], location: Location) -> bool:
def receive_result_flatten(
self, value: Union[T_contra, Sequence[T_contra], Mapping[T_contra, T]], location: Location
) -> bool:
"""
Receive one or more values for gradual execution. When a list is passed, its values will be passed to receive_result
one by one. Otherwise the value itself is passed unmodified.

:return: Whether this collector is complete, i.e. it does not need to receive any further results and its associated
waiter will no longer cause progress. Once this is signalled, this instance should get no further results.
"""
for subvalue in value if isinstance(value, list) else [value]:
if isinstance(value, list):
_iterator = value
elif isinstance(value, dict):
_iterator = value.keys() # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to type this

Copy link
Contributor

Choose a reason for hiding this comment

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

I see what you did here, but I don't feel that we should propagate the dicts this deeply. For one thing, dicts aren't always flattenable. While in the DSL we have no nested lists, we can have dicts inside lists. In other words [[1], [2]] is equivalent to [1, 2], but [{"a": "b"}] is not equivalent to {"a": "b"} or to ["a"].

I can imagine it may be difficult to understand the full implications of this, considering that this is quite an advanced method in a codebase you're only partially familiar with. The model below might help illustrate it. When compiled on master, it prints @{'a': 1, 'b': 2}. On your branch, it prints that, preceded with a and b. The fact that it prints both is actually due to the lack of a consistency safeguard in the for loop. The list comprehension does have it. If such a safeguard where there, the compiler would raise an exception instead. Either way, this is clearly not the intention.

d = {"a": 1, "b": 2}
for i in [d]:
    std::print(i)
end

All this said, what would be the appropriate location to handle this? I was going to say For.execute() , which is mostly just a type check, and GradualFor.receive_result. But as I type this I realize that GradualFor.receive_result will also need some way to determine whether it should or should not iterate any dict values it receives. And I don't have an immediate solution for it. I'll get back to this tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm starting to think that this will be a major hurdle, and that we should go for the plugin after all. We can discuss it tomorrow.

else:
_iterator = [value]

for subvalue in _iterator:
done: bool = self.receive_result(subvalue, location)
if done:
return True
Expand Down
29 changes: 26 additions & 3 deletions tests/compiler/test_loops.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_for_error(snippetcompiler):
implementation none for std::Entity:
end
""",
"A for loop can only be applied to lists and relations. Hint: 'a' resolves to "
"A for loop can only be applied to lists, dictionaries and relations. Hint: 'a' resolves to "
"'__config__::A (instantiated at {dir}/main.cf:6)'. (reported in "
"For(i) ({dir}/main.cf:7))",
)
Expand All @@ -64,7 +64,7 @@ def test_for_error_2(snippetcompiler):
for i in "foo":
end
""",
"A for loop can only be applied to lists and relations. Hint: 'foo' is not a "
"A for loop can only be applied to lists, dictionaries and relations. Hint: 'foo' is not a "
"list. (reported in For(i) ({dir}/main.cf:2))",
)

Expand All @@ -82,7 +82,7 @@ def test_for_error_nullable_list(snippetcompiler):
std::print(element)
end
""",
"A for loop can only be applied to lists and relations. "
"A for loop can only be applied to lists, dictionaries and relations. "
"Hint: 'a.elements' resolves to 'null'. (reported in For(element) ({dir}/main.cf:8))",
ministd=True,
)
Expand Down Expand Up @@ -234,3 +234,26 @@ def test_for_loop_fully_gradual(snippetcompiler):
ministd=True,
)
compiler.do_compile()


def test_dict_keys_iteration(snippetcompiler, capsys):

snippetcompiler.setup_for_snippet(
"""
my_d = {
"B": "F",
"A": "O",
"R": "O"
}
for i in my_d:
std::print(i)
end
""",
autostd=True,
)

capsys.readouterr()
compiler.do_compile()
out, _ = capsys.readouterr()
output = out.strip()
assert output == "\n".join([str(x) for x in "BAR"])