Skip to content
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.

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

textDocument/rename may return duplicates #294

Closed
Yanpas opened this issue Mar 1, 2019 · 3 comments
Closed

textDocument/rename may return duplicates #294

Yanpas opened this issue Mar 1, 2019 · 3 comments
Labels
bug Something isn't working

Comments

@Yanpas
Copy link

Yanpas commented Mar 1, 2019

No matter which mention of a variable I rename I get this warning. Here is formatted strace RPC output:

{
  "jsonrpc": "2.0",
  "id": 391,
  "result": {
    "documentChanges": [
      {
        "textDocument": {
          "uri": "file:///home/yan/dev/blabla.cpp",
          "version": 0
        },
        "edits": [
          {
            "range": {
              "start": { "line": 69, "character": 19 },
              "end": { "line": 69, "character": 25 }
            },
            "newText": "db_path"
          },
          {
            "range": {
              "start": { "line": 70, "character": 71 },
              "end": { "line": 70, "character": 77 }
            },
            "newText": "db_path"
          },
          {
            "range": {
              "start": { "line": 75, "character": 10 },
              "end": { "line": 75, "character": 16 }
            },
            "newText": "db_path"
          },
          {
            "range": {
              "start": { "line": 75, "character": 10 },
              "end": { "line": 75, "character": 16 }
            },
            "newText": "db_path"
          },
          {
            "range": {
              "start": { "line": 78, "character": 10 },
              "end": { "line": 78, "character": 16 }
            },
            "newText": "db_path"
          },
          {
            "range": {
              "start": { "line": 79, "character": 79 },
              "end": { "line": 79, "character": 85 }
            },
            "newText": "db_path"
          },
          {
            "range": {
              "start": { "line": 67, "character": 16 },
              "end": { "line": 67, "character": 22 }
            },
            "newText": "db_path"
          }
        ]
      }
    ]
  }
}

The duplicate is on line 75. The thing is that line 75 has macro (LOGINFO), so deduplicating should be performed by language server

Yanpas pushed a commit to Yanpas/ccls that referenced this issue Mar 1, 2019
Yanpas added a commit to Yanpas/ccls that referenced this issue Mar 1, 2019
MaskRay added a commit that referenced this issue Mar 2, 2019
Mitigate #294 and another case:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 2, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
@MaskRay
Copy link
Owner

MaskRay commented Mar 2, 2019

Mostly fixed by #296

Actually Index{Func,Type,Var}::uses are not guaranteed to be ordered so last_position as used in #294 does not address all duplicates.

The textDocument/rename handler had another bug:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before

#296 fixes the case when WorkingFile *wf is available (textDocument/didOpen has been sent for the document). If wf is nullptr, ideally the handler should load the contents in the filesystem, but that is rare and may have other issues so I didn't address that in #296.

@MaskRay MaskRay closed this as completed Mar 2, 2019
@MaskRay MaskRay changed the title Overlapping ranges are not allowed! warning when renaming variable textDocument/rename may return duplicates Mar 2, 2019
@MaskRay MaskRay added the bug Something isn't working label Mar 2, 2019
@MaskRay MaskRay changed the title textDocument/rename may return duplicates textDocument/rename may return duplicates Mar 2, 2019
MaskRay added a commit that referenced this issue Mar 2, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 2, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 2, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
@Yanpas
Copy link
Author

Yanpas commented Mar 2, 2019

If wf is nullptr, ideally the handler should load the contents in the filesystem

Does it mean that if I rename some function which is mentioned in 3 files and I have opened only 1 file, then those 2 files won't be touched and will remain invalid?

MaskRay added a commit that referenced this issue Mar 2, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
@MaskRay
Copy link
Owner

MaskRay commented Mar 2, 2019

@Yanpas If wf is nullptr (because the file isn't opened), the file will still be touched. It is just that the "identifier literally occurs" check isn't enabled (because ccls doesn't know the file contents). In rare occasions textDocument/rename may corrupt the file.

MaskRay added a commit that referenced this issue Mar 2, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 15, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 15, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 15, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 15, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 18, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 18, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Mar 25, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue May 13, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 24, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 25, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Oct 25, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Nov 10, 2019
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
MaskRay added a commit that referenced this issue Apr 22, 2020
…cro replacement

Mitigate edits in the same place (#294) and:

// textDocument/rename on `f`
void f();
void g() { m(); } // incorrectly rewrote m() before
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants