SE-0386: `package` access modifier

this was actually exactly what i suggested, and very similar to what @allevato pitched earlier in this thread.

to make this concrete, i imagine we would have 3 ways to specify subsystem identities in the package manifest:

extension Package
{
    enum Subsystem
    {
        /// The default global subsystem. The compiler (meaning: SPM) will
        /// replace this with the name of the package.
        case global
        /// The implicit local subsystem. The compiler (meaning: SPM) will
        /// replace this with the name of the module.
        case local
        /// An explicitly named subsystem.
        case named(String)
    }
}

the default would be global, standalone “external” targets like snippets would use local, and nuclear subsystems would use named(_:).

within source code, the new keyword would behave exactly like the proposed package keyword, it would not require any awareness of the subsystem name. (in contrast with @_spi.) we would just need to come up with a different name for it besides “package”.

2 Likes

I'm -0.5 on this change.
This feels like a patch to access control, when it really needs a comprehensive review/overhaul to better meet everyone's needs.

That said it will address a need and shouldn't be actively harmful so I'm hesitant to say it should be stopped for a theoretical review.

1 Like

Super, it seems like a relatively small adjustment to the pitch overall with progressive complexity disclosure which is nice.

One might imply which of these settings should be used as the default based of a targets type also (for test and possible future Benchmark targets) - but that’s really up to the build system.

To bikeshed this I’d avoid adding a new concept here and instead try with something like PackageVisibility, PackageAccessVisibility or AccessVisibility or similar.

1 Like

I’ve only skimmed the thread so far, but as far as naming I wonder if something relational, such as peerpublic, public(peers) or peerprivate would fit without having to invent new terminology for an independent group of modules. It seems the basic concept here is to make a symbol from one module visible to its peers (e.g. other modules in a package).

I like peerprivate the most (as an analogue to fileprivate), since it’s describing private within a scope rather than public only to a certain scope, but I could see an argument that private is several scopes apart from this and it could be better to relate it to the next rank above or below (public or internal, respectively).

Seemed like an interesting route to explore that I haven’t seen suggested so far.

Sorry for the very late reply. Could you elaborate on this? I'm not doubting it would be hard to manage / unproductive, just trying to summarize your feedback for the rest of the workgroup. Is the concern that it be a lot of work to make those sub-targets properly modular (e.g. because you'd have to remove any dependency cycles with other parts of your project)? Or is your concern more about the extra package structure (e.g. manifests, repositories, and so on)?

Would you consider this concern addressed if there was a way to split packages into multiple groupings, or do you think it is wrong to tie them together at all, and so SwiftPM should support groupings that span packages?

Because at that point, you are designing two separate features and giving them the same name.

The design of the current @_spi can be summed up as "the stable ABI is so resilient that clients of a module can generate correct code without even knowing that the SPIs exist, so we'll just leave the SPIs out of the public version of the module and everything will work fine". If you remove the "stable ABI" part of that sentence, the rest of it simply falls apart.

That means your hypothetical "@_spi without module stability" enhancement would need an almost entirely new implementation, and you'd basically have two different features sharing a single syntax. At that point, it seems wiser to admit that these are different things and embrace that so the new feature can be tailor-made for its use cases.

1 Like

I'm still not clear on why that's relevant. I don't think anyone has suggested just renaming @_spi to @spi and calling it a day, or even using the SPI name. Repeating that @_spi wasn't designed for this use-case isn't an explanation for why a feature which looks a lot like it is a bad solution for this problem.

2 Likes

I like that it conveys the access level is within a group, but it's a bit verbose; group is a noun so it looks out of place among all the access modifiers. How about calling it grouped? It's an adjective, short, and still conveys it's for a group of modules.

FWIW, groupinternal is directly analogous to fileprivate.

1 Like

I get that it's analogous but I thought the general consensus was that fileprivate wasn't the best naming either; question is, should we try to come up with a succinct name or keep it analogous to existing modifiers even if it's verbose.

Our concern is about the second point.

In fact, we consider the first point a feature, not an issue. Restructuring the project in order to achieve a proper modularization (that is, code is split into modules, with clear dependencies) without dependency cycles allowed us to disentangle the messy "architecture" that we found when started working on a large existing app. And SPM was a very, very good option to achieve our goal.

In particular, we initially created a single local package within the app project, and started chipping away at the monolith by creating more and more modules within the same package. Our modules (represented by SPM library targets) are of various kinds, for example:

  • features (that usually include business logic and UI)
  • individual services (for example, LocationManager) with "live" instances of services that are eventually injected into the features,
  • app "entry point" and dependency injection management,
  • repositories,
  • common utilities and abstractions,
  • UI components

et cetera. Modules are created and maintained by several teams, working in parallel mostly on different modules, but some modules are shared.

With a single package, though, we quickly encountered 2 problems:

  • the single package manifest was becoming massive and hard to understand and work on; in particular, teams were having a hard time defining new modules or updating the manifest of existing ones;
  • each module could technically depend on every other module, by adding just a line in the manifest, but we wanted to enforce some layering (for example, we don't want feature modules to depend directly on services).

This compelled us to create packages for each "layer" in our architecture, for example we have a package called Features and one called Services: now we can simply enforce that the first has no package dependency on the other, and some extra utilities (like GitHub CODEOWNERS file) help us with team coordination.

Right now have 4 packages, and we're currently fine with the extra package structure, but we will likely have a problem with it if we push this forward.

For example, we realized that some "feature modules" are very large, and we would benefit in splitting them in multiple "groupings of files": for example, a screen that shows a list of things and the filter section for that list are both very large and could benefit from some isolation, with the possibility of sharing some things, while also clarifying what should be the "public" interface of each part (this is like the concept of nuclei from @taylorswift).

We currently use namespaces based on caseless enums, but that doesn't really solve the access problem, it's just for code organization. We could simply split the feature in multiple modules, but each of those would use public for having things visible for the other subcomponent, but in turn it would expose those APIs to the rest of the modules in the package, which is not what we want; also, each individual subcomponent would declare a module importable across the package, but it should really only be importable by the global component (the "feature module") that includes those subcomponents.

Now, if we had something like folderprivate we could work within the feature module and split the files in folders, for each sub component: for example, all types that should only be seen by the subcomponent could be declared as folderprivate.

Alternatively, we could create a package for the feature, and leverage the package access modifier to have things shared across the feature, without being public for other features or parts of the project. But then, we would likely incur in the problem of too many packages and too much SPM structure (not to mention, the "resolving package graph" phase at Xcode startup would likely take longer and longer, and it's already a little annoying for us).

It would be fully addressed if a package was split in multiple groupings, with the possibility of declaring something as visible only within the group.

For example (not a real proposal, just an idea to show what kind of feature would serve us well), if in the package manifest the target had some kind of group parameter, like the following:

.target(
  name: "FirstModule",
  groups: ["GroupOne", "GroupTwo"]
),
.target(
  name: "SecondModule",
  groups: ["GroupOne"]
),
.target(
  name: "ThirdModule",
  groups: ["GroupTwo"]
),

then, in each individual module, I could write something like the following:

// in FirstModule

internal("GroupOne") let visibleInFirstModuleAndSecondModule = 42

internal("GroupTwo") let visibleInFirstModuleAndThirdModule = 43

// in SecondModule

foo(visibleInFirstModuleAndSecondModule) // ok

foo(visibleInFirstModuleAndThirdModule) // error, undefined symbol

// in ThirdModule

bar(visibleInFirstModuleAndThirdModule) // ok

internal("GroupOne") let invalid = 44 // error, module is not part of "GroupOne"
internal("GroupThree") let nonsense = 45 // error, there's no "GroupThree"

About the concept of "groupings that span packages", I personally disagree. I find simpler and better to have a hierarchical access structure, where smaller access groups are strictly contained within larger ones, with some convenient exceptions (for example, accessing private members in extensions, but only within the same file).

I think one of the recurrent contention points when it comes to Swift access control is whether to permit a breakage of the hierarchical structure in some cases (for example, accessing private members in extensions declared in different files than the one where the original type is defined), and I personally disagree with the idea, because I think it would encourage and facilitate the destruction of natural boundaries in a code base, and ultimately increase complexity and development risk, and reduce maintainability.

For similar reasons, I think "module groups" should not span multiple packages.

10 Likes

Thank you, that’s very helpful and quite thorough. I appreciate you taking the time to lay that out.

1 Like

You're welcome, thanks for the questions, and for encouraging a productive conversation on the topic.

1 Like

It seems to me that this can't properly solve the issue it is trying to solve without an addition to the import statement. I'm also not in favour of the current package naming

I suggest the following alternatives that do well both in expressing intent and when communicating/thinking about their effect (unlike the mentioned 'this function is package'):

// ModuleA, same package
concealed func foo() {}

// ModuleB, same package
internal import ModuleA // or ..
revealed import ModuleA // or ..
import revealing ModuleA // or ..
import revealed ModuleA

foo() // ok

// ModuleA, same package
outward func foo() {}

// ModuleB, same package
inward import ModuleA
// ModuleA, same package
exposed func foo() {} 
exposed private func bar() {}

// ModuleB, same package
import exposed ModuleA
// ModuleA, same package
masked func foo() {}

// ModuleB, same package
import unmasked ModuleA // or ..
import unmasking ModuleA

I also like Becca's friend suggestion, but as an adjective, although to be honest this doesn't say much about visibility/access as much as the other ones above:

// ModuleA, same package
friendly func foo() {}

// ModuleB, same package
import friendly ModuleA

With the explicit addition to the import statement we can be sure that example modules or public api testing modules don't accidentally use unwanted symbols.
Using any of the import extra keywords would need to not have any effect or be illegal outside a package and concerning external dependencies, so a

import exposed UIKit

or whatever other keyword variant would either have no effect on the exposed symbols from UIKit or be considered illegal

Personally I think this whole functionality may be better as a modifier rather than a new access level and function similarly to @testable but with fine grained control. As in just the declarations specifically annotated with @exposed (for example) would be made public to a module that imports that module in a specific way, using dunno, @exposed import Module or some better sounding variant. With the caveat that this wouldn't be possible for modules outside the module's package. Correct me if i'm wrong but this way there wouldn't be a need for @usableFromPackageInline

2 Likes

would package declaration expose the declaration in extensions found in other files? Something requested here: Missing a level of access control on class/struct

// File A
public class SomeObject {
  private var propertyA: Int
  package var propertyB: String
  public init() {
    // ...
  }

}

// File B
extension SomeObject {
   func doSomething() {
     propertyB = "some value" // Ok, propertyB is visible at package level, including the defining module
   }
}

But you know what? This whole package access modifier sounds an awful lot like a byproduct of having proper namespaces and with namespaces we'd have submodules as well. Some discussions around namespaces here An Enum Based Approach to Namespaces and here Namespaces × submodules

I think if namespaces could access same package internals and extend them there wouldn't be a need for a package access modifier, forced uses of @testable or @_spi. As in namespaces would be open within a package and final outside the package and thus give all the functionality package access modifier tries to bring

1 Like

I am very late to this party, and realise the review closed weeks ago, so this may well be too late to be pertinent. However, having read the proposal and the thread today, I'll throw this out there.

I think the proposal has significant merit. Given the concerns about the access modifier being too closely tied to Swift Packages, I suggest that an excellent alternative naming for the proposed package access modifier could be local:

open > public > local > internal > fileprivate > private

By default, and perhaps as the only current implementation, local access could be to the modules within a package—implementation exactly as proposed in the proposal. However, the use of this keyword would allow for the potential for new mechanisms to delineate what is local to be introduced in the future. This might for instance include provision for grouping mechanisms within a code base that define specific localities that are not based around Swift Packages.

Within this naming, there could be scope for SPM to provide a mechanism to designate a specific product as nonlocal in some fashion, without this apparently conflicting with the face meaning of package. This could then provide for the use case described for the Examples target that should only exercise public API.

Again, apologies if a contribution well outside the window causes consternation, even given I have no real expectations of relevance in doing so. :kissing_smiling_eyes:

8 Likes

For what it's worth I just had to disable some source-compatibility tests of our projects because the lack of such package visibility. We offer a "testkit" target that prys open internals to provide mocks and other neat tools in the distributed actor cluster, making all those APIs public is not an option, and going through a ton of code and _ it all is also very painful (I tried and gave up just now).

Related issue: ActorTestKit uses @testable import DistributedCluster, causing problems for build -c release · Issue #1113 · apple/swift-distributed-actors · GitHub

If we had package access levels, these APIs would be exactly that: package visible such that we can build the ...TestKit target without relying on @testable imports which causes all kinds of issues in release builds without enabled testability.

8 Likes

SE-0386 has been accepted in part, has been revised, and is now back in review. Please continue any discussion of this feature in the new review thread.

2 Likes