-
Notifications
You must be signed in to change notification settings - Fork 23
Add support for emitting usage as a JSON schema #897
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
base: main
Are you sure you want to change the base?
Conversation
- Vendor a copy of a library defining a JSON schema API which can output as a `Map<String, Object?>`. - Add a `jsonSchema` getter on `ArgParser` which is similar to `usage` but outputs a schema instead of help text. Clients can manually support a `--json-help` or similar argument in the same way they support `--help`.
PR Health
Breaking changes
|
Package | Change | Current Version | New Version | Needed Version | Looking good? |
---|---|---|---|---|---|
args | Breaking | 2.7.0 | 2.7.0 | 3.0.0 Got "2.7.0" expected >= "3.0.0" (breaking changes) |
This check can be disabled by tagging the PR with skip-breaking-check
.
Changelog Entry ❗
Package | Changed Files |
---|---|
package:args | pkgs/args/lib/src/allow_anything_parser.dart pkgs/args/lib/src/arg_parser.dart pkgs/args/lib/src/json_schema.dart |
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check
.
Coverage ⚠️
File | Coverage |
---|---|
pkgs/args/lib/src/allow_anything_parser.dart | 💔 72 % ⬇️ 4 % |
pkgs/args/lib/src/arg_parser.dart | 💚 95 % ⬆️ 1 % |
pkgs/args/lib/src/json_schema.dart | 💚 17 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check
.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
Package | Leaked API symbols |
---|
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files |
---|
no missing headers |
All source files should start with a license header.
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
// Copied from https://github.com/dart-lang/ai/blob/failures-only/pkgs/dart_mcp/lib/src/api/tools.dart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Copied from https://github.com/dart-lang/ai/blob/failures-only/pkgs/dart_mcp/lib/src/api/tools.dart | |
// Copied from https://github.com/dart-lang/ai/blob/main/pkgs/dart_mcp/lib/src/api/tools.dart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use an exact revision, or put a date on when it was copied.
A reference to a mutable file is informative, but really useful.
If the schema changes, it's good to know if we are behind or not.
Add strong language to always use the `run_tests` tool. Add a JSON schema for the CLI arguments to the test runner. The schema was dumped using dart-lang/core#897 Add a test for the conversion of boolean, string, and list of string arguments, as well as overriding the reporter argument.
Has there been a request for this? If we don't accept options being input as JSON, why are we giving a JSON schema for them? |
} | ||
if (extras.isNotEmpty) { | ||
help = [ | ||
if (help != null) help, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use ?help
now?
// for details. All rights reserved. Use of this source code is governed by a | ||
// BSD-style license that can be found in the LICENSE file. | ||
|
||
// Copied from https://github.com/dart-lang/ai/blob/failures-only/pkgs/dart_mcp/lib/src/api/tools.dart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use an exact revision, or put a date on when it was copied.
A reference to a mutable file is informative, but really useful.
If the schema changes, it's good to know if we are behind or not.
string('string'), | ||
num('number'), | ||
int('integer'), | ||
bool('boolean'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why different names and texts?
I can see that null
can't work as a name, but the rest could just use the same name as the enum entry.
Then entry names are not the Dart types (not capitalized, and the Dart type for object
is Map
, not Object
.)
final String typeName; | ||
} | ||
|
||
/// A JSON Schema object defining the any kind of property. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the any"?
/// See https://json-schema.org/understanding-json-schema/reference for the full | ||
/// specification. | ||
/// | ||
/// **Note:** Only a subset of the json schema spec is supported by these types, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json -> JSON
StringSchema? propertyNames, | ||
int? minProperties, | ||
int? maxProperties, | ||
}) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Could also be redirecting constructor : this.fromMap(...);
. No big difference.)
if (description != null) 'description': description, | ||
if (properties != null) 'properties': properties, | ||
if (patternProperties != null) 'patternProperties': patternProperties, | ||
if (required != null) 'required': required, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With null-aware elements:
ObjectSchema.fromMap({
'type': JsonType.object.typeName,
'title': ?title,
'description': ?description,
'properties': ?properties,
'patternProperties': ?patternProperties,
'required': ?required,
'additionalProperties': ?additionalProperties,
'unevaluatedProperties': ?unevaluatedProperties,
'propertyNames': ?propertyNames,
'minProperties': ?minProperties,
'maxProperties': ?maxProperties,
});
int? get maxProperties => _value['maxProperties'] as int?; | ||
} | ||
|
||
/// A JSON Schema definition for a String. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String
-> `String`
Or "... for a [JsonType.string]."
@@ -392,4 +393,48 @@ class ArgParser { | |||
/// Finds the option whose name or alias matches [name], or `null` if no | |||
/// option has that name or alias. | |||
Option? findByNameOrAlias(String name) => options[_aliases[name] ?? name]; | |||
|
|||
Map<String, Object?> get jsonSchema { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation.
Why is this here? What can it be used for? By whom, and where?
].join('\n'); | ||
} | ||
final schema = switch (option.type) { | ||
OptionType.flag => Schema.bool( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just use BoolSchema
directly, and probably drop the redirecting factories.
They're great for discovery, but this schema code here is completely internal, so we don't need that.
const JsonType(this.typeName); | ||
|
||
final String typeName; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we're not using more that ~half of these, and we're not using the allOf
-etc. properties at all.
Do we need a more complicated framework to generate a relatively simple JSON structure?
properties: properties, | ||
required: required, | ||
).asMap(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically just:
var properties = <String, Object?>{};
var required = [];
for (var option in _options.values) {
if (option.hide) continue;
var Option(:name, :type, :help) = option;
final description = [?help,
if (option.defaultsTo != null) 'defaults to "${option.defaultsTo}"'
if (option.allowed?.isNotEmpty ?? false)
'allowed values: ${option.allowed?.join(', ')}'
].join('\n');
properties[name] = {
...switch (type) {
OptionType.bool => const { 'type': 'boolean' },
OptionType.single => const { 'type': 'string' } ,
OptionType.multi => const {
'type': 'list',
'items': {'type': 'string'},
},
}
if (description.isNotEmpty) 'description': description,
};
if (option.mandatory) required.add(name);
}
return {
'type': 'object',
'properties': properties,
if (required.isNotEmpty) 'required': required
};
}
I'm not sure a complete JSONSchema implementation is worth it, especially one we didn't write ourselves, but is vendoring and now have to maintain anyway.
I used this to create a schema for the test runner which we can use in our MCP server. I don't know if it makes sense to land this here since it might be breaking, but I at least wanted a reference implementation on a shared branch.
I had suggested adding support for a JSON argument file to a few folks in a meeting and no one thought it was a good idea. I don't have strong feelings about it either way, it's pretty easy to convert. We could move the utility which does the conversion here if we want that part shared. extension on Map<String, Object?> {
Iterable<String> asCliArgs() sync* {
for (final MapEntry(:key, :value) in entries) {
if (value is List) {
for (final element in value) {
yield '--$key';
yield element as String;
}
continue;
}
yield '--$key';
if (value is bool) continue;
yield value as String;
}
}
} |
Add strong language to always use the `run_tests` tool. Add a JSON schema for the CLI arguments to the test runner. The schema was dumped using dart-lang/core#897 Add a test for the conversion of boolean, string, and list of string arguments, as well as overriding the reporter argument.
as a
Map<String, Object?>
.jsonSchema
getter onArgParser
which is similar tousage
but outputs a schema instead of help text.
Clients can manually support a
--json-help
or similar argument in thesame way they support
--help
.