[QoI] Improve the fix-it for out-of-order arguments
Change the fix-it to move the argument to its correct location in one go.
This happens by removing it from one location and inserting it in the other
(as opposed to the original implementation which swapped one argument with
the preceding one). The commas separating the arguments are adjusted
to match the moved argument.
Add new tests for reordering regular arguments, variadic arguments, and function arguments.
Resolves: SR-4715 rdar://problem/31849281
(cherry picked from commit 00833a234309103dba19d487567cf6fe5d510528)
diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp
index 678f636..1244b36 100644
--- a/lib/Sema/CSDiag.cpp
+++ b/lib/Sema/CSDiag.cpp
@@ -5302,22 +5302,43 @@
SourceLoc diagLoc = firstRange.Start;
+ auto addFixIts = [&](InFlightDiagnostic diag) {
+ diag.highlight(firstRange).highlight(secondRange);
+
+ // Move the misplaced argument by removing it from one location and
+ // inserting it in another location. To maintain argument comma
+ // separation, since the argument is always moving to an earlier index
+ // the preceding comma and whitespace is removed and a new trailing
+ // comma and space is inserted with the moved argument.
+ auto &SM = TC.Context.SourceMgr;
+ auto text = SM.extractText(
+ Lexer::getCharSourceRangeFromSourceRange(SM, firstRange));
+
+ auto removalRange =
+ SourceRange(Lexer::getLocForEndOfToken(
+ SM, tuple->getElement(argIdx - 1)->getEndLoc()),
+ firstRange.End);
+ diag.fixItRemove(removalRange);
+ diag.fixItInsert(secondRange.Start, text.str() + ", ");
+ };
+
+ // There are 4 diagnostic messages variations depending on
+ // labeled/unlabeled arguments.
if (first.empty() && second.empty()) {
- TC.diagnose(diagLoc, diag::argument_out_of_order_unnamed_unnamed,
- argIdx + 1, prevArgIdx + 1)
- .fixItExchange(firstRange, secondRange);
+ addFixIts(TC.diagnose(diagLoc,
+ diag::argument_out_of_order_unnamed_unnamed,
+ argIdx + 1, prevArgIdx + 1));
} else if (first.empty() && !second.empty()) {
- TC.diagnose(diagLoc, diag::argument_out_of_order_unnamed_named,
- argIdx + 1, second)
- .fixItExchange(firstRange, secondRange);
+ addFixIts(TC.diagnose(diagLoc,
+ diag::argument_out_of_order_unnamed_named,
+ argIdx + 1, second));
} else if (!first.empty() && second.empty()) {
- TC.diagnose(diagLoc, diag::argument_out_of_order_named_unnamed, first,
- prevArgIdx + 1)
- .fixItExchange(firstRange, secondRange);
+ addFixIts(TC.diagnose(diagLoc,
+ diag::argument_out_of_order_named_unnamed, first,
+ prevArgIdx + 1));
} else {
- TC.diagnose(diagLoc, diag::argument_out_of_order_named_named, first,
- second)
- .fixItExchange(firstRange, secondRange);
+ addFixIts(TC.diagnose(diagLoc, diag::argument_out_of_order_named_named,
+ first, second));
}
Diagnosed = true;
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
index 12b19e9..9d9f08c 100644
--- a/lib/Sema/CSSimplify.cpp
+++ b/lib/Sema/CSSimplify.cpp
@@ -445,50 +445,46 @@
// If any arguments were provided out-of-order, check whether we have
// violated any of the reordering rules.
if (potentiallyOutOfOrder) {
- // Build a mapping from arguments to parameters.
- SmallVector<unsigned, 4> argumentBindings(numArgs);
- for (paramIdx = 0; paramIdx != numParams; ++paramIdx) {
- for (auto argIdx : parameterBindings[paramIdx])
- argumentBindings[argIdx] = paramIdx;
- }
-
- // Walk through the arguments, determining if any were bound to parameters
- // out-of-order where it is not permitted.
- unsigned prevParamIdx = argumentBindings[0];
- for (unsigned argIdx = 1; argIdx != numArgs; ++argIdx) {
- unsigned paramIdx = argumentBindings[argIdx];
-
- // If this argument binds to the same parameter as the previous one or to
- // a later parameter, just update the parameter index.
- if (paramIdx >= prevParamIdx) {
- prevParamIdx = paramIdx;
- continue;
- }
-
- unsigned prevArgIdx = parameterBindings[prevParamIdx].front();
-
- // First let's double check if out-of-order argument is nothing
- // more than a simple label mismatch, because in situation where
- // one argument requires label and another one doesn't, but caller
- // doesn't provide either, problem is going to be identified as
- // out-of-order argument instead of label mismatch.
- auto ¶meter = params[prevArgIdx];
- if (parameter.hasLabel()) {
- auto expectedLabel = parameter.Label;
- auto argumentLabel = args[argIdx].Label;
-
- // If there is a label but it's incorrect it can only mean
- // situation like this: expected (x, _ y) got (y, _ x).
- if (argumentLabel.empty() ||
- (expectedLabel.compare(argumentLabel) != 0 &&
- args[prevArgIdx].Label.empty())) {
- listener.missingLabel(prevArgIdx);
- return true;
+ unsigned argIdx = 0;
+ // Enumerate the parameters and their bindings to see if any arguments are
+ // our of order
+ for (auto binding : parameterBindings) {
+ for (auto boundArgIdx : binding) {
+ if (boundArgIdx == argIdx) {
+ // If the argument is in the right location, just continue
+ argIdx++;
+ continue;
}
- }
- listener.outOfOrderArgument(argIdx, prevArgIdx);
- return true;
+ // Otherwise, we've found the (first) parameter that has an out of order
+ // argument, and know the indices of the argument the needs to move
+ // (fromArgIdx) and the argument location it should move to (toArgItd).
+ auto fromArgIdx = boundArgIdx;
+ auto toArgIdx = argIdx;
+
+ // First let's double check if out-of-order argument is nothing
+ // more than a simple label mismatch, because in situation where
+ // one argument requires label and another one doesn't, but caller
+ // doesn't provide either, problem is going to be identified as
+ // out-of-order argument instead of label mismatch.
+ auto ¶meter = params[toArgIdx];
+ if (parameter.hasLabel()) {
+ auto expectedLabel = parameter.Label;
+ auto argumentLabel = args[fromArgIdx].Label;
+
+ // If there is a label but it's incorrect it can only mean
+ // situation like this: expected (x, _ y) got (y, _ x).
+ if (argumentLabel.empty() ||
+ (expectedLabel.compare(argumentLabel) != 0 &&
+ !args[toArgIdx].hasLabel())) {
+ listener.missingLabel(toArgIdx);
+ return true;
+ }
+ }
+
+ listener.outOfOrderArgument(fromArgIdx, toArgIdx);
+ return true;
+ }
}
}
diff --git a/test/ClangImporter/objc_parse.swift b/test/ClangImporter/objc_parse.swift
index 5781c2c..a0838fc 100644
--- a/test/ClangImporter/objc_parse.swift
+++ b/test/ClangImporter/objc_parse.swift
@@ -44,7 +44,7 @@
}
// Renaming of redundant parameters.
- b.performAdd(1, withValue:2, withValue2:3, withValue:4) // expected-error{{argument 'withValue' must precede argument 'withValue2'}}
+ b.performAdd(1, withValue:2, withValue2:3, withValue:4) // expected-error{{argument 'withValue' must precede argument 'withValue2'}} {{32-32=withValue:4, }} {{44-57=}}
b.performAdd(1, withValue:2, withValue:4, withValue2: 3)
b.performAdd(1, 2, 3, 4) // expected-error{{missing argument labels 'withValue:withValue:withValue2:' in call}} {{19-19=withValue: }} {{22-22=withValue: }} {{25-25=withValue2: }}
diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift
index e4641e6..edf723b 100644
--- a/test/Constraints/diagnostics.swift
+++ b/test/Constraints/diagnostics.swift
@@ -816,14 +816,14 @@
r27212391(3, 5) // expected-error {{missing argument label 'x' in call}}
r27212391(3, y: 5) // expected-error {{missing argument label 'x' in call}}
-r27212391(3, x: 5) // expected-error {{argument 'x' must precede unnamed argument #1}}
-r27212391(y: 3, x: 5) // expected-error {{argument 'x' must precede argument 'y'}}
+r27212391(3, x: 5) // expected-error {{argument 'x' must precede unnamed argument #1}} {{11-11=x: 5, }} {{12-18=}}
+r27212391(y: 3, x: 5) // expected-error {{argument 'x' must precede argument 'y'}} {{11-11=x: 5, }} {{15-21=}}
r27212391(y: 3, 5) // expected-error {{incorrect argument label in call (have 'y:_:', expected 'x:_:')}}
r27212391(x: 3, x: 5) // expected-error {{extraneous argument label 'x:' in call}}
r27212391(a: 1, 3, y: 5) // expected-error {{missing argument label 'x' in call}}
r27212391(1, x: 3, y: 5) // expected-error {{missing argument label 'a' in call}}
-r27212391(a: 1, y: 3, x: 5) // expected-error {{argument 'x' must precede argument 'y'}}
-r27212391(a: 1, 3, x: 5) // expected-error {{argument 'x' must precede unnamed argument #2}}
+r27212391(a: 1, y: 3, x: 5) // expected-error {{argument 'x' must precede argument 'y'}} {{17-17=x: 5, }} {{21-27=}}
+r27212391(a: 1, 3, x: 5) // expected-error {{argument 'x' must precede unnamed argument #2}} {{17-17=x: 5, }} {{18-24=}}
// SR-1255
func foo1255_1() {
@@ -1055,3 +1055,28 @@
// expected-error@-1 {{value of type 'Expr_28456467' has no member 'hasStateDef'}}
}
}
+
+// rdar://problem/31849281 - Let's play "bump the argument"
+
+struct rdar31849281 { var foo, a, b, c: Int }
+_ = rdar31849281(a: 101, b: 102, c: 103, foo: 104) // expected-error {{argument 'foo' must precede argument 'a'}} {{18-18=foo: 104, }} {{40-50=}}
+
+_ = rdar31849281(a: 101, c: 103, b: 102, foo: 104) // expected-error {{argument 'foo' must precede argument 'a'}} {{18-18=foo: 104, }} {{40-50=}}
+_ = rdar31849281(foo: 104, a: 101, c: 103, b: 102) // expected-error {{argument 'b' must precede argument 'c'}} {{36-36=b: 102, }} {{42-50=}}
+
+_ = rdar31849281(b: 102, c: 103, a: 101, foo: 104) // expected-error {{argument 'foo' must precede argument 'b'}} {{18-18=foo: 104, }} {{40-50=}}
+_ = rdar31849281(foo: 104, b: 102, c: 103, a: 101) // expected-error {{argument 'a' must precede argument 'b'}} {{28-28=a: 101, }} {{42-50=}}
+
+func var_31849281(_ a: Int, _ b: Int..., c: Int) {}
+var_31849281(1, c: 10, 3, 4, 5, 6, 7, 8, 9) // expected-error {{unnamed argument #3 must precede argument 'c'}} {{17-17=3, 4, 5, 6, 7, 8, 9, }} {{22-43=}}
+
+func fun_31849281(a: (Bool) -> Bool, b: (Int) -> (String), c: [Int?]) {}
+fun_31849281(c: [nil, 42], a: { !$0 }, b: { (num: Int) -> String in return "\(num)" })
+// expected-error @-1 {{argument 'a' must precede argument 'c'}} {{14-14=a: { !$0 }, }} {{26-38=}}
+fun_31849281(a: { !$0 }, c: [nil, 42], b: { (num: Int) -> String in return String(describing: num) })
+// expected-error @-1 {{argument 'b' must precede argument 'c'}} {{26-26=b: { (num: Int) -> String in return String(describing: num) }, }} {{38-101=}}
+fun_31849281(a: { !$0 }, c: [nil, 42], b: { "\($0)" })
+// expected-error @-1 {{argument 'b' must precede argument 'c'}} {{26-26=b: { "\\($0)" }, }} {{38-54=}}
+
+func f_31849281(x: Int, y: Int, z: Int) {}
+f_31849281(42, y: 10, x: 20) // expected-error {{argument 'x' must precede unnamed argument #1}} {{12-12=x: 20, }} {{21-28=}}
diff --git a/test/Constraints/function.swift b/test/Constraints/function.swift
index d6e661c..7610786 100644
--- a/test/Constraints/function.swift
+++ b/test/Constraints/function.swift
@@ -36,7 +36,7 @@
// <rdar://problem/17652759> Default arguments cause crash with tuple permutation
func testArgumentShuffle(_ first: Int = 7, third: Int = 9) {
}
-testArgumentShuffle(third: 1, 2) // expected-error {{unnamed argument #2 must precede argument 'third'}} {{21-29=2}} {{31-32=third: 1}}
+testArgumentShuffle(third: 1, 2) // expected-error {{unnamed argument #2 must precede argument 'third'}} {{21-21=2, }} {{29-32=}}
diff --git a/test/Constraints/keyword_arguments.swift b/test/Constraints/keyword_arguments.swift
index 48a35e4..8cf9760 100644
--- a/test/Constraints/keyword_arguments.swift
+++ b/test/Constraints/keyword_arguments.swift
@@ -82,7 +82,7 @@
// -------------------------------------------
// Out-of-order keywords
// -------------------------------------------
-allkeywords1(y: 1, x: 2) // expected-error{{argument 'x' must precede argument 'y'}} {{14-18=x: 2}} {{20-24=y: 1}}
+allkeywords1(y: 1, x: 2) // expected-error{{argument 'x' must precede argument 'y'}} {{14-14=x: 2, }} {{18-24=}}
// -------------------------------------------
// Default arguments
@@ -102,9 +102,9 @@
defargs1(x: 1, z: 3)
// Using defaults (out-of-order, error by SE-0060)
-defargs1(z: 3, y: 2, x: 1) // expected-error{{argument 'y' must precede argument 'z'}} {{10-14=y: 2}} {{16-20=z: 3}}
-defargs1(x: 1, z: 3, y: 2) // expected-error{{argument 'y' must precede argument 'z'}} {{16-20=y: 2}} {{22-26=z: 3}}
-defargs1(y: 2, x: 1) // expected-error{{argument 'x' must precede argument 'y'}} {{10-14=x: 1}} {{16-20=y: 2}}
+defargs1(z: 3, y: 2, x: 1) // expected-error{{argument 'x' must precede argument 'z'}} {{10-10=x: 1, }} {{20-26=}}
+defargs1(x: 1, z: 3, y: 2) // expected-error{{argument 'y' must precede argument 'z'}} {{16-16=y: 2, }} {{20-26=}}
+defargs1(y: 2, x: 1) // expected-error{{argument 'x' must precede argument 'y'}} {{10-10=x: 1, }} {{14-20=}}
// Default arguments "boxed in".
func defargs2(first: Int, x: Int = 1, y: Int = 2, z: Int = 3, last: Int) { }
@@ -116,12 +116,12 @@
defargs2(first: 1, last: 4)
// Using defaults in the middle (out-of-order, error by SE-0060)
-defargs2(first: 1, z: 3, x: 1, last: 4) // expected-error{{argument 'x' must precede argument 'z'}} {{20-24=x: 1}} {{26-30=z: 3}}
-defargs2(first: 1, z: 3, y: 2, last: 4) // expected-error{{argument 'y' must precede argument 'z'}} {{20-24=y: 2}} {{26-30=z: 3}}
+defargs2(first: 1, z: 3, x: 1, last: 4) // expected-error{{argument 'x' must precede argument 'z'}} {{20-20=x: 1, }} {{24-30=}}
+defargs2(first: 1, z: 3, y: 2, last: 4) // expected-error{{argument 'y' must precede argument 'z'}} {{20-20=y: 2, }} {{24-30=}}
// Using defaults that have moved past a non-defaulted parameter
-defargs2(x: 1, first: 1, last: 4) // expected-error{{argument 'first' must precede argument 'x'}} {{10-14=first: 1}} {{16-24=x: 1}}
-defargs2(first: 1, last: 4, x: 1) // expected-error{{argument 'x' must precede argument 'last'}} {{20-27=x: 1}} {{29-33=last: 4}}
+defargs2(x: 1, first: 1, last: 4) // expected-error{{argument 'first' must precede argument 'x'}} {{10-10=first: 1, }} {{14-24=}}
+defargs2(first: 1, last: 4, x: 1) // expected-error{{argument 'x' must precede argument 'last'}} {{20-20=x: 1, }} {{27-33=}}
// -------------------------------------------
// Variadics
@@ -135,7 +135,7 @@
variadics1(x: 1, y: 2, 1, 2, 3)
// Using various (out-of-order)
-variadics1(1, 2, 3, 4, 5, x: 6, y: 7) // expected-error{{argument 'x' must precede unnamed argument #1}} {{12-25=x: 6}} {{27-31=1, 2, 3, 4, 5}}
+variadics1(1, 2, 3, 4, 5, x: 6, y: 7) // expected-error{{argument 'x' must precede unnamed argument #1}} {{12-12=x: 6, }} {{25-31=}}
func variadics2(x: Int, y: Int = 2, z: Int...) { } // expected-note {{'variadics2(x:y:z:)' declared here}}
@@ -150,7 +150,7 @@
// Using variadics (out-of-order)
variadics2(z: 1, 2, 3, y: 2) // expected-error{{missing argument for parameter 'x' in call}}
-variadics2(z: 1, 2, 3, x: 1) // expected-error{{argument 'x' must precede argument 'z'}} {{12-22=x: 1}} {{24-28=z: 1, 2, 3}}
+variadics2(z: 1, 2, 3, x: 1) // expected-error{{argument 'x' must precede argument 'z'}} {{12-12=x: 1, }} {{22-28=}}
func variadics3(_ x: Int..., y: Int = 2, z: Int = 3) { }
@@ -173,8 +173,8 @@
variadics3()
// Using variadics (out-of-order)
-variadics3(y: 0, 1, 2, 3) // expected-error{{unnamed argument #2 must precede argument 'y'}} {{12-16=1, 2, 3}} {{18-25=y: 0}}
-variadics3(z: 1, 1) // expected-error{{unnamed argument #2 must precede argument 'z'}} {{12-16=1}} {{18-19=z: 1}}
+variadics3(y: 0, 1, 2, 3) // expected-error{{unnamed argument #2 must precede argument 'y'}} {{12-12=1, 2, 3, }} {{16-25=}}
+variadics3(z: 1, 1) // expected-error{{unnamed argument #2 must precede argument 'z'}} {{12-12=1, }} {{16-19=}}
func variadics4(x: Int..., y: Int = 2, z: Int = 3) { }
@@ -198,7 +198,7 @@
// Using variadics (in-order, some missing)
variadics4(y: 0, x: 1, 2, 3) // expected-error{{extra argument in call}}
-variadics4(z: 1, x: 1) // expected-error{{argument 'x' must precede argument 'z'}} {{12-16=x: 1}} {{18-22=z: 1}}
+variadics4(z: 1, x: 1) // expected-error{{argument 'x' must precede argument 'z'}} {{12-12=x: 1, }} {{16-22=}}
func variadics5(_ x: Int, y: Int, _ z: Int...) { } // expected-note {{'variadics5(_:y:_:)' declared here}}
@@ -209,7 +209,7 @@
variadics5(1, y: 2, 1, 2, 3)
// Using various (out-of-order)
-variadics5(1, 2, 3, 4, 5, 6, y: 7) // expected-error{{argument 'y' must precede unnamed argument #2}} {{15-28=y: 7}} {{30-34=2, 3, 4, 5, 6}}
+variadics5(1, 2, 3, 4, 5, 6, y: 7) // expected-error{{argument 'y' must precede unnamed argument #2}} {{15-15=y: 7, }} {{28-34=}}
variadics5(y: 1, 2, 3, 4, 5, 6, 7) // expected-error{{missing argument for parameter #1 in call}}
func variadics6(x: Int..., y: Int = 2, z: Int) { } // expected-note 4 {{'variadics6(x:y:z:)' declared here}}
@@ -233,7 +233,7 @@
variadics6() // expected-error{{missing argument for parameter 'z' in call}}
func outOfOrder(_ a : Int, b: Int) {
- outOfOrder(b: 42, 52) // expected-error {{unnamed argument #2 must precede argument 'b'}} {{14-19=52}} {{21-23=b: 42}}
+ outOfOrder(b: 42, 52) // expected-error {{unnamed argument #2 must precede argument 'b'}} {{14-14=52, }} {{19-23=}}
}
// -------------------------------------------
