Skip to content

[Diagnostics] Fixits for extra arguments in nullary call #16652

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

Closed
wants to merge 1 commit into from
Closed

[Diagnostics] Fixits for extra arguments in nullary call #16652

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
24 changes: 13 additions & 11 deletions lib/Sema/CSDiag.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4101,19 +4101,21 @@ class ArgumentMatcher : public MatchCallArgumentListener {
TC.diagnose(loc, diag::extra_trailing_closure_in_call)
.highlight(arg->getSourceRange());
else if (Parameters.empty()) {
auto Paren = dyn_cast<ParenExpr>(ArgExpr);
Expr *SubExpr = nullptr;
if (Paren) {
SubExpr = Paren->getSubExpr();
}
auto diag = TC.diagnose(loc, diag::extra_argument_to_nullary_call);
diag.highlight(ArgExpr->getSourceRange());

auto lastSym =
Lexer::getCharSourceRangeFromSourceRange(TC.Context.SourceMgr,
ArgExpr->getEndLoc());
auto rightParen = getTokenText(tok::r_paren);

if (SubExpr && CandidateInfo.CS.getType(SubExpr) &&
CandidateInfo.CS.getType(SubExpr)->isVoid()) {
TC.diagnose(loc, diag::extra_argument_to_nullary_call)
.fixItRemove(SubExpr->getSourceRange());
if (TC.Context.SourceMgr.extractText(lastSym) == rightParen) {
diag.fixItRemoveChars(ArgExpr->getStartLoc().getAdvancedLoc(1),
ArgExpr->getEndLoc());
} else {
TC.diagnose(loc, diag::extra_argument_to_nullary_call)
.highlight(ArgExpr->getSourceRange());
diag.fixItInsertAfter(ArgExpr->getEndLoc(), rightParen);
diag.fixItRemove(SourceRange(ArgExpr->getStartLoc().getAdvancedLoc(1),
ArgExpr->getEndLoc()));
}
} else if (name.empty())
TC.diagnose(loc, diag::extra_argument_positional)
Expand Down
4 changes: 2 additions & 2 deletions test/Constraints/diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ class CurriedClass {

let c = CurriedClass()
_ = c.method1
c.method1(1) // expected-error {{argument passed to call that takes no arguments}}
c.method1(1) // expected-error {{argument passed to call that takes no arguments}} {{11-12=}}
_ = c.method2(1)
_ = c.method2(1.0) // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
c.method2(1)(2)
Expand Down Expand Up @@ -428,7 +428,7 @@ let _: (Int) -> (Int, Color) = { ($0, .Unknown("")) } // expected-error {{missin
let _: Color = .Unknown("") // expected-error {{missing argument label 'description:' in call}} {{25-25=description: }}
let _: Color = .Unknown // expected-error {{member 'Unknown' expects argument of type '(description: String)'}}
let _: Color = .Unknown(42) // expected-error {{missing argument label 'description:' in call}}
let _ : Color = .rainbow(42) // expected-error {{argument passed to call that takes no arguments}}
let _ : Color = .rainbow(42) // expected-error {{argument passed to call that takes no arguments}} {{26-28=}}
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, but IMO, we should not remove users code like this. This might be just a mistype of function name.
Removing () is implemented in #9583 because of migration need and it probably reflects users intent.


let _ : (Int, Float) = (42.0, 12) // expected-error {{cannot convert value of type 'Double' to specified type 'Int'}}

Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/tuple_arguments.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,7 @@ do {
f(a: logNoOptional() as ((()) -> Void)) // expected-error {{cannot convert value of type '(()) -> Void' to expected argument type '(() -> Void)?'}}

func g() {}
g(()) // expected-error {{argument passed to call that takes no arguments}}
g(()) // expected-error {{argument passed to call that takes no arguments}} {{5-7=}}
Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Over here, the error is raised and the test passes.


func h(_: ()) {} // expected-note {{'h' declared here}}
h() // expected-error {{missing argument for parameter #1 in call}}
Expand Down
10 changes: 10 additions & 0 deletions test/decl/func/functions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ func parenPatternInArg((a): Int) -> Int { // expected-error {{expected parameter
}
parenPatternInArg(0) // expected-error {{argument passed to call that takes no arguments}}

func nullaryFunctionFixit() {}
nullaryFunctionFixit(())
Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Here, however, the same error has to be raised, but it doesn't happen. When I run the compiler, it does raise an error. When I run the test, it passes without expecting any error.


nullaryFunctionFixit(0, 0, "hello")
// expected-error@-1 {{argument passed to call that takes no arguments}} {{22-35=}}
nullaryFunctionFixit(0, 0, "hello"
// expected-error@-1 {{argument passed to call that takes no arguments}} {{22-35=}} {{35-35=)}}
// expected-note@-2 {{to match this opening '('}}

// expected-error@+1 {{expected ')' in expression list}}
var nullaryClosure: (Int) -> Int = {_ in 0}
_ = nullaryClosure(0)

Expand Down