Skip to content

[NFC] Fix typo, make formatting in PubGrub code consistent #7717

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

Merged
merged 1 commit into from
Jun 26, 2024
Merged
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
9 changes: 6 additions & 3 deletions Sources/PackageGraph/Resolution/PubGrub/Assignment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ public struct Assignment: Equatable {

/// An assignment made during decision making.
public static func decision(_ term: Term, decisionLevel: Int) -> Assignment {
assert(term.requirement.isExact, "Cannot create a decision assignment with a non-exact version selection: \(term.requirement)")
assert(
term.requirement.isExact,
"Cannot create a decision assignment with a non-exact version selection: \(term.requirement)"
)

return self.init(
term: term,
Expand Down Expand Up @@ -68,9 +71,9 @@ extension Assignment: CustomStringConvertible {
public var description: String {
switch self.isDecision {
case true:
return "[Decision \(self.decisionLevel): \(self.term)]"
"[Decision \(self.decisionLevel): \(self.term)]"
case false:
return "[Derivation: \(self.term) ← \(self.cause?.description ?? "-")]"
"[Derivation: \(self.term) ← \(self.cause?.description ?? "-")]"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ struct DiagnosticReportBuilder {
private var lineNumbers: [Incompatibility: Int] = [:]
private let provider: ContainerProvider

init(root: DependencyResolutionNode, incompatibilities: [DependencyResolutionNode: [Incompatibility]], provider: ContainerProvider) {
init(
root: DependencyResolutionNode,
incompatibilities: [DependencyResolutionNode: [Incompatibility]],
provider: ContainerProvider
) {
self.rootNode = root
self.incompatibilities = incompatibilities
self.provider = provider
Expand All @@ -41,20 +45,20 @@ struct DiagnosticReportBuilder {
try self.visit(rootCause)
} else {
assertionFailure("Unimplemented")
self.record(
try self.record(
rootCause,
message: try self.description(for: rootCause),
message: self.description(for: rootCause),
isNumbered: false
)
}

var content: String = ""
var content = ""
let padding = self.lineNumbers.isEmpty ? 0 : "\(Array(self.lineNumbers.values).last!) ".count

for (idx, line) in self.lines.enumerated() {
content += String(repeating: " ", count: padding)
if line.number != -1 {
content += String(repeating: " ", count: padding)
content += String(repeating: " ", count: padding)
content += " (\(line.number)) "
}
content += line.message.prefix(1).capitalized
Expand Down Expand Up @@ -86,9 +90,9 @@ struct DiagnosticReportBuilder {
let otherLine = self.lineNumbers[cause.other]

if let conflictLine, let otherLine {
self.record(
try self.record(
incompatibility,
message: "\(incompatibilityDesc) because \(try self.description(for: cause.conflict)) (\(conflictLine)) and \(try self.description(for: cause.other)) (\(otherLine).",
message: "\(incompatibilityDesc) because \(self.description(for: cause.conflict)) (\(conflictLine)) and \(self.description(for: cause.other)) (\(otherLine).",
isNumbered: isNumbered
)
} else if conflictLine != nil || otherLine != nil {
Expand All @@ -106,9 +110,9 @@ struct DiagnosticReportBuilder {
}

try self.visit(withoutLine)
self.record(
try self.record(
incompatibility,
message: "\(conjunction)\(incompatibilityDesc) because \(try self.description(for: withLine)) \(line).",
message: "\(conjunction)\(incompatibilityDesc) because \(self.description(for: withLine)) \(line).",
isNumbered: isNumbered
)
} else {
Expand All @@ -127,9 +131,9 @@ struct DiagnosticReportBuilder {
} else {
try self.visit(cause.conflict, isConclusion: true)
try self.visit(cause.other)
self.record(
try self.record(
incompatibility,
message: "\(conjunction)\(incompatibilityDesc) because \(try self.description(for: cause.conflict)) (\(self.lineNumbers[cause.conflict]!)).",
message: "\(conjunction)\(incompatibilityDesc) because \(self.description(for: cause.conflict)) (\(self.lineNumbers[cause.conflict]!)).",
isNumbered: isNumbered
)
}
Expand All @@ -139,9 +143,9 @@ struct DiagnosticReportBuilder {
let ext = cause.conflict.cause.isConflict ? cause.other : cause.conflict
let derivedLine = self.lineNumbers[derived]
if let derivedLine {
self.record(
try self.record(
incompatibility,
message: "\(incompatibilityDesc) because \(try self.description(for: ext)) and \(try self.description(for: derived)) (\(derivedLine)).",
message: "\(incompatibilityDesc) because \(self.description(for: ext)) and \(self.description(for: derived)) (\(derivedLine)).",
isNumbered: isNumbered
)
} else if self.isCollapsible(derived) {
Expand All @@ -150,27 +154,28 @@ struct DiagnosticReportBuilder {
return
}

let collapsedDerived = derivedCause.conflict.cause.isConflict ? derivedCause.conflict : derivedCause.other
let collapsedDerived = derivedCause.conflict.cause.isConflict ? derivedCause.conflict : derivedCause
.other
let collapsedExt = derivedCause.conflict.cause.isConflict ? derivedCause.other : derivedCause.conflict

try self.visit(collapsedDerived)
self.record(
try self.record(
incompatibility,
message: "\(conjunction)\(incompatibilityDesc) because \(try self.description(for: collapsedExt)) and \(try self.description(for: ext)).",
message: "\(conjunction)\(incompatibilityDesc) because \(self.description(for: collapsedExt)) and \(self.description(for: ext)).",
isNumbered: isNumbered
)
} else {
try self.visit(derived)
self.record(
try self.record(
incompatibility,
message: "\(conjunction)\(incompatibilityDesc) because \(try self.description(for: ext)).",
message: "\(conjunction)\(incompatibilityDesc) because \(self.description(for: ext)).",
isNumbered: isNumbered
)
}
} else {
self.record(
try self.record(
incompatibility,
message: "\(incompatibilityDesc) because \(try self.description(for: cause.conflict)) and \(try self.description(for: cause.other)).",
message: "\(incompatibilityDesc) because \(self.description(for: cause.conflict)) and \(self.description(for: cause.other)).",
isNumbered: isNumbered
)
}
Expand Down Expand Up @@ -213,13 +218,16 @@ struct DiagnosticReportBuilder {
return "package '\(versionedDependency.identity)' is required using a stable-version but '\(versionedDependency.identity)' depends on an unstable-version package '\(unversionedDependency.identity)'"
case .incompatibleToolsVersion(let version):
let term = incompatibility.terms.first!
return "\(try self.description(for: term, normalizeRange: true)) contains incompatible tools version (\(version))"
return try "\(self.description(for: term, normalizeRange: true)) contains incompatible tools version (\(version))"
}

let terms = incompatibility.terms
if terms.count == 1 {
let term = terms.first!
let prefix = try hasEffectivelyAnyRequirement(term) ? term.node.nameForDiagnostics : self.description(for: term, normalizeRange: true)
let prefix = try hasEffectivelyAnyRequirement(term) ? term.node.nameForDiagnostics : self.description(
for: term,
normalizeRange: true
)
return "\(prefix) " + (term.isPositive ? "cannot be used" : "is required")
} else if terms.count == 2 {
let term1 = terms.first!
Expand All @@ -233,12 +241,12 @@ struct DiagnosticReportBuilder {
}
}

let positive = try terms.filter(\.isPositive).map { try description(for: $0) }
let negative = try terms.filter { !$0.isPositive }.map { try description(for: $0) }
let positive = try terms.filter(\.isPositive).map { try self.description(for: $0) }
let negative = try terms.filter { !$0.isPositive }.map { try self.description(for: $0) }
if !positive.isEmpty, !negative.isEmpty {
if positive.count == 1 {
let positiveTerm = terms.first { $0.isPositive }!
return "\(try self.description(for: positiveTerm, normalizeRange: true)) practically depends on \(negative.joined(separator: " or "))"
return try "\(self.description(for: positiveTerm, normalizeRange: true)) practically depends on \(negative.joined(separator: " or "))"
} else {
return "if \(positive.joined(separator: " and ")) then \(negative.joined(separator: " or "))"
}
Expand Down Expand Up @@ -351,8 +359,8 @@ struct DiagnosticReportBuilder {
}
}

private extension DependencyResolutionNode {
var nameForDiagnostics: String {
extension DependencyResolutionNode {
fileprivate var nameForDiagnostics: String {
"'\(self.package.identity)'"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ extension Incompatibility: CustomStringConvertible {
}
}

public extension Incompatibility {
extension Incompatibility {
/// Every incompatibility has a cause to explain its presence in the
/// derivation graph. Only the root incompatibility uses `.root`. All other
/// incompatibilities are either obtained from dependency constraints,
Expand All @@ -89,7 +89,7 @@ public extension Incompatibility {
/// │{root 1.0.0}│
/// └────────────┘
/// ```
indirect enum Cause: Equatable, Hashable {
public indirect enum Cause: Equatable, Hashable {
public struct ConflictCause: Hashable {
public let conflict: Incompatibility
public let other: Incompatibility
Expand All @@ -110,7 +110,10 @@ public extension Incompatibility {
case noAvailableVersion

/// A version-based dependency contains unversioned-based dependency.
case versionBasedDependencyContainsUnversionedDependency(versionedDependency: PackageReference, unversionedDependency: PackageReference)
case versionBasedDependencyContainsUnversionedDependency(
versionedDependency: PackageReference,
unversionedDependency: PackageReference
)

/// The package's tools version is incompatible.
case incompatibleToolsVersion(ToolsVersion)
Expand Down
42 changes: 21 additions & 21 deletions Sources/PackageGraph/Resolution/PubGrub/PartialSolution.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import OrderedCollections
import struct TSCUtility.Version

/// The partial solution is a constantly updated solution used throughout the
/// dependency resolution process, tracking know assignments.
/// dependency resolution process, tracking known assignments.
public struct PartialSolution {
var root: DependencyResolutionNode?

Expand All @@ -37,37 +37,37 @@ public struct PartialSolution {

/// The current decision level.
public var decisionLevel: Int {
decisions.count - 1
self.decisions.count - 1
}

public init(assignments: [Assignment] = []) {
self.assignments = assignments
for assignment in assignments {
register(assignment)
self.register(assignment)
}
}

/// A list of all packages that have been assigned, but are not yet satisfied.
public var undecided: [Term] {
_positive.values.filter { !decisions.keys.contains($0.node) }
self._positive.values.filter { !self.decisions.keys.contains($0.node) }
}

/// Create a new derivation assignment and add it to the partial solution's
/// list of known assignments.
public mutating func derive(_ term: Term, cause: Incompatibility) {
let derivation = Assignment.derivation(term, cause: cause, decisionLevel: self.decisionLevel)
self.assignments.append(derivation)
register(derivation)
self.register(derivation)
}

/// Create a new decision assignment and add it to the partial solution's
/// list of known assignments.
public mutating func decide(_ node: DependencyResolutionNode, at version: Version) {
decisions[node] = version
self.decisions[node] = version
let term = Term(node, .exact(version))
let decision = Assignment.decision(term, decisionLevel: decisionLevel)
let decision = Assignment.decision(term, decisionLevel: self.decisionLevel)
self.assignments.append(decision)
register(decision)
self.register(decision)
}

/// Populates the _positive and _negative properties with the assignment.
Expand All @@ -76,17 +76,17 @@ public struct PartialSolution {
let pkg = term.node

if let positive = _positive[pkg] {
_positive[term.node] = positive.intersect(with: term)
self._positive[term.node] = positive.intersect(with: term)
return
}

let newTerm = _negative[pkg].flatMap { term.intersect(with: $0) } ?? term
let newTerm = self._negative[pkg].flatMap { term.intersect(with: $0) } ?? term

if newTerm.isPositive {
_negative[pkg] = nil
_positive[pkg] = newTerm
self._negative[pkg] = nil
self._positive[pkg] = newTerm
} else {
_negative[pkg] = newTerm
self._negative[pkg] = newTerm
}
}

Expand All @@ -95,7 +95,7 @@ public struct PartialSolution {
public func satisfier(for term: Term) throws -> Assignment {
var assignedTerm: Term?

for assignment in assignments {
for assignment in self.assignments {
guard assignment.term.node == term.node else {
continue
}
Expand All @@ -114,24 +114,24 @@ public struct PartialSolution {
public mutating func backtrack(toDecisionLevel decisionLevel: Int) {
var toBeRemoved: [(Int, Assignment)] = []

for (idx, assignment) in zip(0..., assignments) {
for (idx, assignment) in zip(0..., self.assignments) {
if assignment.decisionLevel > decisionLevel {
toBeRemoved.append((idx, assignment))
}
}

for (idx, remove) in toBeRemoved.reversed() {
let assignment = assignments.remove(at: idx)
let assignment = self.assignments.remove(at: idx)
if assignment.isDecision {
decisions.removeValue(forKey: remove.term.node)
self.decisions.removeValue(forKey: remove.term.node)
}
}

// FIXME: We can optimize this by recomputing only the removed things.
_negative.removeAll()
_positive.removeAll()
for assignment in assignments {
register(assignment)
self._negative.removeAll()
self._positive.removeAll()
for assignment in self.assignments {
self.register(assignment)
}
}

Expand Down
Loading