Respect line directives in SARIF logger #71454
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
| "version": "42.42.42.42", | ||
| "fileVersion": "4.9.0-dev (<developer build>)", | ||
| "semanticVersion": "42.42.42", |
There was a problem hiding this comment.
Can't hard code these values cause they will change. Even the version will change between local and CI (-dev vs. -ci). Need to either filter these out or fill them with interpolation.
| var source = $$""" | ||
| public class C | ||
| { | ||
| #line 123 "D:\otherfile.cs" |
There was a problem hiding this comment.
This makes me a bit nervous cause there are cases where #line can behave differently if it can / can't find a file. Hard coding a path like this means it's possible on some machines the file may exist while on others it won't. Prefer instead if we definitively do / don't put a file on disk at this location via Temp.CreateDirectory().CreateFile and then use interpolation to put the known file path.
| FileLinePositionSpan span = diagnosticLocation.GetMappedLineSpan(); | ||
| var path = diagnosticLocation.SourceTree is { } tree | ||
| ? tree.GetDisplayPath(diagnosticLocation.SourceSpan, resolver) | ||
| : span.Path; |
There was a problem hiding this comment.
Why did you decide to use mapped paths here? Thhe mental model I had for this change is that the sarif location should match the location that we print on the command line for errors. I didn't think that we used mapped paths for that (could be wrong)
There was a problem hiding this comment.
I was mistaken. I had an impression that /pathmap was used to report diagnostics on the command line. The ambiguity between "path mapping" for line directives and the command line doesn't help here.
RikkiGibson
changed the title
Closes #71449