Skip to content

Conversation

@tibvdm
Copy link
Collaborator

@tibvdm tibvdm commented Oct 2, 2024

This pull requests:

  • Enables Heterogenous arguments for the Haskell language. Explicit typing in combination with ambiguous types, enables this future. This also allows for test plans using multiple types for the same function.
  • Adds parentheses around explicit typings. This fixes an issue where invalid syntax was generated due to this explicit typing (see issue Remove types from Haskell judging in TESTed #541, which is also solved by this PR)
  • Remove the explicit typing from the Dodona feedback, offering a clearer output to the user.

@tibvdm tibvdm requested a review from jorg-vr October 3, 2024 07:39
class Haskell(Language):
def __init__(self, config: Optional["GlobalConfig"]):
super().__init__(config)
self.clean_types_regex = re.compile(r"\(([^:\s]*)\s*::\s*([A-Z][a-zA-Z0-9]*)\)")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a constant instead of a variable,

So CLEAN_TYPES_REGEX =

Also is this regex completly save? Eg is it possible that it also matches a type expression within a string?

If typing is optional, it would probably be better not to add the type declarations in 'generators.py', instead of adding them there, running the tests with typing and then obfuscating those types to the end user.

# Handle some advanced types.
if value.type == AdvancedSequenceTypes.TUPLE:
assert isinstance(value, SequenceType)
return f"({convert_arguments(value.data)})"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where I would remove the typing information.

But i am not an expert on TESTed nor haskell so I am not sure it will work

Copy link
Contributor

@jorg-vr jorg-vr left a comment

Choose a reason for hiding this comment

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

see previous comments

@BrentBlanckaert BrentBlanckaert mentioned this pull request Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants