haskell-nix / hnix

A Haskell re-implementation of the Nix expression language
https://hackage.haskell.org/package/hnix
BSD 3-Clause "New" or "Revised" License
752 stars 115 forks source link

Please, refactor the module grouping #778

Open Anton-Latukha opened 3 years ago

Anton-Latukha commented 3 years ago
  1. There is some strange not really grouped code around REPL which seems halfway there, some of it was largely taken from Stephen Dehl, and code seems rarely touched and rarely used overall.

For example Nix.Type.* modules.

  1. Some modules should be renamed.
    • There is Nix.Expr.Types and Nix.Types, last should be renamed to something like Hindley-Milner, which they is/for.
    • There is Standart, Normal. Standart should be better explained/documented. Normal is Normalize, since it consists of normalization functions, maybe it belongs to Expr, maybe to some entity together with Reduce
    • Frames probably belongs to Render/
    • There should be a large group of modules that are ParserOREval: Parser, Exec, Eval, Builtins, Pretty, Fresh, Standard, Normal?, Reduce?, Lint?

Overall modules around executable/eval are not grouped - they should be. Better grouping allows:

Anton-Latukha commented 3 years ago

https://github.com/haskell-nix/hnix/issues/779#issuecomment-751281485

I think monorepo + split package could help in conjunction because then people that only need one part are not affected by breaking changes in other parts.

So the very first step - is to group modules on the directory/file level, that would logically form pre-packaging.

Anton-Latukha commented 3 years ago

Atoms belong to the Expr, since exprs are super to atoms. I do not imagine how to use language atoms without language around them (except but reimplementing the language).

Also I think that there is a big benefit to separating between itself the exoskeleton that is an executable implementation and the Nix expression parsing. Expressions are a separate package. Parsing of expressions and what is between executable and the expressions - are in the separate package. And the CLI executable that is an exoskeleton around all that - in the separate package.

That would allow to people to use Expr, or use the parsing logic and code for their own purposes without the code of the executable.

There of course also should be an extra package for side-stuff. And all that side-stuff depends on the language and internal language processing logic, but extra does not use executable, executable would require extra for some keys.

Anton-Latukha commented 3 years ago

Render and Frame are grouped, but there is no API use between Render and Frame. Frames & Frame and should be grouped together. I doubt Render and Frame &Frames are all one group, but may be grouped such.

Anton-Latukha commented 3 years ago

Made a draft of how modules group. 4 groups: Expr, Eval, Exec and Extra, where with to become a package. The structure of modules, also with respect of their dependencies looks like this:

Utils/Fix1.hs        <- Utils/Fix1.hs
Utils/Utils.hs       <- Utils.hs

Expr/Atoms.hs             <- Atoms.hs
Expr/Expr.hs              <- Expr.hs
Expr/String/Strings.hs    <- Expr/Strings.hs
Expr/Types/Types.hs       <- Expr/Types.hs

Parser/Types/Annotated.hs <- Expr/Types/Annotated.hs
Parser/Parser.hs          <- Parser.hs
Parser/Render.hs          <- Render.hs
Parser/TH.hs                <- TH.hs

Inference/Assumption.hs   <- Type/Assumption.hs
Inference/Env.hs          <- Type/Env.hs
Inference/Infer.hs        <- Type/Infer.hs
Inference/LICENSE         <- Type/LICENSE
Inference/Type.hs         <- Type/Type.hs

Thunk/Thunk.hs       <- Thunk.hs
Thunk/Fresh.hs       <- Fresh.hs

Value/Equal.hs       <- Value/Equal.hs
Value/Monad.hs       <- Value/Monad.hs
Value/Value.hs       <- Value.hs

Context/String.hs     <- String.hs
Context/Context.hs           <- Context.hs
Context/Scope.hs             <- Scope.hs
Context/Cited.hs       <- Cited.hs

MonadFileRender.hs     <- Render.hs
Exec/CacheFiles.hs             <- Cache.hs

Frames/Frame.hs     <- Frames.hs
Frames/Render.hs      <- Render/Frame.hs

Eval/Builtins.hs          <- Builtins.hs
Eval/Coerce.hs            <- String/Coerce.hs
Eval/Convert.hs           <- Convert.hs
Eval/Eval.hs              <- Eval.hs
Eval/Normal.hs            <- Normal.hs
Eval/Reduce.hs            <- Reduce.hs
Eval/Standard.hs          <- Standard.hs
Eval/Var.hs               <- Var.hs

Print/Pretty.hs            <- Pretty.hs

Exec/ThunkImplementation.hs       <- Thunk/Basic.hs
Exec/CitedImplementation.hs       <- Cited/Basic.hs
Exec/Nix.hs               <- Nix.hs
Exec/Exec.hs              <- Exec.hs
Exec/Effects/Basic.hs     <- Effects/Basic.hs
Exec/Effects/Effects.hs   <- Effects.hs
Exec/Fresh/Basic.hs       <- Fresh/Basic.hs

Bin/Options/Options.hs   <- Options.hs
Bin/Options/Parser.hs    <- Options/Parser.hs

Extra/Json.hs             <- Json.hs
Extra/Lint.hs             <- Lint.hs
Extra/XML.hs              <- XML.hs

Of course, some renaming, additional and minor changes may apply.

Of course, I already forgot how sadly the hierarchical module names and the directories are hardcode coupled. This old hard coupling design choice today seems a very poor design choice. I thought only to move files, but It is not possible to move an inch any file in the project without fully breaking&renaming the interface for the moved files for the project and everybody downstream.

So, probably this needs to be done at once with the package splitting (#328).

jwiegley commented 3 years ago

This all sounds pretty healthy to me, @Anton-Latukha .

Anton-Latukha commented 3 years ago

Some related recommendations:

https://www.haskellforall.com/2021/05/module-organization-guidelines-for.html?m=1