Some delta tracking improvements. Still not perfect but probably good enough. Also hide refs that have been deleted in source code.

src/command_line.cc

@@ -33,7 +33,6 @@ const int kNumIndexers = 8 - 1;
 const int kQueueSizeBytes = 1024 * 8;
 const int kMaxWorkspaceSearchResults = 1000;
 
-
 QueryableFile* FindFile(QueryableDatabase* db, const std::string& filename) {
   auto it = db->usr_to_symbol.find(filename);
   if (it != db->usr_to_symbol.end())
@@ -82,13 +81,45 @@ optional<QueryableLocation> GetDefinitionSpellingOfSymbol(QueryableDatabase* db,
   return nullopt;
 }
 
-lsRange GetLsRange(WorkingFile* working_file, const Range& location) {
+optional<QueryableLocation> GetDefinitionExtentOfSymbol(QueryableDatabase* db, const QueryTypeId& id) {
+  return GetQueryable(db, id)->def.definition_extent;
+}
+optional<QueryableLocation> GetDefinitionExtentOfSymbol(QueryableDatabase* db, const QueryFuncId& id) {
+  return GetQueryable(db, id)->def.definition_extent;
+}
+optional<QueryableLocation> GetDefinitionExtentOfSymbol(QueryableDatabase* db, const QueryVarId& id) {
+  return GetQueryable(db, id)->def.definition_extent;
+}
+optional<QueryableLocation> GetDefinitionExtentOfSymbol(QueryableDatabase* db, const SymbolIdx& symbol) {
+  switch (symbol.kind) {
+  case SymbolKind::File:
+    // TODO: If line 1 is deleted the file won't show up in, ie, workspace symbol search results.
+    return QueryableLocation(QueryFileId(symbol.idx), Range(false /*is_interesting*/, Position(1, 1), Position(1, 1)));
+  case SymbolKind::Type:
+    return db->types[symbol.idx].def.definition_extent;
+  case SymbolKind::Func:
+    return db->funcs[symbol.idx].def.definition_extent;
+  case SymbolKind::Var:
+    return db->vars[symbol.idx].def.definition_extent;
+  case SymbolKind::Invalid: {
+    assert(false && "unexpected");
+    break;
+  }
+  }
+  return nullopt;
+}
+
+optional<lsRange> GetLsRange(WorkingFile* working_file, const Range& location) {
   if (!working_file) {
     return lsRange(
       lsPosition(location.start.line - 1, location.start.column - 1),
       lsPosition(location.end.line - 1, location.end.column - 1));
   }
 
+  // TODO: Should we also consider location.end.line?
+  if (working_file->IsDeletedDiskLine(location.start.line))
+    return nullopt;
+
   return lsRange(
     lsPosition(working_file->GetBufferLineFromDiskLine(location.start.line) - 1, location.start.column - 1),
     lsPosition(working_file->GetBufferLineFromDiskLine(location.end.line) - 1, location.end.column - 1));
@@ -104,15 +135,16 @@ lsDocumentUri GetLsDocumentUri(QueryableDatabase* db, QueryFileId file_id) {
   return lsDocumentUri::FromPath(path);
 }
 
-lsLocation GetLsLocation(QueryableDatabase* db, WorkingFiles* working_files, const QueryableLocation& location) {
+optional<lsLocation> GetLsLocation(QueryableDatabase* db, WorkingFiles* working_files, const QueryableLocation& location) {
   std::string path;
   lsDocumentUri uri = GetLsDocumentUri(db, location.path, &path);
-  lsRange range = GetLsRange(working_files->GetFileByFilename(path), location.range);
+  optional<lsRange> range = GetLsRange(working_files->GetFileByFilename(path), location.range);
-  return lsLocation(uri, range);
+  if (!range)
+    return nullopt;
+  return lsLocation(uri, *range);
 }
 
-// Returns a symbol. The symbol will have the location pointing to the
+// Returns a symbol. The symbol will have *NOT* have a location assigned.
-// definition.
 lsSymbolInformation GetSymbolInfo(QueryableDatabase* db, WorkingFiles* working_files, SymbolIdx symbol) {
   lsSymbolInformation info;
 
@@ -121,15 +153,12 @@ lsSymbolInformation GetSymbolInfo(QueryableDatabase* db, WorkingFiles* working_f
       QueryableFile* def = symbol.ResolveFile(db);
       info.name = def->def.usr;
       info.kind = lsSymbolKind::File;
-      info.location.uri.SetPath(def->def.usr);
       break;
     }
     case SymbolKind::Type: {
       QueryableTypeDef* def = symbol.ResolveType(db);
       info.name = def->def.qualified_name;
       info.kind = lsSymbolKind::Class;
-      if (def->def.definition_extent.has_value())
-        info.location = GetLsLocation(db, working_files, def->def.definition_extent.value());
       break;
     }
     case SymbolKind::Func: {
@@ -142,20 +171,12 @@ lsSymbolInformation GetSymbolInfo(QueryableDatabase* db, WorkingFiles* working_f
       else {
         info.kind = lsSymbolKind::Function;
       }
-
-      if (def->def.definition_extent.has_value()) {
-        info.location = GetLsLocation(db, working_files, def->def.definition_extent.value());
-      }
       break;
     }
     case SymbolKind::Var: {
       QueryableVarDef* def = symbol.ResolveVar(db);
       info.name = def->def.qualified_name;
       info.kind = lsSymbolKind::Variable;
-
-      if (def->def.definition_extent.has_value()) {
-        info.location = GetLsLocation(db, working_files, def->def.definition_extent.value());
-      }
       break;
     }
     case SymbolKind::Invalid: {
@@ -180,7 +201,10 @@ void AddCodeLens(
   const char* singular,
   const char* plural) {
   TCodeLens code_lens;
-  code_lens.range = GetLsRange(working_file, loc.range);
+  optional<lsRange> range = GetLsRange(working_file, loc.range);
+  if (!range)
+    return;
+  code_lens.range = *range;
   code_lens.command = lsCommand<lsCodeLensCommandArguments>();
   code_lens.command->command = "superindex.showReferences";
   code_lens.command->arguments.uri = GetLsDocumentUri(db, loc.path);
@@ -193,7 +217,10 @@ void AddCodeLens(
       continue;
     if (only_interesting && !use.range.interesting)
       continue;
-    unique_uses.insert(GetLsLocation(db, working_files, use));
+    optional<lsLocation> location = GetLsLocation(db, working_files, use);
+    if (!location)
+      continue;
+    unique_uses.insert(*location);
   }
   code_lens.command->arguments.locations.assign(unique_uses.begin(),
     unique_uses.end());
@@ -527,7 +554,7 @@ void QueryDbMainLoop(
 
   std::vector<std::unique_ptr<BaseIpcMessage>> messages = language_client->GetMessages(&language_client->for_server);
   for (auto& message : messages) {
-    std::cerr << "[querydb] Processing message " << static_cast<int>(message->method_id) << std::endl;
+    //std::cerr << "[querydb] Processing message " << static_cast<int>(message->method_id) << std::endl;
 
     switch (message->method_id) {
     case IpcId::Quit: {
@@ -636,8 +663,11 @@ void QueryDbMainLoop(
         if (ref.loc.range.start.line >= target_line && ref.loc.range.end.line <= target_line &&
           ref.loc.range.start.column <= target_column && ref.loc.range.end.column >= target_column) {
           optional<QueryableLocation> location = GetDefinitionSpellingOfSymbol(db, ref.idx);
-          if (location)
+          if (location) {
-            response.result.push_back(GetLsLocation(db, working_files, location.value()));
+            optional<lsLocation> ls_location = GetLsLocation(db, working_files, location.value());
+            if (ls_location)
+              response.result.push_back(*ls_location);
+          }
           break;
         }
       }
@@ -661,7 +691,10 @@ void QueryDbMainLoop(
       std::cerr << "File outline size is " << file->def.outline.size() << std::endl;
       for (SymbolRef ref : file->def.outline) {
         lsSymbolInformation info = GetSymbolInfo(db, working_files, ref.idx);
-        info.location = GetLsLocation(db, working_files, ref.loc);
+        optional<lsLocation> location = GetLsLocation(db, working_files, ref.loc);
+        if (!location)
+          continue;
+        info.location = *location;
         response.result.push_back(info);
       }
 
@@ -753,8 +786,17 @@ void QueryDbMainLoop(
           break;
         }
 
-        if (db->qualified_names[i].find(query) != std::string::npos)
+        if (db->qualified_names[i].find(query) != std::string::npos) {
-          response.result.push_back(GetSymbolInfo(db, working_files, db->symbols[i]));
+          lsSymbolInformation info = GetSymbolInfo(db, working_files, db->symbols[i]);
+          optional<QueryableLocation> location = GetDefinitionExtentOfSymbol(db, db->symbols[i]);
+          if (!location)
+            continue;
+          optional<lsLocation> ls_location = GetLsLocation(db, working_files, *location);
+          if (!ls_location)
+            continue;
+          info.location = *ls_location;
+          response.result.push_back(info);
+        }
       }
 
       SendOutMessageToClient(language_client, response);
@@ -917,7 +959,7 @@ void LanguageServerStdinLoop(IpcMessageQueue* ipc) {
     case IpcId::TextDocumentDocumentSymbol:
     case IpcId::TextDocumentCodeLens:
     case IpcId::WorkspaceSymbol:
-      std::cerr << "Spending message " << (int)message->method_id << std::endl;
+      //std::cerr << "Sending message " << (int)message->method_id << std::endl;
       ipc->SendMessage(&ipc->for_server, message->method_id, *message.get());
       break;
     }

src/working_files.cc

@@ -27,23 +27,19 @@ int LineCount(std::string::const_iterator start, const std::string::const_iterat
   return count;
 }
 
-bool IsNextTokenNewline(const std::string& str, int start) {
+bool IsPreviousTokenNewline(const std::string& str, int start) {
-  while (start < str.size()) {
+  return start == 0 || str[start - 1] == '\n';
-    // Peek a token ahead for \r\n. We cannot check a token back because
+}
-    // isalpha('\r') returns false.
-    if ((start + 1 < str.size()) && str[start] == '\r' && str[start + 1] == '\n')
-      return true;
 
-    if (str[start] == '\n')
+int DetermineDiskLineForChange(WorkingFile* f, int start_offset, int buffer_line, int line_delta) {
-      return true;
+  // If we're adding a newline that means we will introduce a new virtual newline
-
+  // below. That's the case *except* if we are moving the current line down.
-    if (!isalpha(str[start]))
+  if (!IsPreviousTokenNewline(f->content, start_offset)) {
-      return false;
+    //std::cerr << " Applying newline workaround" << std::endl;
-
+    ++buffer_line;
-    ++start;
   }
 
-  return true;
+  return f->GetDiskLineFromBufferLine(buffer_line);
 }
 
 }  // namespace
@@ -186,7 +182,7 @@ void WorkingFiles::OnChange(const Ipc_TextDocumentDidChange::Params& change) {
     return;
   }
 
-  // TODO: we should probably pay attention to versioning.
+  //std::cerr << "VERSION " << change.textDocument.version << std::endl;
 
   for (const Ipc_TextDocumentDidChange::lsTextDocumentContentChangeEvent& diff : change.contentChanges) {
     //std::cerr << "Applying rangeLength=" << diff.rangeLength;
@@ -197,26 +193,28 @@ void WorkingFiles::OnChange(const Ipc_TextDocumentDidChange::Params& change) {
     }
     else {
       int start_offset = GetOffsetForPosition(diff.range.start, file->content);
-      int previous_line_count = diff.range.end.line - diff.range.start.line;
+      int old_line_count = diff.range.end.line - diff.range.start.line;
-      int replace_line_count = LineCount(diff.text.begin(), diff.text.end());
+      int new_line_count = LineCount(diff.text.begin(), diff.text.end());
 
-      int line_delta = replace_line_count - previous_line_count;
+      assert(old_line_count == LineCount(file->content.begin() + start_offset, file->content.begin() + start_offset + diff.rangeLength + 1));
+
+      int line_delta = new_line_count - old_line_count;
       if (line_delta != 0) {
+        //std::cerr << " diff.range.start.line=" << diff.range.start.line+1 << ", diff.range.start.character=" << diff.range.start.character << std::endl;
+        //std::cerr << " diff.range.end.line=" << diff.range.end.line+1 << ", diff.range.end.character=" << diff.range.end.character << std::endl;
+        //std::cerr << " old_line_count=" << old_line_count << ", new_line_count=" << new_line_count << std::endl;
+        //std::cerr << " disk_line(diff.range.start.line)=" << file->GetDiskLineFromBufferLine(diff.range.start.line+1) << std::endl;
+        //std::cerr << " disk_line(diff.range.end.line)=" << file->GetDiskLineFromBufferLine(diff.range.end.line+1) << std::endl;
+
         // language server stores line counts starting at 0, we start at 1
         int buffer_line = diff.range.start.line + 1;
-
+        int disk_line = DetermineDiskLineForChange(file, start_offset, buffer_line, line_delta);
-        // If we're adding a newline but not actually shifting any of the
-        // contents of this line, we should mark the text prepended on the next
-        // line.
-        if (IsNextTokenNewline(file->content, start_offset))
-          ++buffer_line;
-
-        int disk_line = file->GetDiskLineFromBufferLine(buffer_line);
 
         bool found = false;
         for (int i = 0; i < file->changes.size(); ++i) {
           auto& change = file->changes[i];
-          if (change.disk_line == disk_line) {
+          if (disk_line == change.disk_line ||
+             (line_delta >= 1 && disk_line - 1 == change.disk_line && change.delta < 0) /* handles joining two lines and then resplitting them */ ) {
             change.delta += line_delta;
             found = true;
           }
@@ -227,14 +225,16 @@ void WorkingFiles::OnChange(const Ipc_TextDocumentDidChange::Params& change) {
         if (!found)
           file->changes.push_back(WorkingFile::Change(disk_line, line_delta));
 
-        //std::cerr << "(changes.size()=" << file->changes.size() << ") Inserted delta=" << line_delta << " at disk_line=" << disk_line << " with buffer_line=" << buffer_line << std::endl;
-        //for (auto& change : file->changes)
-        //  std::cerr << "  disk_line=" << change.disk_line << " delta=" << change.delta << std::endl;
-
         std::sort(file->changes.begin(), file->changes.end(),
           [](const WorkingFile::Change& a, const WorkingFile::Change& b) {
           return a.disk_line < b.disk_line;
         });
+
+        //std::cerr << " APPLIED" << std::endl;
+        //std::cerr << " Inserted delta=" << line_delta << " at disk_line=" << disk_line << " with buffer_line=" << buffer_line << std::endl;
+        //std::cerr << " changes.size()=" << file->changes.size() << std::endl;
+        //for (auto& change : file->changes)
+        //  std::cerr << "  disk_line=" << change.disk_line << " delta=" << change.delta << std::endl;
       }
 
 
@@ -557,4 +557,66 @@ TEST_CASE("deleted_line past-start") {
   CHECK(f.IsDeletedDiskLine(5) == false);
 }
 
+TEST_CASE("wip") {
+  /*
+    VERSION 4
+     diff.range.start.line=2, diff.range.start.character=0
+     diff.range.end.line=  3, diff.range.end.character=  0
+     diff.text=""
+     Applying newline workaround
+     APPLIED
+     Inserted delta=-1 at disk_line=4 with buffer_line=4
+     changes.size()=1
+      disk_line=4 delta=-1
+
+
+    VERSION 7
+     diff.range.start.line=2, diff.range.start.character=0
+     diff.range.end.line=  2, diff.range.end.character=  0
+     diff.text="daaa\n"
+     Applying newline workaround
+     APPLIED
+     Inserted delta=1 at disk_line=5 with buffer_line=4
+     changes.size()=2
+      disk_line=4 delta=-1
+      disk_line=5 delta=1
+  */
+
+  WorkingFile f("", "");
+
+  /*
+  delete line 4
+  buffer line 4 -> disk line 5
+  */
+
+  f.changes.push_back(WorkingFile::Change(4, -1));
+
+  CHECK(f.GetDiskLineFromBufferLine(4) == 5);
+}
+
+TEST_CASE("DetermineDiskLineForChange") {
+  // aaa
+  // bbb
+  WorkingFile f("", "aaa\nbbb");
+
+  // Adding a line so we have
+  //  aaa
+  //
+  //  bbb
+  CHECK(DetermineDiskLineForChange(&f, 3 /*start_offset*/, 1 /*buffer_line*/, 1 /*line_delta*/) == 2);
+
+  // Adding a line so we have
+  //  
+  //  aaa
+  //  bbb
+  CHECK(DetermineDiskLineForChange(&f, 0 /*start_offset*/, 1 /*buffer_line*/, 1 /*line_delta*/) == 1);
+
+  // Adding a line so we have
+  //  a
+  //  aa
+  //  bbb
+  CHECK(DetermineDiskLineForChange(&f, 1 /*start_offset*/, 1 /*buffer_line*/, 1 /*line_delta*/) == 2);
+
+}
+
 TEST_SUITE_END();