Skip to content

[Sema] Improve the note on imports with an access-level when it causes an error #64243

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
Mar 9, 2023
Merged

[Sema] Improve the note on imports with an access-level when it causes an error #64243

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
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,11 @@ ERROR(access_level_conflict_with_exported,none,
NOTE(module_imported_here,none,
"module %0 imported as '%select{private|fileprivate|internal|package|%ERROR|%ERROR}1' here",
(Identifier, AccessLevel))
NOTE(decl_import_via_here,none,
"%0 %1 imported as "
"'%select{private|fileprivate|internal|package|%ERROR|%ERROR}2' "
"from %3 here",
(DescriptiveDeclKind, DeclName, AccessLevel, Identifier))

// Opaque return types
ERROR(opaque_type_invalid_constraint,none,
Expand Down
7 changes: 4 additions & 3 deletions lib/Sema/ResilienceDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,10 @@ bool TypeChecker::diagnoseInlinableDeclRefAccess(SourceLoc loc,
if (problematicImport.has_value() &&
diagAccessLevel == importAccessLevel) {
Context.Diags.diagnose(problematicImport->accessLevelLoc,
diag::module_imported_here,
problematicImport->module.importedModule->getName(),
problematicImport->accessLevel);
diag::decl_import_via_here,
D->getDescriptiveKind(), diagName,
problematicImport->accessLevel,
problematicImport->module.importedModule->getName());
}

return (downgradeToWarning == DowngradeToWarning::No);
Expand Down
106 changes: 63 additions & 43 deletions lib/Sema/TypeCheckAccess.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,15 +343,26 @@ static void highlightOffendingType(InFlightDiagnostic &diag,
/// Emit a note on \p limitImport when it restricted the access level
/// of a type.
static void noteLimitingImport(ASTContext &ctx,
const ImportAccessLevel limitImport) {
const ImportAccessLevel limitImport,
const TypeRepr *complainRepr) {
if (!limitImport.has_value())
return;

assert(limitImport->accessLevel != AccessLevel::Public &&
"a public import shouldn't limit the access level of a decl");
ctx.Diags.diagnose(limitImport->accessLevelLoc, diag::module_imported_here,
limitImport->module.importedModule->getName(),
limitImport->accessLevel);

if (auto ITR = dyn_cast_or_null<IdentTypeRepr>(complainRepr)) {
const ValueDecl *VD = ITR->getBoundDecl();
ctx.Diags.diagnose(limitImport->accessLevelLoc, diag::decl_import_via_here,
DescriptiveDeclKind::Type,
VD->getName(),
limitImport->accessLevel,
limitImport->module.importedModule->getName());
} else {
ctx.Diags.diagnose(limitImport->accessLevelLoc, diag::module_imported_here,
limitImport->module.importedModule->getName(),
limitImport->accessLevel);
}
}

void AccessControlCheckerBase::checkGenericParamAccess(
Expand Down Expand Up @@ -435,7 +446,7 @@ void AccessControlCheckerBase::checkGenericParamAccess(
Context.Diags.diagnose(ownerDecl, diagID, ownerDecl->getDescriptiveKind(),
accessControlErrorKind == ACEK::Requirement);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(Context, minImportLimit);
noteLimitingImport(Context, minImportLimit, complainRepr);
return;
}

Expand All @@ -450,7 +461,7 @@ void AccessControlCheckerBase::checkGenericParamAccess(
contextAccess, minDiagAccessLevel, isa<FileUnit>(DC),
accessControlErrorKind == ACEK::Requirement);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(Context, minImportLimit);
noteLimitingImport(Context, minImportLimit, complainRepr);
}

void AccessControlCheckerBase::checkGenericParamAccess(
Expand Down Expand Up @@ -483,7 +494,7 @@ void AccessControlCheckerBase::checkGlobalActorAccess(const Decl *D) {
auto diag = D->diagnose(diag::global_actor_not_usable_from_inline,
D->getDescriptiveKind(), VD->getName());
highlightOffendingType(diag, complainRepr);
noteLimitingImport(D->getASTContext(), importLimit);
noteLimitingImport(D->getASTContext(), importLimit, complainRepr);
return;
}

Expand All @@ -495,7 +506,7 @@ void AccessControlCheckerBase::checkGlobalActorAccess(const Decl *D) {
D->getDescriptiveKind(), VD->getName(),
diagAccessLevel, globalActorDecl->getName());
highlightOffendingType(diag, complainRepr);
noteLimitingImport(D->getASTContext(), importLimit);
noteLimitingImport(D->getASTContext(), importLimit, complainRepr);
});
}

Expand Down Expand Up @@ -576,12 +587,11 @@ class AccessControlChecker : public AccessControlCheckerBase,
if (downgradeToWarning == DowngradeToWarning::Yes)
diagID = diag::pattern_type_access_inferred_warn;
auto &DE = theVar->getASTContext().Diags;
auto diag = DE.diagnose(NP->getLoc(), diagID, theVar->isLet(),
isTypeContext, isExplicit, theVarAccess,
isa<FileUnit>(theVar->getDeclContext()),
diagAccessLevel, theVar->getInterfaceType());
diag.flush();
noteLimitingImport(theVar->getASTContext(), importLimit);
DE.diagnose(NP->getLoc(), diagID, theVar->isLet(),
isTypeContext, isExplicit, theVarAccess,
isa<FileUnit>(theVar->getDeclContext()),
diagAccessLevel, theVar->getInterfaceType());
noteLimitingImport(theVar->getASTContext(), importLimit, complainRepr);
});
}

