Skip to content
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

Run composer validate from different commands #7492

Closed
staabm opened this issue Jul 25, 2018 · 6 comments
Closed

Run composer validate from different commands #7492

staabm opened this issue Jul 25, 2018 · 6 comments
Labels
Milestone

Comments

@staabm
Copy link
Contributor

staabm commented Jul 25, 2018

As discussed in #5940 (comment) I would like to propose to internally call „composer validate“ from different commands, which can help/should help to find errors caused by invalid composer.json more easily.

I guess it would help in the following commands:

  • at the end of composer diagnose
  • when composer update/install runs into a error

What do you guys think?

@Seldaek
Copy link
Member

Seldaek commented Jul 26, 2018

  • diagnose already does at least a partial schema check in checkComposerSchema
  • Factory::createComposer also does a LAX_SCHEMA validation of the schema when loading the composer.json, which applies to all commands. But this only outputs errors, not warnings as that'd be very invasive.

If you wanna do more why not as long as it doesn't nag people needlessly.. I don't think this problem occurs that often tbh but obviously it's hard to tell.

@staabm
Copy link
Contributor Author

staabm commented Jul 26, 2018

Factory::createComposer also does a LAX_SCHEMA validation of the schema when loading the composer.json, which applies to all commands

is this LAX validation done, because of performance problems or do not all operations require a strict schema?

If you wanna do more why not as long as it doesn't nag people needlessly.. I don't think this problem occurs that often tbh but obviously it's hard to tell.

in the regular used commands like composer install/update I would only add this additional check in case of errors, so the regular happy path shouldn't see any more errors/warnings then before. at least this would be the goal.

@Seldaek
Copy link
Member

Seldaek commented Jul 26, 2018

Lax just means this:

            $schemaData->additionalProperties = true;
            $schemaData->required = array();

So we don't check for typo in top level property names, and we don't force name to be present. It's not that big a difference.

@stof
Copy link
Contributor

stof commented Jul 26, 2018

note that composer validate performs more validation than only the schema validation.

@Seldaek Seldaek added this to the Nice To Have milestone Oct 14, 2020
@Seldaek
Copy link
Member

Seldaek commented May 24, 2022

Closing as IMO doing a full validate is probably overkill, but we have narrowed down the schema quite a bit, and do validate composer.json + auth.json now, and also load the root package with a validating array loader which does 90% of validate command stuff.

@Seldaek Seldaek closed this as completed May 24, 2022
@staabm
Copy link
Contributor Author

staabm commented May 24, 2022

Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants