feat: Add CustomDumpCoreLocation trait

[maximkrouk]

I'm not sure if that's the reason, but seems like dependency on swift-custom-dump is the reason for a warning in AppStoreConnect
The warning prompts to add "location usage description" plist key even tho the app doesn't use CoreLocation APIs
swift-custom-dump is the only place where CoreLocation is imported, so this may fix the issue 🤔

Even if it doesn't I feel like it's a good thing to provide such extensions as opt-ins anyway c:

[mbrandonw]

Hi @maximkrouk, we can't really take this change since it is a breaking one. If anything it would need to be a CustomDumpOmitCoreLocation trait to omit the inclusion of core location APIs.

But can you please explain more about what exactly is happening in App Store Connect? Can you share screenshots/messages you are seeing? And is it just a warning? Not an error?

[maximkrouk]

Yep, that would require a major update, I understand

I guess CustomDumpOmitCoreLocation trait should work for now, and afaiu changing it to CustomDumpCoreLocation in next major update should be free for those who enable CustomDumpOmitCoreLocation

I'll update the PR later, also I'll test the upload with a fork to provide more context
Meanwhile the warning:

90683: Missing purpose string in Info.plist. Your app’s code references one or more APIs that access sensitive user data, or the app has one or more entitlements that permit such access. The Info.plist file for the “xxx” bundle should contain a NSLocationWhenInUseUsageDescription key with a user-facing purpose string explaining clearly and completely why your app needs the data. If you’re using external libraries or SDKs, they may reference APIs that require a purpose string. While your app might not use these APIs, a purpose string is still required. For details, visit: https://developer.apple.com/documentation/uikit/protecting_the_user_s_privacy/requesting_access_to_protected_resources.

[mbrandonw]

Hey @maximkrouk, actually @stephencelis mentioned that you could use default traits to keep it as CustomDumpCoreLocation instead of CustomDumpOmitCoreLocation. So if you want to do that, plus add a #if compiler(<6.1) check to always include CoreLocation when not using the newest Swift, then we would be happy to accept this. Then you could provide an empty collection of traits when depending on CustomDump to clear out the defaults.

If you are feeling brave you could also add similar traits for some of the other frameworks we have in this library (e.g. CoreMotion, GameKit, etc.). But no pressure for that, happy to take just CoreLocation for now (or do the others in a followup PR).

[maximkrouk]

Hmm, actually I just noticed that my implementation is likely to remove CoreLocation helpers completely for Swift 6.1 😅

Package traits is a pretty fresh feature and I may not understand smth correctly, but doesn't it make sense to opt-out non-core traits by default?

For example

• package A doesn't specify any traits and imports swift-custom-dump just for it's core functionality
• package B imports package A, transitively depending on swift-custom-dump
• package B specifies swift-custom-dump dependency to explicitly provide empty traits array to disable CoreLocation trait

Afaiu compiler will see that the trait is required for package A, therefore swift-custom-dump will be compiled with a redundant trait. This can be fixed on package A level, but there wouldn't be such situation if swift-custom-dump didn't enable optional traits by default 🤔

Maybe I'm missing smth 💁‍♂️
Evolution-wise I find the approach with Omit trait more compelling

• ✅ it will allow for opt-outs without introducing breaking changes
• ⚠️ Omit traits are less ergonomic than default ones
• Potential ergonomics are sacrificed in the name of minor improvement in compiler performance and ability to opt-out CoreLocation 😅
• ✅ Ergonomics can be restored in next major update
  • Users of Omit trait will be able to remove it, but since it will take no effect, removal is basically optional
  • Users of CoreLocation will have to enable the trait explicitly (as traits intended to be used)

Note

Swift version check is needed anyway

[maximkrouk]

UPD:

The only real downside I see is that libraries should never enable the Omit trait, since afaiu it will opt-out CoreLocation completely with no way to restore it on the application level 🤔

[mbrandonw]

Hmm, actually I just noticed that my implementation is likely to remove CoreLocation helpers completely for Swift 6.1 😅

That is what my comment about #if compiler(<6.1) was about. You can make it so that Swift <6.1 always gets extensions, and then in Swift 6.2 you can override traits to opt out.

Package traits is a pretty fresh feature and I may not understand smth correctly, but doesn't it make sense to opt-out non-core traits by default?

Yes that would be nice, but that's a breaking change. What I am suggesting is a non-breaking change, allows those who care about the App Store Connect warning to fix it, and then in 2.0 traits will be opt-in instead of opt-out.

[maximkrouk]

Alright, I can update it, but I also want to check if Traits are propagated to child dependencies, afaiu - they are not, so if my check (probably on Monday) succeeds we could dump the "CustomDump" prefix to keep traits cleaner

Note

I took Package.swift from swift-configuration as a reference, and they do use simple names for traits, so I think it would be nice to get rid of the prefix if it can't possible conflict with other packages' traits with the same name

I also have a few other PRs that could benefit from this check:

• feat: Expose reference APIs swift-sharing#205
• feat: Sharing trait swift-navigation#331

[maximkrouk]

Locally tests failed because of date format btw 🤔
Not sure if I should include the fix (for 3 test Suites):

[maximkrouk]

Some details about the update:

• I included both CoreLocation and OmitCoreLocation traits
• CoreLocation is enabled by default
  • I added a readme note with a recommendation to specify it explicitly, the note might need your editing
• The only way to disable CoreLocation is to specify OmitCoreLocation
  • The change is permanent so libraries should not specify it
  • It should be specified on application level when needed
• Currently explicitly specifying and array of empty traits will not remove CoreLocation APIs (since OmitCoreLocation is missing)
• Major release
  • Could remove OmitCoreLocation completely
  • Could remove CoreLocation from default traits
  • If both changes are in place - they won't affect clients with explicitly defined traits
  • Clients that rely on default traits will have to specify CoreLocation explicitly

Note

- I didn't check if it removes AppStoreConnect warning yet
- I didn't check if it works with nested dependencies yet (but it should)
- I checked that it works correctly with a direct dependency

[x-sheep]

Can this problem be solved by adding a PrivacyInfo.xcprivacy file to this library instead? If the file is present (and valid) Xcode can hopefully infer that this library handles CoreLocation types without accessing the user's location data.

[maximkrouk]

I think traits make sense anyway, however if xcprivacy manifest supports specifying such info, it probably should be added as well 🤔