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: check files key in track-level config #435

Merged
merged 44 commits into from
Oct 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
9f50c4d
Implement the config.json files section linting.
bobtfish Oct 4, 2021
dc240cc
Add more tests
bobtfish Oct 6, 2021
2e82742
Use a HashSet
bobtfish Oct 6, 2021
82ac0da
Ability to extract multiple placeholders
bobtfish Oct 6, 2021
48e99ce
tests(lint): prefer a `check` block
ee7 Oct 7, 2021
4085ca0
tests(lint): prefer method call syntax for `len`
ee7 Oct 7, 2021
fe82c4f
tests(lint): DRY cases for single placeholder
ee7 Oct 7, 2021
a12bf22
tests(lint): bikeshed test name
ee7 Oct 7, 2021
8a89d84
tests(lint): make call order match declaration order
ee7 Oct 7, 2021
c793924
tests(lint): add test that raises IndexDefect
ee7 Oct 7, 2021
8432a61
lint(validators): fix IndexDefect
ee7 Oct 7, 2021
94400e2
lint(validators): group the mutable globals
ee7 Oct 7, 2021
fbf5faf
lint(validators): indent consistently
ee7 Oct 7, 2021
61f6692
lint(validators): fix long line
ee7 Oct 7, 2021
e5385eb
lint(validators): prefer `notin`
ee7 Oct 7, 2021
5041aaa
lint(validators): move global into proc
ee7 Oct 7, 2021
59317b5
lint(validators): s/placeholder/ph
ee7 Oct 7, 2021
0273683
lint(validators): optimize `extractPlaceholders`
ee7 Oct 7, 2021
41d31f2
lint(validators): prefer `let`
ee7 Oct 7, 2021
817f67a
lint(validators): rename parameter
ee7 Oct 7, 2021
8a80087
lint(validators): prefer `setLen`
ee7 Oct 7, 2021
3ffce9e
lint(validators): make `phStart` a bool, and rename
ee7 Oct 7, 2021
51dcf9b
tests(lint): add tests
ee7 Oct 7, 2021
9975b0f
lint(validators): make iterator private
ee7 Oct 8, 2021
0435e44
tests(lint): use seq comparison
ee7 Oct 8, 2021
e9c1d44
lint(track_config): tweak indentation style
ee7 Oct 8, 2021
6df6fe6
lint(track_config): tweak `hasObject` use
ee7 Oct 8, 2021
f3b544c
lint(validators): prefer `func`
ee7 Oct 8, 2021
9ea0422
move testing-only extractPlaceholders procedure
ee7 Oct 8, 2021
8ffd252
tests(lint): add more `extractPlaceholders` tests
ee7 Oct 8, 2021
c889846
tests(lint): move a test case
ee7 Oct 8, 2021
d7b61d3
tests(lint): improve comments
ee7 Oct 8, 2021
8b6e9cc
Revert "lint(track_config): tweak `hasObject` use"
ee7 Oct 12, 2021
60e26ad
lint(validators): add warning for possible bad placeholder usage
ee7 Oct 12, 2021
b5f8033
tests(lint): check multiple uses of same placeholder
ee7 Oct 12, 2021
a1e8efd
lint(validators): remove final `.` from error msg
ee7 Oct 12, 2021
fe72242
lint(validators): bikeshed backticks
ee7 Oct 12, 2021
24a42ba
lint(validators): specify valid placeholders only once
ee7 Oct 12, 2021
60267c4
lint(validators): return false quicker
ee7 Oct 12, 2021
99ae34d
tests(lint): test placeholder with valid substring
ee7 Oct 12, 2021
7d7cabd
tests(lint): test `isFilesPattern` with multiple placeholders
ee7 Oct 12, 2021
faffd0f
lint(validators): use "patterns" and "placeholders" consistently
ee7 Oct 13, 2021
9febbcd
lint(validators): nitpick "files pattern"
ee7 Oct 13, 2021
01a0c18
tests(lint): test multiple placeholders that are different
ee7 Oct 13, 2021
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
26 changes: 26 additions & 0 deletions src/lint/track_config.nim
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,31 @@ proc hasValidOnlineEditor(data: JsonNode; path: Path): bool =
]
result = allTrue(checks)

