Linting integration

Hecate commented

2020-05-19 12:47:11 +02:00

Contributor

This commit brings in an HLint configuration file
and several recommended modifications such as:

End-of-line extra spaces removal;
Import lines ordering;
Redundant ($) removal;
Generalisation of (++) and map to (<>) and fmap;
Preferring pure over return;
Removing extraenous extensions.

And finally, a stylish-haskell helper script
that detects if code files are dirty. Can be useful for CI,
although manually calling it can be nice if you would rather
first implement then beautify.

Regarding the imports, I am of the school that says “non-local imports are noise, put them on top, and keep the local imports as close as your code as possible”.
I am not here to preach stuff, but I thought you'd like to know that this is “A Thing”.

Cheers!

This commit brings in an HLint configuration file and several recommended modifications such as: * End-of-line extra spaces removal; * Import lines ordering; * Redundant `($)` removal; * Generalisation of `(++)` and `map` to `(<>)` and `fmap`; * Preferring `pure` over `return`; * Removing extraenous extensions. And finally, a `stylish-haskell` helper script that detects if code files are dirty. Can be useful for CI, although manually calling it can be nice if you would rather first implement then beautify. Regarding the imports, I am of the school that says “non-local imports are noise, put them on top, and keep the local imports as close as your code as possible”. I am not here to preach stuff, but I thought you'd like to know that this is “A Thing”. Cheers!

🚀 1

schmittlauch commented

2020-05-19 13:16:50 +02:00

Owner

Thanks for these opinionated insights, they're a good starting point for Haskell newbies like me.

I need some clarifications though:

Import lines ordering;

Regarding the imports, I am of the school that says “non-local imports are noise, put them on top, and keep the local imports as close as your code as possible”.

So you're basically recommending to put imports of haskell platform and other libraries first, and the module imports from the same package should appear last? I already tried to follow that rule, but may not have been consistent.
What is the import oordering you suggest? base ==> haskell-platform ==> package local imports?

Preferring pure over return;

Why should pure be preferred over return in general? I always tried to use both of them depending on the context: When writing applicative-style functions, I used pure to make the applicative nature of the functions obvious, when messing around inside monads (e.g. with do notation) I used return to highlight the monad stuff, even when `pure could've been enough (example: Maybe).

Thanks for these opinionated insights, they're a good starting point for Haskell newbies like me. I need some clarifications though: > - Import lines ordering; > Regarding the imports, I am of the school that says “non-local imports are noise, put them on top, and keep the local imports as close as your code as possible”. So you're basically recommending to put imports of haskell platform and other libraries first, and the module imports from the same package should appear last? I already tried to follow that rule, but may not have been consistent. What is the import oordering you suggest? base ==> haskell-platform ==> package local imports? > - Preferring pure over return; Why should pure be preferred over return in general? I always tried to use both of them depending on the context: When writing applicative-style functions, I used `pure` to make the applicative nature of the functions obvious, when messing around inside monads (e.g. with `do` notation) I used `return` to highlight the monad stuff, even when `pure could've been enough (example: Maybe).

Hecate commented

2020-05-19 13:32:02 +02:00

Author

Contributor

What is the import oordering you suggest?

Two separated blocks for non-hash2pub modules and hash2pub modules.
Whether or not they belong to the Haskell platform is irrelevant:
They will get ordered alphabetically whilst staying in their lane.

See:

import Control.Monad
import Data.Time

import Hash2Pub.ASN1Coding
import Hash2Pub.FediChord

Why should pure be preferred over return in general? I always tried to use both of them depending on the context: When writing applicative-style functions, I used pure to make the applicative nature of the functions obvious, when messing around inside monads (e.g. with do notation) I used return to highlight the monad stuff,
even when pure could've been enough (example: Maybe).

This is an interesting point, and I see why you do that. However, there are two benefits, on two different dimensions, for preferring pure to return:

return has a strong, strong history of being a keyword in imperative languages to send back the value produced by the function, and this semantic overlap has been described as, at best confusing, as worse outright dangerous because newcomers start to ascribe some kind of magic properties.
The “Monad of no Return” proposal by Ben Gamari explains in length why return is mostly an artifact of the past:

Consequently, the Monad class is left with a now redundant return
method as a historic artifact, as there's no compelling reason to
have pure and return implemented differently.
More to the point, this redundancy violates the
"making illegal states unrepresentable" idiom: Due to the default
implementation of return this redundancy leads to
error-prone situations which aren't caught by the compiler; for instance, when
return is removed while the Applicative instance is left with a
pure = return definition, this leads to a cyclic definition which
can be quite tedious to debug as it only manifests at runtime by a
hanging process.
Traditionally, return is often used where pure would suffice
today, forcing a Monad constraint even if a weaker Applicative
would have sufficed. As a result, language extensions like ApplicativeDo[3] have to
rewrite return to weaken its Monad m => constraint to
Applicative m => in order to benefit existing code at the cost
of introducing magic behavior at the type level.

>What is the import oordering you suggest? Two separated blocks for non-hash2pub modules and hash2pub modules. Whether or not they belong to the Haskell platform is irrelevant: They will get ordered alphabetically whilst staying in their lane. See: ```haskell import Control.Monad import Data.Time import Hash2Pub.ASN1Coding import Hash2Pub.FediChord ``` --- >Why should pure be preferred over return in general? I always tried to use both of them depending on the context: When writing applicative-style functions, I used pure to make the applicative nature of the functions obvious, when messing around inside monads (e.g. with do notation) I used return to highlight the monad stuff, >even when `pure` could've been enough (example: Maybe). This is an interesting point, and I see why you do that. However, there are two benefits, on two different dimensions, for preferring `pure` to `return`: * `return` has a strong, strong history of being a keyword in imperative languages to send back the value produced by the function, and this semantic overlap has been described as, at best confusing, as worse outright dangerous because newcomers start to ascribe some kind of magic properties. * The [“*Monad of no Return*”](https://gitlab.haskell.org/ghc/ghc/-/wikis/proposal/monad-of-no-return#current-situation) proposal by Ben Gamari explains in length why `return` is mostly an artifact of the past: >**Consequently, the Monad class is left with a now redundant return method as a historic artifact, as there's no compelling reason to have pure and return implemented differently.** More to the point, this redundancy violates the "making illegal states unrepresentable" idiom: Due to the default implementation of return this redundancy leads to error-prone situations which aren't caught by the compiler; for instance, when return is removed while the Applicative instance is left with a pure = return definition, this leads to a cyclic definition which can be quite tedious to debug as it only manifests at runtime by a hanging process. Traditionally, return is often used where pure would suffice today, forcing a Monad constraint even if a weaker Applicative would have sufficed. As a result, language extensions like ApplicativeDo[3] have to rewrite return to weaken its Monad m => constraint to Applicative m => in order to benefit existing code at the cost of introducing magic behavior at the type level.

schmittlauch referenced this pull request from a commit

2020-05-19 16:36:00 +02:00

