Improving the Rust representer

Hello,

I made this PR to make the representer for Rust working. I didn’t change any normalization rule (except some little fixes) and so I want to open a brainstorm on what you be nice to add in the representer.

There is already a list of normalization that should be done but I think we can have even more.

If you are a non-rust developer you can still help us if you are aware of normalization made by an other language and we can discuss if it’s possible to add it to Rust.

For Rust aficionados, you can propose normalization that could be nice to implement here is a first list of what I think could be nice to implement :

  • A clippy pass (don’t know if it’s possible to run it from Rust code or we will have to use a binary) (that covers a lot of different normalization so I didn’t add them above)
  • Sort function in the file alphabetically by their function name
  • Remove comments
  • Generic names
  • Fully explicit type to match inference with explicit type (don’t know if it’s possible or it requires rewrite a bit of compiler xD)
  • Constraints on generics alphabetically ordered
    (I will update the list when I have more ideas)

@senekor do you think I should do 1 PR per new “representer new rule” or batch them in few weeks ?

Thanks for everyone that will take the time to participate

Sounds great!

do you think I should do 1 PR per new “representer new rule” or batch them in few weeks ?

I think @ErikSchierboom can answer this question better. Do representations only get rerun when the version is incremented? In that case, we could merge as many PRs as we want and only bump the version once at the end.

A few of the points I don’t understand yet:

A clippy pass

How does clippy factor into the representation of a solution?

Generic names

Do you mean something like?

fn foo<Foo>(f: Foo) {}
// turns into
fn foo<PLACEHOLDER_GENERIC_1>(f: PLACEHOLDER_GENERIC_1) {}

Fully explicit type to match inference with explicit type

Yeah, that sounds extremely hard. How would you figure out which type annotations are necessary and which aren’t? Actually perform Rust’s type inference?

I also think there are situations where

  • type annotations are not necessary for the problem to compile
  • reasonable people can disagree about whether to add a type annotation anyway or not.

Consider the following snippet. A redundant type annotation improves the locality of potential compiler errors in the future.

For me clippy will allow us to group solutions that are the same but one is written in a “non-idiomatic” way with the one approved by clippy. Maybe it’s too far in the normalization but I think this could be nice in this kind of situation for example : Clippy Lints

Yes !

I see what you mean and maybe I don’t fully understand well the role of the representer. For me the representer allow to group solutions that are not unique and, for me, solution with type explicit and inferred can be grouped (if it’s the only difference). But I think it might be too complex so anyway I don’t know if it worth the debate.

Thanks for all your answers

Yeah, I think we should consider the purpose of the representer for a moment.

As far as I understand, the representer has two main purposes:

  1. The community solutions page is not overflowing with identical solutions, those with equal representation are grouped.
  2. Mentors can give feedback on representations, which applies automatically to all solutions with that representation.

With these purposes in mind, I think we should be careful not to normalize two solutions where one may be considered better than the other.

For example, let’s say a student submits a solution which doesn’t adhere to clippy standards. but it get’s lumped together with ones that do, so mentors are never presented the worse version to give feedback on. (ignoring the fact that I’m planning to give automated clippy feedback via the analyzer)

On the community solutions side, recall that the first solution of those that share a representation will actually be shown to users. So, if the first solution doesn’t adhere to clippy lints, that’s actually the one that will be shown and those solutions which are better will be hidden from users.

With that in mind, I think we should only normalize solutions which obviously have no meaningful difference.

Ohhhh okey, didn’t know about this and so I totally agree with you so clippy and type explicit I will remove them from the list (I have added a little new one)

On a side note after thinking about what you explained, shouldn’t be useful to have a v2 of the representer that allow to match code that are similar but one more idiomatic have of kind of score and automatically show the solution that is similar to you but with a better score. I think it could be insane to learn better as it’s the same code as your solution but more idiomatic it’s not a complete different code, but that’s an other subject (if you have time to give you thoughts on that it would be nice)

It sounds equally cool and hard to implement :wink:

On some level, I would say this is exactly what clippy does, right? Independent of Exericsm. It takes some Rust code, and provides you with another solution that is equivalent, but slightly better. Other linters do this for other languages.

From that perspective, the most efficient approach seems to me to integrate linters in our analyzers.

Concerning the list item “Remove comments”, I think this is already implemented? Just judging based on this test case, without reading the source.

Yeah, you are true xD I think too difficult

Yes but it don’t test the comment in function etc… will add more test case

1 Like

Agreed/confirmed :slight_smile:

I have opened a PR for the improvements listed on this discussion : Add new formatting rules by AurelienFT · Pull Request #30 · exercism/rust-representer · GitHub