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

Conversation

ee7
Copy link
Member

@ee7 ee7 commented Feb 3, 2021

This PR implements the concept/<slug>/links.json part of the spec:

  • 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, non-blank string
  • The "[].icon_url" property is optional
  • The "[].icon_url" value must be an URL

To-do:

@ee7 ee7 force-pushed the lint-concepts-links branch 2 times, most recently from 655ff42 to 67d2eea Compare March 18, 2021 10:31
@ee7
Copy link
Member Author

ee7 commented Mar 18, 2021

@ErikSchierboom Just to confirm, what should configlet lint do if it encounters:

  1. A links.json file that is empty? For example, here - there are 4 of these in total across Exercism. Currently, this PR prints
JSON parsing error:
./concepts/enumeration/links.json(1, 0) Error: { expected

In the future we could detect empty files and print a better error message.

  1. A links.json file that is an empty array? For example, here - there are 238 of these in total across Exercism. Right now this PR prints e.g.
Array is empty: root:
./concepts/const-readonly/links.json

but it should't complain about this, right?

@ee7 ee7 marked this pull request as ready for review March 18, 2021 12:04
@ErikSchierboom
Copy link
Member

A links.json file that is empty?

That is an error.

A links.json file that is an empty array?

That's fine.

@ee7
Copy link
Member Author

ee7 commented Mar 19, 2021

A links.json file that is empty?

That is an error.

A links.json file that is an empty array?

That's fine.

Done.

This PR now causes the following diff to the configlet lint output, per track:

Tracks that no longer exit with 0

haxe
java
python

clojure

+File is empty, but must contain at least the empty array, `[]`:
+./concepts/basics/links.json
+

haxe

These files are an object at the top level, see e.g. https://github.com/exercism/haxe/blob/717a4d3cd01ad1c8439501b64b9c3ff46b3f59e7/concepts/abstract-types/links.json

+Not an array: root:
+./concepts/abstract-types/links.json
+
+Not an array: root:
+./concepts/array-comprehension/links.json
+
+Not an array: root:
+./concepts/arrays/links.json
+
+Not an array: root:
+./concepts/basics/links.json
+
+Not an array: root:
+./concepts/bit-manipulation/links.json
+
+Not an array: root:
+./concepts/chars/links.json
+
+Not an array: root:
+./concepts/classes/links.json
+
+Not an array: root:
+./concepts/constructors/links.json
+
+Not an array: root:
+./concepts/datetimes/links.json
+
+Not an array: root:
+./concepts/do-while-loops/links.json
+
+Not an array: root:
+./concepts/enums/links.json
+
+Not an array: root:
+./concepts/equality/links.json
+
+Not an array: root:
+./concepts/exceptions/links.json
+
+Not an array: root:
+./concepts/final/links.json
+
+Not an array: root:
+./concepts/flag-enums/links.json
+
+Not an array: root:
+./concepts/floating-point-numbers/links.json
+
+Not an array: root:
+./concepts/for-loops/links.json
+
+Not an array: root:
+./concepts/foreach-loops/links.json
+
+Not an array: root:
+./concepts/generic-types/links.json
+
+Not an array: root:
+./concepts/if-statements/links.json
+
+Not an array: root:
+./concepts/indexers/links.json
+
+Not an array: root:
+./concepts/inheritance/links.json
+
+Not an array: root:
+./concepts/integral-numbers/links.json
+
+Not an array: root:
+./concepts/interfaces/links.json
+
+Not an array: root:
+./concepts/iterators/links.json
+
+Not an array: root:
+./concepts/lambda/links.json
+
+Not an array: root:
+./concepts/lists/links.json
+
+Not an array: root:
+./concepts/maps/links.json
+
+Not an array: root:
+./concepts/nullability/links.json
+
+Not an array: root:
+./concepts/numbers/links.json
+
+Not an array: root:
+./concepts/operator-overloading/links.json
+
+Not an array: root:
+./concepts/option-type/links.json
+
+Not an array: root:
+./concepts/optional-arguments/links.json
+
+Not an array: root:
+./concepts/pattern-matching/links.json
+
+Not an array: root:
+./concepts/properties/links.json
+
+Not an array: root:
+./concepts/randomness/links.json
+
+Not an array: root:
+./concepts/reflection/links.json
+
+Not an array: root:
+./concepts/regular-expressions/links.json
+
+Not an array: root:
+./concepts/rest-args/links.json
+
+Not an array: root:
+./concepts/string-buffer/links.json
+
+Not an array: root:
+./concepts/switch-expressions/links.json
+
+Not an array: root:
+./concepts/templates/links.json
+
+Not an array: root:
+./concepts/ternary-operators/links.json
+
+Not an array: root:
+./concepts/throw-expressions/links.json
+
+Not an array: root:
+./concepts/time/links.json
+
+Not an array: root:
+./concepts/typedefs/links.json
+
+Not an array: root:
+./concepts/using-statements/links.json
+
+Not an array: root:
+./concepts/while-loops/links.json

java

link key used instead of url, see: https://github.com/exercism/java/blob/2cbf4223fc42b869f455ed4311208ef306026e6b/concepts/for-loops/links.json

+Missing key: 'url':
+./concepts/for-loops/links.json
+
+Missing key: 'url':
+./concepts/foreach-loops/links.json

python

Invalid capitalization of the url key: https://github.com/exercism/python/blob/ed7dadddd94a969ad2739ac1a4b115c3bbe459a7/concepts/numbers/links.json#L35 (JSON is case-sensitive)

+Missing key: 'url':
+./concepts/numbers/links.json

ruby

+
+File is empty, but must contain at least the empty array, `[]`:
+./concepts/enumeration/links.json
+
+File is empty, but must contain at least the empty array, `[]`:
+./concepts/exceptions/links.json
+
+File is empty, but must contain at least the empty array, `[]`:
+./concepts/ostruct/links.json

swift

See https://github.com/exercism/swift/blob/88c32b55ec5a2ad555236a1155818deb616171f4/concepts/shorthand-arguments/links.json

+
+Missing file:
+./concepts/characters/links.json
+
+Missing file:
+./concepts/classes/links.json
+
+Missing file:
+./concepts/initializers/links.json
+
+Missing file:
+./concepts/opaque-indices/links.json
+
+Missing key: 'url':
+./concepts/shorthand-arguments/links.json
+
+Missing key: 'description':
+./concepts/shorthand-arguments/links.json
+
+Missing file:
+./concepts/strings/links.json
+
+Missing file:
+./concepts/structs/links.json

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Two minor nits

src/lint/concepts.nim Outdated Show resolved Hide resolved
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.

src/lint/concepts.nim Outdated Show resolved Hide resolved
ee7 added 3 commits March 19, 2021 17:25
We can do without this optimization - about 97% of the URLs in
`links.json` files so far start with `https://` anyway.
@ee7 ee7 merged commit 01a09d8 into exercism:main Mar 19, 2021
@ee7 ee7 deleted the lint-concepts-links branch March 19, 2021 16:53
@ee7 ee7 changed the title lint: add linting of concepts links.json lint(concepts): implement links.json checks Mar 19, 2021
BethanyG added a commit to exercism/python that referenced this pull request Mar 19, 2021
`URL` --> `url. 
Python had a configlet linting error :  `Invalid capitalization of the url key:`, as referenced in exercism/configlet#171.  
This fixes that error.
BethanyG added a commit to exercism/python that referenced this pull request Mar 20, 2021
`URL` --> `url. 
Python had a configlet linting error :  `Invalid capitalization of the url key:`, as referenced in exercism/configlet#171.  
This fixes that error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants