att_amend_desc() drops version constraints when a package appears under two types in DESCRIPTION

[VincentGuyader]

The invariant

A version constraint written in DESCRIPTION must never disappear from an att_amend_desc() run. If a maintainer wrote pkg (>= 1.2.0), that constraint is load-bearing and attachment has no basis to silently reset it to *.

That invariant is violated today whenever a package appears under two types in DESCRIPTION (for example Imports: pkg (>= 1.2.0) and Suggests: pkg). The two rows are collapsed by row position rather than by a rule, so the constraint survives or is silently replaced by * depending on where the rows happen to sit in the frame.

The type conflict has a correct answer too: a package present in both Imports and Suggests should resolve to Imports, the stronger requirement. That is a semantic precedence, not an ordering question. But the version is the part that must not be lost.

Where

R/att_to_description.R, lines 367-374, in att_to_desc_from_is():

deps_new <- data.frame(
  type = c(rep("Imports", length(imports)), rep("Suggests", length(suggests))),
  package = all_packages, stringsAsFactors = FALSE) %>%
  merge(deps_orig[,c("package", "version")],
        by = "package", sort = TRUE, all.x = TRUE, all.y = FALSE) %>%
  .[,c("type", "package", "version")] %>%
  .[order(.$type, .$package), , drop = FALSE] %>%
  .[!duplicated(.$package),]

deps_orig may hold several rows for the same package (one per type). The merge() is on package only, so it multiplies the row: one output row per original type, all carrying the type assigned by the scan but different version values. order(.$type, .$package) cannot separate them, since those duplicate rows share the same type and the same package. !duplicated() therefore keeps whichever row merge() happened to emit first.

Evidence: the outcome flips on row position alone

Traced on a copy of inst/dummypackage with glue pinned in Imports, glue also being listed in Suggests:

### deps_orig (glue rows) ###
      type package  version
2  Imports    glue >= 1.2.0
5 Suggests    glue        *

### scan result ###
imports  contains glue: FALSE
suggests contains glue: TRUE

### deps_new (glue rows) ###
       type package version
11 Suggests    glue       *

Result: Imports: glue (>= 1.2.0) becomes Suggests: glue. The pin is gone and no message is emitted, because glue is still present in the output, so it never enters the removed set reported at lines 416-420.

Running the identical merge / order / !duplicated pipeline on a minimal two-row deps_orig keeps >= 1.2.0 instead. Same code, opposite result, decided purely by the position of the rows in the full frame.

Expected behaviour

When collapsing several DESCRIPTION rows for one package:

1. Preserve the version constraint through the collapse. A constraint present on any of the collapsed rows must appear on the result. If several rows carry constraints, keep the most restrictive one rather than the first encountered. A constraint is only ever dropped when the maintainer removes it from DESCRIPTION.
2. Resolve the type by precedence, Imports winning over Suggests, rather than by position.
3. Never let the row order in deps_orig change the result.

Point 1 is the invariant; points 2 and 3 are how it is achieved. Note that 1 is independent of the type: even if the package legitimately ends up in Suggests, a version written by hand in DESCRIPTION should still be carried over rather than reset to *.

Note on scope

Scan-level precedence already behaves correctly: a package detected in both R/ and tests/ lands in Imports, verified on dummypackage. That path works because order(.$type, ...) sorts "Imports" before "Suggests" alphabetically, so the dedup keeps the right row. It is worth making that precedence explicit rather than relying on the alphabetical accident, since it silently stops working if a type is ever renamed or a third type is introduced.

This is distinct from #139: there is no scan coverage problem here. The package is correctly detected. The defect is entirely in how duplicate DESCRIPTION rows are collapsed, and it reproduces without involving inst/ or any unscanned directory.

Repro

pkg <- <copy of inst/dummypackage>
d <- desc::desc(file = file.path(pkg, "DESCRIPTION"))
d$set_dep("glue", type = "Imports", version = ">= 1.2.0")
d$write()

attachment::att_amend_desc(path = pkg, document = FALSE, use.config = FALSE)

desc::desc_get_deps(file.path(pkg, "DESCRIPTION"))
#> glue is now "Suggests" with version "*"; the ">= 1.2.0" pin is lost, silently