Sema: Warn on resilient uses of @_implementationOnly as deprecated

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.
This commit is contained in:
Alexis Laferrière
2024-04-21 10:47:09 -07:00
parent f4d73275cb
commit ff797cfd66
4 changed files with 65 additions and 9 deletions

View File

@@ -1167,6 +1167,14 @@ WARNING(implementation_only_requires_library_evolution,none,
"using '@_implementationOnly' without enabling library evolution " "using '@_implementationOnly' without enabling library evolution "
"for %0 may lead to instability during execution", "for %0 may lead to instability during execution",
(Identifier)) (Identifier))
WARNING(implementation_only_deprecated_explicit,none,
"'@_implementationOnly' is deprecated, use 'internal import' "
"and family instead",
())
WARNING(implementation_only_deprecated_implicit,none,
"'@_implementationOnly' is deprecated, use a bare import "
"as 'InternalImportsByDefault' is enabled",
())
ERROR(module_allowable_client_violation,none, ERROR(module_allowable_client_violation,none,
"module %0 doesn't allow importation from module %1", "module %0 doesn't allow importation from module %1",

View File

@@ -592,17 +592,22 @@ struct AttributedImport {
/// if the access level was implicit or explicit. /// if the access level was implicit or explicit.
SourceRange accessLevelRange; SourceRange accessLevelRange;
/// Location of the `@_implementationOnly` attribute if set.
SourceRange implementationOnlyRange;
AttributedImport(ModuleInfo module, SourceLoc importLoc = SourceLoc(), AttributedImport(ModuleInfo module, SourceLoc importLoc = SourceLoc(),
ImportOptions options = ImportOptions(), ImportOptions options = ImportOptions(),
StringRef filename = {}, ArrayRef<Identifier> spiGroups = {}, StringRef filename = {}, ArrayRef<Identifier> spiGroups = {},
SourceRange preconcurrencyRange = {}, SourceRange preconcurrencyRange = {},
std::optional<AccessLevel> docVisibility = std::nullopt, std::optional<AccessLevel> docVisibility = std::nullopt,
AccessLevel accessLevel = AccessLevel::Public, AccessLevel accessLevel = AccessLevel::Public,
SourceRange accessLevelRange = SourceRange()) SourceRange accessLevelRange = SourceRange(),
SourceRange implementationOnlyRange = SourceRange())
: module(module), importLoc(importLoc), options(options), : module(module), importLoc(importLoc), options(options),
sourceFileArg(filename), spiGroups(spiGroups), sourceFileArg(filename), spiGroups(spiGroups),
preconcurrencyRange(preconcurrencyRange), docVisibility(docVisibility), preconcurrencyRange(preconcurrencyRange), docVisibility(docVisibility),
accessLevel(accessLevel), accessLevelRange(accessLevelRange) { accessLevel(accessLevel), accessLevelRange(accessLevelRange),
implementationOnlyRange(implementationOnlyRange) {
assert(!(options.contains(ImportFlags::Exported) && assert(!(options.contains(ImportFlags::Exported) &&
options.contains(ImportFlags::ImplementationOnly)) || options.contains(ImportFlags::ImplementationOnly)) ||
options.contains(ImportFlags::Reserved)); options.contains(ImportFlags::Reserved));
@@ -613,7 +618,8 @@ struct AttributedImport {
: AttributedImport(module, other.importLoc, other.options, : AttributedImport(module, other.importLoc, other.options,
other.sourceFileArg, other.spiGroups, other.sourceFileArg, other.spiGroups,
other.preconcurrencyRange, other.docVisibility, other.preconcurrencyRange, other.docVisibility,
other.accessLevel, other.accessLevelRange) { } other.accessLevel, other.accessLevelRange,
other.implementationOnlyRange) { }
friend bool operator==(const AttributedImport<ModuleInfo> &lhs, friend bool operator==(const AttributedImport<ModuleInfo> &lhs,
const AttributedImport<ModuleInfo> &rhs) { const AttributedImport<ModuleInfo> &rhs) {
@@ -623,7 +629,8 @@ struct AttributedImport {
lhs.spiGroups == rhs.spiGroups && lhs.spiGroups == rhs.spiGroups &&
lhs.docVisibility == rhs.docVisibility && lhs.docVisibility == rhs.docVisibility &&
lhs.accessLevel == rhs.accessLevel && lhs.accessLevel == rhs.accessLevel &&
lhs.accessLevelRange == rhs.accessLevelRange; lhs.accessLevelRange == rhs.accessLevelRange &&
lhs.implementationOnlyRange == rhs.implementationOnlyRange;
} }
AttributedImport<ImportedModule> getLoaded(ModuleDecl *loadedModule) const { AttributedImport<ImportedModule> getLoaded(ModuleDecl *loadedModule) const {

View File

@@ -586,8 +586,10 @@ UnboundImport::UnboundImport(ImportDecl *ID)
if (ID->getAttrs().hasAttribute<TestableAttr>()) if (ID->getAttrs().hasAttribute<TestableAttr>())
import.options |= ImportFlags::Testable; import.options |= ImportFlags::Testable;
if (ID->getAttrs().hasAttribute<ImplementationOnlyAttr>()) if (auto attr = ID->getAttrs().getAttribute<ImplementationOnlyAttr>()) {
import.options |= ImportFlags::ImplementationOnly; import.options |= ImportFlags::ImplementationOnly;
import.implementationOnlyRange = attr->Range;
}
import.accessLevel = ID->getAccessLevel(); import.accessLevel = ID->getAccessLevel();
if (auto attr = ID->getAttrs().getAttribute<AccessControlAttr>()) { if (auto attr = ID->getAttrs().getAttribute<AccessControlAttr>()) {
@@ -811,7 +813,21 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
// We exempt some imports using @_implementationOnly in a safe way from // We exempt some imports using @_implementationOnly in a safe way from
// packages that cannot be resilient. // packages that cannot be resilient.
if (import.options.contains(ImportFlags::ImplementationOnly) && if (import.options.contains(ImportFlags::ImplementationOnly) &&
!SF.getParentModule()->isResilient() && topLevelModule && import.implementationOnlyRange.isValid()) {
if (SF.getParentModule()->isResilient()) {
// Encourage replacing `@_implementationOnly` with `internal import`.
if (ctx.LangOpts.hasFeature(Feature::InternalImportsByDefault)) {
auto inFlight =
ctx.Diags.diagnose(import.importLoc,
diag::implementation_only_deprecated_implicit);
inFlight.fixItRemove(import.implementationOnlyRange);
} else {
auto inFlight =
ctx.Diags.diagnose(import.importLoc,
diag::implementation_only_deprecated_explicit);
inFlight.fixItReplace(import.implementationOnlyRange, "internal");
}
} else if ( // Non-resilient
!(((targetName.str() == "CCryptoBoringSSL" || !(((targetName.str() == "CCryptoBoringSSL" ||
targetName.str() == "CCryptoBoringSSLShims") && targetName.str() == "CCryptoBoringSSLShims") &&
(importerName.str() == "Crypto" || (importerName.str() == "Crypto" ||
@@ -820,11 +836,13 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
((targetName.str() == "CNIOBoringSSL" || ((targetName.str() == "CNIOBoringSSL" ||
targetName.str() == "CNIOBoringSSLShims") && targetName.str() == "CNIOBoringSSLShims") &&
importerName.str() == "NIOSSL"))) { importerName.str() == "NIOSSL"))) {
ctx.Diags.diagnose(import.importLoc, ctx.Diags.diagnose(import.importLoc,
diag::implementation_only_requires_library_evolution, diag::implementation_only_requires_library_evolution,
importerName); importerName);
}
} }
// Report public imports of non-resilient modules from a resilient module.
if (import.options.contains(ImportFlags::ImplementationOnly) || if (import.options.contains(ImportFlags::ImplementationOnly) ||
import.accessLevel < AccessLevel::Public) import.accessLevel < AccessLevel::Public)
return; return;

View File

@@ -0,0 +1,23 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t --leading-lines
// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
// RUN: -enable-library-evolution -swift-version 5 \
// RUN: -emit-module-path %t/Lib.swiftmodule
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -enable-library-evolution -swift-version 5 \
// RUN: -verify -verify-additional-prefix swift-5-
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -enable-library-evolution -swift-version 5 \
// RUN: -enable-upcoming-feature InternalImportsByDefault \
// RUN: -verify -verify-additional-prefix default-to-internal-
//--- Lib.swift
public struct SomeType {}
//--- Client.swift
@_implementationOnly import Lib
// expected-swift-5-warning @-1 {{'@_implementationOnly' is deprecated, use 'internal import' and family instead}} {{1-21=internal}}
// expected-default-to-internal-warning @-2 {{'@_implementationOnly' is deprecated, use a bare import as 'InternalImportsByDefault' is enabled}} {{1-22=}}
internal func foo(_: SomeType) {}