- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5
Add reverse implementors index to the partial SCIP loader #23
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
Changes from 4 commits
3eccfde
              3b662f2
              06d9684
              b749b5a
              92a8b8c
              5f90bcb
              279437c
              beeb9a4
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -24,6 +24,7 @@ type PartialIndex interface { | |
| GetSymbolInformation(symbol string) (*model.SymbolInformation, string, error) | ||
| GetSymbolInformationFromDescriptors(descriptors []model.Descriptor, version string) (*model.SymbolInformation, string, error) | ||
| References(symbol string) (map[string][]*model.Occurrence, error) | ||
| GetImplementationSymbols(symbol string) ([]string, error) | ||
| Tidy() error | ||
| } | ||
|  | ||
|  | @@ -57,19 +58,24 @@ type PartialLoadedIndex struct { | |
| indexFolder string | ||
| pool *scanner.BufferPool | ||
| onDocumentLoaded func(*model.Document) | ||
|  | ||
| // ImplementorsBySymbol maps abstract/interface symbol -> set of implementing symbols | ||
| implementorsMu sync.RWMutex | ||
| ImplementorsBySymbol map[string]map[string]struct{} | ||
| 
      Comment on lines
    
      +63
     to 
      +64
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this put as a map on the main index type because there isn't an easy way to attach it to a treeNode during the indexing process? We probably want to change this when we come up with the multiple step index load process. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the prefeix tree merging happens after scanning, and the nodes are versioned and pruned, cross-tree mutability may complicate concurrency and correctness | ||
| } | ||
|  | ||
| // NewPartialLoadedIndex creates a new PartialLoadedIndex | ||
| func NewPartialLoadedIndex(indexFolder string) PartialIndex { | ||
| return &PartialLoadedIndex{ | ||
| PrefixTreeRoot: NewSymbolPrefixTree(), | ||
| DocTreeNodes: make(map[string]*docNodes), | ||
| LoadedDocuments: make(map[string]*model.Document), | ||
| updatedDocs: make(map[string]int64), | ||
| docToIndex: make(map[string]string, 0), | ||
| indexFolder: indexFolder, | ||
| pool: scanner.NewBufferPool(1024, 12), | ||
| onDocumentLoaded: func(*model.Document) {}, | ||
| PrefixTreeRoot: NewSymbolPrefixTree(), | ||
| DocTreeNodes: make(map[string]*docNodes), | ||
| LoadedDocuments: make(map[string]*model.Document), | ||
| updatedDocs: make(map[string]int64), | ||
| docToIndex: make(map[string]string, 0), | ||
| indexFolder: indexFolder, | ||
| pool: scanner.NewBufferPool(1024, 12), | ||
| onDocumentLoaded: func(*model.Document) {}, | ||
| ImplementorsBySymbol: make(map[string]map[string]struct{}), | ||
| } | ||
| } | ||
|  | ||
|  | @@ -130,6 +136,7 @@ func (p *PartialLoadedIndex) LoadIndex(indexPath string, indexReader scanner.Sci | |
| localDocTreeNodes := make(map[string]*docNodes) | ||
| localUpdatedDocs := make(map[string]int64) | ||
| localDocToIndex := make(map[string]string) | ||
| localImplementorsBySymbol := make(map[string]map[string]struct{}) | ||
|  | ||
| loadScanner := &scanner.IndexScannerImpl{ | ||
| Pool: p.pool, | ||
|  | @@ -157,6 +164,17 @@ func (p *PartialLoadedIndex) LoadIndex(indexPath string, indexReader scanner.Sci | |
| modelInfo := mapper.ScipSymbolInformationToModelSymbolInformation(info) | ||
| leafNode, isNew := localPrefixTree.AddSymbol(docPath, modelInfo, p.revision.Load()) | ||
|  | ||
| // Populate reverse implementors for quick lookup (impl -> abs) | ||
| for _, rel := range modelInfo.Relationships { | ||
| if rel != nil && rel.IsImplementation { | ||
| abs := rel.Symbol | ||
| if localImplementorsBySymbol[abs] == nil { | ||
| localImplementorsBySymbol[abs] = make(map[string]struct{}) | ||
| } | ||
| localImplementorsBySymbol[abs][modelInfo.Symbol] = struct{}{} | ||
| } | ||
| } | ||
|  | ||
| if isNew { | ||
| localDocTreeNodes[docPath].nodes = append(localDocTreeNodes[docPath].nodes, leafNode) | ||
| } | ||
|  | @@ -179,6 +197,7 @@ func (p *PartialLoadedIndex) LoadIndex(indexPath string, indexReader scanner.Sci | |
| p.mergeDocTreeNodes(localDocTreeNodes) | ||
| p.mergeUpdatedDocs(localUpdatedDocs) | ||
| p.mergeDocToIndex(localDocToIndex) | ||
| p.mergeImplementors(localImplementorsBySymbol) | ||
| }() | ||
| return loadScanner.ScanIndexReader(indexReader) | ||
| } | ||
|  | @@ -280,6 +299,23 @@ func (p *PartialLoadedIndex) mergeDocToIndex(localDocToIndex map[string]string) | |
| } | ||
| } | ||
|  | ||
| // mergeImplementors merges a local reverse implementors map into the main index | ||
| func (p *PartialLoadedIndex) mergeImplementors(local map[string]map[string]struct{}) { | ||
| if len(local) == 0 { | ||
| return | ||
| } | ||
| p.implementorsMu.Lock() | ||
| defer p.implementorsMu.Unlock() | ||
| for abs, impls := range local { | ||
| if p.ImplementorsBySymbol[abs] == nil { | ||
| p.ImplementorsBySymbol[abs] = make(map[string]struct{}) | ||
| } | ||
| for impl := range impls { | ||
| p.ImplementorsBySymbol[abs][impl] = struct{}{} | ||
| } | ||
| } | ||
| } | ||
|  | ||
| // LoadDocument loads a document into the PartialLoadedIndex | ||
| func (p *PartialLoadedIndex) LoadDocument(relativeDocPath string) (*model.Document, error) { | ||
| p.loadedDocsMu.RLock() | ||
|  | @@ -364,6 +400,22 @@ func (p *PartialLoadedIndex) loadDocumentFromIndexFolder(relativeDocPath string) | |
| return doc, nil | ||
| } | ||
|  | ||
| // GetImplementationSymbols returns the list of implementing symbols for a given abstract/interface symbol | ||
| func (p *PartialLoadedIndex) GetImplementationSymbols(symbol string) ([]string, error) { | ||
| p.implementorsMu.RLock() | ||
| defer p.implementorsMu.RUnlock() | ||
| set := p.ImplementorsBySymbol[symbol] | ||
| if set == nil { | ||
| return []string{}, nil | ||
| } | ||
| res := make([]string, 0, len(set)) | ||
| for s := range set { | ||
| res = append(res, s) | ||
| } | ||
| sort.Strings(res) | ||
| return res, nil | ||
| } | ||
|  | ||
| // Tidy prunes nodes for documents that were updated in the current revision | ||
| func (p *PartialLoadedIndex) Tidy() error { | ||
| // Acquire the modification mutex to prevent new index loads during cleanup | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -319,6 +319,89 @@ func (p *partialScipRegistry) GetSymbolOccurrence(uri uri.URI, pos protocol.Posi | |
| return nil, errors.New("not implemented") | ||
| } | ||
|  | ||
| // Implementation implements Registry. | ||
| func (p *partialScipRegistry) Implementation(sourceURI uri.URI, pos protocol.Position) ([]protocol.Location, error) { | ||
| doc, err := p.Index.LoadDocument(p.uriToRelativePath(sourceURI)) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to load document %s: %s", sourceURI, err) | ||
| return nil, err | ||
| } | ||
| if doc == nil { | ||
| return nil, nil | ||
| } | ||
|  | ||
| sourceOccurrence := GetOccurrenceForPosition(doc.Occurrences, pos) | ||
| if sourceOccurrence == nil { | ||
| return nil, nil | ||
| } | ||
|  | ||
| // Local symbols typically don't have implementation relationships | ||
| if scip.IsLocalSymbol(sourceOccurrence.Symbol) { | ||
| return []protocol.Location{}, nil | ||
| } | ||
|  | ||
| locations := make([]protocol.Location, 0) | ||
|  | ||
| // Fast path: use reverse implementors index | ||
| implementors, err := p.Index.GetImplementationSymbols(sourceOccurrence.Symbol) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to get implementing symbols for %s: %s", sourceOccurrence.Symbol, err) | ||
| } else if len(implementors) > 0 { | ||
| for _, implSym := range implementors { | ||
| implementingSymbol, err := model.ParseScipSymbol(implSym) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to parse implementing symbol %s: %s", implSym, err) | ||
| continue | ||
| } | ||
| implOcc, err := p.GetSymbolDefinitionOccurrence( | ||
| mapper.ScipDescriptorsToModelDescriptors(implementingSymbol.Descriptors), | ||
| implementingSymbol.Package.Version, | ||
| ) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to get definition for implementing symbol %s: %s", implSym, err) | ||
| continue | ||
| } | ||
| if implOcc != nil && implOcc.Occurrence != nil { | ||
| locations = append(locations, *mapper.ScipOccurrenceToLocation(implOcc.Location, implOcc.Occurrence)) | ||
| } | ||
| } | ||
| return locations, nil | ||
| } | ||
|  | ||
| // Fallback: traverse relationships on the abstract symbol if the reverse index is empty or there is error | ||
| symbolInformation, _, err := p.Index.GetSymbolInformation(sourceOccurrence.Symbol) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to get symbol information for %s: %s", sourceOccurrence.Symbol, err) | ||
| return nil, err | ||
| } | ||
| if symbolInformation == nil { | ||
| return []protocol.Location{}, nil | ||
| } | ||
|  | ||
| for _, relationship := range symbolInformation.Relationships { | ||
| if relationship.IsImplementation { | ||
| implementingSymbol, err := model.ParseScipSymbol(relationship.Symbol) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to parse implementing symbol %s: %s", relationship.Symbol, err) | ||
| continue | ||
| } | ||
| implOcc, err := p.GetSymbolDefinitionOccurrence( | ||
| mapper.ScipDescriptorsToModelDescriptors(implementingSymbol.Descriptors), | ||
| implementingSymbol.Package.Version, | ||
| ) | ||
| if err != nil { | ||
| p.logger.Errorf("failed to get definition for implementing symbol %s: %s", relationship.Symbol, err) | ||
| continue | ||
| } | ||
| if implOcc != nil && implOcc.Occurrence != nil { | ||
| locations = append(locations, *mapper.ScipOccurrenceToLocation(implOcc.Location, implOcc.Occurrence)) | ||
| } | ||
| } | ||
| } | ||
|          | ||
|  | ||
| return locations, nil | ||
| } | ||
|  | ||
| func (p *partialScipRegistry) uriToRelativePath(uri uri.URI) string { | ||
| rel, err := filepath.Rel(p.WorkspaceRoot, uri.Filename()) | ||
| if err != nil { | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Match the other methods by naming this
Implementations