add fix-its for a bare regex literal with leading and/or trailing spaces

[AppAppWorks]

fix #1479

[ahoppen]

You should be able to remove the TODO in RegexLiteralLexer.swift:259 as well.

[AppAppWorks]

You should be able to remove the TODO in RegexLiteralLexer.swift:259 as well.

I've found two other TODOs in RegexLiteralLexer.swift:210-212 about .spaceAtEndOfRegexLiteral

[ahoppen]

I've found two other TODOs in RegexLiteralLexer.swift:210-212 about .spaceAtEndOfRegexLiteral

Good catch. You can remove them as well.

[ahoppen]

Looks good to me, I’d just like to see the following two addressed:

• add fix-its for a bare regex literal with leading and/or trailing spaces #2744 (comment)
• add fix-its for a bare regex literal with leading and/or trailing spaces #2744 (comment)

[AppAppWorks]

I've found two other TODOs in RegexLiteralLexer.swift:210-212 about .spaceAtEndOfRegexLiteral

Good catch. You can remove them as well.

// TODO: We ought to have a fix-it that suggests #/.../#. We could
// suggest escaping, but that would be wrong if the user has written (?x).
// TODO: Should we suggest #/.../# for space-as-first character too?

I don't quite understand what "that would be wrong if the user has written (?x)." means, I am hesitating to remove these two TODOs.

[hamishknight]

I don't quite understand what "that would be wrong if the user has written (?x)." means, I am hesitating to remove these two TODOs

We support an "extended syntax mode" in regex literals, which can be enabled with (?x) and makes whitespace non-semantic, e.g /(?x) a b c/ is equivalent to /abc/. This is relevant for the trailing space case because if you have e.g
/(?x) /, providing a fix-it of /(?x)\ / would change the meaning of the regex (previously it was empty, now it matches a literal space). This likely isn't a common case, but I think the easiest way of handling it is to only suggest adding # to the regex delimiters for the trailing space case, which won't change the semantic meaning. Checking for the use of (?x) isn't entirely trivial since it can appear anywhere in the pattern, can appear with other matching options, and can be disabled later on in the pattern.

[AppAppWorks]

I don't quite understand what "that would be wrong if the user has written (?x)." means, I am hesitating to remove these two TODOs

We support an "extended syntax mode" in regex literals, which can be enabled with (?x) and makes whitespace non-semantic, e.g /(?x) a b c/ is equivalent to /abc/. This is relevant for the trailing space case because if you have e.g /(?x) /, providing a fix-it of /(?x)\ / would change the meaning of the regex (previously it was empty, now it matches a literal space). This likely isn't a common case, but I think the easiest way of handling it is to only suggest adding # to the regex delimiters for the trailing space case, which won't change the semantic meaning. Checking for the use of (?x) isn't entirely trivial since it can appear anywhere in the pattern, can appear with other matching options, and can be disabled later on in the pattern.

Thanks for your detailed explanation!

Considering that the parser of now will never emit diagnostics for the trailing space case alone, I guess we can safely discard the two TODOs ?

[hamishknight]

Considering that the parser of now will never emit diagnostics for the trailing space case alone, I guess we can safely discard the two TODOs ?

IIRC it can be emitted for things like this:

let x = /, /

(Oh huh I never added a test for that, my bad)

[AppAppWorks]

Considering that the parser of now will never emit diagnostics for the trailing space case alone, I guess we can safely discard the two TODOs ?

IIRC it can be emitted for things like this:

let x = /, /

(Oh huh I never added a test for that, my bad)

I got it, I will revert back to my earlier design that the presence of a trailing space will mandate only "convert to extended regex literal with '#'" as the only Fix-it.

[ahoppen]

Could you also address this comment?

• https://github.com/swiftlang/swift-syntax/pull/2744/files#r1688783702

[AppAppWorks]

Done, please check.

[ahoppen]

@swift-ci Please test

[ahoppen]

@swift-ci Please test Windows

[ahoppen]

@swift-ci Please test

[AppAppWorks]

The check statuses have stayed unchanged for 5 days, could there be any problem with the CI?

[bnbarham]

@swift-ci please test