Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint(concepts): implement links.json checks #171

Merged
merged 4 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 65 additions & 1 deletion src/lint/concepts.nim
Original file line number Diff line number Diff line change
@@ -1,6 +1,70 @@
import std/os
import std/[json, os, strutils]
import ".."/helpers
import "."/validators

proc isUrlLike(s: string): bool =
## Returns true if `s` starts with `https://`, `http://` or `www`.
# We probably only need simplistic URL checking, and we want to avoid using
# Nim's stdlib regular expressions in order to avoid a dependency on PCRE.
s.startsWith("https://") or s.startsWith("http://") or s.startsWith("www")

proc isValidLinkObject(data: JsonNode, context: string, path: string): bool =
## Returns true if `data` is a `JObject` that satisfies all of the below:
## - has a `url` key, with a value that is a URL-like string.
## - has a `description` key, with a value that is a non-empty, non-blank
## string.
## - if it has a `icon_url` key, the corresponding value is a URL-like string.
if isObject(data, context, path):
result = true

if checkString(data, "url", path):
let s = data["url"].getStr()
if not isUrlLike(s):
result.setFalseAndPrint("Not a valid URL: " & s, path)
else:
result = false

if not checkString(data, "description", path):
result = false

if data.hasKey("icon_url"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this call? Doesn't checkString gracefully handle optional values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this call?

With the current checkString, unfortunately yes.

The full context is:

    if data.hasKey("icon_url"):
      if checkString(data, "icon_url", path, isRequired = false):
        let s = data["icon_url"].getStr()

icon_url is optional, so we call checkString with isRequired = false. That means that checkString returns true if icon_url is missing, which means the data["icon_url"].getStr() would throw an exception if we don't have the first line.

It is indeed pretty ugly, and we're performing hasKey twice.

In the future I'm thinking about splitting checkString into hasString and isString (and the same for checkBoolean, and checkInteger), similar to the previous split of checkArray into hasArray and isArray.

if checkString(data, "icon_url", path, isRequired = false):
let s = data["icon_url"].getStr()
if not isUrlLike(s):
result.setFalseAndPrint("Not a valid URL: " & s, path)
else:
result = false
else:
result.setFalseAndPrint("At least one element of the top-level array is " &
"not an object: " & $data[context], path)

proc isValidLinksFile(data: JsonNode, path: string): bool =
result = isArrayOf(data, "", path, isValidLinkObject, isRequired = false)

proc isEveryConceptLinksFileValid*(trackDir: string): bool =
let conceptsDir = trackDir / "concepts"
result = true

if dirExists(conceptsDir):
for subdir in getSortedSubdirs(conceptsDir):
let linksPath = subdir / "links.json"
if fileExists(linksPath):
if not isEmptyOrWhitespace(readFile(linksPath)):
let j =
try:
parseFile(linksPath) # Shows the filename in the exception message.
except CatchableError:
result.setFalseAndPrint("JSON parsing error",
getCurrentExceptionMsg())
continue
if not isValidLinksFile(j, linksPath):
result = false
else:
result.setFalseAndPrint("File is empty, but must contain at least " &
"the empty array, `[]`", linksPath)
else:
result.setFalseAndPrint("Missing file", linksPath)

proc conceptFilesExist*(trackDir: string): bool =
## Returns true if every subdirectory in `trackDir/concepts` has the required
## files.
Expand Down
8 changes: 5 additions & 3 deletions src/lint/lint.nim
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@ proc lint*(conf: Conf) =
let b2 = conceptExerciseFilesExist(trackDir)
let b3 = practiceExerciseFilesExist(trackDir)
let b4 = conceptFilesExist(trackDir)
let b5 = isEveryConceptExerciseConfigValid(trackDir)
let b6 = isEveryPracticeExerciseConfigValid(trackDir)
let b5 = isEveryConceptLinksFileValid(trackDir)
let b6 = isEveryConceptExerciseConfigValid(trackDir)
let b7 = isEveryPracticeExerciseConfigValid(trackDir)

if b1 and b2 and b3 and b4 and b5 and b6:
if b1 and b2 and b3 and b4 and b5 and b6 and b7:
echo """
Basic linting finished successfully:
- config.json exists and is valid JSON
- config.json has these valid fields: language, slug, active, blurb, version, tags
- Every concept has the required .md files and links.json file
- Every concept links.json file is valid
- Every concept exercise has the required .md files and a .meta/config.json file
- Every concept exercise .meta/config.json file is valid
- Every practice exercise has the required .md files and a .meta/config.json file
Expand Down