CodeConnect/SourceBrowser

Links to symbol definitions are incorrect.

Closed this issue · 8 comments

I click on the "parent" argument variable on this line:
http://sourcebrowser.io/Browse/CodeConnect/SourceBrowser/SourceBrowser.Generator/DocumentWalker.cs#33

and it takes me here:
http://sourcebrowser.io/Browse/CodeConnect/SourceBrowser/SourceBrowser.Generator/Transformers/AbstractWorkspaceVisitor.cs#35

instead of to the line where the argument is declared. Similar things happen for links on other symbols too.

Sorry for the delay.

That's really weird. I'll take a look at this now.

All good, no rush on my end. Just noticed this while checking out what you guys had done

I think the issue happens in CSWalker.cs
at "localLink.ReferencedSymbolName = symbol.ToString();" in ProcessSymbolUsage

The symbol we're getting is SourceBrowser.Generator.Model.IProjectItem and we know that symbol.Kind is Parameter. Is it possible that the current model doesn't support links to parameters? When we're building the link, we're only assigning the literal name "SourceBrowser.Generator.Model.IProjectItem" to localLink.ReferencedSymbolName. I think the issue is that this information is not enough to accurately point to the parameter definition.

btw, I'm catching the token in question in VisitToken:

        if (token.Value?.ToString() == "parent")
        {
            int test = 3; // set breakpoint here
        }

It's a combination of two bugs.

  1. We improperly handle parameters. We're giving them the fully qualified name of their type. (ie. SourceBrowser.Generator.Model.IProjectItem) We should be linking them to the method for which they are a parameter.
  2. We assume there's only one definition for each type when building links. (We've been ignoring partial types and classes). This means that all 'IProjectItem' parameters in the entire projection will point to a single definition. In this case it's http://sourcebrowser.io/Browse/CodeConnect/SourceBrowser/SourceBrowser.Generator/Transformers/AbstractWorkspaceVisitor.cs#35

I'll fix 1, which will fix this.

The second will require some changes in both the Generator and the Site. I'll add an issue for that if one doesn't exist already.

In case 2 it doesn't actually point to definition of IProjectItem. It points to one of the methods that accepts that type as parameter. Is it a fluke, or an incorrect implementation of case 1?

It's because every method that accepts IProjectItem is incorrectly saying that IProjectIem is defined there. But it has to choose just one, so it looks like happened to choose this method. In theory it could have randomly chosen the correct definition of IProjectItem.

Alright. So we need to compare the fully qualified name of the token's parent method with names of methods that say that define IProjectItem?

We're not handling the declaration of parameters correctly. They are getting a fully qualified name identical to their type. So in

public void Test(JQuerySelector selector) 
{
}

When we process selector, we're erroneously giving it the fully qualified name "Selenium.WebDriver.Extensions.JQuerySelector" which means anything that links to JQuerySelector might link here.

This should be an easy fix, thankfully.