Skip to content

Commit a884fe1

Browse files
LucianoPAlmeidaLucianoPAlmeida
authored
Merge pull request #34744 from LucianoPAlmeida/correct-test-case
[Sema] Emitting diagnostic for SR-11535 and moving it to a fix format
2 parents 353e6fe + e4d4c0b commit a884fe1

File tree

7 files changed

+246
-100
lines changed

7 files changed

+246
-100
lines changed

‎include/swift/Sema/CSFix.h

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class ConstraintLocator;
4646
class ConstraintLocatorBuilder;
4747
enum class ConversionRestrictionKind;
4848
class Solution;
49+
struct MemberLookupResult;
4950

5051
/// Describes the kind of fix to apply to the given constraint before
5152
/// visiting it.
@@ -289,6 +290,10 @@ enum class FixKind : uint8_t {
289290
/// Treat empty and single-element array literals as if they were incomplete
290291
/// dictionary literals when used as such.
291292
TreatArrayLiteralAsDictionary,
293+
294+
/// Explicitly specify the type to disambiguate between possible member base
295+
/// types.
296+
SpecifyBaseTypeForOptionalUnresolvedMember,
292297
};
293298

294299
class ConstraintFix {
@@ -2143,6 +2148,34 @@ class AllowRefToInvalidDecl final : public ConstraintFix {
21432148
ConstraintLocator *locator);
21442149
};
21452150

2151+
/// Diagnose if the base type is optional, we're referring to a nominal
2152+
/// type member via the dot syntax and the member name matches
2153+
/// Optional<T>.{member} or a .none member inferred as non-optional static
2154+
/// member e.g. let _ : Foo? = .none where Foo has a static member none.
2155+
class SpecifyBaseTypeForOptionalUnresolvedMember final : public ConstraintFix {
2156+
SpecifyBaseTypeForOptionalUnresolvedMember(ConstraintSystem &cs,
2157+
DeclNameRef memberName,
2158+
ConstraintLocator *locator)
2159+
: ConstraintFix(cs, FixKind::SpecifyBaseTypeForOptionalUnresolvedMember,
2160+
locator, /*isWarning=*/true),
2161+
MemberName(memberName) {}
2162+
DeclNameRef MemberName;
2163+
2164+
public:
2165+
std::string getName() const override {
2166+
const auto name = MemberName.getBaseName();
2167+
return "specify unresolved member optional base type explicitly '" +
2168+
name.userFacingName().str() + "'";
2169+
}
2170+
2171+
bool diagnose(const Solution &solution, bool asNote = false) const override;
2172+
2173+
static SpecifyBaseTypeForOptionalUnresolvedMember *
2174+
attempt(ConstraintSystem &cs, ConstraintKind kind, Type baseTy,
2175+
DeclNameRef memberName, FunctionRefKind functionRefKind,
2176+
MemberLookupResult result, ConstraintLocator *locator);
2177+
};
2178+
21462179
} // end namespace constraints
21472180
} // end namespace swift
21482181

‎lib/Sema/CSApply.cpp

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -2862,105 +2862,9 @@ namespace {
28622862
if (!result)
28632863
return nullptr;
28642864

2865-
// Check for ambiguous member if the base is an Optional
2866-
if (baseTy->getOptionalObjectType()) {
2867-
diagnoseAmbiguousNominalMember(baseTy, result);
2868-
}
2869-
28702865
return coerceToType(result, resultTy, cs.getConstraintLocator(expr));
28712866
}
2872-
2873-
/// Diagnose if the base type is optional, we're referring to a nominal
2874-
/// type member via the dot syntax and the member name matches
2875-
/// Optional<T>.{member name}
2876-
void diagnoseAmbiguousNominalMember(Type baseTy, Expr *result) {
2877-
if (auto baseTyUnwrapped = baseTy->lookThroughAllOptionalTypes()) {
2878-
// Return if the base type doesn't have a nominal type decl
2879-
if (!baseTyUnwrapped->getNominalOrBoundGenericNominal()) {
2880-
return;
2881-
}
2882-
2883-
// Otherwise, continue digging
2884-
if (auto DSCE = dyn_cast<DotSyntaxCallExpr>(result)) {
2885-
auto calledValue = DSCE->getCalledValue();
2886-
auto isOptional = false;
2887-
Identifier memberName;
2888-
2889-
// Try cast the assigned value to an enum case
2890-
//
2891-
// This will always succeed if the base is Optional<T> & the
2892-
// assigned case comes from Optional<T>
2893-
if (auto EED = dyn_cast<EnumElementDecl>(calledValue)) {
2894-
isOptional = EED->getParentEnum()->isOptionalDecl();
2895-
memberName = EED->getBaseIdentifier();
2896-
}
2897-
2898-
// Return if the enum case doesn't come from Optional<T>
2899-
if (!isOptional) {
2900-
return;
2901-
}
2902-
2903-
// Look up the enum case in the unwrapped type to check if it exists
2904-
// as a member
2905-
auto baseTyNominalDecl = baseTyUnwrapped
2906-
->getNominalOrBoundGenericNominal();
2907-
auto results = TypeChecker::lookupMember(
2908-
baseTyNominalDecl->getModuleContext(), baseTyUnwrapped,
2909-
DeclNameRef(memberName), defaultMemberLookupOptions);
2910-
2911-
// Filter out any functions, instance members, enum cases with
2912-
// associated values or variables whose type does not match the
2913-
// contextual type.
2914-
results.filter([&](const LookupResultEntry entry, bool isOuter) {
2915-
if (auto member = entry.getValueDecl()) {
2916-
if (isa<FuncDecl>(member))
2917-
return false;
2918-
if (member->isInstanceMember())
2919-
return false;
2920-
if (auto EED = dyn_cast<EnumElementDecl>(member)) {
2921-
return !EED->hasAssociatedValues();
2922-
}
2923-
if (auto VD = dyn_cast<VarDecl>(member)) {
2924-
auto baseType = DSCE->getType()->lookThroughAllOptionalTypes();
2925-
return VD->getInterfaceType()->isEqual(baseType);
2926-
}
2927-
}
2928-
2929-
// Filter out anything that's not one of the above. We don't care
2930-
// if we have a typealias named 'none' or a struct/class named
2931-
// 'none'.
2932-
return false;
2933-
});
2934-
2935-
if (results.empty()) {
2936-
return;
2937-
}
29382867

2939-
auto &de = cs.getASTContext().Diags;
2940-
if (auto member = results.front().getValueDecl()) {
2941-
// Emit a diagnostic with some fix-its
2942-
auto baseTyName = baseTy->getCanonicalType().getString();
2943-
auto baseTyUnwrappedName = baseTyUnwrapped->getString();
2944-
auto loc = DSCE->getLoc();
2945-
auto startLoc = DSCE->getStartLoc();
2946-
de.diagnoseWithNotes(
2947-
de.diagnose(loc, swift::diag::optional_ambiguous_case_ref,
2948-
baseTyName, baseTyUnwrappedName, memberName.str()),
2949-
[&]() {
2950-
de.diagnose(loc,
2951-
swift::diag::optional_fixit_ambiguous_case_ref)
2952-
.fixItInsert(startLoc, "Optional");
2953-
de.diagnose(
2954-
loc,
2955-
swift::diag::type_fixit_optional_ambiguous_case_ref,
2956-
baseTyUnwrappedName, memberName.str())
2957-
.fixItInsert(startLoc, baseTyUnwrappedName);
2958-
});
2959-
}
2960-
}
2961-
}
2962-
}
2963-
29642868
private:
29652869
/// A list of "suspicious" optional injections that come from
29662870
/// forced downcasts.

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7095,3 +7095,55 @@ bool InvalidReturnInResultBuilderBody::diagnoseAsError() {
70957095

70967096
return true;
70977097
}
7098+
7099+
bool MemberMissingExplicitBaseTypeFailure::diagnoseAsError() {
7100+
auto UME = castToExpr<UnresolvedMemberExpr>(getAnchor());
7101+
auto memberName = UME->getName().getBaseIdentifier().str();
7102+
auto &DE = getASTContext().Diags;
7103+
auto &solution = getSolution();
7104+
7105+
auto selected = solution.getOverloadChoice(getLocator());
7106+
auto baseType =
7107+
resolveType(selected.choice.getBaseType()->getMetatypeInstanceType());
7108+
7109+
SmallVector<Type, 4> optionals;
7110+
auto baseTyUnwrapped = baseType->lookThroughAllOptionalTypes(optionals);
7111+
7112+
if (!optionals.empty()) {
7113+
auto baseTyName = baseType->getCanonicalType().getString();
7114+
auto baseTyUnwrappedName = baseTyUnwrapped->getString();
7115+
auto loc = UME->getLoc();
7116+
auto startLoc = UME->getStartLoc();
7117+
7118+
DE.diagnoseWithNotes(
7119+
DE.diagnose(loc, diag::optional_ambiguous_case_ref, baseTyName,
7120+
baseTyUnwrappedName, memberName),
7121+
[&]() {
7122+
DE.diagnose(UME->getDotLoc(), diag::optional_fixit_ambiguous_case_ref)
7123+
.fixItInsert(startLoc, "Optional");
7124+
DE.diagnose(UME->getDotLoc(),
7125+
diag::type_fixit_optional_ambiguous_case_ref,
7126+
baseTyUnwrappedName, memberName)
7127+
.fixItInsert(startLoc, baseTyUnwrappedName);
7128+
});
7129+
} else {
7130+
auto baseTypeName = baseType->getCanonicalType().getString();
7131+
auto baseOptionalTypeName =
7132+
OptionalType::get(baseType)->getCanonicalType().getString();
7133+
7134+
DE.diagnoseWithNotes(
7135+
DE.diagnose(UME->getLoc(), diag::optional_ambiguous_case_ref,
7136+
baseTypeName, baseOptionalTypeName, memberName),
7137+
[&]() {
7138+
DE.diagnose(UME->getDotLoc(),
7139+
diag::type_fixit_optional_ambiguous_case_ref,
7140+
baseOptionalTypeName, memberName)
7141+
.fixItInsert(UME->getDotLoc(), baseOptionalTypeName);
7142+
DE.diagnose(UME->getDotLoc(),
7143+
diag::type_fixit_optional_ambiguous_case_ref,
7144+
baseTypeName, memberName)
7145+
.fixItInsert(UME->getDotLoc(), baseTypeName);
7146+
});
7147+
}
7148+
return true;
7149+
}

‎lib/Sema/CSDiagnostics.h

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2352,6 +2352,35 @@ class InvalidReturnInResultBuilderBody final : public FailureDiagnostic {
23522352
bool diagnoseAsError() override;
23532353
};
23542354

2355+
/// Diagnose if the base type is optional, we're referring to a nominal
2356+
/// type member via the dot syntax and the member name matches
2357+
/// Optional<T>.{member_name} or an unresolved `.none` inferred as a static
2358+
/// non-optional member base but could be an Optional<T>.none. So we enforce
2359+
/// explicit type annotation to avoid ambiguity.
2360+
///
2361+
/// \code
2362+
/// enum Enum<T> {
2363+
/// case bar
2364+
/// static var none: Enum<Int> { .bar }
2365+
/// }
2366+
/// let _: Enum<Int>? = .none // Base inferred as Optional.none, suggest
2367+
/// // explicit type.
2368+
/// let _: Enum? = .none // Base inferred as static member Enum<Int>.none,
2369+
/// // emit warning suggesting explicit type.
2370+
/// let _: Enum = .none // Ok
2371+
/// \endcode
2372+
class MemberMissingExplicitBaseTypeFailure final : public FailureDiagnostic {
2373+
DeclNameRef Member;
2374+
2375+
public:
2376+
MemberMissingExplicitBaseTypeFailure(const Solution &solution,
2377+
DeclNameRef member,
2378+
ConstraintLocator *locator)
2379+
: FailureDiagnostic(solution, locator), Member(member) {}
2380+
2381+
bool diagnoseAsError() override;
2382+
};
2383+
23552384
} // end namespace constraints
23562385
} // end namespace swift
23572386

‎lib/Sema/CSFix.cpp

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,3 +1673,100 @@ IgnoreResultBuilderWithReturnStmts::create(ConstraintSystem &cs, Type builderTy,
16731673
return new (cs.getAllocator())
16741674
IgnoreResultBuilderWithReturnStmts(cs, builderTy, locator);
16751675
}
1676+
1677+
bool SpecifyBaseTypeForOptionalUnresolvedMember::diagnose(
1678+
const Solution &solution, bool asNote) const {
1679+
MemberMissingExplicitBaseTypeFailure failure(solution, MemberName,
1680+
getLocator());
1681+
return failure.diagnose(asNote);
1682+
}
1683+
1684+
SpecifyBaseTypeForOptionalUnresolvedMember *
1685+
SpecifyBaseTypeForOptionalUnresolvedMember::attempt(
1686+
ConstraintSystem &cs, ConstraintKind kind, Type baseTy,
1687+
DeclNameRef memberName, FunctionRefKind functionRefKind,
1688+
MemberLookupResult result, ConstraintLocator *locator) {
1689+
1690+
if (kind != ConstraintKind::UnresolvedValueMember)
1691+
return nullptr;
1692+
1693+
// None or only one viable candidate, there is no ambiguity.
1694+
if (result.ViableCandidates.size() <= 1)
1695+
return nullptr;
1696+
1697+
// Only diagnose those situations for static members.
1698+
if (!baseTy->is<MetatypeType>())
1699+
return nullptr;
1700+
1701+
// Don't diagnose for function members e.g. Foo? = .none(0).
1702+
if (functionRefKind != FunctionRefKind::Unapplied)
1703+
return nullptr;
1704+
1705+
Type underlyingBaseType = baseTy->getMetatypeInstanceType();
1706+
if (!underlyingBaseType->getNominalOrBoundGenericNominal())
1707+
return nullptr;
1708+
1709+
if (!underlyingBaseType->getOptionalObjectType())
1710+
return nullptr;
1711+
1712+
auto unwrappedType = underlyingBaseType->lookThroughAllOptionalTypes();
1713+
bool allOptionalBaseCandidates = true;
1714+
auto filterViableCandidates =
1715+
[&](SmallVector<OverloadChoice, 4> &candidates,
1716+
SmallVector<OverloadChoice, 4> &viableCandidates,
1717+
bool &allOptionalBase) {
1718+
for (OverloadChoice choice : candidates) {
1719+
if (!choice.isDecl())
1720+
continue;
1721+
1722+
auto memberDecl = choice.getDecl();
1723+
if (isa<FuncDecl>(memberDecl))
1724+
continue;
1725+
if (memberDecl->isInstanceMember())
1726+
continue;
1727+
1728+
allOptionalBase &= bool(choice.getBaseType()
1729+
->getMetatypeInstanceType()
1730+
->getOptionalObjectType());
1731+
1732+
if (auto EED = dyn_cast<EnumElementDecl>(memberDecl)) {
1733+
if (!EED->hasAssociatedValues())
1734+
viableCandidates.push_back(choice);
1735+
} else if (auto VD = dyn_cast<VarDecl>(memberDecl)) {
1736+
if (unwrappedType->hasTypeVariable() ||
1737+
VD->getInterfaceType()->isEqual(unwrappedType))
1738+
viableCandidates.push_back(choice);
1739+
}
1740+
}
1741+
};
1742+
1743+
SmallVector<OverloadChoice, 4> viableCandidates;
1744+
filterViableCandidates(result.ViableCandidates, viableCandidates,
1745+
allOptionalBaseCandidates);
1746+
1747+
// Also none or only one viable candidate after filtering candidates, there is
1748+
// no ambiguity.
1749+
if (viableCandidates.size() <= 1)
1750+
return nullptr;
1751+
1752+
// Right now, name lookup only unwraps a single layer of optionality, which
1753+
// for cases where base type is a multi-optional type e.g. Foo?? it only
1754+
// finds optional base candidates. To produce the correct warning we perform
1755+
// an extra lookup on unwrapped type.
1756+
if (!allOptionalBaseCandidates)
1757+
return new (cs.getAllocator())
1758+
SpecifyBaseTypeForOptionalUnresolvedMember(cs, memberName, locator);
1759+
1760+
MemberLookupResult unwrappedResult =
1761+
cs.performMemberLookup(kind, memberName, MetatypeType::get(unwrappedType),
1762+
functionRefKind, locator,
1763+
/*includeInaccessibleMembers*/ false);
1764+
SmallVector<OverloadChoice, 4> unwrappedViableCandidates;
1765+
filterViableCandidates(unwrappedResult.ViableCandidates,
1766+
unwrappedViableCandidates, allOptionalBaseCandidates);
1767+
if (unwrappedViableCandidates.empty())
1768+
return nullptr;
1769+
1770+
return new (cs.getAllocator())
1771+
SpecifyBaseTypeForOptionalUnresolvedMember(cs, memberName, locator);
1772+
}

0 commit comments

Comments
 (0)