Fix a couple of exceptions encountered when formatting documents with preprocessor directives #11373
Conversation
… preprocessor directives
| var annotation = trivia.GetAnnotations(MarkerId).Single(); | ||
| if (!int.TryParse(annotation.Data, out var projectedIndex)) | ||
| // We only expect one annotation because we built the entire trivia with a single annotation, but | ||
| // we need to be defensive here. Annotations are a little hard to work with though, so apologies for | ||
| // the slightly odd method of validation. | ||
| using var enumerator = trivia.GetAnnotations(MarkerId).GetEnumerator(); | ||
| enumerator.MoveNext(); | ||
| var annotation = enumerator.Current; | ||
| // We shouldn't be able to enumerate any more, and we should be able to parse our data out of the annotation. | ||
| if (enumerator.MoveNext() || | ||
| !int.TryParse(annotation.Data, out var projectedIndex)) |
There was a problem hiding this comment.
The defensive code seems fine to me, but what are the conditions where multiple annotations are returned?
There was a problem hiding this comment.
Given this document with source mappings at each $$
<div>
@{$$
#if DEBUG
$$}
@{$$
#endif
$$}
</div>
If DEBUG is not defined, then the whole span between directives is a single "DisabledTextTrivia", but obviously it contains two mappings, so two points to track.
| <div /> | ||
| @{ | ||
| #if DEBUG | ||
| } |
There was a problem hiding this comment.
huh... this looks wrong to me :/
There was a problem hiding this comment.
I don't understand this either TBH. Is the preprocessor directive increasing the indent level? It doesn't seem to in standalone C# file. E.g.
if (true)
{
#if DEBUG
if (true)
{
#endif
}
}
formatted becomes
if (true)
{
#if DEBUG
if (true)
{
#endif
}
}
which is what I would expect
There was a problem hiding this comment.
To be very clear, this PR doesn't attempt to actually adjust how this document is formatted, it just makes the formatting engine not crash. The {get;set;} being spaced out correctly here is providing value to the user that they didn't get before.
Having said that, in the tests there are no preprocessor defines (actually we have no way to pass them to the Razor compiler at all I don't think) so the code in the #if is not parsed or formatted by Roslyn. Pretend it doesn't exist, and the result looks right.
| linePositionSpan = default; | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think due to FUSE, there was a zero-width marker token at the end of the document which upset this algorithm. It's entirely possible this never happens in real life, but being defensive doesn't hurt.
|
Thanks for the reviews. I wanted to get this in before the new formatting engine, since it won't hit this path at all, but on type formatting still could. |
Fixes #11372
When you assume, you make a fool out of people who use the
.Single()method and expect a good time.