refactor(parser): Refactor toml dependency validation

[adamgemmell]

What does this PR try to resolve?

The match statement in to_dependency_source_id() was too complex, especially when extended for builtin dependencies (#16960). This PR moves the checks for malformed toml dependencies to a helper function which allows simplifying the match statement considerably.

The PR should preserve the behaviour of the TOML parsing, even for non-intuitive cases like those tested for in alt_registry. The PR does not preserve the order in which errors are checked.

This PR is part of a chain:

• feat(builtin-deps): Add unstable builtin-dependencies feature #17497
• refactor(parser): Refactor toml dependency validation #17502
• feat(builtin-deps): Add builtin dependencies manifest syntax #17498
• fix(builtin-deps): Builtin dependencies manifest validation #17499
• test(builtin-deps): Additional manifest regression tests #17500

How to test and review this PR?

Review commit-by-commit for simpler diffs.

[rustbot]

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @Muscraft, @epage, @weihanglo
• @Muscraft, @epage, @weihanglo expanded to Muscraft, epage, weihanglo
• Random selection from Muscraft, epage, weihanglo

[weihanglo]

Thanks for the refactor! It does improve a bit but not substantially. The nested if-else blocks are also not really readable.

I think we probably should leave the match statement there and move forward like what Ed has suggested. We can always come back later to fix this structurally.

Not a blocker really, and sorry that you've already spent time on this refactor 😞

View changes since the review

[adamgemmell]

No worries, I was just trying something. I agree that ideally something more substantial should happen, but this PR does avoid adding an extra dimension to the match statement for builtins. I can't find Ed's suggestion for this, do you have a link?