jfmengels / elm-review-documentation

Provides elm-review rules to help with the quality and correctness of your Elm project's documentation

elm-review-documentation

Provides elm-review rules to help with the quality and correctness of your Elm project's documentation, both for packages and applications.

Provided rules

• Docs.ReviewAtDocs - Reports problems with the usages of @docs.
• Docs.NoMissing - Reports missing documentation for functions and types.
• Docs.ReviewLinksAndSections - Reports problems with links and sections in Elm projects.
• 🔧 Docs.UpToDateReadmeLinks - Reports links in the README.md that do not point to the current version of the package.

Configuration

module ReviewConfig exposing (config)

import Docs.NoMissing exposing (exposedModules, onlyExposed)
import Docs.ReviewAtDocs
import Docs.ReviewLinksAndSections
import Docs.UpToDateReadmeLinks
import Review.Rule exposing (Rule)


config : List Rule
config =
    [ Docs.NoMissing.rule
        { document = onlyExposed
        , from = exposedModules
        }
    , Docs.ReviewLinksAndSections.rule
    , Docs.ReviewAtDocs.rule
    , Docs.UpToDateReadmeLinks.rule
    ]

Advice for package authors

You can ask the compiler to check whether the documentation is valid (as in, in a publishable state) if you're developing a package, by running elm make --docs=docs.json. It will fail the compilation if documentation is missing, so you might not be able to enable it if the project is far from ready yet. I do recommend enabling this as soon as you can, so that you can work on the documentation as you are working on the project, not right before releasing when it might feel like a chore.

Additionally, running the previous command generates a docs.json file which I recommend you commit into the project, as that will allow people to look at the documentation using elm-doc-preview and give you feedback before you release.

Try it out

You can try the example configuration above out by running the following command:

elm-review --template jfmengels/elm-review-documentation/example

Thanks

Thanks to @lue-bird for helping out with Docs.ReviewLinksAndSections.

elm-review-documentation's Issues

New rule: Detect duplicate sections

What the rule should do:

Report duplicate sections in README and module documentations.

What problems does it solve:

Duplicate sections

Example of things the rule would report:

{-|
# section

# section

## init

@docs init

-}

{-| this is init -}
init = 1

Example of things the rule would not report:

{-|
# section

## Initialization

@docs init

-}

{-| this is init -}
init = 1

When (not) to enable this rule:

If you are working on an application and it has no documentation?

I am looking for:

• Someone to implement it (I can provide help and guidance)
• Question: Do you think this should be merged with another rule? If so, which one?

Make the test script run on Windows

The current test script is

elm make --docs=docs.json && elm-format src/ tests/ --validate && elm-test && elm-review && npm run package-tests -s

But && does not work on Windows, so this prevents Windows users from helping out on this package.

I know of npm-run-all which helps make scripts run on Windows. It does require using npm scripts, so every part will need to be defined in a separate npm script. That has its benefits, but if you can come up with a different solution, let me know.

If we go with using npm-run-all, the task would be to

• Create a script for every part of the test script

• Add npm-run-all as a dependency

• Replace the test script with one like npm-run-all make verify-format ...

• Make sure the scripts are run sequentially. Running them in parallel would have little performance benefits but make failures quite hard to understand I think.

• See if everything

• Optional/bonus: Make the CI run the tests on Windows (and Mac?) to make sure that works. I think we'd need to change the runs-on field for test in .github/workflows/test.yml to [ ubuntu-latest, windows-latest, macos-latest ] but I haven't tried it out. More info here.

The idea is that all of this work, if proved successful, will then be the default setup for all review packages when created by elm-review new-package. That can be a separate PR for jfmengels/node-elm-review, but I think this is a good first step before generalizing.

New rule: Detect links to non-existing sections

What the rule should do:

Detect links to non-existing sections/functions/types in the documentation, in the README and in the modules.

What problems does it solve:

This would help maintaining documentation (it would notice outdated documentation), and notice when you add a link with an error in the path to the section

I don't think the rule should provide an automatic fix, but I think it could provide suggestions ("Did you mean '#init'?")

Example of things the rule would report:

A [section](#section), if that section does not exist

Example of things the rule would not report:

# section

A [section](#section)

A [section](./ModuleName#section) from another file, if it exists in that file.

Link to image [](./image.png)

{-|
@docs init

Look at the [init](#init) function
-}

{-| the init function -}
init : Model
init = 1

When (not) to enable this rule:

If you are working on an application and it has no documentation?

I am looking for:

• A good rule name
• Someone to implement it (I can provide help and guidance)

(More unorganized thoughts over at https://trello.com/c/BKfdmp8v/2-noinvalidreferences-noinvalidlinks-make-sure-that-all-references-to-sections-are-valid)

Docs.ReviewLinksAndSections could allow for “absolute path” links to other packages

Currently, the Docs.ReviewLinksAndSections rule complains if I have a link such as /packages/elm/core/latest/Basics#%7C%3E linking to the documentation for |>, and wants me to convert it to a full URL starting from the protocol.

My observation is that the “absolute path” pattern is useful because it not only works in the official Elm packages website, but also in dmy's fork, without taking you out of the website.

When not to enable this rule:

Maybe this doesn't make sense for people who generate their documentation and host it somewhere else. Not sure if there's people doing that, though. Otherwise, I think it should be pretty safe?

I am looking for:

• Feedback. Should I attempt a PR?
• Tips and help on how to implement it

Much love for elm-review and all the useful rules!

Docs.ReviewAtDocs Allows All Exposed Functions Not to Be Documented

Summary

The documentation for Docs.ReviewAtDocs rule says

Once there are no more issues of malformed @docs, the rule will report about:

  Missing @docs for exposed elements

but this doesn't seem to be the case if no functions are exposed.

Actual Behavior

As an example, run npm run review in the example I've attached as
no-docs-example.zip
and you'll see the following actual output:

-- ELM-REVIEW ERROR ---------------- src/ModuleWithOneExposedFunctionDoc.elm:3:7

Docs.ReviewAtDocs: Missing @docs reference for exposed `anotherThing`

2|     ( oneThing
3|     , anotherThing
         ^^^^^^^^^^^^
4|     )

There is no @docs reference for this element. Maybe you exposed or renamed it
recently.

Please add a @docs reference to it the module documentation (the one at the top
of the module) like this:

{-|
@docs anotherThing
-}

I found 1 error in 1 file.

Expected Behavior

But I would expect, when running npm run review, that in addition to the error above there would be a message about how ModuleWithoutExposedFunctionDocs fails to expose both oneThing and anotherThing

New rule: Detect syntax errors in code examples

What the rule should do:

Detect syntax errors in code examples in module rules and in the README.

What problems does it solve:

If you don't use `elm-verify-examples, you have no way to know if your examples are correct. Checking that they are syntactically correct is a reasonable simple check.

I think it can be done relatively simple by using elm-syntax to parse the example.

Example of things the rule would report:

Any syntax error in code examples.

Maybe it would allow simple expressions? Behind a configuration maybe?

When (not) to enable this rule:

I am looking for:

• A good rule name. NoSyntaxErrorInExample?
• Someone to implement it (I can provide help and guidance)

Docs.NoMissing: Add more configuration options

This is an issue to gather feedback and ideas.

The Docs.NoMissing rule has some configuration options that I think would work fine for packages, but probably not so much for application projects.

I can't imagine that some projects will require documentation for all functions and types in their project, but there is no concept of exposed modules there, so either this rule will be used there with a lot of ignoreRuleFor*, or it needs more configuration options.

I'm imagining a list of explicit modules or explicit directories, kind of like the following, though I'm not sure this will be sufficient either.

rule =
	{ document = everything
	, from =
        [ moduleNamed "Some.Module"
        , modulesUnder "Some.Folder"
        ]
    }

Alternatively, the rule could take a function ModuleName -> Bool that acts as a predicate, which would be very configurable but also potentially a bit annoying to configure.

I would love to see what options you'd like to have and/or what you think is best!

New rule: Make sure that all the exposed things in the module have a corresponding @docs and vice-versa

What the rule should do:

Report problems around misuse of @docs

What problems does it solve:

• Documentation not being ready for publication or simply out of date (for applications)
• Documentation being completely broken because @docs were misused.

Example of things the rule would report:

module A exposing (a, b)
{-|
Here, both a and c should be reported: a is exposed but not documentated, and c is documentation but not exposed
@docs b, c 
-}

import ...

module A exposing (a, b)
{-|@docs a -- should not appear on the first line, otherwise docs will look broken

  @docs b -- should appear at the start

-- should have at least one thing
@docs

-- should only contain valid types/functions
@docs 1
@docs A(..)

-- should be separated by `,`
@docs c d

-- should not document the same thing twice
@docs e
@docs e
-}

import ...

{-|
@docs -- should only appear in the module documentation, not in function/types' documentation
-}
init = ()

Example of things the rule would not report:

module A exposing (a, b)
{-|
@docs a, b
@docs c
-}
import ...

{-| something
-}
init = ()

When (not) to enable this rule:

If you are working on an application and it has no documentation?

I am looking for:

• A good rule name. NoIncorrectDocsReferences?
• Someone to implement it (I can provide help and guidance)

Don't report links in code blocks

Applies to

• Docs.ReviewLinksAndSections
• Docs.UpToDateReadmeLinks

If you include a link to a code example:
```elm
module A exposing (A)
{-| Helpers for [`B.B`](B#B)
-}
import B
```

it triggers both rules

Docs.ReviewLinksAndSections: Link points to non-existing module Expression

202|     module A exposing (A)
203|     {-| Helpers for [`B.B`](B#B)
                           ^^^
204|     -}

This is a dead link.

Docs.UpToDateReadmeLinks: Found relative link to a module in README

202|     module A exposing (A)
203|     {-| Helpers for [`B.B`](B#B)
                           ^^^
204|     -}

Relative links to other modules from the README don't work when looking at the
docs from GitHub or the likes.

I suggest to run elm-review --fix to change the link to an absolute link.

which is not correct because these are not real links that are rendered as such, they are code.

Treat this as a very low prio as checking for it is a bit of work and should only be quite rare in practice.

For implementation we could for example remove all sections from starting-spaced ``` until the next spaced ``` before checking.

New rule: NoMissingDocumentation

What the rule should do:

Report missing documentation for functions and types, and consider empty documentation as no documentation ({-| -})

What problems does it solve:

For package authors: Notice missing documentation issues for you before you run elm publish or elm make --docs=docs.json (if you're not aware of that)

Also, I see a lot of {-| -} which is very unhelpful for the user. It might have been a temporary solution to get the compiler happy, and they forgot to go back to it. I often see it for mapN functions where after the documented map and map2, the documentation for map3 and later becomes "obvious". But having documentation there is still useful for users seeing the documentation in an editor for instance.

For application developers: There is currently no way to enforce documentation out of the box, which is a shame when you want a certain amount of documentation in your application.

Example of things the rule would report:

someFunction =
    great Things

{-| -}
someOtherFunction =
    other (great Things)

Example of things the rule would not report:

{-| someFunction does great things
-}
someFunction =
    great Things

Configuration

Currently two options will be supported

• onlyExposed: which requires documentation for all exposed things of a module (for packages, only for packages in exposed-modules)
• everything: You need to document everything. I suspect this won't be as useful, but maybe people will want to enable this rule in several important files and ignore the rest of the modules (the rule could be duplicated in their configuration)

When (not) to enable this rule:

If you are working on an application and you don't care about it being documented.

Current state

The rule is in progress, the work is available in the no-missing branch.

• Do not report non-exposed elements from exposed modules when using OnlyExposed
• Write documentation for the configuration options

I am looking for

• Do you think more options should exist? Would it make sense (for package developers) to have a onlyModule option, where you are only required to provide a documentation for the module, not for the exposed things?

Only the first version in a line will be replaced

For Documentation.ReadmeLinksPointToCurrentVersion, if we change the version, only the first link will be searched through and reported. I haven't dug into it further, but this is my theory at this point.

Failing SSCCE

[NoUnused.Parameters](https://package.elm-lang.org/packages/jfmengels/elm-review-unused/1.0.1/NoUnused-Dependencies) and [NoUnused.Patterns](https://package.elm-lang.org/packages/jfmengels/elm-review-unused/1.0.1/NoUnused-Dependencies).

Rule to validate that the bumped version has been added to CHANGELOG.md

What the rule should do:

Check the CHANGELOG.md file for an entry corresponding to the current version specified in elm.json.

What problems does it solve:

Rather similar to UpToDateReadmeLinks this rule could help package authors catch missing information they meant to add to CHANGELOG.md between running elm bump and elm publish.

We could also consider requiring conformance of CHANGELOG.md to keepachangelog.com as part of this rule. For example we could enforce the following format for entries:

## [0.3.0] - 2015-12-03

When (not) to enable this rule:

If you do not want to maintain a CHANGELOG.md and want consumers of your library to guess what has changed between versions.

I am looking for:

Mostly documenting an idea I had and trying to open a discussion around if it is a good one.

I may find the time to work on this in the near future, but if someone else wants to tackle it go for it!

New rule: Report what looks like a title but isn't

What the rule should do:

Report when (module/function/type) documentation contains what looks like a title but isn't.

What problems does it solve:

Fixes a Markdown issue that can surprise documentation authors.

Example of things the rule would report:

module A exposing (..)
{-|

#Title

-}

a = 1

Here's an example of the result: https://package.elm-lang.org/packages/stil4m/elm-syntax/7.2.6/Elm-Syntax-Infix (#Types)

Multiplying the number of #'s

Example of things the rule would not report:

module A exposing (..)
{-|

# Title

-}

a = 1

When (not) to enable this rule:

This can probably have some false positives, if authors genuinely want to start a line with #.

I am looking for:

• General feedback
• Information as to whether there is a maximum number of # for titles.
• Information as to whether this will be a problem in practice
• Information: Is this issue only a problem with the current Markdown implementation of the packages website, or is this part of the GitHub Flavored Markdown spec as well?

(cc @dillonkearns who might know some of the answers)

Docs.ReviewLinksAndSections should suggest what seems closest

When the rule reports links to unknown things, I think it would be valuable if the rule suggested what is closest to it (using something like the Levenshtein distance).

For an unknown module, suggest the module name with the name that comes closest.
For an unknown section, suggest the section from the same module with the name that comes closest.

New rule: No pointing to resources on master

What the rule should do:

Forbid links to resources on the master branch, for resources hosted on GitHub. These links should be reported in module documentation and in the README

What problems does it solve:

I often see READMEs containing an image stored on the master branch to something like
https://raw.githubusercontent.com/jfmengels/elm-review/master/documentation/images/elm-review-report-hq.png

The problem with that is that if that images is ever changed/moved/removed on the master branch, then the image will disappear from the documentation.
If instead, we made the link point to an image hosted in a Git tag or a SHA, then the image would stay there forever, like
https://raw.githubusercontent.com/jfmengels/elm-review/2.0.1/documentation/images/elm-review-report-hq.png

I think we can apply this to other resources, like Markdown or any kind of files hosted on GitHub (through raw.githubusercontent.com or github.com (like https://github.com/jfmengels/elm-review/blob/master/documentation/images/elm-review-report-hq.png)

Example of things the rule would report:

{-|

![](https://raw.githubusercontent.com/jfmengels/elm-review/master/documentation/images/elm-review-report-hq.png)

-}

Example of things the rule would not report:

{-|

![](https://raw.githubusercontent.com/jfmengels/elm-review/2.0.1/documentation/images/elm-review-report-hq.png)

-}

When (not) to enable this rule:

I am looking for:

• Feedback: The problem will be for resources not yet published. Ideally we want to use a Git tag as the reference, but before you release the first version with that image, you will likely have to use a Git SHA or something. Do you see better solutions to that?
• A good rule name
• Someone to implement it (I can provide help and guidance)