No problem and thanks for getting it across the line!

While all the tests from the Python translation where there, they were in the wrong order (under the wrong test names). All of this has been rectified now.

This updates the tests to not mandate a specific error type; only implementing Debug is now required.

I've taken care of the last details in a fork and approved the translation.

These are the changes I made:

• cleaned up with rustfmt and clippy
• removed source fieldl from RSUProgram struct in the setup, since it is not part of the mandatory interface.
• reorganized tests to put these before the generator code
• inserted missed Messer::Expr(ExprMesser::Cmd) entry in the compiler faults profile
• made test case lists const, and accept slices instead of Vec references for profiles
• added clarifing messages to assert_eq! calls.

This is an amazing piece of work, however, thank you so very much for all the hard work!

Wow, thank you! To be honest, I didn't expect this to actually get reviewed, so I appreciate you taking the time to go through it. I should have time to address these issues in the next week or so.

Sorry for the huge delays but I've finally managed to give this the full review it deserves. I think only a few minor issues remain!

I've grouped my comments into sections:

Clippy findings

The following definitions are not used by anything and can go:

• The solver.TokenScope struct
• The generator.ast_to_string() function
• The messer.ExprMesser.Cmd enum variant

Error type in solutions

Can we change the result returned for errors more flexible? In my Rust translation for the previous RoboScript kata I only require that the error type E in Result<.., E> implements std::fmt::Display. You can't quite express that as a trait bound but I documented it in the setup as follows:

// You can augment the error type any way you like, and it doesn't matter
// what it is named. The only thing that does matter is that it _must_
// implement the std::fmt::Display trait. E.g. you are free to use
// `std::result::Result<String, &str>` if you feel that that is
// easier.
type RS3Result<T> = std::result::Result<T, RS3Error>;

pub struct RS3Error;

impl std::fmt::Display for RS3Error {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "Something went wrong!")
    }
}

and the tests then do use trait bounds:

fn assert_path_equals<E>(actual: Result<String, E>, expected: Result<&str, &str>)
where
    E: std::fmt::Display,
{
    // ...
}

where actual is a result from the solution-under-test to be compared to expected; the &str in the expected result is there purely as an indicator of what kind of error the solution could have raised when an Ok() result was produced. match patterns then separate out testing Ok() results from when an Err() is produced, Err values are never compared directly.

Assertion messages

The Rust assert_* messages are not clear about what value is the expected and what is the actual value, so we always provide a custom message.

So, don't do this:

assert_eq!(
    RSUProgram::new(case.0).get_tokens(),
    Ok(case.1.iter().map(|&s| s.into()).collect())
);

and instead do this:

assert_eq!(
    RSUProgram::new(case.0).get_tokens(),
    Ok(case.1.iter().map(|&s| s.into()).collect()),
    "\nYour result (left) did not match the expected output (right)"
);

Overall impression

I think this is almost ready to go live. If you no longer feel it is worth your time to address the above minor issues, I can do these too in a new fork of your translation. Just let me know!

I don't see it as a duplicate as having to find 2 numbers adds complexity, and you are correct in that my intention was for naive solutions to be too slow. I've clearly missed the mark, so I'll rework it and try again.

Indeed, I tried other naive solutions but missed the most obvious of all - using set operations. Looks like I need to increase the amount of tests.

Happy to receive any feedback/criticism. Once the kata is stable, I'll translate to other languages (Rust, C, C++, JS, TS).

Honestly I've not thought about this in close to 2 years. Last I recall it was ready to publish pending review per my most recent comment, however nobody who is fluent in Rust was willing to review it. I wrote all of this when I was new to Rust so it likely needs a good once over to make sure I didn't make any goofy mistakes, particularly in the AST/random test generation.

What is the status now, 2 years on?

Resolved merge conflicts in description. I hope no earlier description updates got reverted. If so, let me know.

Merged description conflicts and approved a fork. Full translation honour/ranking points go to you as original author of the translation.

I would of course love to solve in python as well, however it seems to me that time has passed. This kata is practically 10 years old and now IMHO, it is not 1kuy. Maybe you should just do something based on this task, but heavily refined (I mean a completely new publication)? For example, make it a performance task, add: JOIN, LEFT JOIN, RIGHT JOIN, DISTINCT, CASE, EXISTS.... These are just as examples of what could be implemented and improve this kata.

By the way as a performance option, if there will be large data volumes and different sized tables, the right JOIN will make a difference. If you do it nicely and add significant differences from this kata, I think it would be a great new kata.

p.s. I wonder if anyone has responded on Discord though? What kind of response did you get?