Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Config][FrameworkBundle] Generate JSON schema for config#59620

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
valtzu wants to merge1 commit intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromvaltzu:dx-config-schema

Conversation

valtzu
Copy link
Contributor

@valtzuvaltzu commentedJan 26, 2025
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#59603
LicenseMIT

Generate JSON schema in%kernel.build_dir%/config.schema.json for YAML config when the container is built to improve DX.

To discuss:

  1. Is there any way to deal with PHP enums (!php/enum yaml tag)? At the moment they are just ignored/excluded // just allowed a string!php/enum ...
  2. Should we include# $schema: path/to/schema.json in generated yaml files? probably no / up to Flex etc
  3. Any edge-cases I missed?
  4. Should we rather bumpsymfony/config version constraint insymfony/framework-bundle than check for class existence? This is kinda optional stuff so I thought we don't need to enforce it. already bumped in[FrameworkBundle] bump symfony/config requirement to 7.3 #59649

GromNaN, chapterjason, PierreCapel, and Jean-Beru reacted with thumbs up emoji
@GromNaN
Copy link
Member

GromNaN commentedJan 26, 2025
edited
Loading

Thank you very much for working on this feature.
I gave it a try and it works really well.

  • The per-environment configuration need to be added. This is the same as the current root definition (that need to be a type), with key following this pattern:when@[a-zA-Z0-9]+
  • The JSON schema file will be different for each environment, because the bundles are not the sames. The JSON schema must allow any additional root keys forwhen@ other environments.

Here is the files that are not validated by the generated JSON schema.

doctrine.yaml

  • Somedoctrine configuration are not recognized.
image

framework.yaml

routing.yaml

  • "Missing required property 'resource'. Since the configurations are merged from multiple files (and defaults) properties cannot be required in a single Yaml file.

validator.yaml

@valtzu
Copy link
ContributorAuthor

valtzu commentedJan 26, 2025
edited
Loading

Thanks for quick review/comment@GromNaN!

Resolvedwhen@(though it does not account for env-specific extensions yet), removed"required": [...] from the schema, added usingequivalentValues.

The validator issue still on to-do list, not sure on what basis to allow the empty array –maybeuseAttributeAsKey just addedmaxItems: 0 to non-prototyped arrays.

The doctrine issue seems to be caused bynormalization in a closure, some possible solutions below 👇

  1. always allow extra keys / have"additionalProperties": true in the schema (for every subschema), not to cause false-positives on keys that for whatever reason don't appear in the schema
  2. do some manual override / manually defined schema to cover this special case.
  3. convert(dynamically?)the xml schema to json schema(not sure if it matches yaml one-to-one)
GromNaN reacted with thumbs up emoji

@GromNaN
Copy link
Member

Resolvedwhen@(though it does not account for env-specific extensions yet), removed"required": [...] from the schema, added usingequivalentValues.

Thanks, perfect.

The validator issue still on to-do list, not sure on what basis to allow the empty array – maybeuseAttributeAsKey 🤔

I think the JSON schema can be a bit more strict than the config validation rules. But this error may be hiding something else.

The doctrine issue seems to be caused bynormalization in a closure, some possible solutions below 👇

  1. always allow extra keys / have"additionalProperties": true in the schema (for every subschema), not to cause false-positives on keys that for whatever reason don't appear in the schema

Arbitrary additional properties are not allowed by the Config component. This would fail the purpose of the JSON schema.

  1. do some manual override / manually defined schema to cover this special case.

It's the last option if you really can't do any better.

  1. convert(dynamically?)the xml schema to json schema(not sure if it matches yaml one-to-one)

I'd prefer to be able to generate the xsd rather than use it.

Maybe we don't need to support this for a first version, since there's an alternative syntax (more verbose).

@valtzuvaltzuforce-pushed thedx-config-schema branch 2 times, most recently from84f3965 to008ee37CompareJanuary 27, 2025 22:23
Copy link
Member

@GromNaNGromNaN left a comment

Choose a reason for hiding this comment

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

I did a quick code review.

valtzu reacted with thumbs up emoji
@valtzuvaltzuforce-pushed thedx-config-schema branch 3 times, most recently from3cfc28e tode09b68CompareJanuary 29, 2025 18:55
@valtzu
Copy link
ContributorAuthor

Did a whole lot of cleanup based on the comments.

I also managed to workaround the Doctrine issue by allowing any type when$node->getNormalizedTypes() containsExprBuilder::TYPE_ANY.

This of course means we don't get a schema for those, but I think it's better than false-positives when using example configuration.

Also added a deprecation message to the schema which shows up quite nicely now

image

GromNaN and alamirault reacted with thumbs up emoji

@GromNaN
Copy link
Member

Awesome, the deprecation message is exactly why I want such schema validation.

@valtzu
Copy link
ContributorAuthor

valtzu commentedFeb 3, 2025
edited
Loading

I'm a little stuck with the support for env-specific extensions: since the schema is generated inside the kernel, it does not seem possible to generate schemas for other environments. And since the kernel class is not really exposed anywhere, it's not trivial to create multiple instances of the kernel with different envs (not that it would be a great idea anyway).

Best I could think of is

  1. always only build schema for the current environment
  2. in addition to saving the schema into single file inside env-specific directory, also insert a reference into common file outside env-specific dir

This could look something like

var/cache/dev/config.schema.jsonvar/cache/test/config.schema.jsonvar/cache/whatever/config.schema.jsonvar/config.schema.json # references all files above, like {"when@dev": { "$ref": "cache/dev/config.schema.json" } }

The downside with this solution is that unless you build the container for those other envs, you also don't get the schema for those. Also, I doubt that many people build the container locally for any env butdev &test.


If someone has a better approach, great!

Copy link
Member

@GromNaNGromNaN left a comment

Choose a reason for hiding this comment

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

We could look atbundles.php to find all the environments and bundles used, but that wouldn't cover generic use cases (if bundles are registered directly in Kernel.php, for example).

We could also scan all the directories and find all the classes that implement theBundleInterface, but that would require scanning composer autoloader and would take up too many bundles.

I think the idea of generating independent schema files for each env is more realist:

  • generatevar/cache/dev/config.schema.json,var/cache/test/config.schema.json andvar/cache/prod/config.schema.json.
  • when a file is generated, create or update the parentvar/cache/config.schema.json to include the generated fields.

I tried this with PHPStorm, but the file loading doesn't work yet.

{"$schema":"http://json-schema.org/draft-06/schema#","allOf": [    {"$ref":"./dev/config.schema.json" },    {"$ref":"./test/config.schema.json" }  ]}

@valtzuvaltzuforce-pushed thedx-config-schema branch 2 times, most recently from8715b5d to003b6f9CompareFebruary 18, 2025 21:16
@valtzu
Copy link
ContributorAuthor

valtzu commentedFeb 18, 2025
edited
Loading

I tried this with PHPStorm, but the file loading doesn't work yet.

Yeah, that is sad. I ended up putting everything in a single file,%kernel.build_dir%/../config.schema.json.

Also switched fromarray to\stdClass because after merging configs between envs I was getting[] where{} was expected, breaking the whole schema.

So now you'd need to run

bin/console --env=devbin/console --env=testbin/console --env=prod

etc to populate all the configs – though in reality I think most projects have all the bundles & extensions available indev, thus nothing extra exceptdev build should be needed.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@GromNaNGromNaNGromNaN requested changes

Assignees
No one assigned
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

[DX][Config] Generate JSON schema for Yaml configuration
4 participants
@valtzu@GromNaN@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp