[Sema] Merge diagnostics about package modifier outside of a package

Show the same error for both decls and imports when using the package
access level and no package name is set. Also brings up this check to
run once on decls and avoid repeated diagnostics.

Author: xymus

include/swift/AST/DiagnosticsSema.def

@@ -1935,10 +1935,7 @@ 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; "
+      "the package access level requires a package name; "
       "set it with the compiler flag -package-name",
       ())
 ERROR(invalid_decl_attribute,none,

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);

lib/Sema/TypeCheckAttr.cpp

@@ -996,8 +996,8 @@ bool AttributeChecker::visitAbstractAccessControlAttr(
     return true;
   }
 
+  SourceFile *File = D->getDeclContext()->getParentSourceFile();
   if (auto importDecl = dyn_cast<ImportDecl>(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,14 @@ 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);
+    return true;
+  }
+
   return false;
 }
 

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -1965,14 +1965,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
                      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();

test/Sema/package-import-no-package-name.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 requires a package name; set it with the compiler flag -package-name}}
+  package func explicitlyPackage() {} // expected-error {{the package access level 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() {}
+}

test/attr/package-modifier-outside-of-package.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 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 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