-
Notifications
You must be signed in to change notification settings - Fork 1.7k
C# Cleanup and refactoring #5131
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
Conversation
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.
Two questions, otherwise LGTM.
csharp/extractor/Semmle.Extraction.CSharp/Entities/Method.AstLineCounter.cs
Outdated
Show resolved
Hide resolved
|
||
namespace Semmle.Extraction.CSharp.Populators | ||
{ | ||
internal class AstLineCounter : CSharpSyntaxVisitor<LineCounts> |
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.
Now Method
no longer needs to be partial
.
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.
Ahh, good point. Sorry...
b41ebcc
to
74db2cc
Compare
74db2cc
to
2de7fbe
Compare
@hvitved I rebased this, and fixed the build issue after the rebase. Let's see if the tests pass, and if so, then it's ready to be merged. |
This PR is the first in multiple steps to remove nullability warnings. Currently when the context is created for CIL extraction,
null
is passed as scope and compilation, which results in two nullability warnings. To overcome this, I think the best approach would be to have dedicated source and CIL extraction context classes. This PR only moves around functionalities. Most importantly, thestatic
Location
factories are moved to instance methods on theContext
, which will later allow us to override them.Differences job