lint: add kebab-case checks (#274)

This commit implements some stricter checks for the track-level
`config.json` file.

`configlet lint` now checks that every value of every below key is a
"kebab-case string":
- `slug`
- `exercises.concept[].slug`
- `exercises.concept[].concepts`
- `exercises.concept[].prerequisites`
- `exercises.practice[].slug`
- `exercises.practice[].practices`
- `exercises.practice[].prerequisites`
- `exercises.foregone`
- `concepts[].slug`

We define a "kebab-case string" as a string that matches the regular
expression:
  `^[a-z0-9]+(-[a-z0-9]+)*$`

See:
- https://github.com/exercism/docs/blob/4a42d3399139/building/configlet/lint.md#rule-configjson-file-is-valid
- https://github.com/exercism/docs/pull/113/files

src/lint/track_config.nim

@@ -74,14 +74,14 @@ const
 proc isValidConceptExercise(data: JsonNode; context: string; path: Path): bool =
   if isObject(data, context, path):
     let checks = [
-      hasString(data, "slug", path, context),
+      hasString(data, "slug", path, context, checkIsKebab = true),
       hasString(data, "name", path, context),
       hasString(data, "uuid", path, context),
       hasBoolean(data, "deprecated", path, context, isRequired = false),
       hasArrayOfStrings(data, "concepts", path, context,
-                        allowedArrayLen = 0..int.high),
+                        allowedArrayLen = 0..int.high, checkIsKebab = true),
       hasArrayOfStrings(data, "prerequisites", path, context,
-                        allowedArrayLen = 0..int.high),
+                        allowedArrayLen = 0..int.high, checkIsKebab = true),
       hasString(data, "status", path, context, isRequired = false,
                 allowed = statuses),
     ]
@@ -91,15 +91,15 @@ proc isValidPracticeExercise(data: JsonNode; context: string;
                              path: Path): bool =
   if isObject(data, context, path):
     let checks = [
-      hasString(data, "slug", path, context),
+      hasString(data, "slug", path, context, checkIsKebab = true),
       hasString(data, "name", path, context),
       hasString(data, "uuid", path, context),
       hasBoolean(data, "deprecated", path, context, isRequired = false),
       hasInteger(data, "difficulty", path, context, allowed = 0..10),
       hasArrayOfStrings(data, "practices", path, context,
-                        allowedArrayLen = 0..int.high),
+                        allowedArrayLen = 0..int.high, checkIsKebab = true),
       hasArrayOfStrings(data, "prerequisites", path, context,
-                        allowedArrayLen = 0..int.high),
+                        allowedArrayLen = 0..int.high, checkIsKebab = true),
       hasString(data, "status", path, context, isRequired = false,
                 allowed = statuses),
     ]
@@ -114,15 +114,16 @@ proc hasValidExercises(data: JsonNode; path: Path): bool =
                  allowedLength = 0..int.high),
       hasArrayOf(exercises, "practice", path, isValidPracticeExercise, k,
                  allowedLength = 0..int.high),
-      hasArrayOfStrings(exercises, "foregone", path, k, isRequired = false),
+      hasArrayOfStrings(exercises, "foregone", path, k, isRequired = false,
+                        checkIsKebab = true),
     ]
     result = allTrue(checks)
 
 proc isValidConcept(data: JsonNode; context: string; path: Path): bool =
   if isObject(data, context, path):
     let checks = [
       hasString(data, "uuid", path, context),
-      hasString(data, "slug", path, context),
+      hasString(data, "slug", path, context, checkIsKebab = true),
       hasString(data, "name", path, context),
     ]
     result = allTrue(checks)
@@ -153,7 +154,7 @@ proc isValidTrackConfig(data: JsonNode; path: Path): bool =
   if isObject(data, "", path):
     let checks = [
       hasString(data, "language", path),
-      hasString(data, "slug", path),
+      hasString(data, "slug", path, checkIsKebab = true),
       hasBoolean(data, "active", path),
       hasString(data, "blurb", path, maxLen = 400),
       hasInteger(data, "version", path, allowed = 3..3),

src/lint/validators.nim

@@ -1,4 +1,4 @@
-import std/[json, os, sets, streams, strformat, strutils, unicode]
+import std/[json, os, parseutils, sets, streams, strformat, strutils, unicode]
 import ".."/helpers
 
 func allTrue*(bools: openArray[bool]): bool =
@@ -75,9 +75,47 @@ proc isUrlLike(s: string): bool =
 const
   emptySetOfStrings = initHashSet[string](0)
 
+func isKebabCase*(s: string): bool =
+  ## Returns true if `s` is a kebab-case string. By our definition, `s` must:
+  ## - Have a non-zero length
+  ## - Start and end with a character in `[a-z0-9]`
+  ## - Consist only of characters in `[a-z0-9-]`
+  ## - Not contain consecutive `-` characters
+  ##
+  ## This is equivalent to matching the below regular expression:
+  ##
+  ## `^[a-z0-9]+(?:-[a-z0-9]+)*$`.
+  ##
+  ## However, this func's implementation is faster than one that uses
+  ## `re.match`, and doesn't add a PCRE dependency.
+  runnableExamples:
+    assert isKebabCase("hello")
+    assert isKebabCase("hello-world")
+    assert isKebabCase("123") # Can contain only digits.
+    assert not isKebabCase("") # Cannot be the empty string.
+    assert not isKebabCase("hello world") # Cannot contain a space.
+    assert not isKebabCase("hello_world") # Cannot contain an underscore.
+    assert not isKebabCase("helloWorld") # Cannot contain an uppercase letter.
+    assert not isKebabCase("hello--world") # Cannot contain consecutive dashes.
+    assert not isKebabCase("hello!") # Cannot contain a special character.
+
+  const lowerAndDigits = {'a'..'z', '0'..'9'}
+  let sLen = s.len
+  var i = 0
+  while i < sLen:
+    if s[i] == '-':
+      return false
+    i += s.skipWhile(lowerAndDigits, start = i)
+    if i == sLen:
+      return true
+    elif s[i] == '-':
+      inc i
+    else:
+      return false
+
 proc isString*(data: JsonNode; key: string; path: Path; context: string;
                isRequired = true; allowed = emptySetOfStrings;
-               checkIsUrlLike = false; maxLen = int.high;
+               checkIsUrlLike = false; maxLen = int.high; checkIsKebab = false;
                isInArray = false): bool =
   result = true
   case data.kind
@@ -101,6 +139,16 @@ proc isString*(data: JsonNode; key: string; path: Path; context: string;
         result.setFalseAndPrint(&"Not a valid URL: {q s}", path)
     elif s.len > 0:
       if not isEmptyOrWhitespace(s):
+        if checkIsKebab:
+          if not isKebabCase(s):
+            let msg =
+              if isInArray:
+                &"The {format(context, key)} array contains {q s}, but every " &
+                 "value must be lowercase and kebab-case"
+              else:
+                &"The {format(context, key)} value is {q s}, but it must be " &
+                 "a lowercase and kebab-case string"
+            result.setFalseAndPrint(msg, path)
         if not hasValidRuneLength(s, key, path, context, maxLen):
           result = false
       else:
@@ -131,10 +179,11 @@ proc isString*(data: JsonNode; key: string; path: Path; context: string;
 
 proc hasString*(data: JsonNode; key: string; path: Path; context = "";
                 isRequired = true; allowed = emptySetOfStrings;
-                checkIsUrlLike = false; maxLen = int.high): bool =
+                checkIsUrlLike = false; maxLen = int.high;
+                checkIsKebab = false): bool =
   if data.hasKey(key, path, context, isRequired):
     result = isString(data[key], key, path, context, isRequired, allowed,
-                      checkIsUrlLike, maxLen)
+                      checkIsUrlLike, maxLen, checkIsKebab = checkIsKebab)
   elif not isRequired:
     result = true
 
@@ -143,7 +192,8 @@ proc isArrayOfStrings*(data: JsonNode;
                        path: Path;
                        isRequired = true;
                        allowed: HashSet[string];
-                       allowedArrayLen: Slice): bool =
+                       allowedArrayLen: Slice;
+                       checkIsKebab: bool): bool =
   ## Returns true in any of these cases:
   ## - `data` is a `JArray` with length in `allowedArrayLen` that contains only
   ##   non-empty, non-blank strings.
@@ -157,7 +207,7 @@ proc isArrayOfStrings*(data: JsonNode;
       if arrayLen in allowedArrayLen:
         for item in data:
           if not isString(item, context, path, "", isRequired, allowed,
-                          isInArray = true):
+                          checkIsKebab = checkIsKebab, isInArray = true):
             result = false
       else:
         let msgStart = &"The {q context} array has length {arrayLen}, " &
@@ -184,14 +234,16 @@ proc hasArrayOfStrings*(data: JsonNode;
                         context = "";
                         isRequired = true;
                         allowed = emptySetOfStrings;
-                        allowedArrayLen = 1..int.high): bool =
+                        allowedArrayLen = 1..int.high;
+                        checkIsKebab = false): bool =
   ## Returns true in any of these cases:
   ## - `isArrayOfStrings` returns true for `data[key]`.
   ## - `data` lacks the key `key` and `isRequired` is false.
   if data.hasKey(key, path, context, isRequired):
     let contextAndKey = joinWithDot(context, key)
     result = isArrayOfStrings(data[key], contextAndKey, path, isRequired,
-                              allowed, allowedArrayLen)
+                              allowed, allowedArrayLen,
+                              checkIsKebab = checkIsKebab)
   elif not isRequired:
     result = true
 

tests/all_tests.nim

@@ -1,5 +1,6 @@
 import "."/[
   test_binary,
+  test_lint,
   test_probspecs,
   test_uuid,
 ]

tests/test_lint.nim

@@ -0,0 +1,70 @@
+import std/unittest
+import "."/[lint/validators]
+
+proc main =
+  suite "isKebabCase":
+    test "invalid kebab-case strings":
+      check:
+        # Some short, invalid strings
+        not isKebabCase("")
+        not isKebabCase(" ")
+        not isKebabCase("-")
+        not isKebabCase("_")
+        not isKebabCase("--")
+        not isKebabCase("---")
+        not isKebabCase("a ")
+        not isKebabCase(" a")
+        not isKebabCase("a-")
+        not isKebabCase("-a")
+        not isKebabCase("--a")
+        not isKebabCase("a--")
+        not isKebabCase("-a-")
+        not isKebabCase("a--b")
+        # Containing character not in [a-z0-9]
+        not isKebabCase("&")
+        not isKebabCase("&str")
+        not isKebabCase("hello!")
+        # Invalid dash usage
+        not isKebabCase("hello-world-")
+        not isKebabCase("-hello-world")
+        not isKebabCase("-hello-world-")
+        not isKebabCase("hello--world")
+        not isKebabCase("hello---world")
+        # Invalid separator: space
+        not isKebabCase("hello world")
+        not isKebabCase("hello World")
+        not isKebabCase("Hello world")
+        not isKebabCase("Hello World")
+        not isKebabCase("HELLO WORLD")
+        # Invalid separator: underscore
+        not isKebabCase("hello_world")
+        not isKebabCase("hello_World")
+        not isKebabCase("Hello_world")
+        not isKebabCase("Hello_World")
+        not isKebabCase("HELLO_WORLD")
+        # Containing uppercase, with dash
+        not isKebabCase("hello-World")
+        not isKebabCase("Hello-world")
+        not isKebabCase("Hello-World")
+        not isKebabCase("HELLO-WORLD")
+        # Containing uppercase, with no separator
+        not isKebabCase("helloWorld")
+        not isKebabCase("Helloworld")
+        not isKebabCase("HelloWorld")
+        not isKebabCase("HELLOWORLD")
+
+    test "valid kebab-case strings":
+      check:
+        isKebabCase("a")
+        isKebabCase("1")
+        isKebabCase("123")
+        isKebabCase("123-456")
+        isKebabCase("hello-123")
+        isKebabCase("123-hello")
+        isKebabCase("hello")
+        isKebabCase("hello-world")
+        isKebabCase("hello-world-hello")
+        isKebabCase("hello-world-hello-world")
+
+main()
+{.used.}