Which common checks should be running on every solutions?

[jiegillet]

Analyzer checks can be very useful for learning exercises, when you want to guide the student towards a recommended solution, but they can also be very useful for teaching about idiomatic code, community standards and that sort of things, across all exercises.

Here, I would like to discuss which checks would be worth implementing, and which checks would be too preachy. For example, the use of camelCase is so prevalent in Elm, that any other style should be commented on. On the other hand, you wouldn't want to ban <| even though |> is the preferred standard.

The standard analyzer tool in Elm is elm-review and people are encouraged to shared their rules as packages, so I searched for those and came up with this list. I already removed some that are too domain specific for Exercism (ports, JSON, HTML...) or too strict (forbid type alias constructors, ban always...) or some duplicates.

I don't know yet how these would get implemented, I just want to talk about which kind of checks we would like. For now, all of these are my opinion, I look forward to discussions to narrow the options down or add new ideas.

Legend: ✅ yes, 🚫 no, ❓ maybe, 🔍 investigate further.

Rule	Package	Add?	Comment
Provides elm-review rules to detect debug code	jfmengels/elm-review-debug	✅	No Debug in prod [Added in #68 ]
Provides elm-review rules to simplify Elm code	jfmengels/elm-review-simplify	✅	Things like not True --> False [Added in #29 ]
Provides elm-review rules to detect unused elements in your Elm project	jfmengels/elm-review-unused	✅	Removes dead code [Added in #29 ]
elm-review rule to ensure your code uses camelCase.	sparksp/elm-review-camelcase	✅	Strong standard [Added in #71]
elm-review rule to forbid certain words in comments.	sparksp/elm-review-forbidden-words	✅	Can be used to comment on -- TODO comments
Provide elm-review rules to follow some of my personal code style preferences	jfmengels/elm-review-code-style	❓	Minor (NoSimpleLetBody)
elm-review rules for working with the Unit type	mthadley/elm-review-unit	❓	Minor (always matches ())
A rule for elm-review that discourages long one-line Imports.	r-k-b/no-long-import-lines	❓	Tedious without elm-format
elm-review rules to ensure sortable code is sorted in the "proper" order.	SiriusStarr/elm-review-no-unsorted	❓	Tedious without elm-review --fix
elm-review rule enforce consistent import aliases.	sparksp/elm-review-imports	❓	NoInconsistentAliases not relevant, NoModuleOnExposedNames maybe
Forbid division operations that produce unwanted values or runtime exceptions	vkfisher/elm-review-no-unsafe-division	❓	Too demanding?
Provide an elm-review rule to measure the cognitive complexity of a function.	jfmengels/elm-review-cognitive-complexity	🔍	Confusing for the average student. Only for the hardest exercises?
Provides common linting rules for elm-review	jfmengels/elm-review-common	🔍	Some maybe (NoExposingEverything...) some no (NoMissingTypeAnnotationInLetIn...)
Provides elm-review rules to report performance problems in your Elm project	jfmengels/elm-review-performance	🔍	For performance-oriented exercises only? (nth-prime, palindrome-products)
Customizable elm-review rules for allowable pipeline styles.	SiriusStarr/elm-review-pipeline-styles	🚫	Too opinionated
Provides elm-review rules to help with the quality of the documentation	jfmengels/elm-review-documentation	🚫	Documentation quality is not really about language fluency
elm-review rule to enforce documentation for every top level declaration	ContaSystemer/elm-review-no-missing-documentation	🚫	Documentation quality is not really about language fluency
"elm-review" rule to forbid Regex package usage in favour of Parser package	ContaSystemer/elm-review-no-regex	🚫	elm/regex is available
Provides elm-review rules to forbid the use of import aliases	fysiweb/elm-review-no-import-as	🚫	Too strict
Provides elm-review rules to keep record fields and constructors sorted	fysiweb/elm-review-sorted	🚫	Alphabetical is not always the best order
Provides an elm-review rule to prohibit redundant usage of ++	truqu/elm-review-noredundantconcat	🚫	Covered by jfmengels/elm-review-simplify
Provides an elm-review rule to prohibit redundant usage of ::	truqu/elm-review-noredundantcons	🚫	Covered by jfmengels/elm-review-simplify
Provides elm-review rules to disallow single-pattern case expressions.	SiriusStarr/elm-review-no-single-pattern-case	🚫	Covered by jfmengels/elm-review-simplify

[ErikSchierboom]

Awesome list! Just in case you didn't know, we support different types of analyzer comments (see the docs):

• essential: We will soft-block students until they have addressed this comment
• actionable: Any comment that gives a specific instruction to a user to improve their solution
• informative: Comments that give information, but do not necessarily expect students to use it. For example, in Ruby, if > someone uses String Concatenation in TwoFer, we also tell them about String Formatting, but don't suggest that it is a better option.
• celebratory: Comments that tell users they've done something right, either as a general comment on the solution, or on a technique.

With these different types, one can have some comments be more like suggestions, whereas others are considered (soft-)blocking.

On the other hand, you wouldn't want to ban <| even though |> is the preferred standard.

Indeed. What you could do though is to have this be an informative command. Something that the student should be aware of, but doesn't necessarily need to change (BTW F# has this same guidance).

Looking at the above list, here are some thoughts:

Provides elm-review rules to keep record fields and constructors sorted

Is this something elm-format will/can do automatically?

Provide an elm-review rule to measure the cognitive complexity of a function.

I think I'd not implement this rule. Many students won't understand what this means I think. While it is good to know about cognitive complexity, I'm not sure this provides much benefit for the average student.

Provides common linting rules for elm-review

There are probably some rules that are fairly well-known and that could be of use to the student.

Provides elm-review rules to help with the quality of the documentation

I wouldn't implement this. Documentation quality is not really about language fluency (improving that is our goal at exercism) I feel.

Provides elm-review rules to report performance problems in your Elm project

I agree that this could be useful for some harder or some performance-oriented exercises (e.g. nth-prime or palindrome-products require a fairly efficient implementation).

elm-review rule to forbid certain words in comments.

According to your comment, you'd want to comment if you find a comment that starts with -- TODO?

elm-review rule enforce consistent import aliases.

What does this mean?

[jiegillet]

Thank you for the comment!

You're right, comment types provide an extra nuance that we could certainly use. For common checks like these though, I'm thinking there shouldn't be any essential or comments, mostly informative and possibly some actionable. I'm not sure we can have celebratory ones either.

Provides elm-review rules to keep record fields and constructors sorted

Is this something elm-format will/can do automatically?

No, elm-format doesn't do that, however I believe it can be done with elm-review --fix.

Provide an elm-review rule to measure the cognitive complexity of a function.

I think I'd not implement this rule. Many students won't understand what this means I think. While it is good to know about cognitive complexity, I'm not sure this provides much benefit for the average student.

You're right, the average student wouldn't get it. I was thinking that maybe we could use it just for some harder exercises, but I'm not too sure...

Provides common linting rules for elm-review

There are probably some rules that are fairly well-known and that could be of use to the student.

Yes, I think so too, although some in that package may not be that well known...

Provides elm-review rules to help with the quality of the documentation

I wouldn't implement this. Documentation quality is not really about language fluency (improving that is our goal at exercism) I feel.

Yes, you're probably right.

Provides elm-review rules to report performance problems in your Elm project

I agree that this could be useful for some harder or some performance-oriented exercises (e.g. nth-prime or palindrome-products require a fairly efficient implementation).

Good idea!

elm-review rule to forbid certain words in comments.

According to your comment, you'd want to comment if you find a comment that starts with -- TODO?

Yes, we have this one in the Elixir track. Another example for using this package (but not relevant to us) is to find empty markdown tickboxes -- - [ ] in the comments or on the README, I think it's a cool idea.

elm-review rule enforce consistent import aliases.

What does this mean?

It pushes users to always use the same import aliases (import Json.Decode as Decode) for the same packages across modules. This one is not too relevant because currently all exercises are implemented in a single module.
It also warns users if they used the full module path (Json.Decode.decodeString) rather than the alias (Decode.decodeString). I kinda like this one, but it might be a bit opinionated.

I'll edit my initial list from your feedback.

[ErikSchierboom]

Yes, we have this one in the Elixir track. Another example for using this package (but not relevant to us) is to find empty markdown tickboxes -- - [ ] in the comments or on the README, I think it's a cool idea.

Ah. I'm not opposed to it, just curious.

[jiegillet]

Just a thought: maybe there could be a mechanism for suppressing common checks for some specific exercises, like the simpler concept exercises.