[Generator] Wrapper for Strings Constructed during Code Generation #20098
Conversation
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
dustin-wojciechowski
force-pushed
the
dev/dustin/code-blocks
branch
from
281b8ca to
f9370d0
Compare
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…s for CodeBlock, changed LineBlock inheritance
dustin-wojciechowski
force-pushed
the
dev/dustin/code-blocks
branch
from
ec9b174 to
e80f685
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
src/bgen/CodeBlocks/IfBlock.cs
Outdated
| CodeBlock? ElseBlock = null; | ||
| public IfBlock (string condition) | ||
| { | ||
| HeaderText = "if (" + condition + ")"; |
| allParameters.Append (", "); | ||
| } | ||
| } | ||
| HeaderText = methodSignature + "(" + allParameters + ")"; |
There was a problem hiding this comment.
A lot of string concatenation in this class.
tests/generator/CodeBlockTests.cs
Outdated
| MethodBlock methodBlock = new (methodName, methodArguments); | ||
| methodBlock.AddLine (inputText1); | ||
| methodBlock.AddLine (inputText2); |
There was a problem hiding this comment.
In general how test code is written doesn't matter all that much, except that for these changes the test code is an example of what kind of code we'll have to write in the generator, so the nicer test code we can write, the nicer code we can write in the generator too.
With that in mind: while using variables for text is generally a good idea, in this case it just makes the code much harder to read, because you have to go back and forth between the declaration and the usage to determine what's going on.
If something like this were possible, it would look much nicer (using a collection initializer):
var methodBlock = new MethodBlock ("public void Foobinate", "int count", "bool isFoo")
{
"int fooCount = 1;",
"string[] fooNames = new [] {\"foo\"};",
};Now if you squint a little bit, it's relatively easy to see what will be generated.
| List<ICodeBlock> blocks = new List<ICodeBlock> () { new LineBlock ("int fooCount = 1;"), new LineBlock ("string[] fooNames = new [] {\"foo\"};") }; | ||
| MethodBlock methodBlock = new ("public void Foobinate", blocks, "int count", "bool isFoo"); |
There was a problem hiding this comment.
Like above, something like this would be easier to read (one reason being that the layout here is in the same order the code will be generated, you don't have to reorder chunks of text in your head to make sense of it):
MethodBlock methodBlock = new ("public void Foobinate", "int count", "bool isFoo") {
new LineBlock ("int fooCount = 1;"),
new LineBlock ("string [] ...;"),
};Just to be clear: this is how I'd like to use the new API in the generator, and the idea is to change the API to make something like this possible.
tests/generator/CodeBlockTests.cs
Outdated
| "}\n"; | ||
| List<ICodeBlock> blocks = new (); | ||
| blocks.Add (new LineBlock (line1)); | ||
| IfBlock ifBlock = new (ifConditionText, blocks); |
There was a problem hiding this comment.
Same:
var ifBlock = new IfBlock ("fooCount == 5") {
"Console.WriteLine(fooCount);",
};
tests/generator/CodeBlockTests.cs
Outdated
| List<ICodeBlock> blocks = new (); | ||
| List<ICodeBlock> elseBlocks = new List<ICodeBlock> () { new LineBlock (line2) }; | ||
| blocks.Add (new LineBlock (line1)); | ||
| IfBlock ifBlock = new (ifConditionText, blocks); | ||
| ifBlock.AddElse (elseBlocks); |
There was a problem hiding this comment.
var ifBlock = new IfBlock ("fooCount == 5") {
"Console.WriteLine(fooCount);"
}.AddElse (
"Console.WriteLine(booCount);"
);
tests/generator/CodeBlockTests.cs
Outdated
| List<ICodeBlock> blocks = new (); | ||
| List<ICodeBlock> elseIfBlocks = new List<ICodeBlock> () { new LineBlock (line2) }; | ||
| List<ICodeBlock> elseBlocks = new List<ICodeBlock> () { new LineBlock (line3) }; | ||
| blocks.Add (new LineBlock (line1)); | ||
| IfBlock ifBlock = new (ifConditionText, blocks); | ||
| ifBlock.AddElseIf (elseIfConditionText, elseIfBlocks); | ||
| ifBlock.AddElse (elseBlocks); |
There was a problem hiding this comment.
var ifBlock = new IfBlock ("fooCount == 5") {
"Console.WriteLine(fooCount);",
}.AddElseIf ("fooCount == 10",
"Console.WriteLine(booCount);"
).AddElse (
"Console.WriteLine(zooCount);"
);
tests/generator/CodeBlockTests.cs
Outdated
| BlockContainer blockContainer = new (); | ||
| blockContainer.AddLine (usingText); | ||
| blockContainer.AddLine (""); |
There was a problem hiding this comment.
BlockContainer block = new {
"using Network";
}or:
BlockContainer block = new ();
block.AddUsing ("Network");
tests/generator/CodeBlockTests.cs
Outdated
| CodeBlock namespaceBlock = new CodeBlock (namespaceText); | ||
| CodeBlock classBlock = new CodeBlock (headerText); | ||
| MethodBlock methodBlock = new MethodBlock (methodName, methodArguments); | ||
| classBlock.AddLine (variableText); | ||
| classBlock.AddBlock (methodBlock); | ||
| namespaceBlock.AddBlock (classBlock); | ||
| blockContainer.AddBlock (namespaceBlock); |
There was a problem hiding this comment.
var block = new CodeBlock ("public namespace FooSpace") {
new CodeBlock ("public class FooClass") {
"public string FooText { get; set; }",
new MethodBlock ("public void Foobinate", "int count", "bool isFoo") { }
}
}
src/bgen/CodeBlocks/IfBlock.cs
Outdated
| Blocks.AddRange (blocks); | ||
| } | ||
|
|
||
| public void AddElseIf (string condition, List<ICodeBlock> blocks) |
There was a problem hiding this comment.
If you make this look like this:
public IfBlock AddElseIf (string condition, List<ICodeBlock> blocks)
{
ElseIfBlocks.Add (new CodeBlock ($"else if ({condition})", blocks));
return this;
}
You get two things - string interpolation reads a little better and the fluent interface lets you write:
var test = new IfBlock ("someTest", blocks)
.AddElse ("someOtherTest", otherBlocks)
.AddElse ("someOtherOtherTest", otherOtherBlocks)
.AddElse ("other code);
There was a problem hiding this comment.
If you make the blocks a params:
public IfBlock AddElseIf (string condition, params ICodeBlock[] blocks)
{
ElseIfBlocks.Add (new CodeBlock ($"else if ({condition})", blocks));
return this;
}you can do:
var test = new IfBlock ("someTest", blocks)
.AddElseIf ("someOtherTest",
new Block ("Console.WriteLine(var1);"),
new Block ("Console.WriteLine(var2);"))
.AddElseIf ("someOtherOtherTest",
new Block ("Console.WriteLine(var3);"))
.AddElse ("other code);
This comment has been minimized.
This comment has been minimized.
|
FWIW I'm not sure we should go in this direction, here runtime people recommend using either Roslyn or StringBuilder/StringWriter/etc (which we're already doing): dotnet/runtimelab#2526 (comment) |
|
Pushed work I had in progress, last things needed would be to replace string concatenation in MethodBlock and clean up the Print inside of IfBlock if this were to be picked up again. Work on this PR is suspended until further notice. |
|
💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 170 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
This is a String Wrapper for generated text. It will handle things like newlines and indents that are currently managed manually and somewhat inconsistently (for example, an indent variable is incremented and decremented alongside the use of \t escape character)
There are still some things I wanted to do but split it into another PR: