Skip to content

Add unique typo corrections to the main diagnostic with a fix-it #15800

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.

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
Apr 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ NOTE(type_declared_here,none,
"type declared here", ())
NOTE(decl_declared_here,none,
"%0 declared here", (DeclName))
NOTE(implicit_member_declared_here,none,
"%1 '%0' is implicitly declared", (StringRef, StringRef))
NOTE(extended_type_declared_here,none,
"extended type declared here", ())

Expand All @@ -64,8 +66,14 @@ ERROR(could_not_find_tuple_member,none,

ERROR(could_not_find_value_member,none,
"value of type %0 has no member %1", (Type, DeclName))
ERROR(could_not_find_value_member_corrected,none,
"value of type %0 has no member %1; did you mean %2?",
(Type, DeclName, DeclName))
ERROR(could_not_find_type_member,none,
"type %0 has no member %1", (Type, DeclName))
ERROR(could_not_find_type_member_corrected,none,
"type %0 has no member %1; did you mean %2?",
(Type, DeclName, DeclName))

ERROR(could_not_find_enum_case,none,
"enum type %0 has no case %1; did you mean %2", (Type, DeclName, DeclName))
Expand Down Expand Up @@ -654,6 +662,9 @@ ERROR(unspaced_unary_operator,none,

ERROR(use_unresolved_identifier,none,
"use of unresolved %select{identifier|operator}1 %0", (DeclName, bool))
ERROR(use_unresolved_identifier_corrected,none,
"use of unresolved %select{identifier|operator}1 %0; did you mean %2?",
(DeclName, bool, DeclName))
NOTE(confusable_character,none,
"%select{identifier|operator}0 '%1' contains possibly confused characters; "
"did you mean to use '%2'?",
Expand Down
21 changes: 13 additions & 8 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -920,12 +920,11 @@ static std::string getScriptFileName(StringRef name, bool isSwiftVersion3) {
return (Twine(name) + langVer + ".json").str();
}

bool ParseMigratorArgs(MigratorOptions &Opts,
const FrontendOptions &FrontendOpts,
const llvm::Triple &Triple,
const bool isSwiftVersion3,
StringRef ResourcePath, const ArgList &Args,
DiagnosticEngine &Diags) {
static bool ParseMigratorArgs(MigratorOptions &Opts,
LangOptions &LangOpts,
const FrontendOptions &FrontendOpts,
StringRef ResourcePath, const ArgList &Args,
DiagnosticEngine &Diags) {
using namespace options;

Opts.KeepObjcVisibility |= Args.hasArg(OPT_migrate_keep_objc_visibility);
Expand All @@ -950,9 +949,13 @@ bool ParseMigratorArgs(MigratorOptions &Opts,
if (auto DataPath = Args.getLastArg(OPT_api_diff_data_file)) {
Opts.APIDigesterDataStorePaths.push_back(DataPath->getValue());
} else {
auto &Triple = LangOpts.Target;
bool isSwiftVersion3 = LangOpts.isSwiftVersion3();

bool Supported = true;
llvm::SmallString<128> dataPath(ResourcePath);
llvm::sys::path::append(dataPath, "migrator");

if (Triple.isMacOSX())
llvm::sys::path::append(dataPath,
getScriptFileName("macos", isSwiftVersion3));
Expand Down Expand Up @@ -991,6 +994,9 @@ bool ParseMigratorArgs(MigratorOptions &Opts,
// supplementary output for the whole compilation instead of one per input,
// so it's probably not worth it.
FrontendOpts.InputsAndOutputs.assertMustNotBeMoreThanOnePrimaryInput();

// Always disable typo-correction in the migrator.
LangOpts.TypoCorrectionLimit = 0;
}

return false;
Expand Down Expand Up @@ -1061,8 +1067,7 @@ bool CompilerInvocation::parseArgs(
return true;
}

if (ParseMigratorArgs(MigratorOpts, FrontendOpts, LangOpts.Target,
LangOpts.isSwiftVersion3(),
if (ParseMigratorArgs(MigratorOpts, LangOpts, FrontendOpts,
SearchPathOpts.RuntimeResourcePath, ParsedArgs, Diags)) {
return true;
}
Expand Down
85 changes: 50 additions & 35 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "CSDiag.h"
#include "CalleeCandidateInfo.h"
#include "MiscDiagnostics.h"
#include "TypoCorrection.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/Initializer.h"
Expand Down Expand Up @@ -1330,25 +1331,20 @@ diagnoseTypeMemberOnInstanceLookup(Type baseObjTy,
/// lower case counterparts are identical.
/// - DeclName is valid when such a correct case is found; invalid otherwise.
static DeclName
findCorrectEnumCaseName(Type Ty, LookupResult &Result,
findCorrectEnumCaseName(Type Ty, TypoCorrectionResults &corrections,
DeclName memberName) {
if (!memberName.isSimpleName())
return DeclName();
if (!Ty->is<EnumType>() &&
!Ty->is<BoundGenericEnumType>())
return DeclName();
llvm::SmallVector<DeclName, 4> candidates;
for (auto &correction : Result) {
DeclName correctName = correction.getValueDecl()->getFullName();
if (!isa<EnumElementDecl>(correction.getValueDecl()))
continue;
if (correctName.getBaseIdentifier().str().equals_lower(
memberName.getBaseIdentifier().str()))
candidates.push_back(correctName.getBaseName());
}
if (candidates.size() == 1)
return candidates.front();
return DeclName();
auto candidate =
corrections.getUniqueCandidateMatching([&](ValueDecl *candidate) {
return (isa<EnumElementDecl>(candidate) &&
candidate->getFullName().getBaseIdentifier().str()
.equals_lower(memberName.getBaseIdentifier().str()));
});
return (candidate ? candidate->getFullName() : DeclName());
}

/// Given a result of name lookup that had no viable results, diagnose the
Expand All @@ -1362,12 +1358,10 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,

// If we found no results at all, mention that fact.
if (result.UnviableCandidates.empty()) {
LookupResult correctionResults;
TypoCorrectionResults corrections(CS.TC, memberName, nameLoc);
auto tryTypoCorrection = [&] {
CS.TC.performTypoCorrection(CS.DC, DeclRefKind::Ordinary, baseObjTy,
memberName, nameLoc.getBaseNameLoc(),
defaultMemberLookupOptions,
correctionResults);
defaultMemberLookupOptions, corrections);
};

// TODO: This should handle tuple member lookups, like x.1231 as well.
Expand All @@ -1382,48 +1376,69 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
tryTypoCorrection();

if (DeclName rightName = findCorrectEnumCaseName(instanceTy,
correctionResults,
corrections,
memberName)) {
diagnose(loc, diag::could_not_find_enum_case, instanceTy,
memberName, rightName)
.fixItReplace(nameLoc.getBaseNameLoc(),
rightName.getBaseIdentifier().str());
return;
}
diagnose(loc, diag::could_not_find_type_member, instanceTy, memberName)
.highlight(baseRange).highlight(nameLoc.getSourceRange());

if (auto correction = corrections.claimUniqueCorrection()) {
auto diagnostic =
diagnose(loc, diag::could_not_find_type_member_corrected,
instanceTy, memberName, correction->CorrectedName);
diagnostic.highlight(baseRange).highlight(nameLoc.getSourceRange());
correction->addFixits(diagnostic);
} else {
diagnose(loc, diag::could_not_find_type_member, instanceTy, memberName)
.highlight(baseRange).highlight(nameLoc.getSourceRange());
}
} else if (auto moduleTy = baseObjTy->getAs<ModuleType>()) {
diagnose(baseExpr->getLoc(), diag::no_member_of_module,
moduleTy->getModule()->getName(), memberName)
.highlight(baseRange)
.highlight(nameLoc.getSourceRange());
return;
} else {
diagnose(loc, diag::could_not_find_value_member,
baseObjTy, memberName)
.highlight(baseRange).highlight(nameLoc.getSourceRange());
tryTypoCorrection();
auto emitBasicError = [&] {
diagnose(loc, diag::could_not_find_value_member,
baseObjTy, memberName)
.highlight(baseRange).highlight(nameLoc.getSourceRange());
};

// Check for a few common cases that can cause missing members.
if (baseObjTy->is<EnumType>() && memberName.isSimpleName("rawValue")) {
auto loc = baseObjTy->castTo<EnumType>()->getDecl()->getNameLoc();
if (loc.isValid()) {
emitBasicError();
diagnose(loc, diag::did_you_mean_raw_type);
return; // Always prefer this over typo corrections.
return;
}
} else if (baseObjTy->isAny()) {
emitBasicError();
diagnose(loc, diag::any_as_anyobject_fixit)
.fixItInsert(baseExpr->getStartLoc(), "(")
.fixItInsertAfter(baseExpr->getEndLoc(), " as AnyObject)");
return;
}

tryTypoCorrection();

if (auto correction = corrections.claimUniqueCorrection()) {
auto diagnostic =
diagnose(loc, diag::could_not_find_value_member_corrected,
baseObjTy, memberName, correction->CorrectedName);
diagnostic.highlight(baseRange).highlight(nameLoc.getSourceRange());
correction->addFixits(diagnostic);
} else {
emitBasicError();
}
}

// Note all the correction candidates.
for (auto &correction : correctionResults) {
CS.TC.noteTypoCorrection(memberName, nameLoc,
correction.getValueDecl());
}
corrections.noteAllCandidates();

// TODO: recover?
return;
Expand Down Expand Up @@ -6803,11 +6818,13 @@ static bool diagnoseKeyPathComponents(ConstraintSystem &CS, KeyPathExpr *KPE,
// If we didn't find anything, try to apply typo-correction.
bool resultsAreFromTypoCorrection = false;
if (!lookup) {
TypoCorrectionResults corrections(TC, componentName,
DeclNameLoc(componentNameLoc));

TC.performTypoCorrection(CS.DC, DeclRefKind::Ordinary, lookupType,
componentName, componentNameLoc,
(lookupType ? defaultMemberTypeLookupOptions
: defaultUnqualifiedLookupOptions),
lookup);
corrections);

if (currentType)
TC.diagnose(componentNameLoc, diag::could_not_find_type_member,
Expand All @@ -6817,10 +6834,8 @@ static bool diagnoseKeyPathComponents(ConstraintSystem &CS, KeyPathExpr *KPE,
componentName, false);

// Note all the correction candidates.
for (auto &result : lookup) {
TC.noteTypoCorrection(componentName, DeclNameLoc(componentNameLoc),
result.getValueDecl());
}
corrections.noteAllCandidates();
corrections.addAllCandidatesToLookup(lookup);

isInvalid = true;
if (!lookup)
Expand Down
37 changes: 25 additions & 12 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "ConstraintSystem.h"
#include "GenericTypeResolver.h"
#include "TypeChecker.h"
#include "TypoCorrection.h"
#include "MiscDiagnostics.h"
#include "swift/AST/ASTVisitor.h"
#include "swift/AST/ASTWalker.h"
Expand Down Expand Up @@ -448,12 +449,6 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
// one, but we should also try to propagate labels into this.
DeclNameLoc nameLoc = UDRE->getNameLoc();

performTypoCorrection(DC, UDRE->getRefKind(), Type(), Name, Loc,
lookupOptions, Lookup);

diagnose(Loc, diag::use_unresolved_identifier, Name, Name.isOperator())
.highlight(UDRE->getSourceRange());

Identifier simpleName = Name.getBaseIdentifier();
const char *buffer = simpleName.get();
llvm::SmallString<64> expectedIdentifier;
Expand All @@ -477,16 +472,34 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
offset += length;
}

if (isConfused) {
auto emitBasicError = [&] {
diagnose(Loc, diag::use_unresolved_identifier, Name, Name.isOperator())
.highlight(UDRE->getSourceRange());
};

bool claimed = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unused; is it meant to be used for something or just vestigial?

if (!isConfused) {
TypoCorrectionResults corrections(*this, Name, nameLoc);
performTypoCorrection(DC, UDRE->getRefKind(), Type(),
lookupOptions, corrections);

if (auto typo = corrections.claimUniqueCorrection()) {
auto diag = diagnose(Loc, diag::use_unresolved_identifier_corrected,
Name, Name.isOperator(), typo->CorrectedName);
diag.highlight(UDRE->getSourceRange());
typo->addFixits(diag);
} else {
emitBasicError();
}

corrections.noteAllCandidates();
} else {
emitBasicError();

diagnose(Loc, diag::confusable_character,
UDRE->getName().isOperator(), simpleName.str(),
expectedIdentifier)
.fixItReplace(Loc, expectedIdentifier);
} else {
// Note all the correction candidates.
for (auto &result : Lookup) {
noteTypoCorrection(Name, nameLoc, result.getValueDecl());
}
}

// TODO: consider recovering from here. We may want some way to suppress
Expand Down
12 changes: 6 additions & 6 deletions lib/Sema/TypeCheckExprObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//
//===----------------------------------------------------------------------===//
#include "TypeChecker.h"
#include "TypoCorrection.h"
#include "swift/Basic/Range.h"

using namespace swift;
Expand Down Expand Up @@ -248,11 +249,12 @@ Optional<Type> TypeChecker::checkObjCKeyPathExpr(DeclContext *dc,
// If we didn't find anything, try to apply typo-correction.
bool resultsAreFromTypoCorrection = false;
if (!lookup) {
TypoCorrectionResults corrections(*this, componentName,
DeclNameLoc(componentNameLoc));
performTypoCorrection(dc, DeclRefKind::Ordinary, lookupType,
componentName, componentNameLoc,
(lookupType ? defaultMemberTypeLookupOptions
: defaultUnqualifiedLookupOptions),
lookup);
corrections);

if (currentType)
diagnose(componentNameLoc, diag::could_not_find_type_member,
Expand All @@ -262,10 +264,8 @@ Optional<Type> TypeChecker::checkObjCKeyPathExpr(DeclContext *dc,
componentName, false);

// Note all the correction candidates.
for (auto &result : lookup) {
noteTypoCorrection(componentName, DeclNameLoc(componentNameLoc),
result.getValueDecl());
}
corrections.noteAllCandidates();
corrections.addAllCandidatesToLookup(lookup);

isInvalid = true;
if (!lookup) break;
Expand Down
19 changes: 8 additions & 11 deletions lib/Sema/TypeCheckGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//===----------------------------------------------------------------------===//
#include "TypeChecker.h"
#include "GenericTypeResolver.h"
#include "TypoCorrection.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/GenericSignatureBuilder.h"
#include "swift/AST/ProtocolConformance.h"
Expand Down Expand Up @@ -147,30 +148,26 @@ Type CompleteGenericTypeResolver::resolveDependentMemberType(
} else {
// Resolve the base to a potential archetype.
// Perform typo correction.
LookupResult corrections;
TypoCorrectionResults corrections(tc, ref->getIdentifier(),
DeclNameLoc(ref->getIdLoc()));
tc.performTypoCorrection(DC, DeclRefKind::Ordinary,
MetatypeType::get(baseTy),
ref->getIdentifier(), ref->getIdLoc(),
NameLookupFlags::ProtocolMembers,
corrections, &builder);

// Filter out non-types.
corrections.filter([](const LookupResultEntry &result) {
return isa<TypeDecl>(result.getValueDecl());
});

// Check whether we have a single type result.
auto singleType = corrections.getSingleTypeResult();
auto singleType = cast_or_null<TypeDecl>(
corrections.getUniqueCandidateMatching([](ValueDecl *result) {
return isa<TypeDecl>(result);
}));

// If we don't have a single result, complain and fail.
if (!singleType) {
Identifier name = ref->getIdentifier();
SourceLoc nameLoc = ref->getIdLoc();
tc.diagnose(nameLoc, diag::invalid_member_type, name, baseTy)
.highlight(baseRange);
for (const auto &suggestion : corrections)
tc.noteTypoCorrection(name, DeclNameLoc(nameLoc),
suggestion.getValueDecl());
corrections.noteAllCandidates();

return ErrorType::get(tc.Context);
}
Expand Down
Loading