Skip to content

Use the package access level instead of @testable imports #1215

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

Closed
wants to merge 2 commits into from
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
//===----------------------------------------------------------------------===//

#if FOUNDATION_FRAMEWORK
@_spi(Unstable) internal import CollectionsInternal
@_spi(Unstable) package import CollectionsInternal
#elseif canImport(_RopeModule)
internal import _RopeModule
package import _RopeModule
#elseif canImport(_FoundationCollections)
internal import _FoundationCollections
package import _FoundationCollections
#endif

extension AttributedString {
internal final class Guts : @unchecked Sendable {
package final class Guts : @unchecked Sendable {
typealias Index = AttributedString.Index
typealias Runs = AttributedString.Runs
typealias AttributeMergePolicy = AttributedString.AttributeMergePolicy
Expand All @@ -30,7 +30,7 @@ extension AttributedString {
typealias _AttributeStorage = AttributedString._AttributeStorage

var version: Version
var string: BigString
package var string: BigString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is only used from one place: NumberFormatStyleTests which could easily do String(self.characters) instead of String(self._guts.string) which would save us from needing to mark this and the imports above as package.

For cases like this, would you rather we update them in a separate change and keep this as just a @testable --> package change, or would you want to incorporate that into this to reduce the number of things that we import as package like the collections modules above?

Copy link
Member Author

@ahoppen ahoppen Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d prefer if this is just a mechanical @testable -> package change. It’s fairly big already and I don’t want to hide any other functionality change in it.

So, if you could make those changes up-front, that would be great.

var runs: _InternalRuns
var trackedRanges: [Range<BigString.Index>]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal import _FoundationCollections
@dynamicMemberLookup
@available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
public struct AttributedString : Sendable {
internal var _guts: Guts
package var _guts: Guts

internal init(_ guts: Guts) {
_guts = guts
Expand Down
2 changes: 1 addition & 1 deletion Sources/FoundationEssentials/Calendar/Calendar.swift
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ public struct Calendar : Hashable, Equatable, Sendable {
}

/// For use by `NSCoding` implementation in `NSCalendar` and `Codable` for `Calendar` only.
internal init(identifier: Calendar.Identifier, locale: Locale, timeZone: TimeZone?, firstWeekday: Int?, minimumDaysInFirstWeek: Int?, gregorianStartDate: Date?) {
package init(identifier: Calendar.Identifier, locale: Locale, timeZone: TimeZone?, firstWeekday: Int?, minimumDaysInFirstWeek: Int?, gregorianStartDate: Date?) {
_calendar = CalendarCache.cache.fixed(identifier: identifier, locale: locale, timeZone: timeZone, firstWeekday: firstWeekday, minimumDaysInFirstWeek: minimumDaysInFirstWeek, gregorianStartDate: gregorianStartDate)
}

Expand Down
60 changes: 30 additions & 30 deletions Sources/FoundationEssentials/Calendar/Calendar_Gregorian.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ extension Date {
timeIntervalSinceReferenceDate / 86400 + Self.julianDayAtDateReference
}

func julianDay() throws (GregorianCalendarError) -> Int {
package func julianDay() throws (GregorianCalendarError) -> Int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see uses of this, the initializer below, or the error type from our tests, was this an accidental addition?

Copy link
Member Author

@ahoppen ahoppen Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s used here:

That being said, the test should probably call julianDay instead of just getting a function reference to it 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah that's probably why I missed it in my search since it didn't actually call the function - ok thanks for the explanation

let jd = (julianDate + 0.5).rounded(.down)
guard jd <= Double(Self.maxJulianDay), jd >= Double(Self.minJulianDay) else {
throw .overflow(nil, self, nil)
Expand All @@ -51,7 +51,7 @@ extension Date {
self.init(julianDate: Double(julianDay))
}

init(julianDate: Double) {
package init(julianDate: Double) {
let secondsSinceJulianReference = (julianDate - Self.julianDayAtDateReference) * 86400
self.init(timeIntervalSinceReferenceDate: secondsSinceJulianReference)
}
Expand Down Expand Up @@ -165,13 +165,13 @@ enum ResolvedDateComponents {


/// Internal-use error for indicating unexpected situations when finding dates.
enum GregorianCalendarError : Error {
package enum GregorianCalendarError : Error {
case overflow(Calendar.Component?, Date? /* failing start date */, Date? /* failing end date */)
case notAdvancing(Date /* next */, Date /* previous */)
}

/// This class is a placeholder and work-in-progress to provide an implementation of the Gregorian calendar.
internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable {
package final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable {

#if canImport(os)
internal static let logger: Logger = {
Expand All @@ -191,7 +191,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable

let inf_ti : TimeInterval = 4398046511104.0

init(identifier: Calendar.Identifier, timeZone: TimeZone?, locale: Locale?, firstWeekday: Int?, minimumDaysInFirstWeek: Int?, gregorianStartDate: Date?) {
package init(identifier: Calendar.Identifier, timeZone: TimeZone?, locale: Locale?, firstWeekday: Int?, minimumDaysInFirstWeek: Int?, gregorianStartDate: Date?) {

// ISO8601 has different default values for time zone, locale, firstWeekday, and minimumDaysInFirstWeek
let defaultTimeZone: TimeZone
Expand Down Expand Up @@ -250,14 +250,14 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
self.identifier = identifier
}

let identifier: Calendar.Identifier
package let identifier: Calendar.Identifier

var locale: Locale?
package var locale: Locale?

var timeZone: TimeZone
package var timeZone: TimeZone

var _firstWeekday: Int?
var firstWeekday: Int {
package var firstWeekday: Int {
set {
precondition(newValue >= 1 && newValue <= 7, "Weekday should be in the range of 1...7")
_firstWeekday = newValue
Expand All @@ -275,7 +275,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
}

var _minimumDaysInFirstWeek: Int?
var minimumDaysInFirstWeek: Int {
package var minimumDaysInFirstWeek: Int {
set {
if newValue < 1 {
_minimumDaysInFirstWeek = 1
Expand All @@ -298,7 +298,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
}
}

func copy(changingLocale: Locale?, changingTimeZone: TimeZone?, changingFirstWeekday: Int?, changingMinimumDaysInFirstWeek: Int?) -> _CalendarProtocol {
package func copy(changingLocale: Locale?, changingTimeZone: TimeZone?, changingFirstWeekday: Int?, changingMinimumDaysInFirstWeek: Int?) -> _CalendarProtocol {
let newTimeZone = changingTimeZone ?? self.timeZone
let newLocale = changingLocale ?? self.locale

Expand All @@ -323,7 +323,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return _CalendarGregorian.init(identifier: identifier, timeZone: newTimeZone, locale: newLocale, firstWeekday: newFirstWeekday, minimumDaysInFirstWeek: newMinDays, gregorianStartDate: nil)
}

func hash(into hasher: inout Hasher) {
package func hash(into hasher: inout Hasher) {
hasher.combine(identifier)
hasher.combine(timeZone)
hasher.combine(firstWeekday)
Expand All @@ -337,7 +337,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable

// Returns the range of a component in Gregorian Calendar.
// When there are multiple possible upper bounds, the smallest one is returned.
func minimumRange(of component: Calendar.Component) -> Range<Int>? {
package func minimumRange(of component: Calendar.Component) -> Range<Int>? {
switch component {
case .era: 0..<2
case .year: 1..<140743
Expand All @@ -362,7 +362,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable

// Returns the range of a component in Gregorian Calendar.
// When there are multiple possible upper bounds, the largest one is returned.
func maximumRange(of component: Calendar.Component) -> Range<Int>? {
package func maximumRange(of component: Calendar.Component) -> Range<Int>? {
switch component {
case .era: return 0..<2
case .year: return 1..<144684
Expand Down Expand Up @@ -497,7 +497,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return ord1..<(ord2 + 1)
}

func range(of smaller: Calendar.Component, in larger: Calendar.Component, for date: Date) -> Range<Int>? {
package func range(of smaller: Calendar.Component, in larger: Calendar.Component, for date: Date) -> Range<Int>? {
func isValidComponent(_ c: Calendar.Component) -> Bool {
return !(c == .calendar || c == .timeZone || c == .weekdayOrdinal || c == .nanosecond)
}
Expand Down Expand Up @@ -865,7 +865,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable

// FIXME: This is almost the same with Calendar_ICU's _locked_start(of:).
// There is a chance of refactoring Calendar_ICU to use this one
func start(of unit: Calendar.Component, at: Date) -> Date? {
package func start(of unit: Calendar.Component, at: Date) -> Date? {
let time = at.timeIntervalSinceReferenceDate

var effectiveUnit = unit
Expand Down Expand Up @@ -922,7 +922,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return (work, daysAdded)
}

func ordinality(of smaller: Calendar.Component, in larger: Calendar.Component, for date: Date) -> Int? {
package func ordinality(of smaller: Calendar.Component, in larger: Calendar.Component, for date: Date) -> Int? {
let result: Int?
do {
result = try _ordinality(of: smaller, in: larger, for: date)
Expand Down Expand Up @@ -1445,7 +1445,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
// No return here to ensure we've covered all cases in switch statements above, even via `default`.
}

func dateInterval(of component: Calendar.Component, for date: Date) -> DateInterval? {
package func dateInterval(of component: Calendar.Component, for date: Date) -> DateInterval? {
let time = date.timeIntervalSinceReferenceDate
var effectiveUnit = component
switch effectiveUnit {
Expand Down Expand Up @@ -1582,7 +1582,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return TimeInterval(totalSecond)
}

func isDateInWeekend(_ date: Date) -> Bool {
package func isDateInWeekend(_ date: Date) -> Bool {
let weekendRange: WeekendRange
if let localeWeekendRange = locale?.weekendRange {
weekendRange = localeWeekendRange
Expand All @@ -1595,7 +1595,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
}

// For testing purpose
internal func isDateInWeekend(_ date: Date, weekendRange: WeekendRange) -> Bool {
package func isDateInWeekend(_ date: Date, weekendRange: WeekendRange) -> Bool {

// First, compare the day of the week
let dayOfWeek = dateComponent(.weekday, from: date)
Expand Down Expand Up @@ -1660,7 +1660,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return true
}

func date(from components: DateComponents) -> Date? {
package func date(from components: DateComponents) -> Date? {
guard _CalendarGregorian.isComponentsInSupportedRange(components) else {

// One or more values exceeds supported date range
Expand All @@ -1682,7 +1682,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return relativeWeekday
}

func numberOfDaysInMonth(_ month: Int, year: Int) -> Int {
package func numberOfDaysInMonth(_ month: Int, year: Int) -> Int {
var month = month
var year = year
if month > 12 {
Expand Down Expand Up @@ -1978,7 +1978,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return weekNo
}

func dateComponents(_ components: Calendar.ComponentSet, from d: Date, in timeZone: TimeZone) -> DateComponents {
package func dateComponents(_ components: Calendar.ComponentSet, from d: Date, in timeZone: TimeZone) -> DateComponents {
let timezoneOffset = timeZone.secondsFromGMT(for: d)
let localDate = d + Double(timezoneOffset)

Expand Down Expand Up @@ -2132,11 +2132,11 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return dc
}

func dateComponents(_ components: Calendar.ComponentSet, from date: Date) -> DateComponents {
package func dateComponents(_ components: Calendar.ComponentSet, from date: Date) -> DateComponents {
dateComponents(components, from: date, in: timeZone)
}

func dateComponent(_ component: Calendar.Component, from date: Date) -> Int {
package func dateComponent(_ component: Calendar.Component, from date: Date) -> Int {
guard let value = dateComponents(.init(single: component), from: date, in: timeZone).value(for: component) else {
preconditionFailure("dateComponents(:from:in:) unexpectedly returns nil for requested component")
}
Expand Down Expand Up @@ -2227,7 +2227,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return nil
}

func add(_ field: Calendar.Component, to date: Date, amount: Int, inTimeZone timeZone: TimeZone) throws (GregorianCalendarError) -> Date {
package func add(_ field: Calendar.Component, to date: Date, amount: Int, inTimeZone timeZone: TimeZone) throws (GregorianCalendarError) -> Date {

guard amount != 0 else {
return date
Expand Down Expand Up @@ -2406,7 +2406,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return newValue
}

func addAndWrap(_ field: Calendar.Component, to date: Date, amount: Int, inTimeZone timeZone: TimeZone) throws (GregorianCalendarError) -> Date {
package func addAndWrap(_ field: Calendar.Component, to date: Date, amount: Int, inTimeZone timeZone: TimeZone) throws (GregorianCalendarError) -> Date {

guard amount != 0 else {
return date
Expand Down Expand Up @@ -2851,7 +2851,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
return result
}

func date(byAdding components: DateComponents, to date: Date, wrappingComponents: Bool) -> Date? {
package func date(byAdding components: DateComponents, to date: Date, wrappingComponents: Bool) -> Date? {
do {
if wrappingComponents {
return try self.date(byAddingAndWrapping: components, to: date)
Expand All @@ -2866,7 +2866,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
// MARK: Differences

// Calendar::fieldDifference
func difference(inComponent component: Calendar.Component, from start: Date, to end: Date) throws (GregorianCalendarError) -> (difference: Int, newStart: Date) {
package func difference(inComponent component: Calendar.Component, from start: Date, to end: Date) throws (GregorianCalendarError) -> (difference: Int, newStart: Date) {
guard end != start else {
return (0, start)
}
Expand Down Expand Up @@ -2944,7 +2944,7 @@ internal final class _CalendarGregorian: _CalendarProtocol, @unchecked Sendable
}


func dateComponents(_ components: Calendar.ComponentSet, from start: Date, to end: Date) -> DateComponents {
package func dateComponents(_ components: Calendar.ComponentSet, from start: Date, to end: Date) -> DateComponents {

var diffsInNano: Int
var curr: Date
Expand Down
4 changes: 2 additions & 2 deletions Sources/FoundationEssentials/CodableUtilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ extension UInt8 {
internal static var _newline: UInt8 { UInt8(ascii: "\n") }
internal static var _tab: UInt8 { UInt8(ascii: "\t") }

internal static var _colon: UInt8 { UInt8(ascii: ":") }
package static var _colon: UInt8 { UInt8(ascii: ":") }
internal static let _semicolon = UInt8(ascii: ";")
internal static var _comma: UInt8 { UInt8(ascii: ",") }

Expand Down Expand Up @@ -175,7 +175,7 @@ extension UInt8 {
return Int(self &- UInt8(ascii: "0"))
}

internal var isLetter: Bool? {
package var isLetter: Bool? {
return (0x41 ... 0x5a) ~= self || (0x61 ... 0x7a) ~= self
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ extension Decimal : ExpressibleByIntegerLiteral {

@available(macOS 10.10, iOS 8.0, watchOS 2.0, tvOS 9.0, *)
extension Decimal: Hashable {
internal subscript(index: UInt32) -> UInt16 {
package subscript(index: UInt32) -> UInt16 {
get {
switch index {
case 0: return _mantissa.0
Expand Down
Loading