Withhold Maven pins whose coordinates a profile redefines

crates/socket-patch-cli/src/commands/vex_sources.rs

@@ -280,9 +280,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) {
         UnattestedKind::MavenProfileUnpatched => (
             NOTE_MAVEN_PROFILE_UNPATCHED,
             "not attested while a profile declares it at, or its <properties> move it to, \
-             another version; the wiring itself is intact, so re-running `scan` does not change \
-             this (drop that profile <version> or property or set it to the pin, or re-patch \
-             with `scan --mode vendored`)",
+             another version, or its <properties> redefine a groupId / artifactId property so \
+             the pin names another package or another declaration names this one; the wiring \
+             itself is intact, so re-running `scan` does not change this (drop that profile \
+             <version> or property or set it to the pin; `scan --mode vendored` rewrites a \
+             literal profile <version>, but neither mode wires a pin a profile's <properties> \
+             can move, and hosted `scan` now writes no pin a profile bypasses)",
         ),
         UnattestedKind::MavenConfigOverride => (
             NOTE_MAVEN_CONFIG_OVERRIDE,

crates/socket-patch-core/src/formats/maven/properties.rs

@@ -109,6 +109,14 @@ pub(crate) fn interpolate_lenient(value: &str, lookup: &dyn Fn(&str) -> Option<S
     expand(value, lookup, &mut Vec::new(), &lookups, true).unwrap_or_else(|| value.to_string())
 }
 
+/// [`interpolate_lenient`] as an [`Interpolate`] (it always gives a text).
+pub(crate) fn interpolate_keeping(
+    value: &str,
+    lookup: &dyn Fn(&str) -> Option<String>,
+) -> Option<String> {
+    Some(interpolate_lenient(value, lookup))
+}
+
 fn expand(
     value: &str,
     lookup: &dyn Fn(&str) -> Option<String>,
@@ -183,19 +191,25 @@ pub(crate) struct Definitions {
     pub(crate) profiles: Vec<usize>,
 }
 
-/// Every nonempty combination of the profiles that may change what
-/// `value` interpolates to — those defining a name reachable from it
+/// Every nonempty combination of the profiles that may change what any of
+/// `values` interpolates to — those defining a name reachable from one
 /// through any definition ([`Definitions`]) — smallest first, each sorted:
 /// Maven activates any number of profiles together, so a value two
-/// profiles move only jointly is caught. Empty when no profile can change
-/// it. `None` when more than [`MAX_COMBINED_PROFILES`] may, or the walk
-/// exceeds its bound: the caller fails closed.
+/// profiles move only jointly is caught, and the texts of one declaration
+/// (its groupId, artifactId and version) are read under the same
+/// combinations. Empty when no profile can change them. `None` when more
+/// than [`MAX_COMBINED_PROFILES`] may, or the walk exceeds its bound: the
+/// caller fails closed.
 pub(crate) fn profile_combinations(
-    value: &str,
+    values: &[&str],
     defs: &dyn Fn(&str) -> Definitions,
 ) -> Option<Vec<Vec<usize>>> {
     let mut seen: BTreeSet<String> = BTreeSet::new();
-    let mut queue: Vec<String> = references(value).map(str::to_string).collect();
+    let mut queue: Vec<String> = values
+        .iter()
+        .flat_map(|v| references(v))
+        .map(str::to_string)
+        .collect();
     let mut relevant: BTreeSet<usize> = BTreeSet::new();
     while let Some(name) = queue.pop() {
         if !seen.insert(name.clone()) {
@@ -350,15 +364,15 @@ mod tests {
             },
         };
         assert_eq!(
-            profile_combinations("${v}", &defs),
+            profile_combinations(&["${v}"], &defs),
             Some(vec![vec![0], vec![1], vec![0, 1]])
         );
-        assert_eq!(profile_combinations("1.0", &defs), Some(vec![]));
+        assert_eq!(profile_combinations(&["1.0"], &defs), Some(vec![]));
         let many = |_: &str| Definitions {
             texts: vec![],
             profiles: (0..=MAX_COMBINED_PROFILES).collect(),
         };
-        assert_eq!(profile_combinations("${v}", &many), None);
+        assert_eq!(profile_combinations(&["${v}"], &many), None);
     }
 
     #[test]

crates/socket-patch-core/src/patch/redirect/maven_index.rs

@@ -6,7 +6,9 @@
 //! shifting offsets, so a pom with many patched dependencies is scanned once,
 //! not once per dependency.
 
-use crate::formats::maven::PomScope;
+use crate::formats::maven::{
+    interpolate_strict, is_executable_classifier, may_shadow, CoordinateAlternative, PomScope,
+};
 use crate::formats::xml::{child_text, children, elements, Element};
 
 /// One `<dependency>` outside ignored sections.
@@ -20,6 +22,21 @@ pub(super) struct IndexedDep {
     pub(super) type_text: Option<String>,
     pub(super) classifier: Option<String>,
     pub(super) in_profile: bool,
+    /// The trimmed `<groupId>` / `<artifactId>` text as written.
+    pub(super) group_text: Option<String>,
+    pub(super) artifact_text: Option<String>,
+    /// How each combination of profiles that moves its coordinates reads
+    /// it ([`PomScope::coordinate_alternatives`], as VEX discovery reads
+    /// it).
+    pub(super) coordinate_alternatives: Vec<CoordinateAlternative>,
+    /// Inside a `<dependencyManagement>`.
+    pub(super) managed: bool,
+    /// Every version the build may resolve for it, as VEX discovery reads
+    /// one ([`PomScope::resolve_in_build`]): with no profile active but the
+    /// one holding it, then each combination of profiles whose
+    /// `<properties>` move it (as written where one leaves it unresolved).
+    /// Empty when it has no `<version>`.
+    pub(super) build_versions: Vec<String>,
 }
 
 /// One live `<repository>`.
@@ -126,19 +143,36 @@ impl PomIndex {
         pom: &str,
         user: &std::collections::BTreeMap<String, String>,
     ) -> Result<PomIndex, String> {
-        let scope = PomScope::with_user_properties(pom, user)?;
+        let mut scope = PomScope::with_user_properties(pom, user)?;
+        // Profile `<properties>` may move a `${…}` coordinate or version, as
+        // VEX discovery reads it (one it cannot read fails closed there).
+        scope.read_profiles();
         let masked = &scope.masked;
+        let managed = elements(masked, "dependencyManagement")?;
         let deps = scope
             .dependencies()?
             .into_iter()
             .map(|d| IndexedDep {
+                coordinate_alternatives: scope.coordinate_alternatives(&d),
+                managed: managed
+                    .iter()
+                    .any(|m| d.element.start >= m.start && d.element.start < m.end),
+                build_versions: d.version_text.as_deref().map_or_else(Vec::new, |text| {
+                    let (base, alternatives) =
+                        scope.resolve_in_build(text, d.element.start, interpolate_strict);
+                    std::iter::once(base.unwrap_or_else(|| text.to_string()))
+                        .chain(alternatives.into_iter().map(|a| a.value))
+                        .collect()
+                }),
                 group: d.group,
                 artifact: d.artifact,
                 version_inner: d.version_inner,
                 version_text: d.version_text,
                 type_text: d.type_text,
                 classifier: d.classifier,
                 in_profile: d.in_profile,
+                group_text: d.group_text,
+                artifact_text: d.artifact_text,
             })
             .collect();
         let mut repos = Vec::new();
@@ -201,6 +235,165 @@ impl PomIndex {
         })
     }
 
+    /// Why a pin of `group:artifact` at `served` (the suffixed version)
+    /// would not hold whenever some combination of profiles is active whose
+    /// `<properties>` redefine a property a `${…}` coordinate names
+    /// ([`IndexedDep::coordinate_alternatives`]), as VEX discovery weighs
+    /// it: a live versioned declaration of it (the one a pin is written
+    /// into) then names another package, or another declaration then names
+    /// its main jar (as [`may_shadow`] reads one) at a version other than
+    /// `served`, or literally one of its executable classifier copies
+    /// (`test-jar` is the `tests` copy) off `served` or with no version of
+    /// its own. A main jar without a version takes the pin.
+    pub(super) fn retargeted(
+        &self,
+        group: &str,
+        artifact: &str,
+        served: Option<&str>,
+    ) -> Option<String> {
+        self.deps.iter().find_map(|d| {
+            let written = format!(
+                "{}:{}",
+                d.group_text.as_deref().unwrap_or_default(),
+                d.artifact_text.as_deref().unwrap_or_default()
+            );
+            if d.group.as_deref() == Some(group) && d.artifact.as_deref() == Some(artifact) {
+                let moved = d.coordinate_alternatives.first()?;
+                return (!d.in_profile && d.classifier.is_none() && d.version_text.is_some()).then(
+                    || {
+                        format!(
+                            "{written} names {group}:{artifact}, but the <properties> of {} \
+                             make it {}:{}",
+                            moved.by, moved.group, moved.artifact
+                        )
+                    },
+                );
+            }
+            let classifier = match (d.type_text.as_deref(), d.classifier.as_deref()) {
+                (None | Some("jar"), c) => c,
+                (Some(t), c) if t.contains("${") => c,
+                (Some("test-jar"), None) => Some("tests"),
+                // An explicit classifier names the copy whatever the type.
+                (Some(_), Some(c)) => Some(c),
+                _ => return None,
+            };
+            d.coordinate_alternatives.iter().find_map(|r| {
+                let literal = r.group == group && r.artifact == artifact;
+                let names = match classifier {
+                    None => {
+                        (literal
+                            || may_shadow((&r.group, &r.artifact), None, None, (group, artifact)))
+                            && r.version.is_some()
+                    }
+                    Some(c) => literal && is_executable_classifier(c),
+                };
+                (names && r.version.as_deref() != served).then(|| {
+                    format!(
+                        "{written}{} declares {}:{} at <version>{}</version> under the \
+                         <properties> of {}",
+                        classifier.map_or(String::new(), |c| format!(" (classifier {c})")),
+                        r.group,
+                        r.artifact,
+                        r.version.as_deref().unwrap_or("(none of its own)"),
+                        r.by
+                    )
+                })
+            })
+        })
+    }
+
+    /// A `<profiles>` declaration of `group:artifact`'s main jar (literal,
+    /// or through a coordinate property this pom does not define,
+    /// [`may_shadow`]) and a version it may resolve other than `served`
+    /// ([`IndexedDep::build_versions`]: its own profile's `<properties>`,
+    /// or any combination of profiles'), as VEX discovery weighs one: an
+    /// active profile's `<dependencies>` merge over the pom's own, so it
+    /// overrides any pin, and its `<dependencyManagement>` overrides one
+    /// only where management governs the pin (`pin_managed`). A pom /
+    /// sources / javadoc typed one carries no code, and one without a
+    /// version takes the pin.
+    pub(super) fn profile_bypass(
+        &self,
+        group: &str,
+        artifact: &str,
+        served: &str,
+        pin_managed: bool,
+    ) -> Option<(&IndexedDep, &str)> {
+        self.deps.iter().find_map(|d| {
+            let (Some(g), Some(a)) = (d.group.as_deref(), d.artifact.as_deref()) else {
+                return None;
+            };
+            let main_jar = d.classifier.is_none()
+                && !d
+                    .type_text
+                    .as_deref()
+                    .is_some_and(|t| matches!(t, "pom" | "java-source" | "javadoc"))
+                && ((g == group && a == artifact)
+                    || may_shadow((g, a), d.type_text.as_deref(), None, (group, artifact)));
+            if !d.in_profile || !main_jar || (d.managed && !pin_managed) {
+                return None;
+            }
+            let version = d.build_versions.iter().find(|v| *v != served)?;
+            Some((d, version.as_str()))
+        })
+    }
+
+    /// The first executable classifier copy of `group:artifact` (not
+    /// `sources` / `javadoc`; live or inside `<profiles>`) that may resolve
+    /// a version other than `served`, with its classifier and that version
+    /// (`None`: none in this pom), as VEX discovery weighs one: the grant
+    /// serves only the main jar, and a classifier copy is its own
+    /// coordinate, which the pin does not retarget. Its versions are its
+    /// own ([`IndexedDep::build_versions`]), else those of a
+    /// `<dependencyManagement>` entry with the same classifier (Maven keys
+    /// management by classifier; a profile's entry while that profile is
+    /// active), and with no live entry the version comes from outside this
+    /// pom. A `test-jar` typed declaration has no classifier and is refused
+    /// as an unsupported packaging (or a profile bypass) instead.
+    pub(super) fn unserved_classifier_copy(
+        &self,
+        group: &str,
+        artifact: &str,
+        served: &str,
+    ) -> Option<(&str, Option<&str>)> {
+        let copies = |managed: bool| {
+            self.deps.iter().filter_map(move |d| {
+                let c = d.classifier.as_deref()?;
+                (d.managed == managed
+                    && d.group.as_deref() == Some(group)
+                    && d.artifact.as_deref() == Some(artifact)
+                    && is_executable_classifier(c))
+                .then_some((d, c))
+            })
+        };
+        // Every version a declaration may resolve (`None`: it has none).
+        fn versions(d: &IndexedDep) -> Vec<Option<&str>> {
+            match d.build_versions.as_slice() {
+                [] => vec![None],
+                all => all.iter().map(|v| Some(v.as_str())).collect(),
+            }
+        }
+        copies(false).find_map(|(d, classifier)| {
+            let resolved: Vec<Option<&str>> = if d.version_text.is_some() {
+                versions(d)
+            } else {
+                let entries = |in_profile: bool| {
+                    copies(true)
+                        .filter(move |(m, c)| *c == classifier && m.in_profile == in_profile)
+                        .flat_map(|(m, _)| versions(m))
+                };
+                let mut live: Vec<Option<&str>> = entries(false).collect();
+                if live.is_empty() {
+                    live.push(None);
+                }
+                live.extend(entries(true));
+                live
+            };
+            let version = resolved.into_iter().find(|v| *v != Some(served))?;
+            Some((classifier, version))
+        })
+    }
+
     pub(super) fn dep(&self, i: usize) -> &IndexedDep {
         &self.deps[i]
     }
@@ -344,6 +537,11 @@ impl PomIndex {
             type_text: None,
             classifier: None,
             in_profile: false,
+            group_text: Some(group.to_string()),
+            artifact_text: Some(artifact.to_string()),
+            coordinate_alternatives: Vec::new(),
+            managed: true,
+            build_versions: vec![version.to_string()],
         });
     }