sammccall added a comment. The difficulty in testing is that:
- JSON-schema is totally unknown in llvm-project, so there are no tools but also no expectation that contributors understand it - llvm-project doesn't like dependencies, particularly those that aren't in C++ or python - having done some digging, interop seems really poor... I tried 3 consumers (`ajv-cli`, `yaml-language-server`, and `yajsv`) and **hit different blocking bugs in all of them** when using obvious, spec-compliant approaches to writing schemas. > My feeling is that, at the end of the day, the schema format is relatively > straightforward compared to other things that a clangd reviewer needs to know But this is additional, it's obscure (unrelated to all the other knowledge), and as it's untested, the reviewer needs to reliably catch problems, including subtle interop problems. It has the "python problem", where it's readable and you can see at a glance why the code is correct - even if it isn't. (Having spent a couple of days with json-schema now, I **don't** think it's straightforward. That file seems simple so far, but there are simple C++ programs too! And it carefully dodges the interop issues, which is *subtle*). Meanwhile, changing config **already** is a sea of boilerplate that's hard to review and bugs/divergences have crept in. (Config settings that are compiled but not parsed, things missing from the web docs, web docs being reworded but internal docs staying the same, etc). --- Fixing this is an unreasonable burden to place on a new contributor, I've taken a shot at this and have something almost working: https://reviews.llvm.org/D140745. It's pretty draft-ish: - That patch just checks in the generated files {`Fragment.inc`, `YAML.inc`, `doc.md`, `schema.json`}, (maybe) that belongs in the build system - the schema is correct but `yaml-language-server` chokes on it due to https://github.com/redhat-developer/yaml-language-server/issues/823, so need to restructure - files would need to be moved around etc - tests need to be added/updated - i'm not sure whether having a json-schema for `schema.yaml` is actually a good idea, it was mostly a learning exercise ================ Comment at: clang-tools-extra/clangd/schema/config.json:20 + "properties": { + "If": { + "description": "Conditions in the If block restrict when a fragment applies.", ---------------- disabling `additionalProperties` probably yields useful diagnostics ================ Comment at: clang-tools-extra/clangd/schema/config.json:25 + "PathMatch": { + "description": "The file being processed must fully match a regular expression.", + "$ref": "#/$defs/stringOrArrayOf" ---------------- Unfortunately it's not valid to specify description together with $ref (implementations are required to ignore it) https://datatracker.ietf.org/doc/html/draft-handrews-json-schema-01#section-8.3 ================ Comment at: clang-tools-extra/clangd/schema/config.json:81 + "description": "Used to define an external index source", + "type": "object", + "oneOf": [ ---------------- external can also be the case-insensitive string "None" ================ Comment at: clang-tools-extra/clangd/schema/config.json:115 + "FullyQualifiedNamespaces": { + "type": "boolean" + } ---------------- this is rather a list of strings ================ Comment at: clang-tools-extra/clangd/schema/config.json:172 + }, + "ParameterNames": { + "description": "A boolean that enables/disables inlay-hints for parameter names in function calls.", ---------------- hmm, there are two more categories here - were they missing somewhere? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140462/new/ https://reviews.llvm.org/D140462 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
