From 49b594100f799dc1e51efd52336b8bf97dc4a8cf Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Thu, 29 May 2025 10:21:49 -0700 Subject: [PATCH 1/4] AST/Sema: Remove code adding `@_spi` to suggested import fix-its. The was never invoked because inaccessibility due to SPI protection level is always diagnosed before missing imports are diagnosed. The functionality could therefore not be tested and should be removed. --- lib/AST/DiagnosticEngine.cpp | 11 ----------- lib/Sema/TypeCheckNameLookup.cpp | 15 ++------------- .../Inputs/MemberImportVisibility/members_A.swift | 3 +++ .../Inputs/MemberImportVisibility/members_B.swift | 3 +++ .../Inputs/MemberImportVisibility/members_C.swift | 3 +++ test/NameLookup/member_import_visibility.swift | 3 +++ 6 files changed, 14 insertions(+), 24 deletions(-) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 1922efe1908c2..a33ef057f2876 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -361,17 +361,6 @@ InFlightDiagnostic &InFlightDiagnostic::fixItAddImport(StringRef ModuleName) { if (bestLoc.isValid()) { llvm::SmallString<64> importText; - - // @_spi imports. - if (Member->isSPI()) { - auto spiGroups = Member->getSPIGroups(); - if (!spiGroups.empty()) { - importText += "@_spi("; - importText += spiGroups[0].str(); - importText += ") "; - } - } - importText += "import "; importText += ModuleName; importText += "\n"; diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp index ae29222013ed4..a8fee7ab9ad3e 100644 --- a/lib/Sema/TypeCheckNameLookup.cpp +++ b/lib/Sema/TypeCheckNameLookup.cpp @@ -940,8 +940,7 @@ diagnoseMissingImportsForMember(const ValueDecl *decl, static void emitMissingImportFixIt(SourceLoc loc, const MissingImportFixItInfo &fixItInfo, - const ValueDecl *decl) { - ASTContext &ctx = decl->getASTContext(); + ASTContext &ctx) { llvm::SmallString<64> importText; // Add flags that must be used consistently on every import in every file. @@ -962,16 +961,6 @@ static void emitMissingImportFixIt(SourceLoc loc, importText += "@_spiOnly "; } - // Add @_spi groups if needed for the declaration. - if (decl->isSPI()) { - auto spiGroups = decl->getSPIGroups(); - if (!spiGroups.empty()) { - importText += "@_spi("; - importText += spiGroups[0].str(); - importText += ") "; - } - } - if (explicitAccessLevel) { importText += getAccessLevelSpelling(*explicitAccessLevel); importText += " "; @@ -1006,7 +995,7 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf, return; for (auto &fixItInfo : fixItInfos) { - emitMissingImportFixIt(bestLoc, fixItInfo, decl); + emitMissingImportFixIt(bestLoc, fixItInfo, ctx); } } diff --git a/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift b/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift index 47cca731c187f..ccbaa997ceac9 100644 --- a/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift +++ b/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift @@ -17,6 +17,9 @@ infix operator <> extension X { public func XinA() { } + @_spi(A) + public func XinA_spi() { } + public var propXinA: Bool { return true } public static func <<<(a: Self, b: Self) -> Self { a } diff --git a/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift b/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift index 8820424fed07b..3b638b2641c5c 100644 --- a/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift +++ b/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift @@ -5,6 +5,9 @@ extension X { public func XinB() { } package func XinB_package() { } + @_spi(B) + public func XinB_spi() { } + public var propXinB: Bool { return true } package var propXinB_package: Bool { return true } diff --git a/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift b/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift index ac233b11f7864..a6784d7254d30 100644 --- a/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift +++ b/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift @@ -5,6 +5,9 @@ import members_B extension X { public func XinC() { } + @_spi(C) + public func XinC_spi() { } + public var propXinC: Bool { return true } public static func <>(a: Self, b: Self) -> Self { a } diff --git a/test/NameLookup/member_import_visibility.swift b/test/NameLookup/member_import_visibility.swift index d9562a7cc7c19..38f6b5dd4683e 100644 --- a/test/NameLookup/member_import_visibility.swift +++ b/test/NameLookup/member_import_visibility.swift @@ -14,13 +14,16 @@ import members_C func testExtensionMembers(x: X, y: Y) { x.XinA() + x.XinA_spi() // expected-error{{'XinA_spi' is inaccessible due to '@_spi' protection level}} y.YinA() x.XinB() // expected-member-visibility-error{{instance method 'XinB()' is not available due to missing import of defining module 'members_B'}} x.XinB_package() // expected-member-visibility-error{{instance method 'XinB_package()' is not available due to missing import of defining module 'members_B'}} + x.XinB_spi() // expected-error{{'XinB_spi' is inaccessible due to '@_spi' protection level}} y.YinB() // expected-member-visibility-error{{instance method 'YinB()' is not available due to missing import of defining module 'members_B'}} x.XinC() + x.XinC_spi() // expected-error{{'XinC_spi' is inaccessible due to '@_spi' protection level}} y.YinC() _ = X(true) From f6124cbe13c68b9ad29c421e95aa9dd766712ee9 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Thu, 29 May 2025 10:22:48 -0700 Subject: [PATCH 2/4] AST: Simplify the interface of `DiagnosticEngine::getBestAddImportFixItLoc()`. For clarity, it should just take a `SourceFile`. --- include/swift/AST/DiagnosticEngine.h | 9 +------- lib/AST/DiagnosticEngine.cpp | 32 ++++++++++++---------------- lib/Sema/TypeCheckNameLookup.cpp | 2 +- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index 41e8b666f4c9f..3b1373f8e545a 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/include/swift/AST/DiagnosticEngine.h @@ -852,9 +852,6 @@ namespace swift { static const char *fixItStringFor(const FixItID id); - /// Get the best location where an 'import' fixit might be offered. - SourceLoc getBestAddImportFixItLoc(const Decl *Member) const; - /// Add a token-based replacement fix-it to the currently-active /// diagnostic. template @@ -1572,11 +1569,7 @@ namespace swift { SourceLoc getDefaultDiagnosticLoc() const { return bufferIndirectlyCausingDiagnostic; } - SourceLoc getBestAddImportFixItLoc(const Decl *Member, - SourceFile *sourceFile) const; - SourceLoc getBestAddImportFixItLoc(const Decl *Member) const { - return getBestAddImportFixItLoc(Member, nullptr); - } + SourceLoc getBestAddImportFixItLoc(SourceFile *sf) const; }; inline SourceManager &InFlightDiagnostic::getSourceManager() { diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index a33ef057f2876..fda0e9cc05f06 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -313,18 +313,12 @@ InFlightDiagnostic::fixItReplaceChars(SourceLoc Start, SourceLoc End, return *this; } -SourceLoc -DiagnosticEngine::getBestAddImportFixItLoc(const Decl *Member, - SourceFile *sourceFile) const { +SourceLoc DiagnosticEngine::getBestAddImportFixItLoc(SourceFile *SF) const { auto &SM = SourceMgr; SourceLoc bestLoc; - - auto SF = - sourceFile ? sourceFile : Member->getDeclContext()->getParentSourceFile(); - if (!SF) { + if (!SF) return bestLoc; - } for (auto item : SF->getTopLevelItems()) { // If we found an import declaration, we want to insert after it. @@ -356,19 +350,21 @@ DiagnosticEngine::getBestAddImportFixItLoc(const Decl *Member, InFlightDiagnostic &InFlightDiagnostic::fixItAddImport(StringRef ModuleName) { assert(IsActive && "Cannot modify an inactive diagnostic"); - auto Member = Engine->ActiveDiagnostic->getDecl(); - SourceLoc bestLoc = Engine->getBestAddImportFixItLoc(Member); + auto decl = Engine->ActiveDiagnostic->getDecl(); + if (!decl) + return *this; - if (bestLoc.isValid()) { - llvm::SmallString<64> importText; - importText += "import "; - importText += ModuleName; - importText += "\n"; + auto bestLoc = Engine->getBestAddImportFixItLoc( + decl->getDeclContext()->getOutermostParentSourceFile()); + if (!bestLoc.isValid()) + return *this; - return fixItInsert(bestLoc, importText); - } + llvm::SmallString<64> importText; + importText += "import "; + importText += ModuleName; + importText += "\n"; - return *this; + return fixItInsert(bestLoc, importText); } InFlightDiagnostic & diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp index a8fee7ab9ad3e..eb8a28b03f775 100644 --- a/lib/Sema/TypeCheckNameLookup.cpp +++ b/lib/Sema/TypeCheckNameLookup.cpp @@ -990,7 +990,7 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf, diagnoseMissingImportsForMember(decl, modulesToImport, sf, loc); auto &ctx = sf->getASTContext(); - SourceLoc bestLoc = ctx.Diags.getBestAddImportFixItLoc(decl, sf); + SourceLoc bestLoc = ctx.Diags.getBestAddImportFixItLoc(sf); if (!bestLoc.isValid()) return; From 0d6a8c8c4358d762a64722891b39b615e75e354a Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Fri, 23 May 2025 11:36:22 -0700 Subject: [PATCH 3/4] Basic: Fix a bug in LangOptions::hasFeature(). Don't skip checking if a feature is enabled for migration when the feature also has an associated language version. --- lib/Basic/LangOptions.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Basic/LangOptions.cpp b/lib/Basic/LangOptions.cpp index 76e63fd9afe4f..5b04e031b3dbd 100644 --- a/lib/Basic/LangOptions.cpp +++ b/lib/Basic/LangOptions.cpp @@ -340,8 +340,10 @@ bool LangOptions::hasFeature(Feature feature, bool allowMigration) const { if (state.isEnabled()) return true; - if (auto version = feature.getLanguageVersion()) - return isSwiftVersionAtLeast(*version); + if (auto version = feature.getLanguageVersion()) { + if (isSwiftVersionAtLeast(*version)) + return true; + } if (allowMigration && state.isEnabledForMigration()) return true; From c99b19e45093dcf38abd5dd4772d7de0fe336792 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Fri, 23 May 2025 11:39:18 -0700 Subject: [PATCH 4/4] AST/Sema: Make MemberImportVisibility a migratable feature. The migration to `MemberImportVisibility` can be performed mechanically by adding missing import declarations, so offer automatic migration for the feature. Resolves rdar://151931597. --- include/swift/AST/DiagnosticGroups.def | 1 + include/swift/AST/DiagnosticsSema.def | 9 +- include/swift/Basic/Features.def | 2 +- include/swift/Basic/LangOptions.h | 6 +- lib/AST/NameLookup.cpp | 7 +- lib/Basic/LangOptions.cpp | 4 + lib/Basic/SupportedFeatures.cpp | 2 + lib/Sema/MiscDiagnostics.cpp | 3 +- lib/Sema/ResilienceDiagnostics.cpp | 4 +- lib/Sema/TypeCheckDeclOverride.cpp | 6 +- lib/Sema/TypeCheckNameLookup.cpp | 109 ++++++++++-- lib/Sema/TypeCheckType.cpp | 7 +- lib/Sema/TypeOfReference.cpp | 3 +- .../member_import_visibility_migrate.swift | 162 ++++++++++++++++++ .../diagnostics/member-import-visibility.md | 15 ++ 15 files changed, 310 insertions(+), 30 deletions(-) create mode 100644 test/NameLookup/member_import_visibility_migrate.swift create mode 100644 userdocs/diagnostics/member-import-visibility.md diff --git a/include/swift/AST/DiagnosticGroups.def b/include/swift/AST/DiagnosticGroups.def index c4607c53ccfe7..e490185cf30fd 100644 --- a/include/swift/AST/DiagnosticGroups.def +++ b/include/swift/AST/DiagnosticGroups.def @@ -49,6 +49,7 @@ GROUP(ErrorInFutureSwiftVersion, "error-in-future-swift-version") GROUP(ExistentialAny, "existential-any") GROUP(ExistentialMemberAccess, "existential-member-access-limitations") GROUP(IsolatedConformances, "isolated-conformances") +GROUP(MemberImportVisibility, "member-import-visibility") GROUP(MultipleInheritance, "multiple-inheritance") GROUP(MutableGlobalVariable, "mutable-global-variable") GROUP(NominalTypes, "nominal-types") diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 0a3e59c5d6d85..8378f016c2607 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -165,16 +165,21 @@ ERROR(init_candidate_inaccessible,none, "'%select{private|fileprivate|internal|package|@_spi|@_spi}1' protection level", (Type, AccessLevel)) -ERROR(candidate_from_missing_import,none, +GROUPED_ERROR(member_from_missing_import,MemberImportVisibility,none, "%kind0 is not available due to missing import of defining module %1", (const ValueDecl *, const ModuleDecl *)) -ERROR(candidate_from_missing_imports_2_or_more,none, +GROUPED_ERROR(member_from_missing_imports_2_or_more,MemberImportVisibility,none, "%kind0 is not available due to missing imports of defining modules " "%2%select{ and|, }1 %3%select{|, and others}1", (const ValueDecl *, bool, const ModuleDecl *, const ModuleDecl *)) NOTE(candidate_add_import,none, "add import of module %0", (const ModuleDecl *)) +GROUPED_WARNING(add_required_import_for_member,MemberImportVisibility,none, + "import of module %0 is required", (const ModuleDecl *)) +NOTE(decl_from_module_used_here,none, + "%kind0 from %1 used here", (const ValueDecl *, const ModuleDecl *)) + ERROR(cannot_pass_rvalue_mutating_subelement,none, "cannot use mutating member on immutable value: %0", (StringRef)) diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index 72dbdf6091547..cf8c7fd531204 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -287,7 +287,7 @@ UPCOMING_FEATURE(GlobalActorIsolatedTypesUsability, 0434, 6) // Swift 7 MIGRATABLE_UPCOMING_FEATURE(ExistentialAny, 335, 7) UPCOMING_FEATURE(InternalImportsByDefault, 409, 7) -UPCOMING_FEATURE(MemberImportVisibility, 444, 7) +MIGRATABLE_UPCOMING_FEATURE(MemberImportVisibility, 444, 7) MIGRATABLE_UPCOMING_FEATURE(InferIsolatedConformances, 470, 7) MIGRATABLE_UPCOMING_FEATURE(NonisolatedNonsendingByDefault, 461, 7) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index 66b79d4636457..ac1fa635bb860 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -873,13 +873,17 @@ namespace swift { /// Returns whether the given feature is enabled. /// - /// If allowMigration is set, also returns true when the feature has been enabled for migration. + /// If allowMigration is set, also returns true when the feature has been + /// enabled for migration. bool hasFeature(Feature feature, bool allowMigration = false) const; /// Returns whether a feature with the given name is enabled. Returns /// `false` if a feature by this name is not known. bool hasFeature(llvm::StringRef featureName) const; + /// Returns whether the given feature is enabled for migration. + bool isMigratingToFeature(Feature feature) const; + /// Enables the given feature (enables in migration mode if `forMigration` /// is `true`). void enableFeature(Feature feature, bool forMigration = false); diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index 0b24e1f2210d8..4163531b4f97b 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -2371,7 +2371,9 @@ ObjCCategoryNameMap ClassDecl::getObjCCategoryNameMap() { /// the given context. static bool shouldRequireImportsInContext(const DeclContext *lookupContext) { auto &ctx = lookupContext->getASTContext(); - if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) + + if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) return false; // Code outside of the main module (which is often synthesized) isn't subject @@ -3591,7 +3593,8 @@ ExtendedNominalRequest::evaluate(Evaluator &evaluator, // inaccessible due to missing imports. The missing imports will be diagnosed // elsewhere. if (referenced.first.empty() && - ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) { + ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) { options |= DirectlyReferencedTypeLookupFlags::IgnoreMissingImports; referenced = directReferencesForTypeRepr(evaluator, ctx, typeRepr, ext->getParent(), options); diff --git a/lib/Basic/LangOptions.cpp b/lib/Basic/LangOptions.cpp index 5b04e031b3dbd..c2cf55af4d8dd 100644 --- a/lib/Basic/LangOptions.cpp +++ b/lib/Basic/LangOptions.cpp @@ -363,6 +363,10 @@ bool LangOptions::hasFeature(llvm::StringRef featureName) const { return false; } +bool LangOptions::isMigratingToFeature(Feature feature) const { + return featureStates.getState(feature).isEnabledForMigration(); +} + void LangOptions::enableFeature(Feature feature, bool forMigration) { if (forMigration) { ASSERT(feature.isMigratable()); diff --git a/lib/Basic/SupportedFeatures.cpp b/lib/Basic/SupportedFeatures.cpp index 19410bce4374e..48ab019b7c718 100644 --- a/lib/Basic/SupportedFeatures.cpp +++ b/lib/Basic/SupportedFeatures.cpp @@ -35,6 +35,8 @@ static std::vector migratableCategories(Feature feature) { return { DiagGroupID::ExistentialAny }; case Feature::InnerKind::InferIsolatedConformances: return { DiagGroupID::IsolatedConformances }; + case Feature::InnerKind::MemberImportVisibility: + return { DiagGroupID::MemberImportVisibility }; case Feature::InnerKind::NonisolatedNonsendingByDefault: return { DiagGroupID::NonisolatedNonsendingByDefault }; case Feature::InnerKind::StrictMemorySafety: diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 906da0cd42264..3b19e2bb418e1 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -6326,7 +6326,8 @@ static void diagnoseMissingMemberImports(const Expr *E, const DeclContext *DC) { }; auto &ctx = DC->getASTContext(); - if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) + if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) return; DiagnoseWalker walker(DC); diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index fa6decc40b4d5..507e2645ad896 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -279,7 +279,9 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D, // Some diagnostics emitted with the `MemberImportVisibility` feature enabled // subsume these diagnostics. if (originKind == DisallowedOriginKind::MissingImport && - ctx.LangOpts.hasFeature(Feature::MemberImportVisibility) && SF) + ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true) && + SF) return false; if (auto accessor = dyn_cast(D)) { diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index c98498c9b9348..e4bce4488a7f7 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1537,7 +1537,8 @@ bool swift::checkOverrides(ValueDecl *decl) { auto &ctx = decl->getASTContext(); if (overridden.empty() && - ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) { + ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) { // If we didn't find anything, try broadening the search by ignoring missing // imports. if (!checkPotentialOverrides(decl, overridden, @@ -2533,7 +2534,8 @@ OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const { // If we didn't find anything, try broadening the search by ignoring missing // imports. if (overridden.empty() && - ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) { + ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) { overridden = computeOverriddenDecls(decl, true); if (!overridden.empty()) { auto first = overridden.front(); diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp index eb8a28b03f775..a4faf527d2f2b 100644 --- a/lib/Sema/TypeCheckNameLookup.cpp +++ b/lib/Sema/TypeCheckNameLookup.cpp @@ -852,6 +852,12 @@ class MissingImportFixItCache { llvm::DenseMap infos; bool internalImportsByDefaultEnabled; +public: + MissingImportFixItCache(SourceFile &sf) + : sf(sf), + internalImportsByDefaultEnabled(sf.getASTContext().LangOpts.hasFeature( + Feature::InternalImportsByDefault)) {}; + MissingImportFixItInfo getFixItInfo(ModuleDecl *mod) { auto existing = infos.find(mod); if (existing != infos.end()) @@ -900,12 +906,6 @@ class MissingImportFixItCache { return info; } -public: - MissingImportFixItCache(SourceFile &sf) : sf(sf) { - internalImportsByDefaultEnabled = sf.getASTContext().LangOpts.hasFeature( - Feature::InternalImportsByDefault); - }; - std::pair, SmallVector> getModulesAndFixIts(ModuleDecl *mod) { @@ -929,20 +929,17 @@ diagnoseMissingImportsForMember(const ValueDecl *decl, ASSERT(count > 0); if (count > 1) { - ctx.Diags.diagnose(loc, diag::candidate_from_missing_imports_2_or_more, - decl, bool(count > 2), modulesToImport[0], - modulesToImport[1]); + ctx.Diags.diagnose(loc, diag::member_from_missing_imports_2_or_more, decl, + bool(count > 2), modulesToImport[0], modulesToImport[1]); } else { - ctx.Diags.diagnose(loc, diag::candidate_from_missing_import, decl, + ctx.Diags.diagnose(loc, diag::member_from_missing_import, decl, modulesToImport.front()); } } -static void emitMissingImportFixIt(SourceLoc loc, - const MissingImportFixItInfo &fixItInfo, - ASTContext &ctx) { - llvm::SmallString<64> importText; - +static void appendMissingImportFixIt(llvm::SmallString<64> &importText, + const MissingImportFixItInfo &fixItInfo, + ASTContext &ctx) { // Add flags that must be used consistently on every import in every file. if (fixItInfo.flags.contains(ImportFlags::ImplementationOnly)) importText += "@_implementationOnly "; @@ -969,6 +966,12 @@ static void emitMissingImportFixIt(SourceLoc loc, importText += "import "; importText += fixItInfo.moduleToImport->getName().str(); importText += "\n"; +} + +static void emitMissingImportNoteAndFixIt( + SourceLoc loc, const MissingImportFixItInfo &fixItInfo, ASTContext &ctx) { + llvm::SmallString<64> importText; + appendMissingImportFixIt(importText, fixItInfo, ctx); ctx.Diags .diagnose(loc, diag::candidate_add_import, fixItInfo.moduleToImport) .fixItInsert(loc, importText); @@ -995,7 +998,7 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf, return; for (auto &fixItInfo : fixItInfos) { - emitMissingImportFixIt(bestLoc, fixItInfo, ctx); + emitMissingImportNoteAndFixIt(bestLoc, fixItInfo, ctx); } } @@ -1024,6 +1027,11 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl, // In lazy typechecking mode just emit the diagnostic immediately without a // fix-it since there won't be an opportunity to emit delayed diagnostics. if (ctx.TypeCheckerOpts.EnableLazyTypecheck) { + // Lazy type-checking and migration for MemberImportVisibility are + // completely incompatible, so just skip the diagnostic entirely. + if (ctx.LangOpts.isMigratingToFeature(Feature::MemberImportVisibility)) + return false; + auto modulesToImport = missingImportsForDefiningModule(definingModule, *sf); if (modulesToImport.empty()) return false; @@ -1036,7 +1044,76 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl, return false; } +void migrateToMemberImportVisibility(SourceFile &sf) { + auto delayedDiags = sf.takeDelayedMissingImportForMemberDiagnostics(); + if (delayedDiags.empty()) + return; + + auto &ctx = sf.getASTContext(); + auto bestLoc = ctx.Diags.getBestAddImportFixItLoc(&sf); + if (bestLoc.isInvalid()) + return; + + // Collect the distinct modules that need to be imported and map them + // to the collection of declarations which are used in the file and belong + // to the module. + llvm::SmallVector modulesToImport; + llvm::SmallDenseMap> + declsByModuleToImport; + for (auto declAndLocs : delayedDiags) { + auto decl = declAndLocs.first; + auto definingModules = missingImportsForDefiningModule( + decl->getModuleContextForNameLookup(), sf); + + for (auto definingModule : definingModules) { + auto existing = declsByModuleToImport.find(definingModule); + if (existing != declsByModuleToImport.end()) { + existing->second.push_back(decl); + } else { + declsByModuleToImport[definingModule] = {decl}; + modulesToImport.push_back(definingModule); + } + } + } + + // Emit one warning for each module that needcs to be imported and emit notes + // for each reference to a declaration from that module in the file. + llvm::sort(modulesToImport, [](ModuleDecl *lhs, ModuleDecl *rhs) -> int { + return lhs->getName().compare(rhs->getName()); + }); + + auto fixItCache = MissingImportFixItCache(sf); + for (auto mod : modulesToImport) { + auto fixItInfo = fixItCache.getFixItInfo(mod); + llvm::SmallString<64> importText; + appendMissingImportFixIt(importText, fixItInfo, ctx); + ctx.Diags.diagnose(bestLoc, diag::add_required_import_for_member, mod) + .fixItInsert(bestLoc, importText); + + auto decls = declsByModuleToImport.find(mod); + if (decls == declsByModuleToImport.end()) + continue; + + for (auto decl : decls->second) { + auto locs = delayedDiags.find(decl); + if (locs == delayedDiags.end()) + continue; + + for (auto loc : locs->second) { + ctx.Diags.diagnose(loc, diag::decl_from_module_used_here, decl, mod); + } + } + } +} + void swift::diagnoseMissingImports(SourceFile &sf) { + // Missing import diagnostics should be emitted differently in "migrate" mode. + if (sf.getASTContext().LangOpts.isMigratingToFeature( + Feature::MemberImportVisibility)) { + migrateToMemberImportVisibility(sf); + return; + } + auto delayedDiags = sf.takeDelayedMissingImportForMemberDiagnostics(); auto fixItCache = MissingImportFixItCache(sf); diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index a8829e4a63d04..286dd884a366c 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -1783,7 +1783,8 @@ resolveUnqualifiedIdentTypeRepr(const TypeResolution &resolution, // If the look up did not yield any results, try again but allow members from // modules that are not directly imported to be accessible. bool didIgnoreMissingImports = false; - if (!globals && ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) { + if (!globals && ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) { lookupOptions |= NameLookupFlags::IgnoreMissingImports; globals = TypeChecker::lookupUnqualifiedType(DC, id, repr->getLoc(), lookupOptions); @@ -2046,8 +2047,8 @@ static Type resolveQualifiedIdentTypeRepr(const TypeResolution &resolution, DC, parentTy, repr->getNameRef(), repr->getLoc(), lookupOptions); // If no members were found, try ignoring missing imports. - if (!memberTypes && - ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) { + if (!memberTypes && ctx.LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) { lookupOptions |= NameLookupFlags::IgnoreMissingImports; memberTypes = TypeChecker::lookupMemberType( DC, parentTy, repr->getNameRef(), repr->getLoc(), lookupOptions); diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index ef2606e8b93fb..376614b94898a 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -2698,7 +2698,8 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator, increaseScore(SK_Unavailable, locator); // If the declaration is from a module that hasn't been imported, note that. - if (getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility)) { + if (getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility, + /*allowMigration=*/true)) { if (!useDC->isDeclImported(decl)) increaseScore(SK_MissingImport, locator); } diff --git a/test/NameLookup/member_import_visibility_migrate.swift b/test/NameLookup/member_import_visibility_migrate.swift new file mode 100644 index 0000000000000..e6b5b7e11de2e --- /dev/null +++ b/test/NameLookup/member_import_visibility_migrate.swift @@ -0,0 +1,162 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnly.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyDefaultedImport.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/PackageUsesOnly.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/PublicUsesOnly.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/PublicUsesOnlyDefaultedImport.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/MixedUses.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyReexported.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyTransitivelyImported.swift +// RUN: %target-swift-frontend -emit-module -o %t %t/ImportsOtherModules.swift -I %t +// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlySPIOnly.swift -I %t +// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyDefaultedImportSPIOnly.swift -I %t +// RUN: %target-swift-frontend -emit-module -o %t %t/PublicUsesOnlySPIOnly.swift -I %t + +// RUN: %target-swift-frontend -typecheck -verify -swift-version 5 \ +// RUN: -primary-file %t/main.swift \ +// RUN: %t/imports.swift \ +// RUN: -I %t -package-name Package \ +// RUN: -enable-upcoming-feature MemberImportVisibility:migrate + +// REQUIRES: swift_feature_MemberImportVisibility + +//--- main.swift + +// FIXME: The access level on the fix-it for PackageUsesOnly is wrong. +import Swift // Just here to anchor the fix-its +// expected-warning {{import of module 'InternalUsesOnly' is required}}{{1-1=internal import InternalUsesOnly\n}} +// expected-warning@-1 {{import of module 'InternalUsesOnlyDefaultedImport' is required}}{{1-1=import InternalUsesOnlyDefaultedImport\n}} +// expected-warning@-2 {{import of module 'PackageUsesOnly' is required}}{{1-1=public import PackageUsesOnly\n}} +// expected-warning@-3 {{import of module 'PublicUsesOnly' is required}}{{1-1=public import PublicUsesOnly\n}} +// expected-warning@-4 {{import of module 'PublicUsesOnlyDefaultedImport' is required}}{{1-1=import PublicUsesOnlyDefaultedImport\n}} +// expected-warning@-5 {{import of module 'MixedUses' is required}}{{1-1=public import MixedUses\n}} +// expected-warning@-6 {{import of module 'InternalUsesOnlyReexported' is required}}{{1-1=internal import InternalUsesOnlyReexported\n}} +// expected-warning@-7 {{import of module 'InternalUsesOnlyTransitivelyImported' is required}}{{1-1=internal import InternalUsesOnlyTransitivelyImported\n}} +// expected-warning@-8 {{import of module 'InternalUsesOnlySPIOnly' is required}}{{1-1=internal import InternalUsesOnlySPIOnly\n}} +// expected-warning@-9 {{import of module 'InternalUsesOnlyDefaultedImportSPIOnly' is required}}{{1-1=@_spiOnly import InternalUsesOnlyDefaultedImportSPIOnly\n}} +// expected-warning@-10 {{import of module 'PublicUsesOnlySPIOnly' is required}}{{1-1=@_spiOnly public import PublicUsesOnlySPIOnly\n}} + +func internalFunc(_ x: Int) { + _ = x.memberInInternalUsesOnly // expected-note {{property 'memberInInternalUsesOnly' from 'InternalUsesOnly' used here}} + _ = x.memberInInternalUsesOnlyDefaultedImport // expected-note {{property 'memberInInternalUsesOnlyDefaultedImport' from 'InternalUsesOnlyDefaultedImport' used here}} + _ = x.memberInMixedUses // expected-note {{property 'memberInMixedUses' from 'MixedUses' used here}} + _ = x.memberInInternalUsesOnlyReexported // expected-note {{property 'memberInInternalUsesOnlyReexported' from 'InternalUsesOnlyReexported' used here}} + _ = x.memberInInternalUsesOnlySPIOnly // expected-note {{property 'memberInInternalUsesOnlySPIOnly' from 'InternalUsesOnlySPIOnly' used here}} + _ = x.memberInInternalUsesOnlyDefaultedImportSPIOnly // expected-note {{property 'memberInInternalUsesOnlyDefaultedImportSPIOnly' from 'InternalUsesOnlyDefaultedImportSPIOnly' used here}} + _ = x.memberInInternalUsesOnlyTransitivelyImported // expected-note {{property 'memberInInternalUsesOnlyTransitivelyImported' from 'InternalUsesOnlyTransitivelyImported' used here}} +} + +@inlinable package func packageInlinableFunc(_ x: Int) { + _ = x.memberInPackageUsesOnly // expected-note {{property 'memberInPackageUsesOnly' from 'PackageUsesOnly' used here}} + _ = x.memberInMixedUses // expected-note {{property 'memberInMixedUses' from 'MixedUses' used here}} +} + +@inlinable public func inlinableFunc(_ x: Int) { + _ = x.memberInPublicUsesOnly // expected-note {{property 'memberInPublicUsesOnly' from 'PublicUsesOnly' used here}} + _ = x.memberInPublicUsesOnlyDefaultedImport // expected-note {{property 'memberInPublicUsesOnlyDefaultedImport' from 'PublicUsesOnlyDefaultedImport' used here}} + _ = x.memberInMixedUses // expected-note {{property 'memberInMixedUses' from 'MixedUses' used here}} + _ = x.memberInPublicUsesOnlySPIOnly // expected-note {{property 'memberInPublicUsesOnlySPIOnly' from 'PublicUsesOnlySPIOnly' used here}} +} + +extension Int { + private func usesTypealiasInInternalUsesOnly_Private(x: TypealiasInInternalUsesOnly) {} // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}} + internal func usesTypealiasInInternalUsesOnly(x: TypealiasInInternalUsesOnly) {} // expected-note {{type alias 'TypealiasInInternalUsesOnly' from 'InternalUsesOnly' used here}} + package func usesTypealiasInPackageUsesOnly(x: TypealiasInPackageUsesOnly) {} // expected-note {{type alias 'TypealiasInPackageUsesOnly' from 'PackageUsesOnly' used here}} + public func usesTypealiasInPublicUsesOnly(x: TypealiasInPublicUsesOnly) {} // expected-note {{type alias 'TypealiasInPublicUsesOnly' from 'PublicUsesOnly' used here}} + public func usesTypealiasInMixedUses(x: TypealiasInMixedUses) {} // expected-note {{type alias 'TypealiasInMixedUses' from 'MixedUses' used here}} + internal func usesTypealiasInMixedUses_Internal(x: TypealiasInMixedUses) {} // expected-note {{type alias 'TypealiasInMixedUses' from 'MixedUses' used here}} +} + +//--- imports.swift + +internal import InternalUsesOnly +import InternalUsesOnlyDefaultedImport +internal import PackageUsesOnly +internal import PublicUsesOnly +import PublicUsesOnlyDefaultedImport +internal import MixedUses +internal import ImportsOtherModules +@_spiOnly public import InternalUsesOnlySPIOnly +@_spiOnly import InternalUsesOnlyDefaultedImportSPIOnly +@_spiOnly public import PublicUsesOnlySPIOnly + +//--- InternalUsesOnly.swift + +extension Int { + public typealias TypealiasInInternalUsesOnly = Self + public var memberInInternalUsesOnly: Int { return self } +} + +//--- InternalUsesOnlyDefaultedImport.swift + +extension Int { + public typealias TypealiasInInternalUsesOnlyDefaultedImport = Self + public var memberInInternalUsesOnlyDefaultedImport: Int { return self } +} + +//--- PackageUsesOnly.swift + +extension Int { + public typealias TypealiasInPackageUsesOnly = Self + public var memberInPackageUsesOnly: Int { return self } +} + +//--- PublicUsesOnly.swift + +extension Int { + public typealias TypealiasInPublicUsesOnly = Self + public var memberInPublicUsesOnly: Int { return self } +} + +//--- PublicUsesOnlyDefaultedImport.swift + +extension Int { + public typealias TypealiasInPublicUsesOnlyDefaultedImport = Self + public var memberInPublicUsesOnlyDefaultedImport: Int { return self } +} + +//--- MixedUses.swift + +extension Int { + public typealias TypealiasInMixedUses = Self + public var memberInMixedUses: Int { return self } +} + +//--- InternalUsesOnlyReexported.swift + +extension Int { + public typealias TypealiasInInternalUsesOnlyReexported = Self + public var memberInInternalUsesOnlyReexported: Int { return self } +} + +//--- InternalUsesOnlyTransitivelyImported.swift + +extension Int { + public typealias TypealiasInInternalUsesOnlyTransitivelyImported = Self + public var memberInInternalUsesOnlyTransitivelyImported: Int { return self } +} + +//--- ImportsOtherModules.swift + +@_exported import InternalUsesOnlyReexported +import InternalUsesOnlyTransitivelyImported + +//--- InternalUsesOnlySPIOnly.swift + +extension Int { + public var memberInInternalUsesOnlySPIOnly: Int { return self } +} + +//--- InternalUsesOnlyDefaultedImportSPIOnly.swift + +extension Int { + public var memberInInternalUsesOnlyDefaultedImportSPIOnly: Int { return self } +} + +//--- PublicUsesOnlySPIOnly.swift + +extension Int { + public var memberInPublicUsesOnlySPIOnly: Int { return self } +} diff --git a/userdocs/diagnostics/member-import-visibility.md b/userdocs/diagnostics/member-import-visibility.md new file mode 100644 index 0000000000000..e22d04af51119 --- /dev/null +++ b/userdocs/diagnostics/member-import-visibility.md @@ -0,0 +1,15 @@ +# Member import visibility + + +This diagnostic group includes the errors that are emitted when a member declaration cannot be accessed because the module defining that member has not been directly imported by the file containing the reference: + +``` +func getFileContents() throws -> String { + // error: initializer 'init(contentsOfFile:)' is not available due to missing import of defining module 'Foundation' + return try String(contentsOfFile: "example.txt") +} +``` + +To resolve the error, you must add an import of the module that defines the member. + +These errors are only emitted when the `MemberImportVisibility` language mode is enabled.