Skip to content

Add services-xml fixer to convert Symfony services.xml to services.yaml#1090

Open
Soner (shyim) wants to merge 1 commit into
mainfrom
claude/trusting-johnson-x7virx
Open

Add services-xml fixer to convert Symfony services.xml to services.yaml#1090
Soner (shyim) wants to merge 1 commit into
mainfrom
claude/trusting-johnson-x7virx

Conversation

@shyim

Copy link
Copy Markdown
Member

The extension validator already warns about deprecated services.xml
files (Symfony deprecated the XML configuration format in 7.4 and
removes it in 8.0) but pointed users to an external tool for the
conversion.

This adds a new internal/symfony package that parses Symfony
services.xml files and emits the equivalent services.yaml, covering
parameters, imports, service definitions (all argument types, tags,
calls, factories, configurators, decoration, deprecation, properties,
bindings), defaults, prototypes, instanceof conditionals, aliases,
inline services and when@env blocks. The conversion refuses anything it
cannot represent instead of silently dropping configuration, keeping
the XML file untouched in that case.

A new "services-xml" verifier tool runs the conversion during
"shopware-cli extension fix" and "shopware-cli project fix" for
platform plugins, including extra bundles and locally imported XML
files (import resources are rewritten to the converted YAML files).
The validation message now points to the built-in fixer.

The generated YAML was verified to be semantically equivalent to the
XML by loading both into Symfony 7.4 ContainerBuilders and comparing
the resulting definitions, aliases and parameters.

https://claude.ai/code/session_014UNSrY9MntZK4VAthDwrTk

The extension validator already warns about deprecated services.xml
files (Symfony deprecated the XML configuration format in 7.4 and
removes it in 8.0) but pointed users to an external tool for the
conversion.

This adds a new internal/symfony package that parses Symfony
services.xml files and emits the equivalent services.yaml, covering
parameters, imports, service definitions (all argument types, tags,
calls, factories, configurators, decoration, deprecation, properties,
bindings), defaults, prototypes, instanceof conditionals, aliases,
inline services and when@env blocks. The conversion refuses anything it
cannot represent instead of silently dropping configuration, keeping
the XML file untouched in that case.

A new "services-xml" verifier tool runs the conversion during
"shopware-cli extension fix" and "shopware-cli project fix" for
platform plugins, including extra bundles and locally imported XML
files (import resources are rewritten to the converted YAML files).
The validation message now points to the built-in fixer.

The generated YAML was verified to be semantically equivalent to the
XML by loading both into Symfony 7.4 ContainerBuilders and comparing
the resulting definitions, aliases and parameters.

https://claude.ai/code/session_014UNSrY9MntZK4VAthDwrTk
@github-code-quality

Copy link
Copy Markdown

Code Coverage Overview

Languages: Go

Go / code-coverage/go-test

The overall coverage in the branch is 53%. The coverage in the branch is 50%.

Show a code coverage summary of the most impacted files.
File f3a6da0 4f43031 +/-
internal/verifier/phpstan.go 17% 20% +3%
internal/verifier/rector.go 4% 9% +5%
internal/verifier/prettier.go 7% 14% +7%
internal/verifi...r/phpcsfixer.go 7% 15% +8%
internal/verifier/tool.go 60% 90% +30%
internal/symfony/convert.go 0% 83% +83%
internal/symfon...convert_file.go 0% 84% +84%
internal/verifi...services_xml.go 0% 88% +88%
internal/symfony/phpize.go 0% 94% +94%
internal/symfon.../servicesxml.go 0% 96% +96%

Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f43031756

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment on lines +952 to +953
case "ignore", "null":
prefix = "@?"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject null on-invalid references

When a service argument/property/call uses XML on-invalid="null", Symfony injects null when the target service is missing, but this conversion emits the YAML @? syntax, which is the ignore strategy. In method calls this changes behavior by removing the call instead of calling it with null (and Symfony documents that the null strategy is not supported by the YAML driver), so the fixer should refuse these XML references rather than silently rewriting them as ignore.

Useful? React with 👍 / 👎.

Comment on lines +585 to +586
if service.DecorationOnInvalid != "" {
mapPut(mapping, "decoration_on_invalid", newString(service.DecorationOnInvalid))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Emit a YAML null for decoration-on-invalid

For XML decorators with decoration-on-invalid="null", this writes the string "null" into decoration_on_invalid. Symfony's YAML loader accepts an actual null value for this option and rejects the string "null", so converted files for that valid XML case fail to load after the fixer removes the original XML.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants