Fix: DiaSession shows async methods to be external
#307
Conversation
|
Verified by Platform tests and manually. |
| @@ -97,10 +97,10 @@ internal static MethodDebugInformationHandle GetMethodDebugInformationHandle(Met | |||
| out int startLineNumber, | |||
| out int endLineNumber) | |||
| { | |||
| var startPoint = methodDebugDefinition.GetSequencePoints().OrderBy(s => s.StartLine).FirstOrDefault(); | |||
| var startPoint = methodDebugDefinition.GetSequencePoints().OrderBy(s => s.StartLine).FirstOrDefault(s => s.IsHidden == false); | |||
There was a problem hiding this comment.
s => s.IsHidden == false
What's the purpose of adding this?
There was a problem hiding this comment.
By following reference SequencePointList.cs, SequencePoint.cs, FullSymbolReader.cs IsHidden denotes special point which is not exists in source file. Should be ignored.
| startLineNumber = startPoint.StartLine; | ||
| var endPoint = | ||
| methodDebugDefinition.GetSequencePoints().OrderByDescending(s => s.StartLine).FirstOrDefault(); | ||
| methodDebugDefinition.GetSequencePoints().OrderByDescending(s => s.StartLine).FirstOrDefault(s => s.IsHidden == false); |
There was a problem hiding this comment.
Instead of calling same API twice, can we just call it once and then use FirstOrDefault and LastOrDefault method
methodDebugDefinition.GetSequencePoints().OrderBy(s => s.StartLine)
There was a problem hiding this comment.
Makes sense, will change it.
| @@ -100,23 +100,22 @@ private void PopulateCacheForTypeAndMethodSymbols(string binaryPath) | |||
| var typesDict = asm.GetTypes().ToDictionary(type => type.FullName); | |||
There was a problem hiding this comment.
What's the purpose of having a dictionary here? I think IEnumerable is what we need.
| var methodInfoDict = typeEntry.Value.GetMethods().ToDictionary(methodInfo => methodInfo.Name); | ||
| // Get declared method infos | ||
| var methodInfoList = ((TypeInfo)typeEntry.Value.GetTypeInfo()).DeclaredMethods; | ||
| var methodInfoDict = new Dictionary<string, MethodInfo>(); |
There was a problem hiding this comment.
Do we really need this dictionary?
There was a problem hiding this comment.
You are right, list will be sufficient.
| <ItemGroup Condition=" '$(TargetFramework)' == 'net46' "> | ||
| <Reference Include="System.Runtime" /> | ||
| <Reference Include="System" /> | ||
| <Reference Include="Microsoft.CSharp" /> | ||
| <Reference Include="System.Xml" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> |
There was a problem hiding this comment.
Why do we need this service?
There was a problem hiding this comment.
VS generates this automatically, to identify it as test project. Explanation here.
|
|
||
| DiaSession diaSession = new DiaSession(assemblyPath); | ||
| DiaNavigationData diaNavigationData = diaSession.GetNavigationData("SampleUnitTestProject3.Class1", "OverLoadededMethod"); | ||
|
|
There was a problem hiding this comment.
Which method's navigation data should it return, there are two methods with the same name?
There was a problem hiding this comment.
Currently it is returning the last method in source file for both full and portable pdb.
|
@AbhitejJohn, @codito @pvlakshm I tried it for XUnit, tests got discovered, but two out of three overloaded tests failed.
|
#293 #280