Skip to content

Commit a636870

Browse files
xymusxymus
committed
[Sema] Report public extensions to non-publicly imported types
Access levels on extensions are special. Let's make sure we report public extensions referencing non-public imported types using the preexisting general exportability checks. Record and remark on the use of the import at the same time.
1 parent 6b34df7 commit a636870

File tree

4 files changed

+156
-11
lines changed

4 files changed

+156
-11
lines changed

‎include/swift/AST/DiagnosticsSema.def‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3463,7 +3463,7 @@ ERROR(decl_from_hidden_module,none,
34633463
"%2 was imported for SPI only|"
34643464
"%2 was not imported by this file|"
34653465
"C++ types from imported module %2 do not support library evolution|"
3466-
"<<ERROR>>}3",
3466+
"%2 was not imported publicly}3",
34673467
(const Decl *, unsigned, Identifier, unsigned))
34683468
ERROR(typealias_desugars_to_type_from_hidden_module,none,
34693469
"%0 aliases '%1.%2' and cannot be used %select{here|"
@@ -6603,7 +6603,7 @@ ERROR(inlinable_decl_ref_from_hidden_module,
66036603
"%2 was imported for SPI only|"
66046604
"%2 was not imported by this file|"
66056605
"C++ APIs from imported module %2 do not support library evolution|"
6606-
"<<ERROR>>}3",
6606+
"%2 was not imported publicly}3",
66076607
(const ValueDecl *, unsigned, Identifier, unsigned))
66086608

66096609
WARNING(inlinable_decl_ref_from_hidden_module_warn,

‎lib/Sema/ResilienceDiagnostics.cpp‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -233,13 +233,41 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
233233
auto definingModule = D->getModuleContext();
234234
auto downgradeToWarning = DowngradeToWarning::No;
235235

236-
auto originKind = getDisallowedOriginKind(
237-
D, where, downgradeToWarning);
238-
if (originKind == DisallowedOriginKind::None)
239-
return false;
236+
auto reason = where.getExportabilityReason();
237+
auto DC = where.getDeclContext();
238+
ASTContext &ctx = DC->getASTContext();
239+
auto originKind = getDisallowedOriginKind(D, where, downgradeToWarning);
240+
241+
// If we got here it was used in API, we can record the use of the import.
242+
ImportAccessLevel import = D->getImportAccessFrom(DC);
243+
if (import.has_value() && reason.has_value()) {
244+
auto SF = DC->getParentSourceFile();
245+
if (SF)
246+
SF->registerAccessLevelUsingImport(import.value(),
247+
AccessLevel::Public);
248+
}
240249

241250
// Access levels from imports are reported with the others access levels.
242-
if (originKind == DisallowedOriginKind::NonPublicImport)
251+
// Except for extensions, we report them here.
252+
if (originKind == DisallowedOriginKind::NonPublicImport &&
253+
reason != ExportabilityReason::ExtensionWithPublicMembers &&
254+
reason != ExportabilityReason::ExtensionWithConditionalConformances)
255+
return false;
256+
257+
if (ctx.LangOpts.EnableModuleApiImportRemarks &&
258+
import.has_value() && where.isExported() &&
259+
reason != ExportabilityReason::General &&
260+
originKind != DisallowedOriginKind::NonPublicImport) {
261+
// These may be reported twice, for the Type and for the TypeRepr.
262+
ModuleDecl *importedVia = import->module.importedModule,
263+
*sourceModule = D->getModuleContext();
264+
ctx.Diags.diagnose(loc, diag::module_api_import,
265+
D, importedVia, sourceModule,
266+
importedVia == sourceModule,
267+
/*isImplicit*/false);
268+
}
269+
270+
if (originKind == DisallowedOriginKind::None)
243271
return false;
244272

245273
auto diagName = D->getName();
@@ -254,11 +282,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
254282
diagName = accessor->getStorage()->getName();
255283
}
256284

257-
ASTContext &ctx = where.getDeclContext()->getASTContext();
258-
259285
auto fragileKind = where.getFragileFunctionKind();
260-
auto reason = where.getExportabilityReason();
261-
262286
if (fragileKind.kind == FragileFunctionKind::None) {
263287
DiagnosticBehavior limit = downgradeToWarning == DowngradeToWarning::Yes
264288
? DiagnosticBehavior::Warning
@@ -288,6 +312,17 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
288312
addMissingImport(loc, D, where);
289313
}
290314

315+
// If limited by an import, note which one.
316+
if (originKind == DisallowedOriginKind::NonPublicImport) {
317+
assert(import.has_value() &&
318+
import->accessLevel < AccessLevel::Public &&
319+
"The import should still be non-public");
320+
ctx.Diags.diagnose(import->importLoc,
321+
diag::decl_import_via_here, D,
322+
import->accessLevel,
323+
import->module.importedModule);
324+
}
325+
291326
return true;
292327
}
293328

‎test/Sema/access-level-import-classic-exportability.swift‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file --leading-lines %s %t
3+
4+
/// Build the libraries.
5+
// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t \
6+
// RUN: -enable-library-evolution
7+
// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t \
8+
// RUN: -enable-library-evolution
9+
// RUN: %target-swift-frontend -emit-module %t/InternalLib.swift -o %t \
10+
// RUN: -enable-library-evolution
11+
// RUN: %target-swift-frontend -emit-module %t/FileprivateLib.swift -o %t \
12+
// RUN: -enable-library-evolution
13+
// RUN: %target-swift-frontend -emit-module %t/PrivateLib.swift -o %t \
14+
// RUN: -enable-library-evolution
15+
16+
/// Check diagnostics.
17+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
18+
// RUN: -package-name TestPackage -swift-version 5 \
19+
// RUN: -enable-experimental-feature AccessLevelOnImport -verify
20+
21+
/// Check diagnostics with library-evolution.
22+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
23+
// RUN: -package-name TestPackage -swift-version 5 \
24+
// RUN: -enable-library-evolution \
25+
// RUN: -enable-experimental-feature AccessLevelOnImport -verify
26+
27+
//--- PublicLib.swift
28+
public struct PublicImportType {
29+
public init() {}
30+
}
31+
32+
//--- PackageLib.swift
33+
public struct PackageImportType {
34+
public init() {}
35+
}
36+
37+
//--- InternalLib.swift
38+
public struct InternalImportType {
39+
public init() {}
40+
}
41+
42+
//--- FileprivateLib.swift
43+
public struct FileprivateImportType {
44+
public init() {}
45+
}
46+
47+
//--- PrivateLib.swift
48+
public struct PrivateImportType {
49+
public init() {}
50+
}
51+
52+
//--- Client.swift
53+
public import PublicLib
54+
package import PackageLib // expected-note 2 {{struct 'PackageImportType' imported as 'package' from 'PackageLib' here}}
55+
internal import InternalLib // expected-note 2 {{struct 'InternalImportType' imported as 'internal' from 'InternalLib' here}}
56+
fileprivate import FileprivateLib // expected-note 2 {{struct 'FileprivateImportType' imported as 'fileprivate' from 'FileprivateLib' here}}
57+
private import PrivateLib // expected-note 2 {{struct 'PrivateImportType' imported as 'private' from 'PrivateLib' here}}
58+
59+
public protocol PublicConstrainedExtensionProto {}
60+
extension Array: PublicConstrainedExtensionProto where Element == PublicImportType {}
61+
62+
extension PublicImportType {
63+
public func foo() {}
64+
}
65+
66+
public protocol PackageConstrainedExtensionProto {}
67+
extension Array: PackageConstrainedExtensionProto where Element == PackageImportType {} // expected-error {{cannot use struct 'PackageImportType' in an extension with conditional conformances; 'PackageLib' was not imported publicly}}
68+
69+
extension PackageImportType { // expected-error {{cannot use struct 'PackageImportType' in an extension with public or '@usableFromInline' members; 'PackageLib' was not imported publicly}}
70+
public func foo() {}
71+
}
72+
73+
public protocol InternalConstrainedExtensionProto {}
74+
extension Array: InternalConstrainedExtensionProto where Element == InternalImportType {} // expected-error {{cannot use struct 'InternalImportType' in an extension with conditional conformances; 'InternalLib' was not imported publicly}}
75+
76+
extension InternalImportType { // expected-error {{cannot use struct 'InternalImportType' in an extension with public or '@usableFromInline' members; 'InternalLib' was not imported publicly}}
77+
public func foo() {}
78+
}
79+
80+
public protocol FileprivateConstrainedExtensionProto {}
81+
extension Array: FileprivateConstrainedExtensionProto where Element == FileprivateImportType {} // expected-error {{cannot use struct 'FileprivateImportType' in an extension with conditional conformances; 'FileprivateLib' was not imported publicly}}
82+
83+
extension FileprivateImportType { // expected-error {{cannot use struct 'FileprivateImportType' in an extension with public or '@usableFromInline' members; 'FileprivateLib' was not imported publicly}}
84+
public func foo() {}
85+
}
86+
87+
public protocol PrivateConstrainedExtensionProto {}
88+
extension Array: PrivateConstrainedExtensionProto where Element == PrivateImportType {} // expected-error {{cannot use struct 'PrivateImportType' in an extension with conditional conformances; 'PrivateLib' was not imported publicly}}
89+
90+
extension PrivateImportType { // expected-error {{cannot use struct 'PrivateImportType' in an extension with public or '@usableFromInline' members; 'PrivateLib' was not imported publicly}}
91+
public func foo() {}
92+
}

‎test/Sema/superfluously-public-imports.swift‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
// RUN: %target-swift-frontend -emit-module %t/ExtensionA.swift -o %t -I %t
1515
// RUN: %target-swift-frontend -emit-module %t/ExtensionB.swift -o %t -I %t
1616
// RUN: %target-swift-frontend -emit-module %t/PropertyWrapper.swift -o %t -I %t
17+
// RUN: %target-swift-frontend -emit-module %t/ExtendedDefinitionPublic.swift -o %t -I %t
18+
// RUN: %target-swift-frontend -emit-module %t/ExtendedDefinitionNonPublic.swift -o %t -I %t
1719
// RUN: %target-swift-frontend -emit-module %t/UnusedImport.swift -o %t -I %t
1820
// RUN: %target-swift-frontend -emit-module %t/UnusedPackageImport.swift -o %t -I %t
1921
// RUN: %target-swift-frontend -emit-module %t/ImportNotUseFromAPI.swift -o %t -I %t
@@ -82,6 +84,12 @@ public struct MyPropertyWrapper<T> {
8284
public init(_ value: T) { self.wrappedValue = value }
8385
}
8486

87+
//--- ExtendedDefinitionPublic.swift
88+
public struct PublicExtendedType {}
89+
90+
//--- ExtendedDefinitionNonPublic.swift
91+
public struct NonPublicExtendedType {}
92+
8593
//--- UnusedImport.swift
8694

8795
//--- UnusedPackageImport.swift
@@ -111,6 +119,8 @@ public import Aliases
111119
public import ExtensionA
112120
public import ExtensionB
113121
public import PropertyWrapper
122+
public import ExtendedDefinitionPublic
123+
public import ExtendedDefinitionNonPublic // expected-warning {{public import of 'ExtendedDefinitionNonPublic' was not used in public declarations or inlinable code}} {{1-8=}}
114124

115125
/// Repeat some imports to make sure we report all of them.
116126
public import UnusedImport // expected-warning {{public import of 'UnusedImport' was not used in public declarations or inlinable code}} {{1-8=}}
@@ -179,6 +189,14 @@ public struct StructUsingPropertyWrapper {
179189
@MyPropertyWrapper(42) public var wrapped: Any // expected-remark 2 {{generic struct 'MyPropertyWrapper' is imported via 'PropertyWrapper'}}
180190
}
181191

192+
extension PublicExtendedType { // expected-remark 2 {{struct 'PublicExtendedType' is imported via 'ExtendedDefinitionPublic'}}
193+
public func foo() {}
194+
}
195+
196+
extension NonPublicExtendedType {
197+
func foo() {}
198+
}
199+
182200
public struct Struct { // expected-remark {{implicitly used struct 'Int' is imported via 'Swift'}}
183201
public var propWithInferredIntType = 42
184202
public var propWithExplicitType: String = "Text" // expected-remark {{struct 'String' is imported via 'Swift'}}

0 commit comments

Comments
 (0)