[Configlet] Define linting rules

[ErikSchierboom]

This issue defines the linting rules that we want the configlet tool to have for v3 tracks:

Lint check: required files being present

The linter should check if all the required files are present. The non-exercise specific files that must be present are:

• config.json
• config/maintainers.json

The Concept Exercise specific files that must be present are:

• exercises/concept/<slug>/.docs/hints.md
• exercises/concept/<slug>/.docs/instructions.md
• exercises/concept/<slug>/.docs/introduction.md
• exercises/concept/<slug>/.meta/config.json

There will be a similar list for Practice Exercises, but we've not yet defined the spec for that.

Each concept listed in the config.json should have the following files:

• concepts/<slug>/about.md
• concepts/<slug>/introduction.md (pending agreement on Update concept introductions spec #2767)
• concepts/<slug>/links.json

Lint check: config.json

The config.json file should have the following checks:

• The file must be valid JSON
• The "language" key is required
• The "language" value must be a non-empty string
• The "slug" key is required
• The "slug" value must be a non-empty, lowercased string using kebab-case
• The "active" key is required
• The "active" value must be a boolean
• The "blurb" key is required
• The "blurb" value must be a non-empty string
• The "version" key is required
• The "version" value must be the integer 3
• The "online_editor.indent_style" key is required
• The "online_editor.indent_style" value must be the string space or tab
• The "online_editor.indent_size" key is required
• The "online_editor.indent_size" value must be a positive integer (>= 0)
• The "exercises" key is required
• The "exercises.concept" key is required
• The "exercises.concept" value must be an array
• The "exercises.concept.slug" key is required
• The "exercises.concept.slug" value must be a non-empty, lowercased string using kebab-case
• The "exercises.concept.slug" value must be unique in "exercises.concept[].slug" and may not exist in "exercises.practice[].slug"
• The "exercises.concept.name" key is required
• The "exercises.concept.name" value must be a non-empty string
• The "exercises.concept.uuid" key is required
• The "exercises.concept.uuid" value must be a unique UUID
• The "exercises.concept.deprecated" key is optional
• The "exercises.concept.deprecated" value must be a boolean value
• The "exercises.concept.deprecated" value must generate a warning if set to false
• The "exercises.concept.concepts" key is required
• The "exercises.concept.concepts" value must be a non-empty array of strings
• The "exercises.concept.concepts" values must be non-empty, lowercased strings using kebab-case
• The "exercises.concept.concepts" values must not have duplicates
• The "exercises.concept.concepts" values must not be in any other concept exercise's "concepts" property
• The "exercises.concept.concepts" values must match the "concepts.slug" property of one of the concepts
• The "exercises.concept.prerequisites" key is required
• The "exercises.concept.prerequisites" value must be a non-empty array of strings for all but one exercise, which can have an empty array as its value
• The "exercises.concept.prerequisites" values must be non-empty, lowercased strings using kebab-case
• The "exercises.concept.prerequisites" values must not have duplicates
• The "exercises.concept.prerequisites" values must match any other concept exercise's "concepts" property values
• The "exercises.concept.prerequisites" values must not match any of the values in the exercise's "exercises.concept.concepts" property
• The "exercises.concept.prerequisites" values must match the "concepts.slug" property of one of the concepts
• There must not be any cycles between "exercises.concept.concepts" and "exercises.concept.prerequisites"
• The "exercises.practice" key is required
• The "exercises.practice" value must be an array
• The "exercises.practice.slug" key is required
• The "exercises.practice.slug" value must be a non-empty, lowercased string using kebab-case
• The "exercises.practice.slug" value must be unique in "exercises.practice[].slug" and may not exist in "exercises.concept[].slug"
• The "exercises.practice.name" key is required
• The "exercises.practice.name" value must be a non-empty string
• The "exercises.practice.uuid" key is required
• The "exercises.practice.uuid" value must be a unique UUID
• The "exercises.practice.deprecated" key is optional
• The "exercises.practice.deprecated" value must be a boolean value
• The "exercises.practice.deprecated" value must generate a warning if set to false
• The "exercises.practice.difficulty" key is required
• The "exercises.practice.difficulty" value must be an integer >= 0 and <= 10
• The "exercises.practice.prerequisites" key is required
• The "exercises.practice.prerequisites" value must be a non-empty array of strings
• The "exercises.practice.prerequisites" values must be non-empty, lowercased strings using kebab-case
• The "exercises.practice.prerequisites" values must not have duplicates
• The "exercises.practice.prerequisites" values must match any concept exercise's "exercises.concept.concepts" values
• The "exercises.practice.prerequisites" values must match the "concepts.slug" property of one of the concepts
• The "exercises.foregone" key is optional
• The "exercises.foregone" value must be a non-empty array of strings
• The "exercises.foregone" values must be non-empty, lowercased strings using kebab-case
• The "exercises.foregone" values must not match any of the concept or practice exercise slugs
• The "concepts" key is required
• The "concepts" value must be an array
• The "concepts" value must have a entry with a matching "slug" property for each concept listed in a concept exercise's "concepts" property
• The "concepts.uuid" key is required
• The "concepts.uuid" value must be a unique UUID
• The "concepts.slug" key is required
• The "concepts.slug" value must be a non-empty, lowercased string using kebab-case
• The "concepts.name" key is required
• The "concepts.name" value must be a non-empty, titleized string
• The "concepts.blurb" key is required
• The "concepts.blurb" value must be a non-empty string
• Each "concepts" value must have a concept/<concepts.slug>/about.md file. Linting rules for this file are specified below.
• Each "concepts" value must have a concept/<concepts.slug>/introduction.md file (pending agreement on Update concept introductions spec #2767). Linting rules for this file are specified below.
• Each "concepts" value must have a concept/<concepts.slug>/links.json file. Linting rules for this file are specified below.

Lint check: config/maintainers.json

• The file must be valid JSON
• The JSON root must be an object
• The "maintainers" key is required
• The "maintainers" value must be an array
• The "maintainers" array elements must use the correct format (TODO: specify)
• There must not be duplicate maintainers

Lint check: exercises/concept/<exercise-slug>/.meta/config.json

• The file must be valid JSON
• The JSON root must be an object
• The "authors" key is required
• The "authors" value must be an non-empty array
• The "authors[].github_username" key is required
• The "authors[].github_username" key must be a non-empty string
• The "authors[].github_username" value is treated case-insensitively
• The "authors[].exercism_username" key is required
• The "authors[].exercism_username" key must be a non-empty string
• The "authors[].github_username" value is treated case-insensitively
• The "contributors" key is optional
• The "contributors" value must be an array
• The "contributors[].github_username" key is required
• The "contributors[].github_username" key must be a non-empty string
• The "contributors[].github_username" value is treated case-insensitively
• The "contributors[].exercism_username" key is required
• The "contributors[].exercism_username" key must be a non-empty string
• The "contributors[].exercism_username" value is treated case-insensitively
• Users can only be listed in either the "authors" or "contributors" array (no overlap)
• The "editor.solution_files" key is required
• The "editor.solution_files" value must be a non-empty array
• The "editor.test_files" key is required
• The "editor.test_files" value must be a non-empty array
• The files listed in the "editor.solution_files" must exist
• The files listed in the "editor.test_files" must exist
• Files can only be listed in either the "editor.solution_files" or "editor.test_files" array (no overlap)
• The "forked_from" key is optional
• The "forked_from" value must be a non-empty array
• The "forked_from" values must be strings formatted as <track-slug>/<exercise-slug> (e.g. fsharp/bird-watcher)
• The "forked_from" values must refer to actually implemented exercises
• The "forked_from" values must be unique
• The "language_versions" key is optional
• The "language_versions" value must be a string

Lint check: exercises/concept/<exercise-slug>/.docs/hints.md

• If there are sub headings, they must start at level two
• All headings must be level two headings
• All headings must be either ## General or ## <task> where <task> matches the exact task heading in the instructions.md
• All hints must be specified as Markdown list items

Lint check: exercises/concept/<exercise-slug>/.docs/instructions.md

• If there are sub headings, they must start at level two
• All tasks must start with a level two heading that starts with a number followed by a dot: ## 1. Do X

Lint check: exercises/concept/<exercise-slug>/.docs/introduction.md

• If there are sub headings, they must start at level two

Lint check: exercises/shared/.docs/cli.md

• If there are sub headings, they must start at level two

Lint check: exercises/shared/.docs/debug.md

• If there are sub headings, they must start at level two

Lint check: concepts/<concept-slug>/about.md

• If there are sub headings, they must start at level two

Lint check: concepts/<concept-slug>/introduction.md (pending agreement on #2767)

• If there are sub headings, they must start at level two

Lint check: concept/<concept-slug>/links.json

• The file must be valid JSON
• The JSON root must be an array
• The "[].url" property is required
• The "[].url" value must be an URL
• The "[].description" property is required
• The "[].description" value must be a non-empty string
• The "[].icon_url" property is optional
• The "[].icon_url" value must be an URL

---

Any thoughts or feedback welcome and appreciated!

[ErikSchierboom]

Note: many of the above linting rules could be defined in a JSON schema, but not all (e.g. the cross-track uniqueness of the UUIDs).

[iHiD]

(cc @exercism/track-maintainers)

[SaschaMann]

The "version" key is required
The "version" value must be the integer 3

If there's only one allowed value, why bother with the field at all?

---

The "online_editor.indent_size" key is required
The "online_editor.indent_size" value must be a positive integer (>= 0)

Should this also be required if it's set to use tabs?

---

The "authors[].github_username" key must be a non-empty string

Would it be useful to check if it's a valid username?

---

The file must be valid Markdown

What would an invalid Markdown file look like?

[lpil]

The version key is so that they version used can be detected when there's a later version 4

[ErikSchierboom]

The "version" key is required

If there's only one allowed value, why bother with the field at all?

As @lpil said, it is to help with possible future updates to the schema. It can also be used to relatively easily detect if one is working with a v2 config.json file.

Should this also be required if it's set to use tabs?

Question for @iHiD!

Would it be useful to check if it's a valid username?

This could definitely be useful.

What would an invalid Markdown file look like?

Not being parseable by a Markdown parser?

[SaschaMann]

Not being parseable by a Markdown parser?

By which Markdown parser? I don't think I've ever encountered an invalid Markdown file. Formatting that is "wrong" in context, sure, but I don't know what an actually invalid file would contain. That's why I'm curious about an example.

[lpil]

The markdown spec says a parser should not reject any document, however it is possible to have incorrect syntax and it'll just render what was typed verbatim. A common one is to get the link syntax wrong.

[ErikSchierboom]

Ah okay, I thought that maybe there was some way to check a Markdown document.

@SaschaMann do you think we should check the links in Markdown documents?

[coriolinus]

Suggest adding:

• exercises.concept.slug must be unique in exercises.concept.slug and may not exist in exercises.practice.slug.
• exercises.practice.slug must be unique in exercises.practice.slug and may not exist in exercises.concept.slug.

The "exercises.concept.deprecated" value must the boolean value true

It would surprise me if modifying true => false ever broke a build, but that's the property we get with this. Suggest that we change "must" to "should": warn if it's false but don't actually fail. Same for practice exercises.

The "exercises.concept.prerequisites" value must be a non-empty array of strings for all but one exercise, which can have an empty array of string as it value

How can you tell in JSON what the contained type of an empty array is?

The "exercises.practice.prerequisites" value must be a non-empty array of strings for all but one exercise, which can have an empty array of string as it value

I do not think it is useful to have any practice exercise with no prerequisites.

The "editor.solution_files" key is required
The "editor.test_files" key is required

This feels like it will take a lot of repetitive busywork to keep up. For every Rust concept exercise, and most Rust practice exercises, editor.solution_files = ["src/lib.rs"], and editor.test_files = ["tests/<slug>.rs"]. Would prefer to be able to set a default pattern with some substitutions, and use that in case the keys are unset for a particular exercise.

All headings must be level two headings

That's not ideal. General-purpose Markdown linters require that a markdown file start with a level-1 heading, and don't skip between levels. Why not just impose that requirement? If this requirement is intended to make it easier to merge templates, it's a better idea to adjust the heading levels of the included markdown file anyway.

I think we'll see better quality markdown if standard Markdown linters do the right thing for our source files.

[ErikSchierboom]

It would surprise me if modifying true => false ever broke a build, but that's the property we get with this. Suggest that we change "must" to "should": warn if it's false but don't actually fail. Same for practice exercises.

Good idea!

How can you tell in JSON what the contained type of an empty array is?

I'll rephrase :)

I do not think it is useful to have any practice exercise with no prerequisites.

It isn't. This is a mistake on my part.

This feels like it will take a lot of repetitive busywork to keep up. For every Rust concept exercise, and most Rust practice exercises, editor.solution_files = ["src/lib.rs"], and editor.test_files = ["tests/.rs"]. Would prefer to be able to set a default pattern with some substitutions, and use that in case the keys are unset for a particular exercise.

Something for @iHiD to consider.

That's not ideal. General-purpose Markdown linters require that a markdown file start with a level-1 heading, and don't skip between levels. Why not just impose that requirement? If this requirement is intended to make it easier to merge templates, it's a better idea to adjust the heading levels of the included markdown file anyway.

Hmmm, I didn't consider this. It depends a bit on how easy the transformation is for @iHiD.

[SaschaMann]

Ah okay, I thought that maybe there was some way to check a Markdown document.

@SaschaMann do you think we should check the links in Markdown documents?

Having warnings instead of errors for syntax that may be wrong, like a reversed link or unmatched asterisks, would be useful. But since there doesn't seem to be anything like an invalid markdown document, perhaps the spec should be rephrased. I don't think this should cause errors, only warnings.

[coriolinus]

It depends a bit on how easy the transformation is

It's pretty easy:

#!/usr/bin/env bash

n="$1"
file="$2"

if [ -z "$n" ] || [ -z "$file" ]; then
    echo "USAGE: $0 N FILE"
    echo
    echo "  N: number of markdown layers to insert"
    echo "  FILE: file on which to work"
    exit 1
fi

n_hashes="$(head -c "$n" < /dev/zero | tr '\0' '#')"
sed_expr="$(printf "s/^#+/%s&/" "$n_hashes")"
sed -i -E -e "$sed_expr" "$file"

[ErikSchierboom]

Having warnings instead of errors for syntax that may be wrong, like a reversed link or unmatched asterisks, would be useful. But since there doesn't seem to be anything like an invalid markdown document, perhaps the spec should be rephrased. I don't think this should cause errors, only warnings.

I've removed it from the spec.

@SaschaMann do you think we should check the links in Markdown documents?

With this I meant should we use something like the markdown link checker that we use for v3? To verify that external resources can actually be reached?

[SaschaMann]

With this I meant should we use something like the markdown link checker that we use for v3? To verify that external resources can actually be reached?

Ah okay. It's something that would have to be run periodically, otherwise you'll get failures for unrelated changes in PRs. Might make more sense to keep it separate for that reason. But I'm not sure.

[ErikSchierboom]

Ah okay. It's something that would have to be run periodically, otherwise you'll get failures for unrelated changes in PRs. Might make more sense to keep it separate for that reason. But I'm not sure.

Yeay. Maybe this should be a GitHub workflow that tracks can integrate and run periodically?

[ee7]

I suggest adding the rule of "each JSON object must contain only the specified keys".

I think the main benefit is for the case of somebody making a typo while trying to add a non-required key. For example:

exercises.practice.depracated = true
exercises.forgone = [bad-slug-1, bad-slug-2]
languageversions = 1

For typoing a required key (e.g. URL instead of url), there'd already be an error of e.g.

missing required key: 'url'

But the error message is better if we print e.g.

missing required key: 'url'
contains invalid key: 'URL'

Somewhat related: we could consider writing something like "note that every key is lowercase". That is: JSON is case-sensitive, our linting will be case-sensitive, and we don't do any normalization of keys in a later step.

Lastly, are there any values that we want to be case-insensitive? There are at least some values that we want to normalize while linting, but that should probably have consistent capitalization. For example:

"authors": [
  {
    "github_username": "ErikSchierboom",
    "exercism_username": "foo"
  }
],
"contributors": [
  {
    "github_username": "erikschierboom",
    "exercism_username": "foo"
  }
]

In this case, we want to print a warning that the "github_username" of "ErikSchierboom" is listed in both "authors" and "contributors", which violates:

Users can only be listed in either the "authors" or "contributors" array (no overlap)

We can't say that "all values must be lowercase" because e.g.:

The "concepts.name" value must be a non-empty, titleized string

and some tracks might have uppercase characters in e.g. editor.solution_files.

[ErikSchierboom]

I suggest adding the rule of "each JSON object must contain only the specified keys".

This sounds good in principle, but some tracks do use custom properties: https://github.com/exercism/ruby/blob/master/config.json#L5 I don't know how common that is, or if we want to support that, but it is worth considering. What do others think of this?

Somewhat related: we could consider writing something like "note that every key is lowercase". That is: JSON is case-sensitive, our linting will be case-sensitive, and we don't do any normalization of keys in a later step.

One thing we could consider is to implement something like what the Git CLI does, where it tries to detect what string the user intended to write versus what was actually written. So if a config.json file contains a Slug key, it could say:

Unknown Slug key found. Did you mean slug?

Lastly, are there any values that we want to be case-insensitive? There are at least some values that we want to normalize while linting, but that should probably have consistent capitalization. For example:

I think the github_username and exercism_username fields are a good example where the values should be treated case-insensitive. I'll add this to the spec.

[ErikSchierboom]

Spec written up in https://github.com/exercism/v3-docs/blob/master/anatomy/tracks/configlet/linting.md