C#: Escape IDs in TRAP label definitions#5848
Conversation
hvitved
force-pushed
the
csharp/trap-key-escape
branch
2 times, most recently
from
3eee4ba to
c3d6eb4
Compare
hvitved
force-pushed
the
csharp/trap-key-escape
branch
from
c3d6eb4 to
fab8400
Compare
|
|
||
| public abstract string NameLabel { get; } | ||
|
|
||
| protected internal void WriteMethodId(TextWriter trapFile, Type parent, string methodName) |
There was a problem hiding this comment.
This is strange. Why was this protected internal?!
| public override void Close() | ||
| => throw new NotImplementedException(); | ||
|
|
||
| public override ValueTask DisposeAsync() | ||
| => throw new NotImplementedException(); |
There was a problem hiding this comment.
Why aren't these calling the wrapped?
There was a problem hiding this comment.
I didn't bother to implement those because we are not using them.
| public override bool Equals(object? obj) | ||
| => wrapped.Equals(obj); |
There was a problem hiding this comment.
Is this okay? Shouldn't disposeUnderlying be included here?
| private readonly TextWriter wrapped; | ||
| private readonly bool disposeUnderlying; | ||
|
|
||
| public EscapingTextWriter(TextWriter wrapped, bool disposeUnderlying = false) |
There was a problem hiding this comment.
Do we need this wrapped TextWriter? Wouldn't the following be enough?
public class EscapingStringWriter : StringWriter
{
public override void Write(char value) => WriteEscaped(value);
private void WriteEscaped(char c)
{ ... }
}
Maybe we would also need to override void Write(char[] buffer, int index, int count), but most probably all other Write* just calls Write(char).
There was a problem hiding this comment.
When writing actual TRAP we are using an underlying StreamWriter, so it is not always a StringWriter.
2 participants
Fixes https://github.com/github/codeql-csharp-team/issues/154
https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/1056/