Skip to content

Sema: Only complain about the import when it does restrict the access level #68644

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.

Sign up for GitHub

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 1 commit into from
Sep 20, 2023
Merged

Sema: Only complain about the import when it does restrict the access level #68644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/Sema/ResilienceDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,

ImportAccessLevel problematicImport = D->getImportAccessFrom(DC);
if (problematicImport.has_value() &&
diagAccessLevel == problematicImport->accessLevel) {
problematicImport->accessLevel < D->getFormalAccess()) {
Context.Diags.diagnose(problematicImport->accessLevelLoc,
diag::decl_import_via_here, D,
problematicImport->accessLevel,
Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/TypeCheckAccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,12 @@ void AccessControlCheckerBase::checkTypeAccessImpl(
static_cast<const IdentTypeRepr*>(complainRepr)->getBoundDecl();
assert(VD && "findTypeWithScope should return bound TypeReprs only");
complainImport = VD->getImportAccessFrom(useDC);

// Don't complain about an import that doesn't restrict the access
// level of the decl. This can happen with imported `package` decls.
if (complainImport.has_value() &&
complainImport->accessLevel >= VD->getFormalAccess())
complainImport = llvm::None;
}

diagnose(problematicAccessScope, complainRepr, downgradeToWarning,
Expand Down
31 changes: 29 additions & 2 deletions test/Sema/access-level-import-diag-priority.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
// RUN: split-file --leading-lines %s %t

/// Build the libraries.
// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t \
// RUN: -package-name pkg
// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t \
// RUN: -package-name pkg
// RUN: %target-swift-frontend -emit-module %t/InternalLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/FileprivateLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/PrivateLib.swift -o %t
Expand All @@ -14,15 +16,20 @@
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -enable-experimental-feature AccessLevelOnImport -verify \
// RUN: -package-name pkg
// RUN: %target-swift-frontend -typecheck %t/PackageTypeImportedAsPackageClient.swift -I %t \
// RUN: -enable-experimental-feature AccessLevelOnImport -verify \
// RUN: -package-name pkg
// RUN: %target-swift-frontend -typecheck %t/LocalVsImportClient.swift -I %t \
// RUN: -enable-experimental-feature AccessLevelOnImport -verify \
// RUN: -package-name pkg

//--- PublicLib.swift
public struct PublicImportType {}
package struct PackageLevelPublicImportedType {}

//--- PackageLib.swift
public struct PackageImportType {}
package struct PackageLevelPackageImportedType {}

//--- InternalLib.swift
public struct InternalImportType {}
Expand Down Expand Up @@ -62,6 +69,26 @@ public func publicFuncUsesPrivateScambled(_ a: PublicImportType, d: FileprivateI
var _: PrivateImportType
}

//--- PackageTypeImportedAsPackageClient.swift
/// Report errors about using package decls in public but don't note the import
/// as it doesn't affect the access level of the decls.

public import PublicLib
package import PackageLib

public func publicFuncUsesPackageLevelPublicImportedType(_ a: PackageLevelPublicImportedType) {} // expected-error {{function cannot be declared public because its parameter uses a package type}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should give the same error if PublicImportType is used here right?

Copy link
Contributor Author

@xymus xymus Sep 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PublicImportType would be ok as long as it's publicly imported. It would show the same error for PackageImportType, with the note.

public func publicFuncUsesPackageLevelPackageImportedType(_ a: PackageLevelPackageImportedType) {} // expected-error {{function cannot be declared public because its parameter uses a package type}}

@inlinable public func funcInlinableReferenceToPublicImportedType() {
var _: PackageLevelPublicImportedType // expected-error {{struct 'PackageLevelPublicImportedType' is package and cannot be referenced from an '@inlinable' function}}
var _: Array<PackageLevelPublicImportedType> // expected-error {{struct 'PackageLevelPublicImportedType' is package and cannot be referenced from an '@inlinable' function}}
}

@inlinable public func funcInlinableReferenceToPackageImportedType() {
var _: PackageLevelPackageImportedType // expected-error {{struct 'PackageLevelPackageImportedType' is package and cannot be referenced from an '@inlinable' function}}
var _: Array<PackageLevelPackageImportedType> // expected-error {{struct 'PackageLevelPackageImportedType' is package and cannot be referenced from an '@inlinable' function}}
}

/// Local vs imports
//--- LocalVsImportClient.swift
public import PublicLib
Expand Down