Skip to content

Commit 3ad108b

Browse files
lattnerlattner
committed
Reapply r29419:
Enhance fixItRemove() to be a bit more careful about what whitespace it leaves around: if the thing it is removing has leading and trailing whitespace already, this nukes an extra space to avoid leaving double spaces or incorrectly indented results. This includes an extra fix for looking off the start of a buffer, which extractText doesn't and can't handle. This fixes <rdar://problem/21045509> Fixit deletes 'let' from non-binding 'if case let' statements, but leaves an extra space Swift SVN r29449
1 parent 9deaf6c commit 3ad108b

19 files changed

+155
-122
lines changed

‎include/swift/AST/DiagnosticEngine.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -380,10 +380,8 @@ namespace swift {
380380

381381
/// \brief Add a token-based removal fix-it to the currently-active
382382
/// diagnostic.
383-
InFlightDiagnostic &fixItRemove(SourceRange R) {
384-
return fixItReplace(R, {});
385-
}
386-
383+
InFlightDiagnostic &fixItRemove(SourceRange R);
384+
387385
/// \brief Add a character-based removal fix-it to the currently-active
388386
/// diagnostic.
389387
InFlightDiagnostic &fixItRemoveChars(SourceLoc Start, SourceLoc End) {

‎lib/AST/DiagnosticEngine.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,50 @@ InFlightDiagnostic &InFlightDiagnostic::fixItInsertAfter(SourceLoc L,
103103
return fixItInsert(L, Str);
104104
}
105105

106+
/// \brief Add a token-based removal fix-it to the currently-active
107+
/// diagnostic.
108+
InFlightDiagnostic &InFlightDiagnostic::fixItRemove(SourceRange R) {
109+
assert(IsActive && "Cannot modify an inactive diagnostic");
110+
if (R.isInvalid() || !Engine) return *this;
111+
112+
// Convert from a token range to a CharSourceRange, which points to the end of
113+
// the token we want to remove.
114+
auto &SM = Engine->SourceMgr;
115+
auto charRange = toCharSourceRange(SM, R);
116+
117+
// If we're removing something (e.g. a keyword), do a bit of extra work to
118+
// make sure that we leave the code in a good place, without extraneous white
119+
// space around its hole. Specifically, check to see there is whitespace
120+
// before and after the end of range. If so, nuke the space afterward to keep
121+
// things consistent.
122+
if (SM.extractText({charRange.getEnd(), 1}) == " ") {
123+
// Check before the string, we have to be careful not to go off the front of
124+
// the buffer.
125+
auto bufferRange =
126+
SM.getRangeForBuffer(SM.findBufferContainingLoc(charRange.getStart()));
127+
bool ShouldRemove = false;
128+
if (bufferRange.getStart() == charRange.getStart())
129+
ShouldRemove = true;
130+
else {
131+
auto beforeChars =
132+
SM.extractText({charRange.getStart().getAdvancedLoc(-1), 1});
133+
ShouldRemove = !beforeChars.empty() && isspace(beforeChars[0]);
134+
}
135+
if (ShouldRemove) {
136+
charRange = CharSourceRange(charRange.getStart(),
137+
charRange.getByteLength()+1);
138+
}
139+
}
140+
Engine->getActiveDiagnostic().addFixIt(Diagnostic::FixIt(charRange, {}));
141+
return *this;
142+
}
143+
144+
106145
InFlightDiagnostic &InFlightDiagnostic::fixItReplace(SourceRange R,
107146
StringRef Str) {
147+
if (Str.empty())
148+
return fixItRemove(R);
149+
108150
assert(IsActive && "Cannot modify an inactive diagnostic");
109151
if (Engine && R.isValid())
110152
Engine->getActiveDiagnostic().addFixIt(

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,18 +1262,8 @@ void swift::fixItAccessibility(InFlightDiagnostic &diag, ValueDecl *VD,
12621262
if (isForSetter && VD->getFormalAccess() == desiredAccess) {
12631263
assert(attr);
12641264
attr->setInvalid();
1265-
if (!attr->Range.isValid())
1266-
return;
1267-
1268-
// Remove the setter attribute along with a possible single trailing space.
1269-
SourceManager &sourceMgr = VD->getASTContext().SourceMgr;
1270-
SourceLoc nextCharLoc = Lexer::getLocForEndOfToken(sourceMgr,
1271-
attr->Range.End);
1272-
StringRef nextChar = sourceMgr.extractText({ nextCharLoc, 1 });
1273-
if (nextChar == " ")
1274-
diag.fixItRemoveChars(attr->Range.Start, nextCharLoc.getAdvancedLoc(1));
1275-
else
1276-
diag.fixItRemove(attr->Range);
1265+
// Remove the setter attribute.
1266+
diag.fixItRemove(attr->Range);
12771267

12781268
} else if (attr) {
12791269
// This uses getLocation() instead of getRange() because we don't want to

‎test/Parse/availability_query.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ if #available(OSX 10.10 || iOS 8.0) {// expected-error {{'||' cannot be used in
8787

8888
// Emit Fix-It removing un-needed >=, for the moment.
8989

90-
if #available(OSX >= 10.10, *) { // expected-error {{version comparison not needed}} {{19-21=}}
90+
if #available(OSX >= 10.10, *) { // expected-error {{version comparison not needed}} {{19-22=}}
9191
}
9292

9393
// <rdar://problem/20904820> Following a "let" condition with #available is incorrectly rejected

‎test/Parse/invalid.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func foo() {
1616
skview! // expected-error 2{{expected ',' separator}} expected-error {{use of unresolved identifier 'skview'}}
1717
} // expected-error {{expected expression in list of expressions}} expected-error {{expected ')' in expression list}}
1818

19-
func test3(a: inout Int) {} // expected-error {{'inout' must appear before the parameter name}}{{12-12=inout }}{{15-20=}}
19+
func test3(a: inout Int) {} // expected-error {{'inout' must appear before the parameter name}}{{12-12=inout }}{{15-21=}}
2020

2121
super.init() // expected-error {{'super' cannot be used outside of class members}} expected-error {{initializers may only be declared within a type}}
2222

‎test/Parse/pattern_without_variables.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ enum SimpleEnum { case Bar }
2222

2323
func testVarLetPattern(a : SimpleEnum) {
2424
switch a {
25-
case let .Bar: break // expected-warning {{'let' pattern has no effect; sub-pattern didn't bind any variables}}
25+
case let .Bar: break // expected-warning {{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{8-12=}}
2626
}
2727
switch a {
2828
case let x: _ = x; break // Ok.
@@ -33,4 +33,7 @@ func testVarLetPattern(a : SimpleEnum) {
3333
switch (a, 42) {
3434
case let (_, x): _ = x; break // ok
3535
}
36+
37+
// expected-warning @+1 {{'if' condition is always true}}
38+
if case let _ = "str" {} // expected-warning {{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}}
3639
}

‎test/Parse/recovery.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func test(a: BadAttributes) -> () {
3939
}
4040

4141
// Here is an extra random close-brace!
42-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
42+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
4343

4444

4545
//===--- Recovery for braced blocks.

‎test/Parse/recovery_library.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,19 @@
44
//===--- Keep this test the first one in the file.
55

66
// Check that we handle multiple consecutive right braces.
7-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
8-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
7+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
8+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
99

1010
func foo() {}
1111
// Check that we handle multiple consecutive right braces.
12-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
13-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
12+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
13+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
1414
func bar() {}
1515

1616
//===--- Recovery for extra braces at top level.
1717
//===--- Keep this test the last one in the file.
1818

1919
// Check that we handle multiple consecutive right braces.
20-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
21-
} // expected-error{{extraneous '}' at top level}} {{1-2=}}
20+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
21+
} // expected-error{{extraneous '}' at top level}} {{1-3=}}
2222

‎test/Parse/trailing-semi.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ struct S {
77
}
88

99
struct SpuriousSemi {
10-
; // expected-error{{unexpected ';' separator}} {{3-4=}}
10+
; // expected-error{{unexpected ';' separator}} {{3-5=}}
1111
var a : Int ; ; // FIXME -- we need to consistently track ','/';' separators
1212
func b () {};
1313
; static func c () {} // FIXME -- we need to consistently track ','/';' separators

‎test/Sema/accessibility.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ private extension PublicStruct {
6464
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}}
6565
private func extImplPrivate() {}
6666
}
67-
public extension InternalStruct { // expected-error {{extension of internal struct cannot be declared public}} {{1-7=}}
67+
public extension InternalStruct { // expected-error {{extension of internal struct cannot be declared public}} {{1-8=}}
6868
public func extMemberPublic() {} // expected-warning {{declaring a public instance method for an internal struct}} {{3-9=internal}}
6969
private func extImplPublic() {}
7070
}
@@ -76,11 +76,11 @@ private extension InternalStruct {
7676
public func extMemberPrivate() {} // expected-warning {{declaring a public instance method in a private extension}}
7777
private func extImplPrivate() {}
7878
}
79-
public extension PrivateStruct { // expected-error {{extension of private struct cannot be declared public}} {{1-7=}}
79+
public extension PrivateStruct { // expected-error {{extension of private struct cannot be declared public}} {{1-8=}}
8080
public func extMemberPublic() {} // expected-warning {{declaring a public instance method for a private struct}} {{3-9=private}}
8181
private func extImplPublic() {}
8282
}
83-
internal extension PrivateStruct { // expected-error {{extension of private struct cannot be declared internal}} {{1-9=}}
83+
internal extension PrivateStruct { // expected-error {{extension of private struct cannot be declared internal}} {{1-10=}}
8484
public func extMemberInternal() {} // expected-warning {{declaring a public instance method in an internal extension}}
8585
private func extImplInternal() {}
8686
}

‎test/attr/attr_autoclosure.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ func func8(@autoclosure inout x: () -> Bool) -> Bool { // expected-error {{@aut
2424

2525

2626
// Should have good QoI:
27-
func migrate1(fp fpx : @autoclosure () -> Int) {} // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{15-15=@autoclosure }} {{24-36=}}
27+
func migrate1(fp fpx : @autoclosure () -> Int) {} // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{15-15=@autoclosure }} {{24-37=}}
2828
struct MethodHolder {
29-
func migrate2(a : Int, _ fp : @autoclosure () -> Int) {} // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{26-26=@autoclosure }} {{33-45=}}
29+
func migrate2(a : Int, _ fp : @autoclosure () -> Int) {} // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{26-26=@autoclosure }} {{33-46=}}
3030
}
31-
func migrate3(fp fp : @autoclosure () -> Int) {} // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{15-15=@autoclosure }} {{23-35=}}
31+
func migrate3(fp fp : @autoclosure () -> Int) {} // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{15-15=@autoclosure }} {{23-36=}}
3232
public func || <T: BooleanType>(
33-
lhs: T, rhs: @autoclosure () -> Bool // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{11-11=@autoclosure }} {{16-28=}}
33+
lhs: T, rhs: @autoclosure () -> Bool // expected-error {{@autoclosure is now an attribute of the parameter declaration, not its type}} {{11-11=@autoclosure }} {{16-29=}}
3434
) -> Bool {
3535
return lhs.boolValue ? true : rhs().boolValue
3636
}

‎test/attr/attr_noreturn.swift

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,37 +3,37 @@
33
@noreturn
44
func exit(_: Int) {}
55

6-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
6+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
77
class InvalidOnClass {}
88

9-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
9+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
1010
struct InvalidOnStruct {}
1111

12-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
12+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
1313
enum InvalidOnEnum {}
1414

15-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
15+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
1616
protocol InvalidOnProtocol {}
1717

1818
struct InvalidOnExtension {}
1919

2020
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}
2121
extension InvalidOnExtension {}
2222

23-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
23+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
2424
var invalidOnVar = 0
2525

26-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
26+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
2727
let invalidOnLet = 0
2828

2929
class InvalidOnClassMembers {
30-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{3-12=}}
30+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{3-13=}}
3131
init() {}
3232

33-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{3-12=}}
33+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{3-13=}}
3434
deinit {}
3535

36-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{3-12=}}
36+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{3-13=}}
3737
subscript(i: Int) -> Int {
3838
get {
3939
return 0
@@ -128,7 +128,7 @@ func testRvalue(lhs: (), rhs: @noreturn () -> ()) -> () {
128128

129129
var fnr: @noreturn (_: Int) -> () = exit
130130
// This might be a desirable syntax, but it does not get properly propagated to SIL, so reject it for now.
131-
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-10=}}
131+
@noreturn // expected-error {{@noreturn may only be used on 'func' declarations}}{{1-11=}}
132132
var fpr: (_: Int) -> () = exit
133133

134134
func testWitnessMethod<T: TestProtocol>(t: T) {

‎test/attr/attr_objc.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ protocol Protocol_Class2 : class {}
2727

2828
//===--- Subjects of @objc attribute.
2929

30-
@objc extension PlainClass { } // expected-error{{@objc cannot be applied to this declaration}}{{1-6=}}
30+
@objc extension PlainClass { } // expected-error{{@objc cannot be applied to this declaration}}{{1-7=}}
3131

3232
@objc
3333
var subject_globalVar: Int // expected-error {{only classes, protocols, methods, initializers, properties, and subscript declarations can be declared @objc}}

‎test/attr/attributes.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,9 @@ weak var weak16 : Class!
178178
@weak var weak17 : Class? // expected-error {{'weak' is a declaration modifier, not an attribute}}
179179

180180

181-
@exported var exportVar: Int // expected-error {{@exported may only be used on 'import' declarations}}{{1-10=}}
182-
@exported func exportFunc() {} // expected-error {{@exported may only be used on 'import' declarations}}{{1-10=}}
183-
@exported struct ExportStruct {} // expected-error {{@exported may only be used on 'import' declarations}}{{1-10=}}
181+
@exported var exportVar: Int // expected-error {{@exported may only be used on 'import' declarations}}{{1-11=}}
182+
@exported func exportFunc() {} // expected-error {{@exported may only be used on 'import' declarations}}{{1-11=}}
183+
@exported struct ExportStruct {} // expected-error {{@exported may only be used on 'import' declarations}}{{1-11=}}
184184

185185

186186
// Function result type attributes.

‎test/decl/func/static_func.swift

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,41 @@
11
// RUN: %target-parse-verify-swift
22

3-
static func gf1() {} // expected-error {{static methods may only be declared on a type}}{{1-7=}}
4-
class func gf2() {} // expected-error {{class methods may only be declared on a type}}{{1-6=}}
3+
static func gf1() {} // expected-error {{static methods may only be declared on a type}}{{1-8=}}
4+
class func gf2() {} // expected-error {{class methods may only be declared on a type}}{{1-7=}}
55

6-
override static func gf3() {} // expected-error {{static methods may only be declared on a type}}{{10-16=}}
7-
// expected-error@-1 {{'override' can only be specified on class members}}{{1-9=}}
8-
override class func gf4() {} // expected-error {{class methods may only be declared on a type}}{{10-15=}}
9-
// expected-error@-1 {{'override' can only be specified on class members}}{{1-9=}}
6+
override static func gf3() {} // expected-error {{static methods may only be declared on a type}}{{10-17=}}
7+
// expected-error@-1 {{'override' can only be specified on class members}}{{1-10=}}
8+
override class func gf4() {} // expected-error {{class methods may only be declared on a type}}{{10-16=}}
9+
// expected-error@-1 {{'override' can only be specified on class members}}{{1-10=}}
1010

11-
static override func gf5() {} // expected-error {{static methods may only be declared on a type}}{{1-7=}}
12-
// expected-error@-1 {{'override' can only be specified on class members}}{{8-16=}}
13-
class override func gf6() {} // expected-error {{class methods may only be declared on a type}}{{1-6=}}
14-
// expected-error@-1 {{'override' can only be specified on class members}}{{7-15=}}
11+
static override func gf5() {} // expected-error {{static methods may only be declared on a type}}{{1-8=}}
12+
// expected-error@-1 {{'override' can only be specified on class members}}{{8-17=}}
13+
class override func gf6() {} // expected-error {{class methods may only be declared on a type}}{{1-7=}}
14+
// expected-error@-1 {{'override' can only be specified on class members}}{{7-16=}}
1515

1616
static gf7() {} // expected-error {{expected declaration}} expected-error {{braced block of statements is an unused closure}} expected-error{{begin with a closure}} expected-note{{discard the result}} expected-error{{type of expression is ambiguous without more context}}
1717
class gf8() {} // expected-error {{expected '{' in class}} expected-error {{braced block of statements is an unused closure}} expected-error{{begin with a closure}} expected-note{{discard the result}} expected-error{{type of expression is ambiguous without more context}}
1818

1919
func inGlobalFunc() {
20-
static func gf1() {} // expected-error {{static methods may only be declared on a type}}{{3-9=}}
21-
class func gf2() {} // expected-error {{class methods may only be declared on a type}}{{3-8=}}
20+
static func gf1() {} // expected-error {{static methods may only be declared on a type}}{{3-10=}}
21+
class func gf2() {} // expected-error {{class methods may only be declared on a type}}{{3-9=}}
2222
}
2323

2424
struct InMemberFunc {
2525
func member() {
26-
static func gf1() {} // expected-error {{static methods may only be declared on a type}}{{5-11=}}
27-
class func gf2() {} // expected-error {{class methods may only be declared on a type}}{{5-10=}}
26+
static func gf1() {} // expected-error {{static methods may only be declared on a type}}{{5-12=}}
27+
class func gf2() {} // expected-error {{class methods may only be declared on a type}}{{5-11=}}
2828
}
2929
}
3030

3131
struct DuplicateStatic {
32-
static static func f1() {} // expected-error{{'static' specified twice}}{{10-16=}}
33-
static class func f2() {} // expected-error{{'class' specified twice}}{{10-15=}}
34-
class static func f3() {} // expected-error{{'static' specified twice}}{{9-15=}} expected-error{{class methods are only allowed within classes; use 'static' to declare a static method}}{{3-8=static}}
35-
class class func f4() {} // expected-error{{'class' specified twice}}{{9-14=}} expected-error{{class methods are only allowed within classes; use 'static' to declare a static method}}{{3-8=static}}
36-
override static static func f5() {} // expected-error{{'static' specified twice}}{{19-25=}} expected-error{{'override' can only be specified on class members}}
37-
static override static func f6() {} // expected-error{{'static' specified twice}}{{19-25=}} expected-error{{'override' can only be specified on class members}}
38-
static static override func f7() {} // expected-error{{'static' specified twice}}{{10-16=}} expected-error{{'override' can only be specified on class members}}
32+
static static func f1() {} // expected-error{{'static' specified twice}}{{10-17=}}
33+
static class func f2() {} // expected-error{{'class' specified twice}}{{10-16=}}
34+
class static func f3() {} // expected-error{{'static' specified twice}}{{9-16=}} expected-error{{class methods are only allowed within classes; use 'static' to declare a static method}}{{3-8=static}}
35+
class class func f4() {} // expected-error{{'class' specified twice}}{{9-15=}} expected-error{{class methods are only allowed within classes; use 'static' to declare a static method}}{{3-8=static}}
36+
override static static func f5() {} // expected-error{{'static' specified twice}}{{19-26=}} expected-error{{'override' can only be specified on class members}}
37+
static override static func f6() {} // expected-error{{'static' specified twice}}{{19-26=}} expected-error{{'override' can only be specified on class members}}
38+
static static override func f7() {} // expected-error{{'static' specified twice}}{{10-17=}} expected-error{{'override' can only be specified on class members}}
3939
static final func f8() {} // expected-error {{only classes and class members may be marked with 'final'}}
4040
}
4141

0 commit comments

Comments
 (0)