[READY] Fix multiline comments and strings issues

[micbou]

When collect_identifiers_from_comments_and_strings is 0, we are replacing comments and strings with empty strings before extracting the identifiers. If one of these comments or strings span multiple lines, the current line number line_num may become invalid and, in that case, the identifier completer will fail to add the previous identifier on the CurrentIdentifierFinished event:

As you can see, the test identifier is properly extracted before the comment but not after.

Another issue with multiline comments and strings is that, when adding an identifier under the cursor on the InsertLeave event, we only remove comments and strings on the current line. This is incorrect if the current line is in the middle of a multiline comment or string:

The test identifier is extracted even though it's inside a comment.

For now, I am updating the tests to showcase both issues. I'll update the PR with the fixes once the builds failed.

---

This change is 

[micbou]

Fix the tests by replacing comments and strings with empty lines instead of empty strings. Also, replace them on the whole content and not just the current line when adding an identifier under the cursor.

---

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[codecov-io]

Codecov Report

Merging #831 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #831      +/-   ##
==========================================
+ Coverage   94.84%   94.84%   +<.01%     
==========================================
  Files          79       79              
  Lines        5390     5396       +6     
  Branches      169      169              
==========================================
+ Hits         5112     5118       +6     
  Misses        230      230              
  Partials       48       48

[puremourning]

Reviewed 1 of 4 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

ycmd/identifier_utils.py, line 179 at r2 (raw file):

def RemoveIdentifierFreeText( text, filetype = None ):
  return CommentAndStringRegexForFiletype( filetype ).sub(

What happens with something like this:

/* test */ realIdentifier

Does it kill realIdentifier?

Point is... does this over remove if the comment is not a whole-line comment?

---

Comments from Reviewable

[micbou]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

ycmd/identifier_utils.py, line 179 at r2 (raw file):

Previously, puremourning (Ben Jackson) wrote…

What happens with something like this:

/* test */ realIdentifier

Does it kill realIdentifier?

Point is... does this over remove if the comment is not a whole-line comment?

No, /* test */ will be matched and ReplaceWithEmptyLines will return ``. See the RemoveIdentifierFreeText_CstyleComments test.

---

Comments from Reviewable

[Valloric]

That's a fantastic RB description, thank you! Made it very easy to understand the problem.

---

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/identifier_utils.py, line 174 at r2 (raw file):

def ReplaceWithEmptyLines( match ):

Maybe regex_match to emphasize it's the match object from re?

---

ycmd/identifier_utils.py, line 179 at r2 (raw file):

Previously, micbou wrote…

No, /* test */ will be matched and ReplaceWithEmptyLines will return ``. See the RemoveIdentifierFreeText_CstyleComments test.

Let's make sure we have a test for that though.

---

Comments from Reviewable

[micbou]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/identifier_utils.py, line 174 at r2 (raw file):

Previously, Valloric (Val Markovic) wrote…

Maybe regex_match to emphasize it's the match object from re?

Done.

---

ycmd/identifier_utils.py, line 179 at r2 (raw file):

Previously, Valloric (Val Markovic) wrote…

Let's make sure we have a test for that though.

Added a test for this case (identifier after a C-style comment).

---

Comments from Reviewable

[puremourning]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/identifier_utils.py, line 179 at r2 (raw file):

Previously, micbou wrote…

Added a test for this case (identifier after a C-style comment).

Ah I think my mobile broke what I was trying to put. On reflecting on the code I'm pretty sure it is fine, but the example I meant was

/*
 */ anIdentifier

Like i said, I'm pretty sure it works,

---

Comments from Reviewable

[puremourning]

assuming th case I mentioned works as expected. Thanks!

---

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[micbou]

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/identifier_utils.py, line 179 at r2 (raw file):

Previously, puremourning (Ben Jackson) wrote…

Ah I think my mobile broke what I was trying to put. On reflecting on the code I'm pretty sure it is fine, but the example I meant was

/*
 */ anIdentifier

Like i said, I'm pretty sure it works,

Updated the test.

---

Comments from Reviewable

[Valloric]

Thanks for the PR!

@zzbot r=puremourning

---

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[zzbot]

📌 Commit 74648f8 has been approved by puremourning

[zzbot]

⌛ Testing commit 74648f8 with merge 2e7a596...

[zzbot]

💔 Test failed - status-travis

[zzbot]

☀️ Test successful - status-travis
Approved by: puremourning
Pushing 2e7a596 to master...