Skip to content

[6.0] Sema: Improve warnings and notes related to access-level on imports #73203

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 7 commits into from
Apr 24, 2024

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Apr 23, 2024

General improvements to diagnostics around access-level on imports:

  • Fix erroneous warning about superfluous package imports providing a type extended in the local file.
  • Warn that @_implementationOnly imports are deprecated and offer fixit to replace with internal import.
  • Note a way to silence warnings about ambiguous implicit access-level on imports. This will be a problem with internal imports by default being pushed out of Swift 6.
  • Note what imports limit the access-level of a decl both on the import and the decl where it triggers an error to make sure it's visible in IDEs.

Scope: Users of access-level on imports.
Risk: Low, this only adds and removes warnings and notes.
Reviewed by @tshortli and @nkcsgexi
Cherry-pick of #73125, #73176, #73179 and #73183
Resolves rdar://126712864&119438201

xymus added 7 commits April 23, 2024 10:56
The warnings on superfluously public/package imports has a hole where
it could report a package import as not being used by package decls
even if it provides a type extended by a package extension. This is
caused by the exportability checker not being enabled for package
decls, thus not triggering the usual logic registering this reference.

Address this issue by adding a check specifically for package extensions.
We can remove this check once we have exportability checking for package
decls.

rdar://126712864
…ings

Adoption InternalImportsByDefault provides a safe access-level by default
to imports, as such ambiguities are not a risk and showing this warning is
superflous. When this warning is shown, make sure we note this alternative.
Report uses of `@_implementationOnly` in resilient modules as deprecated.
With a fixit to replace it with `internal` or delete it when imports
are internal by default.

Uses of `@_implementationOnly` in non-resilient modules is already reported
as being unsafe.
Access-level on imports limit where decls from the target module can
be referenced. This is reported by the typical error about and a note
on the import. However, when using an IDE and editing a large file,
the note on the import is easy to miss. Address this by duplicating
the information on the error line as well so it's never out of the
current viewport.

rdar://119438201
@xymus xymus requested a review from a team as a code owner April 23, 2024 18:08
@xymus
Copy link
Contributor Author

xymus commented Apr 23, 2024

@swift-ci Please test

@xymus xymus added 🍒 release cherry pick Flag: Release branch cherry picks swift 6.0 labels Apr 23, 2024
@xymus
Copy link
Contributor Author

xymus commented Apr 23, 2024

@swift-ci Please test Linux

@xymus
Copy link
Contributor Author

xymus commented Apr 23, 2024

Both macOS and Linux failed under the SourceKitLSPPackageTests.

@swift-ci Please test macOS

@xymus
Copy link
Contributor Author

xymus commented Apr 23, 2024

@swift-ci Please test Linux

@xymus
Copy link
Contributor Author

xymus commented Apr 24, 2024

It looks like infrastructure issues around SourceKitLSPPackageTests and SwiftPMWorkspaceTests.

@swift-ci Please test macOS

@xymus
Copy link
Contributor Author

xymus commented Apr 24, 2024

@swift-ci Please test Linux

@xymus
Copy link
Contributor Author

xymus commented Apr 24, 2024

It should have been fixed.

@swift-ci Please test macOS

@xymus
Copy link
Contributor Author

xymus commented Apr 24, 2024

@swift-ci Please test Linux

@xymus xymus merged commit 9a73306 into swiftlang:release/6.0 Apr 24, 2024
@xymus xymus deleted the access-level-import-diags-6.0 branch April 24, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants