[Diagnostics] Display correct debug note for compound references typo suggestions #6715
Conversation
|
@swift-ci Please smoke test |
| @@ -555,13 +555,16 @@ void TypeChecker::performTypoCorrection(DeclContext *DC, DeclRefKind refKind, | |||
| } | |||
|
|
|||
| static InFlightDiagnostic | |||
| diagnoseTypoCorrection(TypeChecker &tc, DeclNameLoc loc, ValueDecl *decl) { | |||
| diagnoseTypoCorrection(TypeChecker &tc, DeclNameLoc loc, ValueDecl *decl, FunctionRefKind refKind) { | |||
There was a problem hiding this comment.
nit-pick: The style guide for C++ in Swift projects is a 80 character column width. You should format this like so:
diagnoseTypoCorrection(TypeChecker &tc,
DeclNameLoc loc,
ValueDecl *decl,
FunctionRefKind refKind) {Same goes for the line of code below this one: you'll need to split it across two lines for it to fit within 80 characters.
There was a problem hiding this comment.
Oh, thanks! Will update accordingly.
|
@swift-ci please smoke test |
|
@modocache updated with code-style comments. Thanks for the eyes on this! |
| ValueDecl *decl, | ||
| FunctionRefKind refKind) { | ||
|
|
||
| auto name = refKind == FunctionRefKind::Compound ? decl->getFullName() : |
There was a problem hiding this comment.
LLVM style moves the colon to a new line in-line with the else condition and aligns it with the question mark.
auto name = refKind == FunctionRefKind::Compound ? decl->getFullName()
: decl->getName();| // RUN: %target-typecheck-verify-swift | ||
|
|
||
| func foo(x: Int) {} // expected-note * {{did you mean 'foo(x:)'?}} | ||
| let f = foo(_:) // expected-error {{use of unresolved identifier 'foo'}} |
There was a problem hiding this comment.
Please remove this file and add these tests to Sema/typo_correction.swift (sans * on the expected-note).
| @@ -555,13 +555,21 @@ void TypeChecker::performTypoCorrection(DeclContext *DC, DeclRefKind refKind, | |||
| } | |||
|
|
|||
| static InFlightDiagnostic | |||
| diagnoseTypoCorrection(TypeChecker &tc, DeclNameLoc loc, ValueDecl *decl) { | |||
| diagnoseTypoCorrection(TypeChecker &tc, | |||
| DeclNameLoc loc, | |||
There was a problem hiding this comment.
You don't necessarily have to fold the entire signature over, just those that go over the line-limit. If it makes it easier, you can run clang-format with the default LLVM style to have it wrap these for you.
|
@CodaFi Fixed your comments. Thanks! |
| FunctionRefKind refKind) { | ||
|
|
||
| auto name = refKind == FunctionRefKind::Compound ? decl->getFullName() | ||
| : decl->getName(); |
There was a problem hiding this comment.
Here you must add the quotes back or it'll fall any existing typo correction tests that rely on it.
| @@ -2083,7 +2083,8 @@ class TypeChecker final : public LazyResolver { | |||
| unsigned maxResults = 4); | |||
|
|
|||
| void noteTypoCorrection(DeclName name, DeclNameLoc nameLoc, | |||
| const LookupResult::Result &suggestion); | |||
| const LookupResult::Result &suggestion, | |||
| FunctionRefKind refKind = FunctionRefKind::Unapplied); | |||
There was a problem hiding this comment.
I'm still not 100% sure this is the right type for this. For names that aren't function references that go through typo correction this is kind of nonsensical.
There was a problem hiding this comment.
Is there any other type that would work in this case?
|
@OscarSwanros What's the status of this pull request? If you're still interested in this bug fix, could you rebase this pull request and give me a ping? |
|
This patch has fallen behind after a month. @OscarSwanros feel free to reopen this when you find the time. |
|
Oh, sorry @CodaFi — have been swamped and missed your notification. Will try to find the time soon. |
This PR tries to address the issue where functions with multiple arguments would just be diagnosed incorrectly ignoring their attributes.
Resolves #45842.