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

Enhancements to cli.js and CONTRIBUTING.md #4452

Merged
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
20 changes: 20 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- [Ajv non-strict mode](#ajv-non-strict-mode)
- [SchemaSafe](#schemasafe)
- [About `catalog.json`](#about-catalogjson)
- [Avoiding Generic Names](#avoiding-generic-names)
- [Compatible Language Servers and Tools](#compatible-language-servers-and-tools)
- [`redhat-developer/yaml-language-server`](#redhat-developeryaml-language-server)
- [`tamasfe/taplo`](#tamasfetaplo)
Expand Down Expand Up @@ -368,6 +369,25 @@ Sometimes, `catalog.json` is interpreted differently:

And, generally, if a software supports multiple formats, stick with configuration file formats like JSON and avoid JavaScript. See [this](https://github.com/SchemaStore/schemastore/pull/3989) issue.

### Avoid Generic `fileMatch` Patterns

When adding glob patterns to `fileMatch` so language servers can auto-apply schemas, avoid adding generic patterns. For example, [Hugo](https://gohugo.io) used to use `config.toml`:

```jsonc
{
"name": "Hugo",
"description": "Hugo static site generator config file",
"fileMatch": ["config.toml"], // Avoid generic patterns.
"url": "https://json.schemastore.org/hugo.json",
}
```

This would not be accepted because the file detection would have too many false positives, conflicting with other frameworks and personal configurations. There are several ways to fix this:

- Modify the tool to read from a more specific file (Hugo [now reads](https://github.com/gohugoio/hugo/issues/8979) from `hugo.toml` as well)
- Omit `fileMatch` or set it to an empty array (which still allows the user to manually select it)
- Prepend a directory name to the pattern (e.g. `"**/micro/runtime/syntax/*.yaml"`)

## Compatible Language Servers and Tools

### [`redhat-developer/yaml-language-server`](https://github.com/redhat-developer/yaml-language-server)
Expand Down
28 changes: 23 additions & 5 deletions cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ if (argv.SchemaName) {
* @typedef {Object} CatalogJsonEntry
* @property {string} name
* @property {string} description
* @property {string[]} fileMatch
* @property {string[] | undefined} fileMatch
* @property {string} url
* @property {Record<string, string>} versions
*
Expand Down Expand Up @@ -225,7 +225,11 @@ async function forEachFile(/** @type {ForEachTestFile} */ obj) {

const schemaPath = path.join(SchemaDir, schemaName)
const schemaFile = await toFile(schemaPath)
spinner.text = `Running "${obj.actionName}" on file "${schemaFile.path}"`
if (process.env.CI) {
console.info(`Running "${obj.actionName}" on file "${schemaFile.path}"`)
} else {
spinner.text = `Running "${obj.actionName}" on file "${schemaFile.path}"`
}
const data = await obj?.onSchemaFile?.(schemaFile, { spinner })

if (obj?.onPositiveTestFile) {
Expand Down Expand Up @@ -872,8 +876,12 @@ function assertCatalogJsonHasNoDuplicateNames() {
schemaNames.indexOf(catalogEntry.name) !==
schemaNames.lastIndexOf(catalogEntry.name)
) {
const duplicateEntry =
Catalog.schemas[schemaNames.lastIndexOf(catalogEntry.name)]
printErrorAndExit(new Error(), [
`Found two schema entries with duplicate "name" of "${catalogEntry.name}" in file "${CatalogFile}"`,
`The first entry has url "${catalogEntry.url}"`,
`The second entry has url "${duplicateEntry.url}"`,
`Expected the "name" property of schema entries to be unique`,
])
}
Expand Down Expand Up @@ -905,8 +913,8 @@ function assertCatalogJsonHasNoBadFields() {
for (const property of /** @type {const} */ (['name', 'description'])) {
if (catalogEntry?.[property]?.toLowerCase()?.includes('schema')) {
printErrorAndExit(new Error(), [
`Expected the "name" or "description" properties of catalog entries to not include the word "schema"`,
`All files are already schemas, so its meaning is implied`,
`Expected the "name" or "description" properties of entries in "${CatalogFile}" to not include the word "schema"`,
`All specified files are already schemas, so its meaning is implied`,
`If the JSON schema is actually a meta-schema (or some other exception applies), ignore this error by appending to the property "catalogEntryNoLintNameOrDescription" in file "${SchemaValidationFile}"`,
`The invalid entry has a "url" of "${catalogEntry.url}" in file "${CatalogFile}"`,
])
Expand Down Expand Up @@ -949,8 +957,18 @@ function assertCatalogJsonHasNoFileMatchConflict() {
}

if (allFileMatches.includes(fileGlob)) {
const firstEntry = Catalog.schemas.find((entry) =>
entry.fileMatch?.includes(fileGlob),
)
// @ts-expect-error Node v18 supports this.
const lastEntry = Catalog.schemas.findLast((entry) =>
entry.fileMatch?.includes(fileGlob),
)
printErrorAndExit(new Error(), [
`Expected "fileMatch" value of "${fileGlob}" to be unique across all "fileMatch" properties in file "${CatalogFile}"`,
`Found two schema entries with duplicate "fileMatch" entry of "${fileGlob}" in file "${CatalogFile}"`,
`The first entry has url "${firstEntry?.url}"`,
`The second entry has url "${lastEntry?.url}"`,
`Expected all values in "fileMatch" entries to be unique`,
])
}

Expand Down
2 changes: 1 addition & 1 deletion jsconfig.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"compilerOptions": {
"target": "ES2018",
"target": "es2022",
"module": "Node16",
"moduleResolution": "Node16",
"strict": true
Expand Down