lue-bird/elm-review-mini/1.0.1

Review

A review inspects modules, elm.json, dependencies and extra files like README.md from your project and uses the combined knowledge to report problems.

ignoreErrorsForPathsWhere : (String -> Bool) -> Review -> Review

Specify where you don't want errors to be reported in:

  • an external library you copied over and don't want to modify more than necessary
  • generated source code which isn't supposed to be read and or over which you have little control
  • you wrote a review that is very specific and should only be applied for a portion of your codebase

tests/ is never inspected

Everything below is intended for writing a new review

{ inspect : List (Inspect knowledge)
, knowledgeMerge : knowledge -> knowledge -> knowledge
, report : knowledge -> List Error
}
-> Review

First, read "when to write or enable a review". To write a new Review you need to

  • inspect: the parts you want to scan to collect knowledge from. See section inspecting
  • knowledgeMerge: assemble knowledge from multiple you've collected into one
  • report: the final evaluation, turning your combined knowledge into a list of errors

Write tests with Review.Test alongside your implementation and document your review, explaining when (not) to enable the review and adding examples of patterns that will (not) be reported

inspecting

Collect knowledge from parts of the project.

( { source : String
, project : Project
}
-> knowledge
)
-> Inspect knowledge

Collect knowledge from the elm.json project config. You can use the raw source to search for a section you want to highlight in an Error

( List
{ elmJson : Project
, modules : List Module
}
-> knowledge
)
-> Inspect knowledge

Collect knowledge from all project configs and module docs the project directly depends on

( { path : String
, source : String
}
-> knowledge
)
-> Inspect knowledge

Collect knowledge from an available files like README.md or CHANGELOG.md that aren't source modules or the elm.json. The provided path is relative to the project's elm.json and can be used as an Error target file path

( { syntax : File
, source : String
, path : String
}
-> knowledge
)
-> Inspect knowledge

Collect knowledge from a module file.

The path is relative to the project's elm.json and can be used as an Error target file path.

The raw elm file source can be used to preserve existing formatting in a range in case you want to move and copy things around, see sourceExtractInRange.

The syntax field contains a tree-like structure which represents your source code: elm-syntax File. I recommend having the documentation for that package open while writing a review.

syntax.moduleDefinition

The module header (module SomeModuleName exposing (a, b)).

syntax.declarations

The module's declaration statements (someVar = add 1 2, type Bool = True | False, port output : Json.Encode.Value -> Cmd msg).

syntax.comments

  • Module documentation ({-| -})
  • Port documentation comments ({-| -})
  • comments ({- -} and --)

provided in source order. Documentation comments ({-| -}) of value/function/choice type/type alias declarations can be found directly in their syntax data.

If you only need the module documentation, use moduleHeaderDocumentation.

syntax.imports

The module's import statements (import Html as H exposing (div)) in order of their definition. A nice helper to get the exposes: topLevelExposeListToExposes

An example review that forbids importing both Element (elm-ui) and Html.Styled (elm-css) in the same module:

import Elm.Syntax.Import
import Elm.Syntax.Node
import Elm.Syntax.Range
import Review
import FastDict


review : Review.Review
review =
    Review.create
        { inspect =
            [ Review.inspectModule
                (\moduleData ->
                    let
                        importedModuleNames : Dict Elm.Syntax.ModuleName.ModuleName Elm.Syntax.Range.Range
                        importedModuleNames =
                            moduleData.syntax.imports
                                |> List.map
                                    (\(Elm.Syntax.Node.Node _ import_) ->
                                        ( import.moduleName |> Elm.Syntax.Node.value, import.moduleName |> Elm.Syntax.Node.range )
                                    )
                                |> FastDict.fromList
                    in
                    case
                        ( importedModuleNames |> Dict.get [ "Element" ]
                        , importedModuleNames |> Dict.get [ "Html", "Styled" ]
                        )
                    of
                        ( Just elementImportRange, Just _ ) ->
                            FastDict.singleton moduleData.path elementImportRange


                    else
                        FastDict.empty
                )
            ]
        , knowledgeMerge = \a b -> FastDict.union a b
        , report =
            \invalidImportsByPath ->
                invalidImportsByPath
                    |> FastDict.toList
                    |> List.map
                        (\( path, elementImportRange ) ->
                            { path = moduleData.path
                            , message = "both `elm-ui` and `elm-css` used"
                            , details = [ "At fruits.com, we use `elm-ui` in the dashboard application, and `elm-css` in the rest of the code. We want to use `elm-ui` in our new projects, but in projects using `elm-css`, we don't want to use both libraries to keep things simple." ]
                            , range = elementImportRange
                            , fix = []
                            }
                        )
        }

I strongly recommend FastDict for your writing your review instead of Dict because Dict.union and friends have performance footguns you shouldn't need to worry about

reporting

type alias Error =
{ path : String
, range : Range
, message : String
, details : List String
, fix :
List
{ path : String
, edits : List SourceEdit
}
}

A problem to report in a given file and range, possibly suggesting a fix with SourceEdits

Take the elm compiler errors as inspiration in terms of helpfulness:

  • error message: half-sentence on what is undesired here. A user that has encountered this error multiple times should know exactly what to do. Example: "`` is never used" → a user who knows the rule knows that a function can be removed and which one
  • error details: additional information such as the rationale and suggestions for a solution or alternative
  • error report range: The region marked as problematic. Make this section as small as possible. For instance, in a rule that would forbid Debug.log, you would want the error to appear at Debug.log, not on the whole function call
elmJsonPath : String

Relative path to the project elm.json to be used in an Error or expected test error. You could also just use "elm.json" which feels a bit brittle.

= SourceRangeReplacement
{ range : Range
, replacement : String
}

A single edit to be applied to a file's source in order to fix a review error

insertAt : Location -> String -> SourceEdit

Insert something at the given position

Remove the section in between a given range

replaceRange : Range -> String -> SourceEdit

Replace the section in between a given range by something

If you want to insert bigger pieces of dynamic elm code, I recommend either using the-sett/elm-syntax-dsl or mdgriffith/elm-codegen

convenience helpers

Various helpers for elm-syntax and other project data that is often useful when writing a review.

For example, by only listing all immediate sub-parts of a piece of syntax you can choose to traverse it however you need to. E.g. to find all as pattern ranges

findAllAsPatternRanges :
    Elm.Syntax.Node.Node ELm.Syntax.Pattern.Pattern
    -> List ELm.Syntax.Range.Range
findAllAsPatternRanges =
    \patternNode ->
        (case patternNode of
            Elm.Syntax.Node.Node asRange (Elm.Syntax.Pattern.AsPattern _ _) ->
                [ asRange ]


            _ ->
                []
        )
            ++ (patternNode |> Review.patternSubs |> List.concatMap findAllAsPatternRanges)

This fine control allows you to e.g. skip visiting certain parts that you already accounted for.

expressionSubs : Node Expression -> List (Node Expression)

All surface-level child expressions.

If you also want to record the newly available variables by child expression, use expressionSubsWithBindings

Node Expression
-> List
{ expressionNode : Node Expression
, bindings : List String
}

Like expressionSubs with additional info about newly available variables from patterns and let declared value/function names which can be used to for example disambiguate variables you plan to introduce yourself through a fix

patternBindings : Node Pattern -> List String

Recursively find all introduced variables in the pattern (like a and b in ( Just a, { b } ))

patternSubs : Node Pattern -> List (Node Pattern)

All surface-level child patterns

typeSubs : Node TypeAnnotation -> List (Node TypeAnnotation)

All surface-level child types

moduleHeaderDocumentation : File -> Maybe (Node String)

The module's documentation comment at the top of the file, which can be Nothing. elm-syntax has a weird way of giving you the comments of a file, this helper should make it easier

Dict
ModuleName
{ alias : Maybe String
, exposes : Set String
}
-> ( ModuleName, String )
-> Maybe ModuleName

Either the full qualification of a given variant/value/function/type identifier or Nothing if defined locally.

Requires a list of local explicit imports, see importsToExplicit

{ moduleExposes :
Dict
ModuleName
{ typesWithVariantNames : Dict String (Set String)
, simpleNames : Set String
}
, importsExposingAll :
List
{ moduleName : ModuleName
, alias : Maybe String
}
, importsExposingExplicit :
List
{ moduleName : ModuleName
, alias : Maybe String
, typesExposingVariants : Set String
, simpleNames : Set String
}
}
-> Dict
ModuleName
{ alias : Maybe String
, exposes : Set String
}

Regular elm imports don't tell the whole truth of what names can be used in the module declarations. Elm for example uses some imports implicitly, allows exposing (..) and ChoiceTypeWithVariants(..) and merges imports in a specific manner.

importsToExplicit now takes the module exposes of each module (see moduleExposes) and a list of imports exposing (..) and imports exposing explicit (see topLevelExposeListToExposes) from the current module to give you a FastDict with all imports and actual exposes and aliases listed.

Can be used to determineModuleOrigin.

Edge case note: The elm compiler has a bug where the List module does not expose the type List. To make it easier to differentiate between local and imported List type we still say that the List type originates from List. It's then the responsibility of review authors to cope with this edge case when trying to edit the qualification or adding an import.

File
-> { simpleNames : Set String
, typesWithVariantNames : Dict String (Set String)
}

Collect all exposed members from the file

  • simpleNames: values, functions, type alias, opaque types (those without (..)), infix operators (parenthesized)

  • typesWithVariantNames: names of types that expose their variants with (..) paired with what variant names are actually meant by ... It's provided as a FastDict. I strongly recommend using it too for your writing your review instead of Dict because Dict.union and friends have performance footguns you shouldn't need to worry about

List (Node TopLevelExpose)
-> { simpleNames : Set String
, typesExposingVariants : Set String
}

Collect all exposed members from an explicit exposing list of visible exposes

  • simpleNames: values, functions, type alias, opaque types (those without (..)), infix operators (parenthesized)
  • typesWithVariantNames: names of types that expose their variants with (..)
Module
-> { simpleNames : Set String
, typesWithVariantNames : Dict String (Set String)
}
sourceExtractInRange : Range -> String -> String

In case you need the read the source in a range as formatted by the user. This can be nice to keep the user's formatting if you just move code around

sourceRangesOf : String -> String -> List Range

Find all occurrences of a given section in the source in the case you can't access or easily calculate the range

packageElmJsonExposedModules : Exposed -> Set ModuleName

The set of modules in Elm-Project.Exposed (the exposed-modules field in a package elm.json)

Suggestions to add, remove or change helpers welcome!

running

Make elm-review-mini run in a new environment

type Inspect knowledge
= InspectDirectDependencies
( List
{ elmJson : Project
, modules : List Module
}
-> knowledge
)
| InspectElmJson
( { source : String
, project : Project
}
-> knowledge
)
| InspectExtraFile
( { path : String
, source : String
}
-> knowledge
)
| InspectModule
( { syntax : File
, source : String
, path : String
}
-> knowledge
)

How to collect knowledge from scanning a single part of the project

type alias Review =
{ ignoreErrorsForPathsWhere : String -> Bool
, run : Run
}

A construct that can inspect your project files and report errors, see Review.create

run :
Review
-> { elmJson :
{ source : String
, project : Project
}
, directDependencies :
List
{ elmJson : Project
, docsJson : List Module
}
, addedOrChangedExtraFiles :
List
{ path : String
, source : String
}
, addedOrChangedModules :
List
{ path : String
, source : String
, syntax : File
}
, removedExtraFilePaths : List String
, removedModulePaths : List String
}
-> { errorsByPath :
Dict
String
(List
{ range : Range
, message : String
, details : List String
, fixEditsByPath : Dict String (List SourceEdit)
}
)
, nextRun : Run
}

Review a given project and return the errors reported by the given Review as a FastDict along with a next Run that uses cached knowledge wherever it can.

import Review
import SomeConvention


doReview =
    let
        project =
            { addedOrChangedModules =
                [ { path = "src/A.elm", source = "module A exposing (a)\na = 1", syntax = ... }
                , { path = "src/B.elm", source = "module B exposing (b)\nb = 1", syntax = ... }
                ]
            , ...
            }


        runResult =
            Review.run SomeConvention.review project
    in
    doSomethingWith runResult.errorsByPath

(elm/core is automatically part of every project)

The resulting next Run internally keeps a cache to make it faster to re-run the review when only some files have changed. You can store this resulting nextRun in your application state type, e.g.

Review.run
    { run = yourApplicationState.nextRunProvidedByTheLastReviewRun
    , ignoreErrorsForPathsWhere = SomeConvention.review.ignoreErrorsForPathsWhere
    }
    { addedOrChangedModules =
        [ { path = "src/C.elm", source = "module C exposing (c)\nc = 1", syntax = ... }
        ]
    , removedModulePaths = [ "src/B.elm" ]
    , ...
    }
sourceApplyEdits : List SourceEdit -> String -> Result SourceEditError String

Try to apply a set of SourceEdites to the relevant file source, potentially failing with a SourceEditError

= AfterFixIsUnchanged
| FixHasCollisionsInRanges

An undesired situation when trying to apply SourceEdits

type Run
= Run
( { elmJson :
{ source : String
, project : Project
}
, directDependencies :
List
{ elmJson : Project
, docsJson : List Module
}
, addedOrChangedExtraFiles :
List
{ path : String
, source : String
}
, addedOrChangedModules :
List
{ path : String
, source : String
, syntax : File
}
, removedExtraFilePaths : List String
, removedModulePaths : List String
}
-> { errorsByPath :
Dict
String
(List
{ range : Range
, message : String
, details : List String
, fixEditsByPath : Dict String (List SourceEdit)
}
)
, nextRun : Run
}
)

A function that inspects, folds knowledge and reports errors in one go, and provides a recursively-defined future run function that already has the calculated knowledges cached.

Does it need to be so complicated?

Different reviews can have different knowledge types, so how can we put all of them in a single list? No problem, hide this parameter by only storing the combined run function from project files to errors.

But how do we implement caching this way (only re-run inspections for files that changed, otherwise take the already computed knowledge parts)? I know about roughly three options:

  • require users to provide encode/decode pairs to some generic structure. The simplest and fastest seems to be miniBill/elm-codec

    • the ability to use the encoded cache at any time enables write to disk (not a goal of elm-review-mini but having the option is nice)
    • the ability to show the encoded cache in the elm debugger (undeniably cool, though that only works in the browser. Debug.log is likely a nice enough alternative)
    • the ability to export the produced json for external use (not a goal of elm-review-mini but undeniably cool, similar to elm-review insights/extract although less explicit)
    • quite a burden users should preferably not bear
  • use js shenanigans to effectively "cast specific context type to any" and "cast any to specific context type" similar to linsyking/elm-anytype

    • faster than any possible alternative
    • simpler elm internals than any possible alternative
    • the highest possible degree of danger, as incorrect casting is always possible (+ no way to control that Review.run users wire them correctly)
    • custom embeds of running reviews (e.g. if you wanted to create a browser playground) always need additional js hacking which to me is deal breaking
  • provide a future function as a result which knows about the previously calculated knowledges. I was surprised to find that it's almost trivial to implement and even cuts down internal complexity compared to something like the codec one. If you're intrigued and have a week, some smart folks have written an entertaining dialog-blog-ish series about these "Jeremy's interfaces"

    • still very fast (much faster than the codec one)
    • concise internals
    • possibly less flexible for implementing stuff like shared knowledge (I much prefer defunctionalization wherever possible)
    • slightly less obvious than the codec one

I didn't expect to ever need to resort to such a complex feature but here we are. Though I'm always excited to try and experiment with other options (except the codec one which I already tried), issues welcome!