diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 492c428d69be9..c9e00afdeeb19 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1935,12 +1935,9 @@ WARNING(access_control_non_objc_open_member,none, "non-'@objc' %0 in extensions cannot be overridden; use 'public' instead", (DescriptiveDeclKind)) ERROR(access_control_requires_package_name, none, - "%0 has a package access level but no -package-name was specified: %1", - (Identifier, StringRef)) -ERROR(access_control_requires_package_name_import, none, - "package import can only be used from a module with a package name; " - "set it with the compiler flag -package-name", - ()) + "the package access level %select{|used on %1 }0" + "requires a package name; set it with the compiler flag -package-name", + (bool, const Decl*)) ERROR(invalid_decl_attribute,none, "'%0' attribute cannot be applied to this declaration", (DeclAttribute)) ERROR(attr_invalid_on_decl_kind,none, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 0397439d0a9c5..2003a4b55b707 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -4091,21 +4091,6 @@ getAccessScopeForFormalAccess(const ValueDecl *VD, case AccessLevel::Package: { auto pkg = resultDC->getPackageContext(/*lookupIfNotCurrent*/ true); if (!pkg) { - auto srcFile = resultDC->getOutermostParentSourceFile(); - // Check if the file containing package decls is an interface file; if an - // interface file contains package decls, they must be usableFromInline or - // inlinable and are accessed within the defining module, so package-name - // is not needed; do not show diagnostics in such case. - auto shouldSkipDiag = srcFile && srcFile->Kind == SourceFileKind::Interface; - if (!shouldSkipDiag) { - // No package context was found; show diagnostics - auto &d = VD->getASTContext().Diags; - auto filename = srcFile ? srcFile->getFilename() : resultDC->getParentModule()->getBaseIdentifier().str(); - d.diagnose(VD->getLoc(), - diag::access_control_requires_package_name, - VD->getBaseIdentifier(), - filename); - } // Instead of reporting and failing early, return the scope of resultDC to // allow continuation (should still non-zero exit later if in script mode) return AccessScope(resultDC); diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 9751f50e12de4..16be45340b58a 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -996,8 +996,8 @@ bool AttributeChecker::visitAbstractAccessControlAttr( return true; } + SourceFile *File = D->getDeclContext()->getParentSourceFile(); if (auto importDecl = dyn_cast(D)) { - SourceFile *File = D->getDeclContext()->getParentSourceFile(); if (!D->getASTContext().LangOpts.hasFeature(Feature::AccessLevelOnImport) && File && File->Kind != SourceFileKind::Interface) { diagnoseAndRemoveAttr(attr, diag::access_level_on_import_not_enabled); @@ -1019,6 +1019,15 @@ bool AttributeChecker::visitAbstractAccessControlAttr( } } + if (attr->getAccess() == AccessLevel::Package && + D->getASTContext().LangOpts.PackageName.empty() && + File && File->Kind != SourceFileKind::Interface) { + // `package` modifier used outside of a package. + diagnose(attr->getLocation(), diag::access_control_requires_package_name, + isa(D), D); + return true; + } + return false; } diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index a893ef03308ba..ae389d5ef65a2 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -1965,14 +1965,6 @@ class DeclChecker : public DeclVisitor { target->getModuleFilename()); } - // Report use of package import when no package name is set. - if (ID->getAccessLevel() == AccessLevel::Package && - getASTContext().LangOpts.PackageName.empty()) { - auto &diags = ID->getASTContext().Diags; - diags.diagnose(ID->getLoc(), - diag::access_control_requires_package_name_import); - } - // Report the public import of a private module. if (ID->getASTContext().LangOpts.LibraryLevel == LibraryLevel::API) { auto importer = ID->getModuleContext(); diff --git a/test/Sema/package-import-no-package-name.swift b/test/Sema/package-import-no-package-name.swift deleted file mode 100644 index 34395d7243eba..0000000000000 --- a/test/Sema/package-import-no-package-name.swift +++ /dev/null @@ -1,17 +0,0 @@ -/// Report uses of package import without a package. - -// RUN: %empty-directory(%t) -// RUN: split-file --leading-lines %s %t - -// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t -// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \ -// RUN: -enable-experimental-feature AccessLevelOnImport -verify -// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \ -// RUN: -enable-experimental-feature AccessLevelOnImport \ -// RUN: -package-name pkg - -//--- PackageLib.swift -public struct PackageImportType {} - -//--- Client.swift -package import PackageLib // expected-error {{package import can only be used from a module with a package name; set it with the compiler flag -package-name}} diff --git a/test/attr/package-modifier-outside-of-package.swift b/test/attr/package-modifier-outside-of-package.swift new file mode 100644 index 0000000000000..1dff753bff212 --- /dev/null +++ b/test/attr/package-modifier-outside-of-package.swift @@ -0,0 +1,27 @@ +/// Report uses of the package access-level modifier without a package name. + +// RUN: %empty-directory(%t) +// RUN: split-file --leading-lines %s %t + +// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t +// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \ +// RUN: -enable-experimental-feature AccessLevelOnImport -verify +// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \ +// RUN: -enable-experimental-feature AccessLevelOnImport \ +// RUN: -package-name pkg + +//--- PackageLib.swift +public struct PackageImportType {} + +//--- Client.swift +package import PackageLib // expected-error {{the package access level requires a package name; set it with the compiler flag -package-name}} + +package struct PackageStruct { // expected-error {{the package access level used on 'PackageStruct' requires a package name; set it with the compiler flag -package-name}} + package func explicitlyPackage() {} // expected-error {{the package access level used on 'explicitlyPackage()' requires a package name; set it with the compiler flag -package-name}} +} + +public struct PublicStruct {} + +package extension PublicStruct { // expected-error {{the package access level requires a package name; set it with the compiler flag -package-name}} + func implicitlyPackage() {} +} diff --git a/test/diagnostics/package-name-diagnostics.swift b/test/diagnostics/package-name-diagnostics.swift index fc0f4e963033a..0d917cffa874e 100644 --- a/test/diagnostics/package-name-diagnostics.swift +++ b/test/diagnostics/package-name-diagnostics.swift @@ -3,11 +3,11 @@ // Package name should not be empty // RUN: not %target-swift-frontend -typecheck %s -package-name "" 2>&1 | %FileCheck %s -check-prefix CHECK-EMPTY // CHECK-EMPTY: error: package-name is empty -// CHECK-EMPTY: error: 'log' has a package access level but no -package-name was specified: {{.*}}.swift +// CHECK-EMPTY: error: the package access level used on 'log()' requires a package name; set it with the compiler flag -package-name // If package access level is used but no package-name is passed, it should error // RUN: not %target-swift-frontend -typecheck %s 2>&1 | %FileCheck %s -check-prefix CHECK-MISSING -// CHECK-MISSING: error: 'log' has a package access level but no -package-name was specified: {{.*}}.swift +// CHECK-MISSING: error: the package access level used on 'log()' requires a package name; set it with the compiler flag -package-name // Package name can be same as the module name // RUN: %target-swift-frontend -module-name Logging -package-name Logging %s -emit-module -emit-module-path %t/Logging.swiftmodule