proc hasValidFiles(data: JsonNode; path: Path): bool =
const f = "files"
if hasKey(data, f):
if hasObject(data, f, path, isRequired = false):
let checks = [
hasArrayOfStrings(data[f], "solution", path, context = f,
uniqueValues = true, isRequired = false,
checkIsFilesPattern = true),
hasArrayOfStrings(data[f], "test", path, context = f,
uniqueValues = true, isRequired = false,
checkIsFilesPattern = true),
hasArrayOfStrings(data[f], "example", path, context = f,
uniqueValues = true, isRequired = false,
checkIsFilesPattern = true),
hasArrayOfStrings(data[f], "exemplar", path, context = f,
uniqueValues = true, isRequired = false,
checkIsFilesPattern = true),
hasArrayOfStrings(data[f], "editor", path, context = f,
uniqueValues = true, isRequired = false,
checkIsFilesPattern = true),
]
result = allTrue(checks)
else:
result = true

proc hasValidTestRunner(data: JsonNode; path: Path): bool =
const s = "status"
if hasObject(data, s, path):
Expand Down Expand Up @@ -210,6 +235,7 @@ proc satisfiesFirstPass(data: JsonNode; path: Path): bool =
hasInteger(data, "version", path, allowed = 3..3),
hasValidStatus(data, path),
hasValidOnlineEditor(data, path),
hasValidFiles(data, path),
hasValidTestRunner(data, path),
hasValidExercises(data, path),
hasValidConcepts(data, path),
Expand Down
88 changes: 83 additions & 5 deletions src/lint/validators.nim
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,66 @@ func isUuidV4*(s: string): bool =
s[34] in Hex and
s[35] in Hex

iterator extractPlaceholders*(s: string): string =
var i = 0
var expectClosingBrace = false
var ph = ""
while i < s.len:
let c = s[i]
if not expectClosingBrace:
if c == '%':
if i+1 < s.len and s[i+1] == '{':
expectClosingBrace = true
inc i
else:
if c == '}':
yield ph
ph.setLen(0)
expectClosingBrace = false
else:
ph.add c
inc i

const filesPlaceholders = [
"kebab_slug",
"snake_slug",
"camel_slug",
"pascal_slug"
].toHashSet()

func isFilesPattern*(s: string): bool =
if not isEmptyOrWhitespace(s):
result = true
for ph in extractPlaceholders(s):
if ph notin filesPlaceholders:
return false

func list(a: SomeSet[string]; prefix = ""; suffix = ""): string =
## Returns a string that lists the elements of `a`, with `prefix` before
## each element, and `suffix` after each element.
var seen = 0
for item in a:
if seen == 0:
# Use the first item to estimate the final string length.
let estimatedLen = a.len * (item.len + prefix.len + suffix.len + 5)
result = newStringOfCap(estimatedLen)
result.add prefix
result.add item
result.add suffix
inc seen
if seen < a.len-1:
result.add ", "
elif seen == a.len-1:
result.add ", and "

var seenUuids = initHashSet[string](250)
var seenFilePatterns = initHashSet[string](250)

proc isString*(data: JsonNode; key: string; path: Path; context: string;
isRequired = true; allowed = emptySetOfStrings;
checkIsUrlLike = false; maxLen = int.high; checkIsKebab = false;
checkIsUuid = false; isInArray = false): bool =
checkIsUuid = false; isInArray = false;
checkIsFilesPattern = false): bool =
result = true
case data.kind
of JString:
Expand Down Expand Up @@ -224,6 +278,25 @@ proc isString*(data: JsonNode; key: string; path: Path; context: string;
&"A {format(context, key)} value is {q s}, which is not a " &
"lowercased version 4 UUID"
result.setFalseAndPrint(msg, path)
elif checkIsFilesPattern:
if seenFilePatterns.containsOrIncl(s):
let msg =
&"A {format(context, key)} value is {q s}, which is not a unique " &
"`files` entry"
result.setFalseAndPrint(msg, path)
if isFilesPattern(s):
if "%{" in s and "}" notin s:
let msg =
&"A {format(context, key)} value is {q s}, which contains " &
"a possible malformed placeholder. It contains " &
"`%{` but not the terminating `}` character"
warn(msg, path)
else:
const placeholders = list(filesPlaceholders, "%{", "}")
let msg =
&"A {format(context, key)} value is {q s}, which is not a " &
&"valid file pattern. Allowed placeholders are: {placeholders}"
result.setFalseAndPrint(msg, path)
if not hasValidRuneLength(s, key, path, context, maxLen):
result = false
else:
Expand Down Expand Up @@ -270,7 +343,8 @@ proc isArrayOfStrings*(data: JsonNode;
uniqueValues = false;
allowed: HashSet[string];
allowedArrayLen: Slice;
checkIsKebab: bool): bool =
checkIsKebab: bool;
checkIsFilesPattern: 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.
Expand All @@ -286,7 +360,9 @@ proc isArrayOfStrings*(data: JsonNode;

for item in data:
if not isString(item, context, path, "", isRequired, allowed,
checkIsKebab = checkIsKebab, isInArray = true):
checkIsKebab = checkIsKebab,
checkIsFilesPattern = checkIsFilesPattern,
isInArray = true):
result = false
elif uniqueValues:
let itemStr = item.getStr()
Expand Down Expand Up @@ -322,15 +398,17 @@ proc hasArrayOfStrings*(data: JsonNode;
uniqueValues = false;
allowed = emptySetOfStrings;
allowedArrayLen = 1..int.high;
checkIsKebab = false): bool =
checkIsKebab = false;
checkIsFilesPattern = 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,
uniqueValues, allowed, allowedArrayLen,
checkIsKebab = checkIsKebab)
checkIsKebab = checkIsKebab,
checkIsFilesPattern = checkIsFilesPattern)
elif not isRequired:
result = true

Expand Down
80 changes: 80 additions & 0 deletions tests/test_lint.nim
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,89 @@ proc testIsUuidV4 =
else:
check not isUuidV4(uuid)

func extractPlaceholders(s: string): seq[string] =
result = newSeq[string]()
for ph in extractPlaceholders(s):
result.add ph

proc testExtractPlaceholders =
suite "extractPlaceholder":
test "no placeholder":
check:
# 0 placeholder characters
extractPlaceholders("").len == 0
extractPlaceholders("foo").len == 0

# 1 placeholder character
extractPlaceholders("foo%").len == 0
extractPlaceholders("%foo").len == 0
extractPlaceholders("{foo").len == 0
extractPlaceholders("foo}").len == 0

# 2 placeholder characters
extractPlaceholders("%{foo").len == 0
extractPlaceholders("%foo}").len == 0
extractPlaceholders("{foo}").len == 0
extractPlaceholders("%foo{bar").len == 0

# 3 placeholder characters, but badly formed
extractPlaceholders("%foo{bar}").len == 0
extractPlaceholders("%}foo{").len == 0
extractPlaceholders("{%foo}").len == 0

test "one placeholder":
check:
extractPlaceholders("%{}") == @[""]
extractPlaceholders("%{f}") == @["f"]
extractPlaceholders("%{foo}") == @["foo"]
extractPlaceholders("prefix%{foo}") == @["foo"]
extractPlaceholders("%{foo}suffix") == @["foo"]
extractPlaceholders("prefix%{foo}suffix") == @["foo"]
extractPlaceholders("prefix%%{foo}suffix") == @["foo"]
extractPlaceholders("pre%fix%{foo}suffix") == @["foo"]
extractPlaceholders("pre%}fix%{foo}suffix") == @["foo"]
extractPlaceholders("prefix%{foo}s}uffix") == @["foo"]

test "multiple placeholders":
check:
extractPlaceholders("prefix%{foo}bar%{foo}suffix") == @["foo", "foo"]
extractPlaceholders("prefix%{foo}bar%{baz}suffix") == @["foo", "baz"]
extractPlaceholders("prefix%{foo}%{bar}%{baz}suffix") == @["foo", "bar", "baz"]

proc testIsFilesPattern =
suite "isFilesPattern":
test "invalid files patterns":
check:
not isFilesPattern("")
not isFilesPattern(" ")
not isFilesPattern("%{unknown_slug}suffix")
not isFilesPattern("prefix%{unknown_slug}suffix")
not isFilesPattern("prefix%{unknown_slug}")
not isFilesPattern("%{unknown_slug}")
not isFilesPattern("%{ssnake_slug}")
not isFilesPattern("%{snake_slugg}")
not isFilesPattern("somedir/%{snake_slug}/%{unknown_slug}.suffix")

test "valid files patterns":
check:
isFilesPattern("somefile")
isFilesPattern("somefile.go")
isFilesPattern("%{kebab_slug}.js")
isFilesPattern("foo%{snake_slug}bar")
isFilesPattern("foobar%{camel_slug}")
isFilesPattern("%{pascal_slug}")
isFilesPattern("somedir/%{pascal_slug}")
isFilesPattern("somedir/%{pascal_slug}.suffix")
isFilesPattern("somedir/%{pascal_slug}/filename.suffix")
isFilesPattern("somedir/%{pascal_slug}/%{pascal_slug}.suffix")
isFilesPattern("somedir/%{pascal_slug}/%{snake_slug}.suffix")
isFilesPattern("%{pascal_slug}/filename.suffix")

proc main =
testIsKebabCase()
testIsUuidV4()
testExtractPlaceholders()
testIsFilesPattern()

main()
{.used.}