From b578dd87621db071328eb438c0af8d2aa35bc98f Mon Sep 17 00:00:00 2001 From: Zanie Date: Wed, 29 Nov 2023 18:16:36 -0600 Subject: [PATCH 1/9] Add report formatter concept for customization of report output --- src/internal/incompatibility.rs | 6 +- src/report.rs | 200 ++++++++++++++++++++++---------- 2 files changed, 143 insertions(+), 63 deletions(-) diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index b56a3c44..359c6329 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -9,7 +9,9 @@ use std::fmt; use crate::internal::arena::{Arena, Id}; use crate::internal::small_map::SmallMap; use crate::package::Package; -use crate::report::{DefaultStringReporter, DerivationTree, Derived, External}; +use crate::report::{ + DefaultStringReportFormatter, DerivationTree, Derived, External, ReportFormatter, +}; use crate::term::{self, Term}; use crate::version_set::VersionSet; @@ -251,7 +253,7 @@ impl fmt::Display for Incompatibility { write!( f, "{}", - DefaultStringReporter::string_terms(&self.package_terms.as_map()) + DefaultStringReportFormatter {}.format_terms(&self.package_terms.as_map()) ) } } diff --git a/src/report.rs b/src/report.rs index ff0b2d3f..517e09fc 100644 --- a/src/report.rs +++ b/src/report.rs @@ -19,6 +19,11 @@ pub trait Reporter { /// Generate a report from the derivation tree /// describing the resolution failure. fn report(derivation_tree: &DerivationTree) -> Self::Output; + + fn report_with_formatter( + derivation_tree: &DerivationTree, + formatter: &impl ReportFormatter, + ) -> Self::Output; } /// Derivation tree resulting in the impossibility @@ -173,6 +178,49 @@ impl fmt::Display for External { } } +/// Trait for formatting outputs in the reporter. +pub trait ReportFormatter { + /// Output type of the report. + type Output; + + /// Format an [External] incompatibility. + fn format_external(&self, external: &External) -> Self::Output; + + /// Format terms of an incompatibility. + fn format_terms(&self, terms: &Map>) -> Self::Output; +} + +/// Default formatter for the default reporter. +pub struct DefaultStringReportFormatter; + +impl ReportFormatter for DefaultStringReportFormatter { + type Output = String; + + fn format_external(&self, external: &External) -> String { + external.to_string() + } + + fn format_terms(&self, terms: &Map>) -> Self::Output { + let terms_vec: Vec<_> = terms.iter().collect(); + match terms_vec.as_slice() { + [] => "version solving failed".into(), + // TODO: special case when that unique package is root. + [(package, Term::Positive(range))] => format!("{} {} is forbidden", package, range), + [(package, Term::Negative(range))] => format!("{} {} is mandatory", package, range), + [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => { + External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string() + } + [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => { + External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string() + } + slice => { + let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{} {}", p, t)).collect(); + str_terms.join(", ") + " are incompatible" + } + } + } +} + /// Default reporter able to generate an explanation as a [String]. pub struct DefaultStringReporter { /// Number of explanations already with a line reference. @@ -194,8 +242,12 @@ impl DefaultStringReporter { } } - fn build_recursive(&mut self, derived: &Derived) { - self.build_recursive_helper(derived); + fn build_recursive( + &mut self, + derived: &Derived, + formatter: &impl ReportFormatter, + ) { + self.build_recursive_helper(derived, formatter); if let Some(id) = derived.shared_id { if self.shared_with_ref.get(&id).is_none() { self.add_line_ref(); @@ -204,7 +256,11 @@ impl DefaultStringReporter { }; } - fn build_recursive_helper(&mut self, current: &Derived) { + fn build_recursive_helper( + &mut self, + current: &Derived, + formatter: &impl ReportFormatter, + ) { match (current.cause1.deref(), current.cause2.deref()) { (DerivationTree::External(external1), DerivationTree::External(external2)) => { // Simplest case, we just combine two external incompatibilities. @@ -212,16 +268,17 @@ impl DefaultStringReporter { external1, external2, ¤t.terms, + formatter, )); } (DerivationTree::Derived(derived), DerivationTree::External(external)) => { // One cause is derived, so we explain this first // then we add the one-line external part // and finally conclude with the current incompatibility. - self.report_one_each(derived, external, ¤t.terms); + self.report_one_each(derived, external, ¤t.terms, formatter); } (DerivationTree::External(external), DerivationTree::Derived(derived)) => { - self.report_one_each(derived, external, ¤t.terms); + self.report_one_each(derived, external, ¤t.terms, formatter); } (DerivationTree::Derived(derived1), DerivationTree::Derived(derived2)) => { // This is the most complex case since both causes are also derived. @@ -237,19 +294,28 @@ impl DefaultStringReporter { ref2, derived2, ¤t.terms, + formatter, )), // Otherwise, if one only has a line number reference, // we recursively call the one without reference and then // add the one with reference to conclude. (Some(ref1), None) => { - self.build_recursive(derived2); - self.lines - .push(Self::and_explain_ref(ref1, derived1, ¤t.terms)); + self.build_recursive(derived2, formatter); + self.lines.push(Self::and_explain_ref( + ref1, + derived1, + ¤t.terms, + formatter, + )); } (None, Some(ref2)) => { - self.build_recursive(derived1); - self.lines - .push(Self::and_explain_ref(ref2, derived2, ¤t.terms)); + self.build_recursive(derived1, formatter); + self.lines.push(Self::and_explain_ref( + ref2, + derived2, + ¤t.terms, + formatter, + )); } // Finally, if no line reference exists yet, // we call recursively the first one and then, @@ -259,17 +325,21 @@ impl DefaultStringReporter { // recursively call on the second node, // and finally conclude. (None, None) => { - self.build_recursive(derived1); + self.build_recursive(derived1, formatter); if derived1.shared_id.is_some() { self.lines.push("".into()); - self.build_recursive(current); + self.build_recursive(current, formatter); } else { self.add_line_ref(); let ref1 = self.ref_count; self.lines.push("".into()); - self.build_recursive(derived2); - self.lines - .push(Self::and_explain_ref(ref1, derived1, ¤t.terms)); + self.build_recursive(derived2, formatter); + self.lines.push(Self::and_explain_ref( + ref1, + derived1, + ¤t.terms, + formatter, + )); } } } @@ -286,6 +356,7 @@ impl DefaultStringReporter { derived: &Derived, external: &External, current_terms: &Map>, + formatter: &impl ReportFormatter, ) { match self.line_ref_of(derived.shared_id) { Some(ref_id) => self.lines.push(Self::explain_ref_and_external( @@ -293,8 +364,9 @@ impl DefaultStringReporter { derived, external, current_terms, + formatter, )), - None => self.report_recurse_one_each(derived, external, current_terms), + None => self.report_recurse_one_each(derived, external, current_terms, formatter), } } @@ -304,32 +376,38 @@ impl DefaultStringReporter { derived: &Derived, external: &External, current_terms: &Map>, + formatter: &impl ReportFormatter, ) { match (derived.cause1.deref(), derived.cause2.deref()) { // If the derived cause has itself one external prior cause, // we can chain the external explanations. (DerivationTree::Derived(prior_derived), DerivationTree::External(prior_external)) => { - self.build_recursive(prior_derived); + self.build_recursive(prior_derived, formatter); self.lines.push(Self::and_explain_prior_and_external( prior_external, external, current_terms, + formatter, )); } // If the derived cause has itself one external prior cause, // we can chain the external explanations. (DerivationTree::External(prior_external), DerivationTree::Derived(prior_derived)) => { - self.build_recursive(prior_derived); + self.build_recursive(prior_derived, formatter); self.lines.push(Self::and_explain_prior_and_external( prior_external, external, current_terms, + formatter, )); } _ => { - self.build_recursive(derived); - self.lines - .push(Self::and_explain_external(external, current_terms)); + self.build_recursive(derived, formatter); + self.lines.push(Self::and_explain_external( + external, + current_terms, + formatter, + )); } } } @@ -341,13 +419,14 @@ impl DefaultStringReporter { external1: &External, external2: &External, current_terms: &Map>, + formatter: &impl ReportFormatter, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} and {}, {}.", - external1, - external2, - Self::string_terms(current_terms) + formatter.format_external(external1), + formatter.format_external(external2), + formatter.format_terms(current_terms) ) } @@ -358,15 +437,16 @@ impl DefaultStringReporter { ref_id2: usize, derived2: &Derived, current_terms: &Map>, + formatter: &impl ReportFormatter, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} ({}) and {} ({}), {}.", - Self::string_terms(&derived1.terms), + formatter.format_terms(&derived1.terms), ref_id1, - Self::string_terms(&derived2.terms), + formatter.format_terms(&derived2.terms), ref_id2, - Self::string_terms(current_terms) + formatter.format_terms(current_terms) ) } @@ -378,14 +458,15 @@ impl DefaultStringReporter { derived: &Derived, external: &External, current_terms: &Map>, + formatter: &impl ReportFormatter, ) -> String { // TODO: order should be chosen to make it more logical. format!( "Because {} ({}) and {}, {}.", - Self::string_terms(&derived.terms), + formatter.format_terms(&derived.terms), ref_id, - external, - Self::string_terms(current_terms) + formatter.format_external(external), + formatter.format_terms(current_terms) ) } @@ -393,11 +474,12 @@ impl DefaultStringReporter { fn and_explain_external( external: &External, current_terms: &Map>, + formatter: &impl ReportFormatter, ) -> String { format!( "And because {}, {}.", - external, - Self::string_terms(current_terms) + formatter.format_external(external), + formatter.format_terms(current_terms) ) } @@ -406,12 +488,13 @@ impl DefaultStringReporter { ref_id: usize, derived: &Derived, current_terms: &Map>, + formatter: &impl ReportFormatter, ) -> String { format!( "And because {} ({}), {}.", - Self::string_terms(&derived.terms), + formatter.format_terms(&derived.terms), ref_id, - Self::string_terms(current_terms) + formatter.format_terms(current_terms) ) } @@ -420,36 +503,16 @@ impl DefaultStringReporter { prior_external: &External, external: &External, current_terms: &Map>, + formatter: &impl ReportFormatter, ) -> String { format!( "And because {} and {}, {}.", - prior_external, - external, - Self::string_terms(current_terms) + formatter.format_external(prior_external), + formatter.format_external(external), + formatter.format_terms(current_terms) ) } - /// Try to print terms of an incompatibility in a human-readable way. - pub fn string_terms(terms: &Map>) -> String { - let terms_vec: Vec<_> = terms.iter().collect(); - match terms_vec.as_slice() { - [] => "version solving failed".into(), - // TODO: special case when that unique package is root. - [(package, Term::Positive(range))] => format!("{} {} is forbidden", package, range), - [(package, Term::Negative(range))] => format!("{} {} is mandatory", package, range), - [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => { - External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string() - } - [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => { - External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string() - } - slice => { - let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{} {}", p, t)).collect(); - str_terms.join(", ") + " are incompatible" - } - } - } - // Helper functions ######################################################## fn add_line_ref(&mut self) { @@ -469,11 +532,26 @@ impl Reporter for DefaultStringReporter { type Output = String; fn report(derivation_tree: &DerivationTree) -> Self::Output { + let formatter = DefaultStringReportFormatter {}; + match derivation_tree { + DerivationTree::External(external) => formatter.format_external(external), + DerivationTree::Derived(derived) => { + let mut reporter = Self::new(); + reporter.build_recursive(derived, &formatter); + reporter.lines.join("\n") + } + } + } + + fn report_with_formatter( + derivation_tree: &DerivationTree, + formatter: &impl ReportFormatter, + ) -> Self::Output { match derivation_tree { - DerivationTree::External(external) => external.to_string(), + DerivationTree::External(external) => formatter.format_external(&external), DerivationTree::Derived(derived) => { let mut reporter = Self::new(); - reporter.build_recursive(derived); + reporter.build_recursive(derived, formatter); reporter.lines.join("\n") } } From c1006eb1e572a7f7641cb9fa7f9b52327a63e800 Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 10:43:18 -0600 Subject: [PATCH 2/9] Add default implementation --- src/internal/incompatibility.rs | 2 +- src/report.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 359c6329..7e04ec7d 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -253,7 +253,7 @@ impl fmt::Display for Incompatibility { write!( f, "{}", - DefaultStringReportFormatter {}.format_terms(&self.package_terms.as_map()) + DefaultStringReportFormatter::default().format_terms(&self.package_terms.as_map()) ) } } diff --git a/src/report.rs b/src/report.rs index 517e09fc..482e61b2 100644 --- a/src/report.rs +++ b/src/report.rs @@ -191,6 +191,7 @@ pub trait ReportFormatter { } /// Default formatter for the default reporter. +#[derive(Default, Debug)] pub struct DefaultStringReportFormatter; impl ReportFormatter for DefaultStringReportFormatter { @@ -532,7 +533,7 @@ impl Reporter for DefaultStringReporter { type Output = String; fn report(derivation_tree: &DerivationTree) -> Self::Output { - let formatter = DefaultStringReportFormatter {}; + let formatter = DefaultStringReportFormatter::default(); match derivation_tree { DerivationTree::External(external) => formatter.format_external(external), DerivationTree::Derived(derived) => { From 0980f84f2ed8b563b7a1da6d55c1a3ac94e7212b Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 10:56:59 -0600 Subject: [PATCH 3/9] Use `format_external` in `format_terms` --- src/report.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/report.rs b/src/report.rs index 482e61b2..6e3201a1 100644 --- a/src/report.rs +++ b/src/report.rs @@ -209,10 +209,10 @@ impl ReportFormatter for DefaultStringReportF [(package, Term::Positive(range))] => format!("{} {} is forbidden", package, range), [(package, Term::Negative(range))] => format!("{} {} is mandatory", package, range), [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => { - External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string() + self.format_external(&External::FromDependencyOf(p1, r1.clone(), p2, r2.clone())) } [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => { - External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string() + self.format_external(&External::FromDependencyOf(p2, r2.clone(), p1, r1.clone())) } slice => { let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{} {}", p, t)).collect(); From 7ad15c187893612cc5bb8eebe393ecc7cd37951c Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 11:03:59 -0600 Subject: [PATCH 4/9] Add previous example --- examples/unsat_root_message_no_version.rs | 469 ++++++++++++++++++++++ 1 file changed, 469 insertions(+) create mode 100644 examples/unsat_root_message_no_version.rs diff --git a/examples/unsat_root_message_no_version.rs b/examples/unsat_root_message_no_version.rs new file mode 100644 index 00000000..4d7f92a0 --- /dev/null +++ b/examples/unsat_root_message_no_version.rs @@ -0,0 +1,469 @@ +// SPDX-License-Identifier: MPL-2.0 + +use pubgrub::error::PubGrubError; +use pubgrub::range::Range; +use pubgrub::report::Reporter; +use pubgrub::solver::{resolve, OfflineDependencyProvider}; +use pubgrub::version::SemanticVersion; + +use std::fmt::{self, Display}; + +use pubgrub::report::{DerivationTree, Derived, External}; +use pubgrub::term::Term; +use pubgrub::type_aliases::Map; + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum Package { + Root, + Package(String), +} + +impl Display for Package { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Package::Root => write!(f, "root"), + Package::Package(name) => write!(f, "{}", name), + } + } +} + +/// Derivative of [`pubgrub::report::DefaultStringReporter`] for customized display +/// of package resolution errors. +pub struct CustomStringReporter { + /// Number of explanations already with a line reference. + ref_count: usize, + /// Shared nodes that have already been marked with a line reference. + /// The incompatibility ids are the keys, and the line references are the values. + shared_with_ref: Map, + /// Accumulated lines of the report already generated. + lines: Vec, +} + +impl CustomStringReporter { + /// Initialize the reporter. + fn new() -> Self { + Self { + ref_count: 0, + shared_with_ref: Map::default(), + lines: Vec::new(), + } + } + + fn build_recursive(&mut self, derived: &Derived>) { + self.build_recursive_helper(derived); + if let Some(id) = derived.shared_id { + if self.shared_with_ref.get(&id).is_none() { + self.add_line_ref(); + self.shared_with_ref.insert(id, self.ref_count); + } + }; + } + + fn build_recursive_helper(&mut self, current: &Derived>) { + match (&*current.cause1, &*current.cause2) { + (DerivationTree::External(external1), DerivationTree::External(external2)) => { + // Simplest case, we just combine two external incompatibilities. + self.lines.push(Self::explain_both_external( + external1, + external2, + ¤t.terms, + )); + } + (DerivationTree::Derived(derived), DerivationTree::External(external)) => { + // One cause is derived, so we explain this first + // then we add the one-line external part + // and finally conclude with the current incompatibility. + self.report_one_each(derived, external, ¤t.terms); + } + (DerivationTree::External(external), DerivationTree::Derived(derived)) => { + self.report_one_each(derived, external, ¤t.terms); + } + (DerivationTree::Derived(derived1), DerivationTree::Derived(derived2)) => { + // This is the most complex case since both causes are also derived. + match ( + self.line_ref_of(derived1.shared_id), + self.line_ref_of(derived2.shared_id), + ) { + // If both causes already have been referenced (shared_id), + // the explanation simply uses those references. + (Some(ref1), Some(ref2)) => self.lines.push(Self::explain_both_ref( + ref1, + derived1, + ref2, + derived2, + ¤t.terms, + )), + // Otherwise, if one only has a line number reference, + // we recursively call the one without reference and then + // add the one with reference to conclude. + (Some(ref1), None) => { + self.build_recursive(derived2); + self.lines + .push(Self::and_explain_ref(ref1, derived1, ¤t.terms)); + } + (None, Some(ref2)) => { + self.build_recursive(derived1); + self.lines + .push(Self::and_explain_ref(ref2, derived2, ¤t.terms)); + } + // Finally, if no line reference exists yet, + // we call recursively the first one and then, + // - if this was a shared node, it will get a line ref + // and we can simply recall this with the current node. + // - otherwise, we add a line reference to it, + // recursively call on the second node, + // and finally conclude. + (None, None) => { + self.build_recursive(derived1); + if derived1.shared_id.is_some() { + self.lines.push(String::new()); + self.build_recursive(current); + } else { + self.add_line_ref(); + let ref1 = self.ref_count; + self.lines.push(String::new()); + self.build_recursive(derived2); + self.lines + .push(Self::and_explain_ref(ref1, derived1, ¤t.terms)); + } + } + } + } + } + } + + /// Report a derived and an external incompatibility. + /// + /// The result will depend on the fact that the derived incompatibility + /// has already been explained or not. + fn report_one_each( + &mut self, + derived: &Derived>, + external: &External>, + current_terms: &Map>>, + ) { + match self.line_ref_of(derived.shared_id) { + Some(ref_id) => self.lines.push(Self::explain_ref_and_external( + ref_id, + derived, + external, + current_terms, + )), + None => self.report_recurse_one_each(derived, external, current_terms), + } + } + + /// Report one derived (without a line ref yet) and one external. + fn report_recurse_one_each( + &mut self, + derived: &Derived>, + external: &External>, + current_terms: &Map>>, + ) { + match (&*derived.cause1, &*derived.cause2) { + // If the derived cause has itself one external prior cause, + // we can chain the external explanations. + (DerivationTree::Derived(prior_derived), DerivationTree::External(prior_external)) => { + self.build_recursive(prior_derived); + self.lines.push(Self::and_explain_prior_and_external( + prior_external, + external, + current_terms, + )); + } + // If the derived cause has itself one external prior cause, + // we can chain the external explanations. + (DerivationTree::External(prior_external), DerivationTree::Derived(prior_derived)) => { + self.build_recursive(prior_derived); + self.lines.push(Self::and_explain_prior_and_external( + prior_external, + external, + current_terms, + )); + } + _ => { + self.build_recursive(derived); + self.lines + .push(Self::and_explain_external(external, current_terms)); + } + } + } + + // String explanations ##################################################### + + /// Simplest case, we just combine two external incompatibilities. + fn explain_both_external( + external1: &External>, + external2: &External>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} and {}, {}.", + CustomExternal::from_pubgrub(external1.clone()), + CustomExternal::from_pubgrub(external2.clone()), + Self::string_terms(current_terms) + ) + } + + /// Both causes have already been explained so we use their refs. + fn explain_both_ref( + ref_id1: usize, + derived1: &Derived>, + ref_id2: usize, + derived2: &Derived>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} ({}) and {} ({}), {}.", + Self::string_terms(&derived1.terms), + ref_id1, + Self::string_terms(&derived2.terms), + ref_id2, + Self::string_terms(current_terms) + ) + } + + /// One cause is derived (already explained so one-line), + /// the other is a one-line external cause, + /// and finally we conclude with the current incompatibility. + fn explain_ref_and_external( + ref_id: usize, + derived: &Derived>, + external: &External>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} ({}) and {}, {}.", + Self::string_terms(&derived.terms), + ref_id, + CustomExternal::from_pubgrub(external.clone()), + Self::string_terms(current_terms) + ) + } + + /// Add an external cause to the chain of explanations. + fn and_explain_external( + external: &External>, + current_terms: &Map>>, + ) -> String { + format!( + "And because {}, {}.", + CustomExternal::from_pubgrub(external.clone()), + Self::string_terms(current_terms) + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_ref( + ref_id: usize, + derived: &Derived>, + current_terms: &Map>>, + ) -> String { + format!( + "And because {} ({}), {}.", + Self::string_terms(&derived.terms), + ref_id, + Self::string_terms(current_terms) + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_prior_and_external( + prior_external: &External>, + external: &External>, + current_terms: &Map>>, + ) -> String { + format!( + "And because {} and {}, {}.", + CustomExternal::from_pubgrub(prior_external.clone()), + CustomExternal::from_pubgrub(external.clone()), + Self::string_terms(current_terms) + ) + } + + /// Try to print terms of an incompatibility in a human-readable way. + pub fn string_terms(terms: &Map>>) -> String { + let terms_vec: Vec<_> = terms.iter().collect(); + match terms_vec.as_slice() { + [] => "version solving failed".into(), + // TODO: special case when that unique package is root. + [(package @ Package::Root, Term::Positive(_))] => { + format!("{package} is forbidden") + } + [(package @ Package::Root, Term::Negative(_))] => { + format!("{package} is mandatory") + } + [(package @ Package::Package(_), Term::Positive(range))] => { + format!("{package} {range} is forbidden") + } + [(package @ Package::Package(_), Term::Negative(range))] => { + format!("{package} {range} is mandatory") + } + [(p1, Term::Positive(r1)), (p2, Term::Negative(r2))] => { + External::FromDependencyOf(p1, r1.clone(), p2, r2.clone()).to_string() + } + [(p1, Term::Negative(r1)), (p2, Term::Positive(r2))] => { + External::FromDependencyOf(p2, r2.clone(), p1, r1.clone()).to_string() + } + slice => { + let str_terms: Vec<_> = slice.iter().map(|(p, t)| format!("{p} {t}")).collect(); + str_terms.join(", ") + " are incompatible" + } + } + } + + // Helper functions ######################################################## + + fn add_line_ref(&mut self) { + let new_count = self.ref_count + 1; + self.ref_count = new_count; + if let Some(line) = self.lines.last_mut() { + *line = format!("{line} ({new_count})"); + } + } + + fn line_ref_of(&self, shared_id: Option) -> Option { + shared_id.and_then(|id| self.shared_with_ref.get(&id).copied()) + } +} + +impl Reporter> for CustomStringReporter { + type Output = String; + + fn report(derivation_tree: &DerivationTree>) -> Self::Output { + match derivation_tree { + DerivationTree::External(external) => { + CustomExternal::from_pubgrub(external.clone()).to_string() + } + DerivationTree::Derived(derived) => { + let mut reporter = Self::new(); + reporter.build_recursive(derived); + reporter.lines.join("\n") + } + } + } +} + +/// Derivative of [`pubgrub::report::External`] for customized display +/// for internal [`Package`]. +#[allow(clippy::large_enum_variant)] +#[derive(Debug, Clone)] +enum CustomExternal { + /// Initial incompatibility aiming at picking the root package for the first decision. + NotRoot(Package, SemanticVersion), + /// There are no versions in the given set for this package. + NoVersions(Package, Range), + /// Dependencies of the package are unavailable for versions in that set. + UnavailableDependencies(Package, Range), + /// Incompatibility coming from the dependencies of a given package. + FromDependencyOf( + Package, + Range, + Package, + Range, + ), +} + +impl CustomExternal { + fn from_pubgrub(external: External>) -> Self { + match external { + External::NotRoot(p, v) => CustomExternal::NotRoot(p, v), + External::NoVersions(p, vs) => CustomExternal::NoVersions(p, vs), + External::UnavailableDependencies(p, vs) => { + CustomExternal::UnavailableDependencies(p, vs) + } + External::FromDependencyOf(p, vs, p_dep, vs_dep) => { + CustomExternal::FromDependencyOf(p, vs, p_dep, vs_dep) + } + } + } +} + +impl fmt::Display for CustomExternal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::NotRoot(package, version) => { + write!(f, "we are solving dependencies of {package} {version}") + } + Self::NoVersions(package, set) => { + if set == &Range::full() { + write!(f, "there is no available version for {package}") + } else { + write!(f, "there is no version of {package} in {set}") + } + } + Self::UnavailableDependencies(package, set) => { + if set == &Range::full() { + write!(f, "dependencies of {package} are unavailable") + } else { + write!( + f, + "dependencies of {package} at version {set} are unavailable" + ) + } + } + Self::FromDependencyOf(package, package_set, dependency, dependency_set) => { + if package_set == &Range::full() && dependency_set == &Range::full() { + write!(f, "{package} depends on {dependency}") + } else if package_set == &Range::full() { + write!(f, "{package} depends on {dependency} {dependency_set}") + } else if dependency_set == &Range::full() { + if matches!(package, Package::Root) { + // Exclude the dummy version for root packages + write!(f, "{package} depends on {dependency}") + } else { + write!(f, "{package} {package_set} depends on {dependency}") + } + } else { + if matches!(package, Package::Root) { + // Exclude the dummy version for root packages + write!(f, "{package} depends on {dependency} {dependency_set}") + } else { + write!( + f, + "{package} {package_set} depends on {dependency} {dependency_set}" + ) + } + } + } + } + } +} + +fn main() { + let mut dependency_provider = + OfflineDependencyProvider::>::new(); + // Define the root package with a dependency on a package we do not provide + dependency_provider.add_dependencies( + Package::Root, + (0, 0, 0), + vec![( + Package::Package("foo".to_string()), + Range::singleton((1, 0, 0)), + )], + ); + + // Run the algorithm + match resolve(&dependency_provider, Package::Root, (0, 0, 0)) { + Ok(sol) => println!("{:?}", sol), + Err(PubGrubError::NoSolution(mut derivation_tree)) => { + eprintln!("No solution.\n"); + + eprintln!("### Default report:"); + eprintln!("```"); + eprintln!("{}", CustomStringReporter::report(&derivation_tree)); + eprintln!("```\n"); + + derivation_tree.collapse_no_versions(); + eprintln!("### Report with `collapse_no_versions`:"); + eprintln!("```"); + eprintln!("{}", CustomStringReporter::report(&derivation_tree)); + eprintln!("```"); + std::process::exit(1); + } + Err(err) => panic!("{:?}", err), + }; +} From 725ec87c7ef7b1a7efbe90f91375b8d685318aa2 Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 11:11:14 -0600 Subject: [PATCH 5/9] Use new formatter --- examples/unsat_root_message_no_version.rs | 388 ++-------------------- 1 file changed, 33 insertions(+), 355 deletions(-) diff --git a/examples/unsat_root_message_no_version.rs b/examples/unsat_root_message_no_version.rs index 4d7f92a0..91ee20a3 100644 --- a/examples/unsat_root_message_no_version.rs +++ b/examples/unsat_root_message_no_version.rs @@ -6,11 +6,10 @@ use pubgrub::report::Reporter; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::SemanticVersion; -use std::fmt::{self, Display}; - -use pubgrub::report::{DerivationTree, Derived, External}; +use pubgrub::report::{DefaultStringReporter, External, ReportFormatter}; use pubgrub::term::Term; use pubgrub::type_aliases::Map; +use std::fmt::{self, Display}; #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum Package { @@ -27,265 +26,13 @@ impl Display for Package { } } -/// Derivative of [`pubgrub::report::DefaultStringReporter`] for customized display -/// of package resolution errors. -pub struct CustomStringReporter { - /// Number of explanations already with a line reference. - ref_count: usize, - /// Shared nodes that have already been marked with a line reference. - /// The incompatibility ids are the keys, and the line references are the values. - shared_with_ref: Map, - /// Accumulated lines of the report already generated. - lines: Vec, -} - -impl CustomStringReporter { - /// Initialize the reporter. - fn new() -> Self { - Self { - ref_count: 0, - shared_with_ref: Map::default(), - lines: Vec::new(), - } - } - - fn build_recursive(&mut self, derived: &Derived>) { - self.build_recursive_helper(derived); - if let Some(id) = derived.shared_id { - if self.shared_with_ref.get(&id).is_none() { - self.add_line_ref(); - self.shared_with_ref.insert(id, self.ref_count); - } - }; - } - - fn build_recursive_helper(&mut self, current: &Derived>) { - match (&*current.cause1, &*current.cause2) { - (DerivationTree::External(external1), DerivationTree::External(external2)) => { - // Simplest case, we just combine two external incompatibilities. - self.lines.push(Self::explain_both_external( - external1, - external2, - ¤t.terms, - )); - } - (DerivationTree::Derived(derived), DerivationTree::External(external)) => { - // One cause is derived, so we explain this first - // then we add the one-line external part - // and finally conclude with the current incompatibility. - self.report_one_each(derived, external, ¤t.terms); - } - (DerivationTree::External(external), DerivationTree::Derived(derived)) => { - self.report_one_each(derived, external, ¤t.terms); - } - (DerivationTree::Derived(derived1), DerivationTree::Derived(derived2)) => { - // This is the most complex case since both causes are also derived. - match ( - self.line_ref_of(derived1.shared_id), - self.line_ref_of(derived2.shared_id), - ) { - // If both causes already have been referenced (shared_id), - // the explanation simply uses those references. - (Some(ref1), Some(ref2)) => self.lines.push(Self::explain_both_ref( - ref1, - derived1, - ref2, - derived2, - ¤t.terms, - )), - // Otherwise, if one only has a line number reference, - // we recursively call the one without reference and then - // add the one with reference to conclude. - (Some(ref1), None) => { - self.build_recursive(derived2); - self.lines - .push(Self::and_explain_ref(ref1, derived1, ¤t.terms)); - } - (None, Some(ref2)) => { - self.build_recursive(derived1); - self.lines - .push(Self::and_explain_ref(ref2, derived2, ¤t.terms)); - } - // Finally, if no line reference exists yet, - // we call recursively the first one and then, - // - if this was a shared node, it will get a line ref - // and we can simply recall this with the current node. - // - otherwise, we add a line reference to it, - // recursively call on the second node, - // and finally conclude. - (None, None) => { - self.build_recursive(derived1); - if derived1.shared_id.is_some() { - self.lines.push(String::new()); - self.build_recursive(current); - } else { - self.add_line_ref(); - let ref1 = self.ref_count; - self.lines.push(String::new()); - self.build_recursive(derived2); - self.lines - .push(Self::and_explain_ref(ref1, derived1, ¤t.terms)); - } - } - } - } - } - } - - /// Report a derived and an external incompatibility. - /// - /// The result will depend on the fact that the derived incompatibility - /// has already been explained or not. - fn report_one_each( - &mut self, - derived: &Derived>, - external: &External>, - current_terms: &Map>>, - ) { - match self.line_ref_of(derived.shared_id) { - Some(ref_id) => self.lines.push(Self::explain_ref_and_external( - ref_id, - derived, - external, - current_terms, - )), - None => self.report_recurse_one_each(derived, external, current_terms), - } - } - - /// Report one derived (without a line ref yet) and one external. - fn report_recurse_one_each( - &mut self, - derived: &Derived>, - external: &External>, - current_terms: &Map>>, - ) { - match (&*derived.cause1, &*derived.cause2) { - // If the derived cause has itself one external prior cause, - // we can chain the external explanations. - (DerivationTree::Derived(prior_derived), DerivationTree::External(prior_external)) => { - self.build_recursive(prior_derived); - self.lines.push(Self::and_explain_prior_and_external( - prior_external, - external, - current_terms, - )); - } - // If the derived cause has itself one external prior cause, - // we can chain the external explanations. - (DerivationTree::External(prior_external), DerivationTree::Derived(prior_derived)) => { - self.build_recursive(prior_derived); - self.lines.push(Self::and_explain_prior_and_external( - prior_external, - external, - current_terms, - )); - } - _ => { - self.build_recursive(derived); - self.lines - .push(Self::and_explain_external(external, current_terms)); - } - } - } - - // String explanations ##################################################### - - /// Simplest case, we just combine two external incompatibilities. - fn explain_both_external( - external1: &External>, - external2: &External>, - current_terms: &Map>>, - ) -> String { - // TODO: order should be chosen to make it more logical. - format!( - "Because {} and {}, {}.", - CustomExternal::from_pubgrub(external1.clone()), - CustomExternal::from_pubgrub(external2.clone()), - Self::string_terms(current_terms) - ) - } - - /// Both causes have already been explained so we use their refs. - fn explain_both_ref( - ref_id1: usize, - derived1: &Derived>, - ref_id2: usize, - derived2: &Derived>, - current_terms: &Map>>, - ) -> String { - // TODO: order should be chosen to make it more logical. - format!( - "Because {} ({}) and {} ({}), {}.", - Self::string_terms(&derived1.terms), - ref_id1, - Self::string_terms(&derived2.terms), - ref_id2, - Self::string_terms(current_terms) - ) - } - - /// One cause is derived (already explained so one-line), - /// the other is a one-line external cause, - /// and finally we conclude with the current incompatibility. - fn explain_ref_and_external( - ref_id: usize, - derived: &Derived>, - external: &External>, - current_terms: &Map>>, - ) -> String { - // TODO: order should be chosen to make it more logical. - format!( - "Because {} ({}) and {}, {}.", - Self::string_terms(&derived.terms), - ref_id, - CustomExternal::from_pubgrub(external.clone()), - Self::string_terms(current_terms) - ) - } - - /// Add an external cause to the chain of explanations. - fn and_explain_external( - external: &External>, - current_terms: &Map>>, - ) -> String { - format!( - "And because {}, {}.", - CustomExternal::from_pubgrub(external.clone()), - Self::string_terms(current_terms) - ) - } +#[derive(Debug, Default)] +struct CustomReportFormatter; - /// Add an already explained incompat to the chain of explanations. - fn and_explain_ref( - ref_id: usize, - derived: &Derived>, - current_terms: &Map>>, - ) -> String { - format!( - "And because {} ({}), {}.", - Self::string_terms(&derived.terms), - ref_id, - Self::string_terms(current_terms) - ) - } - - /// Add an already explained incompat to the chain of explanations. - fn and_explain_prior_and_external( - prior_external: &External>, - external: &External>, - current_terms: &Map>>, - ) -> String { - format!( - "And because {} and {}, {}.", - CustomExternal::from_pubgrub(prior_external.clone()), - CustomExternal::from_pubgrub(external.clone()), - Self::string_terms(current_terms) - ) - } +impl ReportFormatter> for CustomReportFormatter { + type Output = String; - /// Try to print terms of an incompatibility in a human-readable way. - pub fn string_terms(terms: &Map>>) -> String { + fn format_terms(&self, terms: &Map>>) -> String { let terms_vec: Vec<_> = terms.iter().collect(); match terms_vec.as_slice() { [] => "version solving failed".into(), @@ -315,117 +62,43 @@ impl CustomStringReporter { } } - // Helper functions ######################################################## - - fn add_line_ref(&mut self) { - let new_count = self.ref_count + 1; - self.ref_count = new_count; - if let Some(line) = self.lines.last_mut() { - *line = format!("{line} ({new_count})"); - } - } - - fn line_ref_of(&self, shared_id: Option) -> Option { - shared_id.and_then(|id| self.shared_with_ref.get(&id).copied()) - } -} - -impl Reporter> for CustomStringReporter { - type Output = String; - - fn report(derivation_tree: &DerivationTree>) -> Self::Output { - match derivation_tree { - DerivationTree::External(external) => { - CustomExternal::from_pubgrub(external.clone()).to_string() - } - DerivationTree::Derived(derived) => { - let mut reporter = Self::new(); - reporter.build_recursive(derived); - reporter.lines.join("\n") - } - } - } -} - -/// Derivative of [`pubgrub::report::External`] for customized display -/// for internal [`Package`]. -#[allow(clippy::large_enum_variant)] -#[derive(Debug, Clone)] -enum CustomExternal { - /// Initial incompatibility aiming at picking the root package for the first decision. - NotRoot(Package, SemanticVersion), - /// There are no versions in the given set for this package. - NoVersions(Package, Range), - /// Dependencies of the package are unavailable for versions in that set. - UnavailableDependencies(Package, Range), - /// Incompatibility coming from the dependencies of a given package. - FromDependencyOf( - Package, - Range, - Package, - Range, - ), -} - -impl CustomExternal { - fn from_pubgrub(external: External>) -> Self { + fn format_external(&self, external: &External>) -> String { match external { - External::NotRoot(p, v) => CustomExternal::NotRoot(p, v), - External::NoVersions(p, vs) => CustomExternal::NoVersions(p, vs), - External::UnavailableDependencies(p, vs) => { - CustomExternal::UnavailableDependencies(p, vs) - } - External::FromDependencyOf(p, vs, p_dep, vs_dep) => { - CustomExternal::FromDependencyOf(p, vs, p_dep, vs_dep) - } - } - } -} - -impl fmt::Display for CustomExternal { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::NotRoot(package, version) => { - write!(f, "we are solving dependencies of {package} {version}") + External::NotRoot(package, version) => { + format!("we are solving dependencies of {package} {version}") } - Self::NoVersions(package, set) => { + External::NoVersions(package, set) => { if set == &Range::full() { - write!(f, "there is no available version for {package}") + format!("there is no available version for {package}") } else { - write!(f, "there is no version of {package} in {set}") + format!("there is no version of {package} in {set}") } } - Self::UnavailableDependencies(package, set) => { + External::UnavailableDependencies(package, set) => { if set == &Range::full() { - write!(f, "dependencies of {package} are unavailable") + format!("dependencies of {package} are unavailable") } else { - write!( - f, - "dependencies of {package} at version {set} are unavailable" - ) + format!("dependencies of {package} at version {set} are unavailable") } } - Self::FromDependencyOf(package, package_set, dependency, dependency_set) => { + External::FromDependencyOf(package, package_set, dependency, dependency_set) => { if package_set == &Range::full() && dependency_set == &Range::full() { - write!(f, "{package} depends on {dependency}") + format!("{package} depends on {dependency}") } else if package_set == &Range::full() { - write!(f, "{package} depends on {dependency} {dependency_set}") + format!("{package} depends on {dependency} {dependency_set}") } else if dependency_set == &Range::full() { if matches!(package, Package::Root) { // Exclude the dummy version for root packages - write!(f, "{package} depends on {dependency}") + format!("{package} depends on {dependency}") } else { - write!(f, "{package} {package_set} depends on {dependency}") + format!("{package} {package_set} depends on {dependency}") } } else { if matches!(package, Package::Root) { // Exclude the dummy version for root packages - write!(f, "{package} depends on {dependency} {dependency_set}") + format!("{package} depends on {dependency} {dependency_set}") } else { - write!( - f, - "{package} {package_set} depends on {dependency} {dependency_set}" - ) + format!("{package} {package_set} depends on {dependency} {dependency_set}") } } } @@ -449,18 +122,23 @@ fn main() { // Run the algorithm match resolve(&dependency_provider, Package::Root, (0, 0, 0)) { Ok(sol) => println!("{:?}", sol), - Err(PubGrubError::NoSolution(mut derivation_tree)) => { + Err(PubGrubError::NoSolution(derivation_tree)) => { eprintln!("No solution.\n"); eprintln!("### Default report:"); eprintln!("```"); - eprintln!("{}", CustomStringReporter::report(&derivation_tree)); + eprintln!("{}", DefaultStringReporter::report(&derivation_tree)); eprintln!("```\n"); - derivation_tree.collapse_no_versions(); - eprintln!("### Report with `collapse_no_versions`:"); + eprintln!("### Report with custom formatter:"); eprintln!("```"); - eprintln!("{}", CustomStringReporter::report(&derivation_tree)); + eprintln!( + "{}", + DefaultStringReporter::report_with_formatter( + &derivation_tree, + &CustomReportFormatter + ) + ); eprintln!("```"); std::process::exit(1); } From 5f704db316fb2d9ab9ff28bf9c006dd482ab4e4c Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 11:21:06 -0600 Subject: [PATCH 6/9] Add docs to trait method --- src/report.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/report.rs b/src/report.rs index 6e3201a1..02e59c55 100644 --- a/src/report.rs +++ b/src/report.rs @@ -17,9 +17,11 @@ pub trait Reporter { type Output; /// Generate a report from the derivation tree - /// describing the resolution failure. + /// describing the resolution failure using the default formatter. fn report(derivation_tree: &DerivationTree) -> Self::Output; + /// Generate a report from the derivation tree + /// describing the resolution failure using a custom formatter. fn report_with_formatter( derivation_tree: &DerivationTree, formatter: &impl ReportFormatter, From f964d936bcc70ac5ef09b8ee90fbef09779110ae Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 12:01:51 -0600 Subject: [PATCH 7/9] Clippy --- src/internal/incompatibility.rs | 2 +- src/report.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 7e04ec7d..49037e79 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -253,7 +253,7 @@ impl fmt::Display for Incompatibility { write!( f, "{}", - DefaultStringReportFormatter::default().format_terms(&self.package_terms.as_map()) + DefaultStringReportFormatter.format_terms(&self.package_terms.as_map()) ) } } diff --git a/src/report.rs b/src/report.rs index 02e59c55..a6368c6d 100644 --- a/src/report.rs +++ b/src/report.rs @@ -535,7 +535,7 @@ impl Reporter for DefaultStringReporter { type Output = String; fn report(derivation_tree: &DerivationTree) -> Self::Output { - let formatter = DefaultStringReportFormatter::default(); + let formatter = DefaultStringReportFormatter; match derivation_tree { DerivationTree::External(external) => formatter.format_external(external), DerivationTree::Derived(derived) => { @@ -551,7 +551,7 @@ impl Reporter for DefaultStringReporter { formatter: &impl ReportFormatter, ) -> Self::Output { match derivation_tree { - DerivationTree::External(external) => formatter.format_external(&external), + DerivationTree::External(external) => formatter.format_external(external), DerivationTree::Derived(derived) => { let mut reporter = Self::new(); reporter.build_recursive(derived, formatter); From d05cca3028b53d1e8031328e2dc26bfe32525af1 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 30 Nov 2023 12:55:04 -0600 Subject: [PATCH 8/9] Remove outdated comment in example --- examples/unsat_root_message_no_version.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/unsat_root_message_no_version.rs b/examples/unsat_root_message_no_version.rs index 91ee20a3..7c45b9ee 100644 --- a/examples/unsat_root_message_no_version.rs +++ b/examples/unsat_root_message_no_version.rs @@ -36,7 +36,6 @@ impl ReportFormatter> for CustomReportFormatter let terms_vec: Vec<_> = terms.iter().collect(); match terms_vec.as_slice() { [] => "version solving failed".into(), - // TODO: special case when that unique package is root. [(package @ Package::Root, Term::Positive(_))] => { format!("{package} is forbidden") } From c0c749da47c7d6ce9686a424b85412e6e40fb7a6 Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 30 Nov 2023 13:43:54 -0600 Subject: [PATCH 9/9] Use generic --- src/report.rs | 64 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/src/report.rs b/src/report.rs index a6368c6d..af423d40 100644 --- a/src/report.rs +++ b/src/report.rs @@ -245,10 +245,10 @@ impl DefaultStringReporter { } } - fn build_recursive( + fn build_recursive>( &mut self, derived: &Derived, - formatter: &impl ReportFormatter, + formatter: &F, ) { self.build_recursive_helper(derived, formatter); if let Some(id) = derived.shared_id { @@ -259,10 +259,14 @@ impl DefaultStringReporter { }; } - fn build_recursive_helper( + fn build_recursive_helper< + P: Package, + VS: VersionSet, + F: ReportFormatter, + >( &mut self, current: &Derived, - formatter: &impl ReportFormatter, + formatter: &F, ) { match (current.cause1.deref(), current.cause2.deref()) { (DerivationTree::External(external1), DerivationTree::External(external2)) => { @@ -354,12 +358,12 @@ impl DefaultStringReporter { /// /// The result will depend on the fact that the derived incompatibility /// has already been explained or not. - fn report_one_each( + fn report_one_each>( &mut self, derived: &Derived, external: &External, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) { match self.line_ref_of(derived.shared_id) { Some(ref_id) => self.lines.push(Self::explain_ref_and_external( @@ -374,12 +378,16 @@ impl DefaultStringReporter { } /// Report one derived (without a line ref yet) and one external. - fn report_recurse_one_each( + fn report_recurse_one_each< + P: Package, + VS: VersionSet, + F: ReportFormatter, + >( &mut self, derived: &Derived, external: &External, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) { match (derived.cause1.deref(), derived.cause2.deref()) { // If the derived cause has itself one external prior cause, @@ -418,11 +426,15 @@ impl DefaultStringReporter { // String explanations ##################################################### /// Simplest case, we just combine two external incompatibilities. - fn explain_both_external( + fn explain_both_external< + P: Package, + VS: VersionSet, + F: ReportFormatter, + >( external1: &External, external2: &External, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -434,13 +446,13 @@ impl DefaultStringReporter { } /// Both causes have already been explained so we use their refs. - fn explain_both_ref( + fn explain_both_ref>( ref_id1: usize, derived1: &Derived, ref_id2: usize, derived2: &Derived, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -456,12 +468,16 @@ impl DefaultStringReporter { /// One cause is derived (already explained so one-line), /// the other is a one-line external cause, /// and finally we conclude with the current incompatibility. - fn explain_ref_and_external( + fn explain_ref_and_external< + P: Package, + VS: VersionSet, + F: ReportFormatter, + >( ref_id: usize, derived: &Derived, external: &External, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) -> String { // TODO: order should be chosen to make it more logical. format!( @@ -474,10 +490,14 @@ impl DefaultStringReporter { } /// Add an external cause to the chain of explanations. - fn and_explain_external( + fn and_explain_external< + P: Package, + VS: VersionSet, + F: ReportFormatter, + >( external: &External, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) -> String { format!( "And because {}, {}.", @@ -487,11 +507,11 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_ref( + fn and_explain_ref>( ref_id: usize, derived: &Derived, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) -> String { format!( "And because {} ({}), {}.", @@ -502,11 +522,15 @@ impl DefaultStringReporter { } /// Add an already explained incompat to the chain of explanations. - fn and_explain_prior_and_external( + fn and_explain_prior_and_external< + P: Package, + VS: VersionSet, + F: ReportFormatter, + >( prior_external: &External, external: &External, current_terms: &Map>, - formatter: &impl ReportFormatter, + formatter: &F, ) -> String { format!( "And because {} and {}, {}.",