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

Improve dependency conflict error messages #4094

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,27 @@
with a local password, reducing risk of your Hex password being compromised.
([Louis Pilfold](https://github.com/lpil))

- Improved the error message for dependency resolution failures to more clearly
explain why the conflict occurs. Previously, the error message only listed the
names of the conflicting packages:

```
- mist
- server
- cors_builder
```

Now, the message provides more detailed descriptions:

```
- Package `app` requires `cors_builder` 1.0.0 <= v < 2.0.0
- Conflict with the `mist` package:
- `cors_builder` requires `mist` version 1.0.0 <= v < 2.0.0
- `app` requires `mist` version 3.0.0 <= v < 4.0.0
```

([dinkelspiel](https://github.com/dinkelspiel))

### Language Server

- The language server now provides type information when hovering over argument
Expand Down
72 changes: 54 additions & 18 deletions compiler-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use pubgrub::package::Package;
use pubgrub::report::DerivationTree;
use pubgrub::version::Version;
use std::borrow::Cow;
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::env;
use std::fmt::{Debug, Display};
use std::io::Write;
Expand Down Expand Up @@ -379,29 +379,49 @@ impl Error {
}

pub fn dependency_resolution_failed(error: ResolutionError) -> Error {
fn collect_conflicting_packages<'dt, P: Package, V: Version>(
derivation_tree: &'dt DerivationTree<P, V>,
conflicting_packages: &mut HashSet<&'dt P>,
fn collect_conflicting_packages<P: Package, V: Version>(
derivation_tree: &DerivationTree<P, V>,
conflicting_packages: &mut HashSet<String>,
from_dep_of: &mut HashMap<String, Vec<(String, String)>>,
) {
match derivation_tree {
DerivationTree::External(external) => match external {
pubgrub::report::External::NotRoot(package, _) => {
let _ = conflicting_packages.insert(package);
let _ = conflicting_packages.insert(format!("- {package}"));
}
pubgrub::report::External::NoVersions(package, _) => {
let _ = conflicting_packages.insert(package);
}
pubgrub::report::External::UnavailableDependencies(package, _) => {
let _ = conflicting_packages.insert(package);
}
pubgrub::report::External::FromDependencyOf(package, _, dep_package, _) => {
let _ = conflicting_packages.insert(package);
let _ = conflicting_packages.insert(dep_package);
let _ = conflicting_packages.insert(format!("- {package}"));
}
pubgrub::report::External::UnavailableDependencies(_, _) => {
// let _ = conflicting_packages.insert(format!("- {package}"));
}
pubgrub::report::External::FromDependencyOf(
package,
_,
dep_package,
dep_version,
) => {
let mut vec =
if let Some(current) = from_dep_of.get(&dep_package.to_string()) {
current.to_owned()
} else {
Vec::<(String, String)>::new()
};
vec.push((package.to_string(), dep_version.to_string()));
let _ = from_dep_of.insert(dep_package.to_string(), vec);
}
},
DerivationTree::Derived(derived) => {
collect_conflicting_packages(&derived.cause1, conflicting_packages);
collect_conflicting_packages(&derived.cause2, conflicting_packages);
collect_conflicting_packages(
&derived.cause1,
conflicting_packages,
from_dep_of,
);
collect_conflicting_packages(
&derived.cause2,
conflicting_packages,
from_dep_of,
);
}
}
}
Expand All @@ -411,15 +431,31 @@ impl Error {
derivation_tree.collapse_no_versions();

let mut conflicting_packages = HashSet::new();
collect_conflicting_packages(&derivation_tree, &mut conflicting_packages);
let mut from_dep_of: HashMap<String, Vec<(String, String)>> = HashMap::new();
collect_conflicting_packages(&derivation_tree, &mut conflicting_packages, &mut from_dep_of);

wrap_format!("Unable to find compatible versions for \
the version constraints in your gleam.toml. \
The conflicting packages are:

{}
{}{}{}
",
conflicting_packages.into_iter().map(|s| format!("- {s}")).join("\n"))
conflicting_packages.clone().into_iter().sorted().join("\n"),
if !conflicting_packages.is_empty() {
"\n"
} else {
""
},
from_dep_of.into_iter().sorted_by_key(|(a, _)| a.to_string()).map(|(dependency, conflicts)| {
if conflicts.len() > 1 {
format!("- Conflict with the `{dependency}` package:\n{}", conflicts.into_iter().map(|(package, dependency_version)| format!(" - `{}` requires `{}` version {}", package, dependency, dependency_version)).join("\n"))
} else if let Some((package, dependency_version)) = conflicts.first() {
format!("- Package `{package}` requires `{dependency}` {dependency_version}")
} else {
unreachable!("The from_dep_of hashmap can't exist with an empty vector");
}
}).join("\n")
)
}

ResolutionError::ErrorRetrievingDependencies {
Expand Down
Loading