Merge pull request 'Linting integration' (#17) from Hecate/Hash2Pub:hlint-configuration into master Introduce some code formatting guidelines and enforce them with hlint and stylish: - End-of-line extra spaces removal; - Import lines ordering; - Redundant ($) removal; - Generalisation of (++) and map to (<>) and fmap; - Preferring pure over return; - Removing extraenous extensions.

schmittlauch closed this pull request

2020-05-19 16:36:00 +02:00

schmittlauch commented

2020-05-23 17:53:29 +02:00

Owner

And finally, a stylish-haskell helper script that detects if code files are dirty. Can be useful for CI, although manually calling it can be nice if you would rather first implement then beautify.

@hecate I do not fully understand what's the best way of using stylish-haskell in my workflow:

On the one hand I'm too lazy for calling the tool itself for each single file of my project.
When using the helper script, I usually commit my changes first, then run the heloer script and --amend the formatting changes to the previous commit. This only works though when always committing all changes from the repo, not only those of a single file or even a chosen patch set.

Why does the helper script care about a dirty working directory?

> And finally, a stylish-haskell helper script that detects if code files are dirty. Can be useful for CI, although manually calling it can be nice if you would rather first implement then beautify. @hecate I do not fully understand what's the best way of using `stylish-haskell` in my workflow: On the one hand I'm too lazy for calling the tool itself for each single file of my project. When using the helper script, I usually commit my changes first, then run the heloer script and `--amend` the formatting changes to the previous commit. This only works though when always committing all changes from the repo, not only those of a single file or even a chosen patch set. Why does the helper script care about a dirty working directory?

schmittlauch deleted branch hlint-configuration

2020-05-23 17:53:51 +02:00

Hecate commented

2020-05-23 17:56:15 +02:00

Author

Contributor

@hecate I do not fully understand what's the best way of using stylish-haskell in my workflow:

I have an editor plugin that calls it for the current buffer that I am saving.

The helper script can be run in a CI action or a pre-commit git hook (very useful, I also use that for hlint).

> @hecate I do not fully understand what's the best way of using `stylish-haskell` in my workflow: I have an editor plugin that calls it for the current buffer that I am saving. The helper script can be run in a CI action or a pre-commit git hook (very useful, I also use that for hlint).

schmittlauch commented

2020-05-23 17:57:59 +02:00

Owner

The helper script can be run in a CI action or a pre-commit git hook (very useful, I also use that for hlint).

If run as a pre-commit hook, wouldn't it always complain about a dirty working directory then as before a commit changes aren't commited yet?

> The helper script can be run in a CI action or a pre-commit git hook (very useful, I also use that for hlint). If run as a pre-commit hook, wouldn't it always complain about a dirty working directory then as *before* a commit changes aren't commited yet?

Hecate commented

2020-05-23 18:04:06 +02:00

Author

Contributor

If run as a pre-commit hook, wouldn't it always complain about a dirty working directory then as before a commit changes aren't commited yet?

It will! Sorry for the confusion 😵

What I meant to say is that the script uses the in-place editing feature of stylish-haskell to then detect files that need to be passed through by checking the git status of those files (since they were edited by stylish-haskell, they are now unstaged).

But the important thing here is that since it returns 1 as an exit code, it will abort the commit step and let you apply stylish-haskell on the incriminated files.

Am I clear enough? I haven't had much sleep recently…

>If run as a pre-commit hook, wouldn't it always complain about a dirty working directory then as before a commit changes aren't commited yet? It will! Sorry for the confusion :dizzy_face: What I meant to say is that the script uses the in-place editing feature of `stylish-haskell` to then detect files that need to be passed through by checking the `git status` of those files (since they were edited by `stylish-haskell`, they are now unstaged). But the important thing here is that since it returns 1 as an exit code, it will abort the commit step and let you apply `stylish-haskell` on the incriminated files. Am I clear enough? I haven't had much sleep recently…

schmittlauch commented

2020-05-31 22:09:25 +02:00

Owner

Am I clear enough? I haven't had much sleep recently…

@Hecate Honestly, no. But maybe you had time to catch up on sleep?

I haven't set it up as a pre-commit hook yet as I don't fully understand it, but running it manually before a commit does only complain about unclean working state.

But the important thing here is that since it returns 1 as an exit code, it will abort the commit step and let you apply stylish-haskell on the incriminated files.

Do you mean that I then manually shall invoke stylish-haskell -i on each Haskell file in the repo?
Couldn't I just let the pre-commit script make that do automatically, or are there edge cases where stylish messes up the not-yet-commited changes (e.g. syntax errors, incomplete implementations)?

> Am I clear enough? I haven't had much sleep recently… @Hecate Honestly, no. But maybe you had time to catch up on sleep? I haven't set it up as a pre-commit hook yet as I don't fully understand it, but running it manually before a commit does only complain about unclean working state. > But the important thing here is that since it returns 1 as an exit code, it will abort the commit step and let you apply `stylish-haskell` on the incriminated files. Do you mean that I then manually shall invoke `stylish-haskell -i` on each Haskell file in the repo? Couldn't I just let the pre-commit script make that do automatically, or are there edge cases where stylish messes up the not-yet-commited changes (e.g. syntax errors, incomplete implementations)?

Hecate commented

2020-05-31 22:24:59 +02:00

Author

Contributor

I haven't set it up as a pre-commit hook yet as I don't fully understand it, but running it manually before a commit does only complain about unclean working state.

Then maybe the workflow of this script isn't suited for you. Which editor are you using?

Do you mean that I then manually shall invoke stylish-haskell -i on each Haskell file in the repo? Couldn't I just let the pre-commit script make that do automatically,

NOPE! If you do so, stylish-haskell will edit the files in-place, then return the return code 0, then you'll be prompted to enter the commit's message, which won't include the edited files, because you couldn't add them in the meantime. Basically, you'll have to commit a fixup afterwards.

syntax errors, incomplete implementations)

stylish-haskell will most definitely stumble on syntax errors. My vim bindings apply it each time I save the file, so it gets formatted in-place.

>I haven't set it up as a pre-commit hook yet as I don't fully understand it, but running it manually before a commit does only complain about unclean working state. Then maybe the workflow of this script isn't suited for you. Which editor are you using? >Do you mean that I then manually shall invoke stylish-haskell -i on each Haskell file in the repo? Couldn't I just let the pre-commit script make that do automatically, NOPE! If you do so, stylish-haskell will edit the files in-place, then return the return code 0, then you'll be prompted to enter the commit's message, which won't include the edited files, because you couldn't add them in the meantime. Basically, you'll have to commit a fixup afterwards. >syntax errors, incomplete implementations) stylish-haskell will most definitely stumble on syntax errors. My vim bindings apply it each time I save the file, so it gets formatted in-place.

Linting integration #17