[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
hyperupcall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/1376


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140462

Files:
  clang-tools-extra/clangd/schema/config.json

Index: clang-tools-extra/clangd/schema/config.json
===
--- /dev/null
+++ clang-tools-extra/clangd/schema/config.json
@@ -0,0 +1,192 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema";,
+  "$defs": {
+"stringOrArrayOf": {
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+}
+  },
+  "type": "object",
+  "properties": {
+"If": {
+  "description": "Conditions in the If block restrict when a fragment applies.",
+  "type": "object",
+  "properties": {
+"PathMatch": {
+  "description": "The file being processed must fully match a regular expression.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"PathExclude": {
+  "description": "The file being processed must not fully match a regular expression.",
+  "$ref": "#/$defs/stringOrArrayOf"
+}
+  }
+},
+"CompileFlags": {
+  "description": "Affects how a source file is parsed.",
+  "type": "object",
+  "properties": {
+"Add": {
+  "description": "List of flags to append to the compile command.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"Remove": {
+  "description": "List of flags to remove from the compile command",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"CompilationDatabase": {
+  "description": "Directory to search for compilation database (compile_commands.json etc).",
+  "type": "string"
+},
+"Compiler": {
+  "description": "String to replace the executable name in the compile flags. The name controls flag parsing (clang vs clang-cl), target inference (gcc-arm-noneabi) etc.",
+  "type": "string"
+}
+  }
+},
+"Index": {
+  "description": "Controls how clangd understands code outside the current file.",
+  "type": "object",
+  "properties": {
+"Background": {
+  "description": "Whether files are built in the background to produce a project index. This is checked for translation units only, not headers they include.",
+  "type": "string",
+  "enum": [
+"Build",
+"Skip"
+  ],
+  "default": "Build"
+},
+"External": {
+  "description": "Used to define an external index source",
+  "type": "object",
+  "oneOf": [
+{
+  "properties": {
+"File": {
+  "type": "string"
+},
+"Server": {
+  "type": "string"
+},
+"MountPoint": {
+  "type": "string"
+}
+  },
+  "required": [
+"File"
+  ]
+},
+{
+  "properties": {
+"File": {
+  "type": "string"
+},
+"Server": {
+  "type": "string"
+},
+"MountPoint": {
+  "type": "string"
+}
+  },
+  "required": [
+"Server"
+  ]
+}
+  ]
+}
+  }
+},
+"Style": {
+  "description": "Describes the style of the codebase, beyond formatting",
+  "type": "object",
+  "properties": {
+"FullyQualifiedNamespaces": {
+  "type": "boolean"
+}
+  }
+},
+"Diagnostics": {
+  "type": "object",
+  "properties": {
+"Suppress": {
+  "description": "Diagnostic codes that should be suppressed.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"ClangTidy": {
+  "description": "Configure how clang-tidy runs over your files. The settings are merged with any settings found in .clang-tidy configuration files with the ones from clangd configs taking precedence.",
+  "type": "object",
+  "properties": {
+"Add": {
+  "description": "List of checks to enable, can be globs.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"Remove": {
+  "description": "List of checks to disable, can be globs.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"CheckOptions": {
+ 

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall updated this revision to Diff 485351.
hyperupcall added a comment.

Improve schema correctness


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140462/new/

https://reviews.llvm.org/D140462

Files:
  clang-tools-extra/clangd/schema/config.json

Index: clang-tools-extra/clangd/schema/config.json
===
--- /dev/null
+++ clang-tools-extra/clangd/schema/config.json
@@ -0,0 +1,180 @@
+{
+	"$schema": "http://json-schema.org/draft-07/schema";,
+	"$defs": {
+		"stringOrArrayOf": {
+			"oneOf": [
+{
+	"type": "string"
+},
+{
+	"type": "array",
+	"items": {
+		"type": "string"
+	}
+}
+			]
+		}
+	},
+	"type": "object",
+	"properties": {
+		"If": {
+			"description": "Conditions in the If block restrict when a fragment applies.",
+			"type": "object",
+			"properties": {
+"PathMatch": {
+	"description": "The file being processed must fully match a regular expression.",
+	"$ref": "#/$defs/stringOrArrayOf"
+},
+"PathExclude": {
+	"description": "The file being processed must not fully match a regular expression.",
+	"$ref": "#/$defs/stringOrArrayOf"
+}
+			}
+		},
+		"CompileFlags": {
+			"description": "Affects how a source file is parsed.",
+			"type": "object",
+			"properties": {
+"Add": {
+	"description": "List of flags to append to the compile command.",
+	"$ref": "#/$defs/stringOrArrayOf"
+},
+"Remove": {
+	"description": "List of flags to remove from the compile command",
+	"$ref": "#/$defs/stringOrArrayOf"
+},
+"CompilationDatabase": {
+	"description": "Directory to search for compilation database (compile_commands.json etc).",
+	"oneOf": [
+		{
+			"type": "string"
+		},
+		{
+			"enum": ["Ancestors", "None"]
+		}
+	]
+},
+"Compiler": {
+	"description": "String to replace the executable name in the compile flags. The name controls flag parsing (clang vs clang-cl), target inference (gcc-arm-noneabi) etc.",
+	"type": "string"
+}
+			}
+		},
+		"Index": {
+			"description": "Controls how clangd understands code outside the current file.",
+			"type": "object",
+			"properties": {
+"Background": {
+	"description": "Whether files are built in the background to produce a project index. This is checked for translation units only, not headers they include.",
+	"type": "string",
+	"enum": ["Build", "Skip"],
+	"default": "Build"
+},
+"External": {
+	"description": "Used to define an external index source",
+	"type": "object",
+	"oneOf": [
+		{
+			"properties": {
+"File": {
+	"type": "string"
+}
+			},
+			"required": ["File"]
+		},
+		{
+			"properties": {
+"Server": {
+	"type": "string"
+},
+"MountPoint": {
+	"type": "string"
+}
+			},
+			"required": ["Server"]
+		}
+	]
+}
+			}
+		},
+		"Style": {
+			"description": "Describes the style of the codebase, beyond formatting",
+			"type": "object",
+			"properties": {
+"FullyQualifiedNamespaces": {
+	"type": "boolean"
+}
+			}
+		},
+		"Diagnostics": {
+			"type": "object",
+			"properties": {
+"Suppress": {
+	"description": "Diagnostic codes that should be suppressed.",
+	"$ref": "#/$defs/stringOrArrayOf"
+},
+"ClangTidy": {
+	"description": "Configure how clang-tidy runs over your files. The settings are merged with any settings found in .clang-tidy configuration files with the ones from clangd configs taking precedence.",
+	"type": "object",
+	"properties": {
+		"Add": {
+			"description": "List of checks to enable, can be globs.",
+			"$ref": "#/$defs/stringOrArrayOf"
+		},
+		"Remove": {
+			"description": "List of checks to disable, can be globs.",
+			"$ref": "#/$defs/stringOrArrayOf"
+		},
+		"CheckOptions": {
+			"description": "Key-value pairs of options for clang-tidy checks",
+			"type": "object"
+		},
+		"UnusedIncludes": {
+			"description": "Enables Include Cleaner's unused includes diagnostics.",
+			"type": "string",
+			"enum": ["None", "Strict"],
+			"default": "None"
+		}
+	}
+}
+			}
+		},
+		"Completion": {
+			"type": "object",
+			"properties": {
+"AllScopes": {
+	"description": "Whether code completion should include suggestions from scopes that are not visible. The required scope prefix will be inserted.",
+	"type": "boolean"
+}
+			}
+		},
+		"InlayHints": {
+			"description": "Configures the behaviour of the inlay-hints feature.",
+			"type": "object",
+			"properties": {
+"Enabled": {
+	"description": "A boolean that enables/disables the inlay-hints feature for all kinds, when disabled, configuration for specific kinds are ignored.",
+	"type": "boolean"
+}

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall updated this revision to Diff 485352.
hyperupcall added a comment.

Apply `clang-format`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140462/new/

https://reviews.llvm.org/D140462

Files:
  clang-tools-extra/clangd/schema/config.json

Index: clang-tools-extra/clangd/schema/config.json
===
--- /dev/null
+++ clang-tools-extra/clangd/schema/config.json
@@ -0,0 +1,193 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema";,
+  "$defs": {
+"stringOrArrayOf": {
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+}
+  },
+  "type": "object",
+  "properties": {
+"If": {
+  "description": "Conditions in the If block restrict when a fragment applies.",
+  "type": "object",
+  "properties": {
+"PathMatch": {
+  "description": "The file being processed must fully match a regular expression.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"PathExclude": {
+  "description": "The file being processed must not fully match a regular expression.",
+  "$ref": "#/$defs/stringOrArrayOf"
+}
+  }
+},
+"CompileFlags": {
+  "description": "Affects how a source file is parsed.",
+  "type": "object",
+  "properties": {
+"Add": {
+  "description": "List of flags to append to the compile command.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"Remove": {
+  "description": "List of flags to remove from the compile command",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"CompilationDatabase": {
+  "description": "Directory to search for compilation database (compile_commands.json etc).",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "enum": [
+"Ancestors",
+"None"
+  ]
+}
+  ]
+},
+"Compiler": {
+  "description": "String to replace the executable name in the compile flags. The name controls flag parsing (clang vs clang-cl), target inference (gcc-arm-noneabi) etc.",
+  "type": "string"
+}
+  }
+},
+"Index": {
+  "description": "Controls how clangd understands code outside the current file.",
+  "type": "object",
+  "properties": {
+"Background": {
+  "description": "Whether files are built in the background to produce a project index. This is checked for translation units only, not headers they include.",
+  "type": "string",
+  "enum": [
+"Build",
+"Skip"
+  ],
+  "default": "Build"
+},
+"External": {
+  "description": "Used to define an external index source",
+  "type": "object",
+  "oneOf": [
+{
+  "properties": {
+"File": {
+  "type": "string"
+}
+  },
+  "required": [
+"File"
+  ]
+},
+{
+  "properties": {
+"Server": {
+  "type": "string"
+},
+"MountPoint": {
+  "type": "string"
+}
+  },
+  "required": [
+"Server"
+  ]
+}
+  ]
+}
+  }
+},
+"Style": {
+  "description": "Describes the style of the codebase, beyond formatting",
+  "type": "object",
+  "properties": {
+"FullyQualifiedNamespaces": {
+  "type": "boolean"
+}
+  }
+},
+"Diagnostics": {
+  "type": "object",
+  "properties": {
+"Suppress": {
+  "description": "Diagnostic codes that should be suppressed.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"ClangTidy": {
+  "description": "Configure how clang-tidy runs over your files. The settings are merged with any settings found in .clang-tidy configuration files with the ones from clangd configs taking precedence.",
+  "type": "object",
+  "properties": {
+"Add": {
+  "description": "List of checks to enable, can be globs.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"Remove": {
+  "description": "List of checks to disable, can be globs.",
+  "$ref": "#/$defs/stringOrArrayOf"
+},
+"CheckOptions": {
+  "description": "Key-value pairs of options for clang-tidy checks",
+  "type": "object"
+},
+"UnusedIncludes": {
+  "description": "Enables Include Cleaner's un

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall added a comment.

Thank you for the suggestions - I applied all of the fixes!

As someone who helps maintain a lot of the schemas 
 on schemastore, it's nice when 
schemas are in-tree with their respective project, but I totally understand 
that this can increase complexity, especially for the reasons stated.

In D140462#4014944 , @sammccall wrote:

> given lack of any automation/tests.



In D140462#4014997 , @nridge wrote:

> And perhaps, if someone contributes automated tests for the schema in the 
> future, we could consider upgrading its status to "maintained" at that time?

If you would like me to add tests to verify the schema (for now or later?), is 
there a utility within LLVM to help do so? I saw TableGen was mentioned, but it 
sounds like the schema validation use case is a different issue.

In D140462#4014944 , @sammccall wrote:

> One practical question: AIUI the main point of having this is so it can be 
> provided to the VSCode YAML extension so it understands .clangd files. How 
> does this work mechanically? Does it need to be part of the vscode-clangd 
> repo instead?

I think it's worth mentioning that the JSON schema can be hosted anywhere. As 
already mentioned, the YAML extension automatically downloads and uses the 
schema if it finds a `.clangd` file, and the schema can point to an external 
URL. One example of that in practice is the xunit-2.2.json 

 schema; it simply contains a `$ref` to the actual schema on xunit.net. Whether 
this schema goes in `clangd/schema/config.json`, 
`clangd/contrib/schema/config.json`, or in `vscode-clangd` - on the SchemaStore 
end, all I have to do is update `$ref`.


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

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall marked 2 inline comments as done.
hyperupcall added a comment.

In D140462#4018983 , @sammccall wrote:

> 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. I only have experience with 
> `ajv`, so I suppose it means



> 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*).

I appreciate that you took the time to experiment with those three 
implementations of JSON-schema - perhaps I was naive in thinking that the 
different implementations were more mature. On my side, this makes me want to 
validate the various schemas in SchemaStore with more implementations than just 
`ajv` to improve the experience for everyone.

> ---
>
> 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

This looks pretty neat! I think something like that revision would be a better 
way forward. I will go ahead and upload a new Diff, I suppose just for 
completeness, but I don't expect anything to change much since it doesn't 
address your points about syncing with documentation or the obscurity of the 
specification.


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

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall updated this revision to Diff 487706.
hyperupcall added a comment.

Improve schema correctness and disable `additionalProperties`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140462/new/

https://reviews.llvm.org/D140462

Files:
  clang-tools-extra/clangd/schema/config.json

Index: clang-tools-extra/clangd/schema/config.json
===
--- /dev/null
+++ clang-tools-extra/clangd/schema/config.json
@@ -0,0 +1,272 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema";,
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"If": {
+  "description": "Conditions in the If block restrict when a fragment applies.",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"PathMatch": {
+  "description": "The file being processed must fully match a regular expression.",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+},
+"PathExclude": {
+  "description": "The file being processed must not fully match a regular expression.",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+}
+  }
+},
+"CompileFlags": {
+  "description": "Affects how a source file is parsed.",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"Add": {
+  "description": "List of flags to append to the compile command.",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+},
+"Remove": {
+  "description": "List of flags to remove from the compile command",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+},
+"CompilationDatabase": {
+  "description": "Directory to search for compilation database (compile_commands.json etc).",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "enum": [
+"Ancestors",
+"None"
+  ]
+}
+  ]
+},
+"Compiler": {
+  "description": "String to replace the executable name in the compile flags. The name controls flag parsing (clang vs clang-cl), target inference (gcc-arm-noneabi) etc.",
+  "type": "string"
+}
+  }
+},
+"Index": {
+  "description": "Controls how clangd understands code outside the current file.",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"Background": {
+  "description": "Whether files are built in the background to produce a project index. This is checked for translation units only, not headers they include.",
+  "type": "string",
+  "enum": [
+"Build",
+"Skip"
+  ],
+  "default": "Build"
+},
+"External": {
+  "description": "Used to define an external index source",
+  "oneOf": [
+{
+  "type": "string",
+  "pattern": "[nN][oO][nN][eE]"
+},
+{
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"File": {
+  "type": "string"
+}
+  },
+  "required": [
+"File"
+  ]
+},
+{
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"Server": {
+  "type": "string"
+},
+"MountPoint": {
+  "type": "string"
+}
+  },
+  "required": [
+"Server"
+  ]
+}
+  ]
+}
+  }
+},
+"Style": {
+  "description": "Describes the style of the codebase, beyond formatting",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"FullyQualifiedNamespaces": {
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  }
+},
+"Diagnostics": {
+  "type": "obj

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall abandoned this revision.
hyperupcall added a comment.

I think I abandon  
this revision for the reasons stated above - there is an improved path forward 
at https://reviews.llvm.org/D140745 with autogenerated files.


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