C#: Follow line directives when getting element location#5005
Conversation
tamasvajk
force-pushed
the
feature/follow-line
branch
from
85b0051 to
ff6ad12
Compare
| result = e.getALocation().(SourceLocation) and | ||
| ( | ||
| not exists(e.getALocation().(SourceLocation).getMappedLocation()) or | ||
| e instanceof LineDirective |
There was a problem hiding this comment.
This is needed for line directives that are setting back the default line counting behavior:
y.cs:
#line 5 "x.cs"
var i = 1; // reported at x.cs, line 5
#line default // reported at x.cs, line 6, but y.cs, line 3 makes more sense
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Yep, that would probably be a bit nicer. Would I do it like this?
Location getMappedLocation() {
locations_mapped(this, result) and
not exists(LineDirective l | l.getALocation() = this)
}
| # 17| 4: [BlockStmt] {...} | ||
| # 19| 0: [LocalVariableDeclStmt] ... ...; | ||
| # 19| 0: [LocalVariableDeclExpr] Int32 i | ||
| comments1.cs: | ||
| # 1| 0: [LocalVariableDeclStmt] ... ...; | ||
| # 1| 0: [LocalVariableDeclExpr] Int32 i | ||
| trivia.cs: | ||
| # 19| 0: [TypeMention] int | ||
| # 20| 1: [LocalVariableDeclStmt] ... ...; | ||
| # 20| 0: [LocalVariableDeclExpr] Int32 j | ||
| comments1.cs: | ||
| # 2| 1: [LocalVariableDeclStmt] ... ...; | ||
| # 2| 0: [LocalVariableDeclExpr] Int32 j | ||
| trivia.cs: | ||
| # 20| 0: [TypeMention] int |
There was a problem hiding this comment.
I'm not sure what to do with this. The AST viewer is now also following the #line directives, which maybe makes sense. At the same time, it feels a bit strange that nodes in a tree might belong to different files.
tamasvajk
force-pushed
the
feature/follow-line
branch
from
ff6ad12 to
8f1482d
Compare
tamasvajk
force-pushed
the
feature/follow-line
branch
2 times, most recently
from
e005a74 to
2f51845
Compare
hvitved
left a comment
hvitved
left a comment
There was a problem hiding this comment.
Looks great; only one question.
| result = e.getALocation().(SourceLocation) and | ||
| ( | ||
| not exists(e.getALocation().(SourceLocation).getMappedLocation()) or | ||
| e instanceof LineDirective |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
tamasvajk
force-pushed
the
feature/follow-line
branch
from
70c00c4 to
4214759
Compare
|
@hvitved The latest differences job finished. It seems okay. Now issues show up in non-cs files, like this missing ternary opportunity here: There are three new extraction errors, all relating to reading a file. I pushed an extra commit to fix the logged message. |
…nt extraction related classes
tamasvajk
force-pushed
the
feature/follow-line
branch
from
b7e6ffa to
88d1539
Compare
|
I've rebased this PR and pushed a fix for the failing test. |
2 participants
This PR builds on #4976.The main goal of this PR is to use locations of QL entities in non-generated files when possible. For example
.cshtmlfiles are compiled into.cshtml.g.csfiles, and issues were reported in these generated files as these are the ones passed to csc. With this PR, locations are following#linedirectives. In case of aspnetcore,.cshtml.g.csfiles point to the original.cshtmlfiles.This PR:
#linedirective extraction by including the referenced file in the source archive, and referencing the file in the directive entity,#pragma checksum,Element::getLocationwas improved to take into account#linedirectives.Missing
https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/817/https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/825/https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/827/