Expand Down Expand Up @@ -616,7 +626,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
anyVarAccess, isa<FileUnit>(anyVar->getDeclContext()),
diagAccessLevel);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(anyVar->getASTContext(), importLimit);
noteLimitingImport(anyVar->getASTContext(), importLimit, complainRepr);
});

// Check the property wrapper types.
Expand All @@ -642,7 +652,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
isa<FileUnit>(anyVar->getDeclContext()),
diagAccessLevel);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(anyVar->getASTContext(), importLimit);
noteLimitingImport(anyVar->getASTContext(), importLimit, complainRepr);
});
}
}
Expand Down Expand Up @@ -692,7 +702,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
diagAccessLevel,
isa<FileUnit>(TAD->getDeclContext()));
highlightOffendingType(diag, complainRepr);
noteLimitingImport(TAD->getASTContext(), importLimit);
noteLimitingImport(TAD->getASTContext(), importLimit, complainRepr);
});
}

Expand Down Expand Up @@ -787,7 +797,8 @@ class AccessControlChecker : public AccessControlCheckerBase,
minDiagAccessLevel,
accessControlErrorKind);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(assocType->getASTContext(), minImportLimit);
noteLimitingImport(assocType->getASTContext(), minImportLimit,
complainRepr);
}
}

Expand Down Expand Up @@ -823,7 +834,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
diagAccessLevel,
isa<FileUnit>(ED->getDeclContext()));
highlightOffendingType(diag, complainRepr);
noteLimitingImport(ED->getASTContext(), importLimit);
noteLimitingImport(ED->getASTContext(), importLimit, complainRepr);
});
}
}
Expand Down Expand Up @@ -885,7 +896,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
isa<FileUnit>(CD->getDeclContext()),
superclassLocIter->getTypeRepr() != complainRepr);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(CD->getASTContext(), importLimit);
noteLimitingImport(CD->getASTContext(), importLimit, complainRepr);
});
}
}
Expand Down Expand Up @@ -988,7 +999,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
protocolControlErrorKind, minDiagAccessLevel,
isa<FileUnit>(proto->getDeclContext()), declKind);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(proto->getASTContext(), minImportLimit);
noteLimitingImport(proto->getASTContext(), minImportLimit, complainRepr);
}
}

Expand Down Expand Up @@ -1052,7 +1063,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
auto diag = SD->diagnose(diagID, isExplicit, subscriptDeclAccess,
minDiagAccessLevel, problemIsElement);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(SD->getASTContext(), minImportLimit);
noteLimitingImport(SD->getASTContext(), minImportLimit, complainRepr);
}
}

Expand Down Expand Up @@ -1159,7 +1170,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
functionKind, problemIsResult,
hasInaccessibleParameterWrapper);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(fn->getASTContext(), minImportLimit);
noteLimitingImport(fn->getASTContext(), minImportLimit, complainRepr);
}
}

Expand All @@ -1178,7 +1189,8 @@ class AccessControlChecker : public AccessControlCheckerBase,
auto diag =
EED->diagnose(diagID, EED->getFormalAccess(), diagAccessLevel);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(EED->getASTContext(), importLimit);
noteLimitingImport(EED->getASTContext(), importLimit,
complainRepr);
});
}
}
Expand Down Expand Up @@ -1244,7 +1256,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
auto diag = MD->diagnose(diagID, isExplicit, macroDeclAccess,
minDiagAccessLevel, problemIsResult);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(MD->getASTContext(), minImportLimit);
noteLimitingImport(MD->getASTContext(), minImportLimit, complainRepr);
}
}
};
Expand Down Expand Up @@ -1353,7 +1365,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
}
Ctx.Diags.diagnose(NP->getLoc(), diagID, theVar->isLet(),
isTypeContext, theVar->getInterfaceType());
noteLimitingImport(theVar->getASTContext(), importLimit);
noteLimitingImport(theVar->getASTContext(), importLimit,
complainRepr);
});
}

Expand Down Expand Up @@ -1392,7 +1405,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
auto diag = Ctx.Diags.diagnose(TP->getLoc(), diagID, anyVar->isLet(),
isTypeContext);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(anyVar->getASTContext(), importLimit);
noteLimitingImport(anyVar->getASTContext(), importLimit,
complainRepr);
});

for (auto attr : anyVar->getAttachedPropertyWrappers()) {
Expand All @@ -1403,12 +1417,13 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
[&](AccessScope typeAccessScope,
const TypeRepr *complainRepr,
DowngradeToWarning downgradeToWarning,
ImportAccessLevel importLimit, AccessLevel diagAccessLevel) {
ImportAccessLevel importLimit,
AccessLevel diagAccessLevel) {
auto diag = anyVar->diagnose(
diag::property_wrapper_type_not_usable_from_inline,
anyVar->isLet(), isTypeContext);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(anyVar->getASTContext(), importLimit);
noteLimitingImport(anyVar->getASTContext(), importLimit, complainRepr);
});
}
}
Expand Down Expand Up @@ -1447,7 +1462,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::type_alias_underlying_type_not_usable_from_inline_warn;
auto diag = TAD->diagnose(diagID);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(TAD->getASTContext(), importLimit);
noteLimitingImport(TAD->getASTContext(), importLimit, complainRepr);
});
}

Expand All @@ -1471,7 +1486,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::associated_type_not_usable_from_inline_warn;
auto diag = assocType->diagnose(diagID, ACEK_Requirement);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(assocType->getASTContext(), importLimit);
noteLimitingImport(assocType->getASTContext(), importLimit,
complainRepr);
});
});
checkTypeAccess(assocType->getDefaultDefinitionType(),
Expand All @@ -1487,7 +1503,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::associated_type_not_usable_from_inline_warn;
auto diag = assocType->diagnose(diagID, ACEK_DefaultDefinition);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(assocType->getASTContext(), importLimit);
noteLimitingImport(assocType->getASTContext(), importLimit,
complainRepr);
});

if (assocType->getTrailingWhereClause()) {
Expand All @@ -1506,7 +1523,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::associated_type_not_usable_from_inline_warn;
auto diag = assocType->diagnose(diagID, ACEK_Requirement);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(assocType->getASTContext(), importLimit);
noteLimitingImport(assocType->getASTContext(), importLimit,
complainRepr);
});
}
}
Expand Down Expand Up @@ -1537,7 +1555,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::enum_raw_type_not_usable_from_inline_warn;
auto diag = ED->diagnose(diagID);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(ED->getASTContext(), importLimit);
noteLimitingImport(ED->getASTContext(), importLimit, complainRepr);
});
}
}
Expand Down Expand Up @@ -1583,7 +1601,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
auto diag = CD->diagnose(diagID, superclassLocIter->getTypeRepr() !=
complainRepr);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(CD->getASTContext(), importLimit);
noteLimitingImport(CD->getASTContext(), importLimit, complainRepr);
});
}
}
Expand All @@ -1609,7 +1627,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::protocol_usable_from_inline_warn;
auto diag = proto->diagnose(diagID, PCEK_Refine);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(proto->getASTContext(), importLimit);
noteLimitingImport(proto->getASTContext(), importLimit, complainRepr);
});
});

Expand All @@ -1629,7 +1647,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::protocol_usable_from_inline_warn;
auto diag = proto->diagnose(diagID, PCEK_Requirement);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(proto->getASTContext(), importLimit);
noteLimitingImport(proto->getASTContext(), importLimit, complainRepr);
});
}
}
Expand All @@ -1649,7 +1667,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::subscript_type_usable_from_inline_warn;
auto diag = SD->diagnose(diagID, /*problemIsElement=*/false);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(SD->getASTContext(), importLimit);
noteLimitingImport(SD->getASTContext(), importLimit, complainRepr);
});
}

Expand All @@ -1665,7 +1683,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::subscript_type_usable_from_inline_warn;
auto diag = SD->diagnose(diagID, /*problemIsElement=*/true);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(SD->getASTContext(), importLimit);
noteLimitingImport(SD->getASTContext(), importLimit, complainRepr);
});
}

Expand Down Expand Up @@ -1703,7 +1721,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
/*problemIsResult=*/false,
/*inaccessibleWrapper=*/true);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(fn->getASTContext(), importLimit);
noteLimitingImport(fn->getASTContext(), importLimit,
complainRepr);
});
}
}
Expand All @@ -1720,7 +1739,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
/*problemIsResult=*/false,
/*inaccessibleWrapper=*/false);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(fn->getASTContext(), importLimit);
noteLimitingImport(fn->getASTContext(), importLimit, complainRepr);
});
}

Expand All @@ -1738,7 +1757,7 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
/*problemIsResult=*/true,
/*inaccessibleWrapper=*/false);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(fn->getASTContext(), importLimit);
noteLimitingImport(fn->getASTContext(), importLimit, complainRepr);
});
}
}
Expand All @@ -1762,7 +1781,8 @@ class UsableFromInlineChecker : public AccessControlCheckerBase,
diagID = diag::enum_case_usable_from_inline_warn;
auto diag = EED->diagnose(diagID);
highlightOffendingType(diag, complainRepr);
noteLimitingImport(EED->getASTContext(), importLimit);
noteLimitingImport(EED->getASTContext(), importLimit,
complainRepr);
});
}
}
Expand Down
Loading