Skip to content

Commit 891c666

Browse files
charliermarshcharliermarsh
committed
Add detailed errors for tool.uv.sources deserialization failures
1 parent b6de417 commit 891c666

6 files changed

Lines changed: 214 additions & 111 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/uv-distribution/src/metadata/lowering.rs‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,6 @@ impl LoweredRequirement {
209209
};
210210
(source, marker)
211211
}
212-
Source::CatchAll { .. } => {
213-
// Emit a dedicated error message, which is an improvement over Serde's default error.
214-
return Err(LoweringError::InvalidEntry);
215-
}
216212
};
217213

218214
marker.and(requirement.marker.clone());
@@ -325,11 +321,6 @@ impl LoweredRequirement {
325321
Source::Workspace { .. } => {
326322
return Err(LoweringError::WorkspaceMember);
327323
}
328-
Source::CatchAll { .. } => {
329-
// Emit a dedicated error message, which is an improvement over Serde's default
330-
// error.
331-
return Err(LoweringError::InvalidEntry);
332-
}
333324
};
334325

335326
marker.and(requirement.marker.clone());
@@ -364,8 +355,6 @@ pub enum LoweringError {
364355
UndeclaredWorkspacePackage,
365356
#[error("Can only specify one of: `rev`, `tag`, or `branch`")]
366357
MoreThanOneGitRef,
367-
#[error("Unable to combine options in `tool.uv.sources`")]
368-
InvalidEntry,
369358
#[error("Workspace members are not allowed in non-workspace contexts")]
370359
WorkspaceMember,
371360
#[error(transparent)]

‎crates/uv-distribution/src/metadata/requires_dist.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ mod test {
245245
8 | tqdm = true
246246
| ^^^^
247247
invalid type: boolean `true`, expected an array or map
248-
249248
"###);
250249
}
251250

@@ -281,14 +280,12 @@ mod test {
281280
tqdm = { git = "https://github.com/tqdm/tqdm", ref = "baaaaaab" }
282281
"#};
283282

284-
// TODO(konsti): This should tell you the set of valid fields
285283
assert_snapshot!(format_err(input).await, @r###"
286284
error: TOML parse error at line 8, column 8
287285
|
288286
8 | tqdm = { git = "https://github.com/tqdm/tqdm", ref = "baaaaaab" }
289287
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
290-
data did not match any variant of untagged enum Source
291-
288+
unknown field `ref`, expected one of `git`, `subdirectory`, `rev`, `tag`, `branch`, `url`, `path`, `editable`, `index`, `workspace`, `marker`
292289
"###);
293290
}
294291

@@ -305,14 +302,12 @@ mod test {
305302
tqdm = { path = "tqdm", index = "torch" }
306303
"#};
307304

308-
// TODO(konsti): This should tell you the set of valid fields
309305
assert_snapshot!(format_err(input).await, @r###"
310306
error: TOML parse error at line 8, column 8
311307
|
312308
8 | tqdm = { path = "tqdm", index = "torch" }
313309
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
314-
data did not match any variant of untagged enum Source
315-
310+
cannot specify both `path` and `index`
316311
"###);
317312
}
318313

@@ -349,7 +344,6 @@ mod test {
349344
| ^
350345
invalid string
351346
expected `"`, `'`
352-
353347
"###);
354348
}
355349

@@ -371,8 +365,10 @@ mod test {
371365
|
372366
8 | tqdm = { url = "§invalid#+#*Ä" }
373367
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
374-
data did not match any variant of untagged enum Source
368+
invalid value: string "§invalid#+#*Ä", expected relative URL without a base
369+
375370
371+
in `url`
376372
"###);
377373
}
378374

‎crates/uv-workspace/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ toml = { workspace = true }
3939
toml_edit = { workspace = true }
4040
tracing = { workspace = true }
4141
url = { workspace = true }
42+
serde_json = "1.0.128"
4243

4344
[dev-dependencies]
4445
anyhow = { workspace = true }

‎crates/uv-workspace/src/pyproject.rs‎

Lines changed: 207 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
99
use glob::Pattern;
1010
use owo_colors::OwoColorize;
11-
use serde::{de::IntoDeserializer, Deserialize, Serialize};
11+
use serde::{de::IntoDeserializer, Deserialize, Deserializer, Serialize};
1212
use std::ops::Deref;
1313
use std::path::{Path, PathBuf};
1414
use std::str::FromStr;
@@ -455,7 +455,7 @@ enum SourcesWire {
455455
impl<'de> serde::de::Deserialize<'de> for SourcesWire {
456456
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
457457
where
458-
D: serde::de::Deserializer<'de>,
458+
D: Deserializer<'de>,
459459
{
460460
serde_untagged::UntaggedEnumVisitor::new()
461461
.map(|map| map.deserialize().map(SourcesWire::One))
@@ -520,31 +520,30 @@ impl TryFrom<SourcesWire> for Sources {
520520
}
521521

522522
/// A `tool.uv.sources` value.
523-
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
524-
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
525-
#[serde(rename_all = "kebab-case", untagged, deny_unknown_fields)]
523+
#[derive(Serialize, Debug, Clone, PartialEq, Eq)]
524+
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema), schemars(rename_all = "kebab-case", untagged, deny_unknown_fields))]
526525
pub enum Source {
527526
/// A remote Git repository, available over HTTPS or SSH.
528527
///
529528
/// Example:
530529
/// ```toml
531530
/// flask = { git = "https://github.com/pallets/flask", tag = "3.0.0" }
532531
/// ```
533-
Git {
532+
Git{
534533
/// The repository URL (without the `git+` prefix).
535-
git: Url,
534+
git: Url,
536535
/// The path to the directory with the `pyproject.toml`, if it's not in the archive root.
537-
subdirectory: Option<PortablePathBuf>,
536+
subdirectory: Option<PortablePathBuf>,
538537
// Only one of the three may be used; we'll validate this later and emit a custom error.
539-
rev: Option<String>,
540-
tag: Option<String>,
541-
branch: Option<String>,
538+
rev: Option<String>,
539+
tag: Option<String>,
540+
branch: Option<String>,
542541
#[serde(
543542
skip_serializing_if = "pep508_rs::marker::ser::is_empty",
544543
serialize_with = "pep508_rs::marker::ser::serialize",
545544
default
546545
)]
547-
marker: MarkerTree,
546+
marker: MarkerTree,
548547
},
549548
/// A remote `http://` or `https://` URL, either a wheel (`.whl`) or a source distribution
550549
/// (`.zip`, `.tar.gz`).
@@ -602,24 +601,202 @@ pub enum Source {
602601
)]
603602
marker: MarkerTree,
604603
},
605-
/// A catch-all variant used to emit precise error messages when deserializing.
606-
CatchAll {
607-
git: String,
608-
subdirectory: Option<PortablePathBuf>,
609-
rev: Option<String>,
610-
tag: Option<String>,
611-
branch: Option<String>,
612-
url: String,
613-
path: PortablePathBuf,
614-
index: String,
615-
workspace: bool,
616-
#[serde(
617-
skip_serializing_if = "pep508_rs::marker::ser::is_empty",
618-
serialize_with = "pep508_rs::marker::ser::serialize",
619-
default
620-
)]
621-
marker: MarkerTree,
622-
},
604+
}
605+
606+
/// A custom deserialization implementation for [`Source`]. This is roughly equivalent to
607+
/// `#[serde(untagged)]`, but provides more detailed error messages.
608+
impl<'de> Deserialize<'de> for Source {
609+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
610+
where
611+
D: Deserializer<'de>,
612+
{
613+
#[derive(Deserialize, Debug, Clone)]
614+
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
615+
struct CatchAll {
616+
git: Option<Url>,
617+
subdirectory: Option<PortablePathBuf>,
618+
rev: Option<String>,
619+
tag: Option<String>,
620+
branch: Option<String>,
621+
url: Option<Url>,
622+
path: Option<PortablePathBuf>,
623+
editable: Option<bool>,
624+
index: Option<String>,
625+
workspace: Option<bool>,
626+
#[serde(
627+
skip_serializing_if = "pep508_rs::marker::ser::is_empty",
628+
serialize_with = "pep508_rs::marker::ser::serialize",
629+
default
630+
)]
631+
marker: MarkerTree,
632+
}
633+
634+
// Attempt to deserialize as `CatchAll`.
635+
let CatchAll { git, subdirectory, rev, tag, branch, url, path, editable, index, workspace, marker } = CatchAll::deserialize(deserializer)?;
636+
637+
// If the `git` field is set, we're dealing with a Git source.
638+
if let Some(git) = git {
639+
if index.is_some() {
640+
return Err(serde::de::Error::custom("cannot specify both `git` and `index`"));
641+
}
642+
if workspace.is_some() {
643+
return Err(serde::de::Error::custom("cannot specify both `git` and `workspace`"));
644+
}
645+
if path.is_some() {
646+
return Err(serde::de::Error::custom("cannot specify both `git` and `path`"));
647+
}
648+
if url.is_some() {
649+
return Err(serde::de::Error::custom("cannot specify both `git` and `url`"));
650+
}
651+
if editable.is_some() {
652+
return Err(serde::de::Error::custom("cannot specify both `git` and `editable`"));
653+
}
654+
655+
return Ok(Source::Git {
656+
git,
657+
subdirectory,
658+
rev,
659+
tag,
660+
branch,
661+
marker,
662+
});
663+
}
664+
665+
// If the `url` field is set, we're dealing with a URL source.
666+
if let Some(url) = url {
667+
if index.is_some() {
668+
return Err(serde::de::Error::custom("cannot specify both `url` and `index`"));
669+
}
670+
if workspace.is_some() {
671+
return Err(serde::de::Error::custom("cannot specify both `url` and `workspace`"));
672+
}
673+
if path.is_some() {
674+
return Err(serde::de::Error::custom("cannot specify both `url` and `path`"));
675+
}
676+
if git.is_some() {
677+
return Err(serde::de::Error::custom("cannot specify both `url` and `git`"));
678+
}
679+
if rev.is_some() {
680+
return Err(serde::de::Error::custom("cannot specify both `url` and `rev`"));
681+
}
682+
if tag.is_some() {
683+
return Err(serde::de::Error::custom("cannot specify both `url` and `tag`"));
684+
}
685+
if branch.is_some() {
686+
return Err(serde::de::Error::custom("cannot specify both `url` and `branch`"));
687+
}
688+
if editable.is_some() {
689+
return Err(serde::de::Error::custom("cannot specify both `url` and `editable`"));
690+
}
691+
692+
return Ok(Source::Url {
693+
url,
694+
subdirectory,
695+
marker,
696+
});
697+
}
698+
699+
// If the `path` field is set, we're dealing with a path source.
700+
if let Some(path) = path {
701+
if index.is_some() {
702+
return Err(serde::de::Error::custom("cannot specify both `path` and `index`"));
703+
}
704+
if workspace.is_some() {
705+
return Err(serde::de::Error::custom("cannot specify both `path` and `workspace`"));
706+
}
707+
if git.is_some() {
708+
return Err(serde::de::Error::custom("cannot specify both `path` and `git`"));
709+
}
710+
if url.is_some() {
711+
return Err(serde::de::Error::custom("cannot specify both `path` and `url`"));
712+
}
713+
if rev.is_some() {
714+
return Err(serde::de::Error::custom("cannot specify both `path` and `rev`"));
715+
}
716+
if tag.is_some() {
717+
return Err(serde::de::Error::custom("cannot specify both `path` and `tag`"));
718+
}
719+
if branch.is_some() {
720+
return Err(serde::de::Error::custom("cannot specify both `path` and `branch`"));
721+
}
722+
723+
return Ok(Source::Path {
724+
path,
725+
editable,
726+
marker,
727+
});
728+
}
729+
730+
// If the `index` field is set, we're dealing with a registry source.
731+
if let Some(index) = index {
732+
if workspace.is_some() {
733+
return Err(serde::de::Error::custom("cannot specify both `index` and `workspace`"));
734+
}
735+
if git.is_some() {
736+
return Err(serde::de::Error::custom("cannot specify both `index` and `git`"));
737+
}
738+
if url.is_some() {
739+
return Err(serde::de::Error::custom("cannot specify both `index` and `url`"));
740+
}
741+
if path.is_some() {
742+
return Err(serde::de::Error::custom("cannot specify both `index` and `path`"));
743+
}
744+
if rev.is_some() {
745+
return Err(serde::de::Error::custom("cannot specify both `index` and `rev`"));
746+
}
747+
if tag.is_some() {
748+
return Err(serde::de::Error::custom("cannot specify both `index` and `tag`"));
749+
}
750+
if branch.is_some() {
751+
return Err(serde::de::Error::custom("cannot specify both `index` and `branch`"));
752+
}
753+
if editable.is_some() {
754+
return Err(serde::de::Error::custom("cannot specify both `index` and `editable`"));
755+
}
756+
757+
return Ok(Source::Registry {
758+
index,
759+
marker,
760+
});
761+
}
762+
763+
// If the `workspace` field is set, we're dealing with a workspace source.
764+
if let Some(workspace) = workspace {
765+
if index.is_some() {
766+
return Err(serde::de::Error::custom("cannot specify both `index` and `index`"));
767+
}
768+
if git.is_some() {
769+
return Err(serde::de::Error::custom("cannot specify both `index` and `git`"));
770+
}
771+
if url.is_some() {
772+
return Err(serde::de::Error::custom("cannot specify both `index` and `url`"));
773+
}
774+
if path.is_some() {
775+
return Err(serde::de::Error::custom("cannot specify both `index` and `path`"));
776+
}
777+
if rev.is_some() {
778+
return Err(serde::de::Error::custom("cannot specify both `index` and `rev`"));
779+
}
780+
if tag.is_some() {
781+
return Err(serde::de::Error::custom("cannot specify both `index` and `tag`"));
782+
}
783+
if branch.is_some() {
784+
return Err(serde::de::Error::custom("cannot specify both `index` and `branch`"));
785+
}
786+
if editable.is_some() {
787+
return Err(serde::de::Error::custom("cannot specify both `index` and `editable`"));
788+
}
789+
790+
return Ok(Source::Workspace {
791+
workspace,
792+
marker,
793+
});
794+
}
795+
796+
// If none of the fields are set, we're dealing with an error.
797+
Err(serde::de::Error::custom("expected one of `git`, `url`, `path`, `index`, or `workspace`"))
798+
799+
}
623800
}
624801

625802
#[derive(Error, Debug)]
@@ -763,7 +940,6 @@ impl Source {
763940
Source::Path { marker, .. } => marker.clone(),
764941
Source::Registry { marker, .. } => marker.clone(),
765942
Source::Workspace { marker, .. } => marker.clone(),
766-
Source::CatchAll { marker, .. } => marker.clone(),
767943
}
768944
}
769945
}

0 commit comments

Comments
 (0)