Merge pull request #148 from terrajobst/episode26

Episode 26: Miscellaneous
terrajobst · May 26, 2020 · 974ae47 · 974ae47
2 parents 7223f82 + f50f208
commit 974ae47
Show file tree

Hide file tree

Showing 20 changed files with 147 additions and 66 deletions.
diff --git a/docs/episode-26.md b/docs/episode-26.md
@@ -0,0 +1,64 @@
+# Episode 26
+
+[Video](https://www.youtube.com/watch?v=Y2Gn6qr_twA&list=PLRAdsfhKI4OWNOSfS7EUu5GRAVmze1t2y&index=26) |
+[Pull Request](https://github.com/terrajobst/minsk/pull/148) |
+[Previous](episode-26.md) |
+[Next](episode-27.md)
+
+## Completed items
+
+* Enabled nullable in Minsk.Tests & Minsk.Generators
+* Honored nullable annotations in source generator
+* Filed and fixed various TODOs
+
+## Interesting aspects
+
+### Leveraging nullable annotations inside the source generator
+
+We're now honoring nullable annotations when generating the
+`SyntaxNode.GetChildren()` methods.
+
+For example, consider `ReturnStatementSyntax`. The `Expression` property is
+marked as being nullable (because `return` can be used without an expression).
+
+```C#
+partial class ReturnStatementSyntax : StatementSyntax
+{
+    public SyntaxToken ReturnKeyword { get; }
+    public ExpressionSyntax? Expression { get; }
+}
+```
+
+Our generator now uses the null annotation for the `Expression` property and
+emits a `null` check:
+
+```C#
+partial class ReturnStatementSyntax
+{
+    public override IEnumerable<SyntaxNode> GetChildren()
+    {
+        yield return ReturnKeyword;
+        if (Expression != null)
+            yield return Expression;
+    }
+}
+```
+
+This is simply done by [using Roslyn's `NullableAnnotation`
+property][null-annotations]:
+
+```C#
+var canBeNull = property.NullableAnnotation == NullableAnnotation.Annotated;
+if (canBeNull)
+{
+    writer.WriteLine($"if ({property.Name} != null)");
+    writer.Indent++;
+}
+
+writer.WriteLine($"yield return {property.Name};");
+
+if (canBeNull)
+    writer.Indent--;
+```
+
+[null-annotations]: https://github.com/terrajobst/minsk/blob/877fefa36e184da125fd62942b5797328df79896/src/Minsk.Generators/SyntaxNodeGetChildrenGenerator.cs#L59-L69
diff --git a/src/Minsk.Generators/Minsk.Generators.csproj b/src/Minsk.Generators/Minsk.Generators.csproj
@@ -4,8 +4,6 @@
     <IsPackable>false</IsPackable>
     <IsPublishable>false</IsPublishable>
     <TargetFramework>netstandard2.0</TargetFramework>
-    <!-- TODO: Remove when nullable annotations are added -->
-    <Nullable>Disable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/src/Minsk.Generators/SyntaxNodeGetChildrenGenerator.cs b/src/Minsk.Generators/SyntaxNodeGetChildrenGenerator.cs
@@ -29,6 +29,9 @@ public void Execute(SourceGeneratorContext context)
             var separatedSyntaxListType = compilation.GetTypeByMetadataName("Minsk.CodeAnalysis.Syntax.SeparatedSyntaxList`1");
             var syntaxNodeType = compilation.GetTypeByMetadataName("Minsk.CodeAnalysis.Syntax.SyntaxNode");
 
+            if (immutableArrayType == null || separatedSyntaxListType == null || syntaxNodeType == null)
+                return;
+
             var types = GetAllTypes(compilation.Assembly);
             var syntaxNodeTypes = types.Where(t => !t.IsAbstract && IsPartial(t) && IsDerivedFrom(t, syntaxNodeType));
 
@@ -53,15 +56,25 @@ public void Execute(SourceGeneratorContext context)
                                 {
                                     if (IsDerivedFrom(propertyType, syntaxNodeType))
                                     {
+                                        var canBeNull = property.NullableAnnotation == NullableAnnotation.Annotated;
+                                        if (canBeNull)
+                                        {
+                                            indentedTextWriter.WriteLine($"if ({property.Name} != null)");
+                                            indentedTextWriter.Indent++;
+                                        }
+
                                         indentedTextWriter.WriteLine($"yield return {property.Name};");
+
+                                        if (canBeNull)
+                                            indentedTextWriter.Indent--;
                                     }
                                     else if (propertyType.TypeArguments.Length == 1 &&
                                              IsDerivedFrom(propertyType.TypeArguments[0], syntaxNodeType) &&
                                              SymbolEqualityComparer.Default.Equals(propertyType.OriginalDefinition, immutableArrayType))
                                     {
                                         indentedTextWriter.WriteLine($"foreach (var child in {property.Name})");
                                         indentedTextWriter.WriteLine($"{indentString}yield return child;");
-                                        
+
                                     }
                                     else if (SymbolEqualityComparer.Default.Equals(propertyType.OriginalDefinition, separatedSyntaxListType) &&
                                              IsDerivedFrom(propertyType.TypeArguments[0], syntaxNodeType))
@@ -127,12 +140,14 @@ private void GetAllTypes(List<INamedTypeSymbol> result, INamespaceOrTypeSymbol s
 
         private bool IsDerivedFrom(ITypeSymbol type, INamedTypeSymbol baseType)
         {
-            while (type != null)
+            var current = type;
+
+            while (current != null)
             {
-                if (SymbolEqualityComparer.Default.Equals(type, baseType))
+                if (SymbolEqualityComparer.Default.Equals(current, baseType))
                     return true;
 
-                type = type.BaseType;
+                current = current.BaseType;
             }
 
             return false;

diff --git a/src/Minsk.Tests/CodeAnalysis/AnnotatedText.cs b/src/Minsk.Tests/CodeAnalysis/AnnotatedText.cs
@@ -69,7 +69,7 @@ public static string[] UnindentLines(string text)
 
             using (var reader = new StringReader(text))
             {
-                string line;
+                string? line;
                 while ((line = reader.ReadLine()) != null)
                     lines.Add(line);
             }

diff --git a/src/Minsk.Tests/CodeAnalysis/Syntax/LexerTests.cs b/src/Minsk.Tests/CodeAnalysis/Syntax/LexerTests.cs
@@ -144,9 +144,9 @@ public static IEnumerable<object[]> GetTokenPairsWithSeparatorData()
         {
             var fixedTokens = Enum.GetValues(typeof(SyntaxKind))
                                   .Cast<SyntaxKind>()
-                                  .Select(k => (kind: k, text: SyntaxFacts.GetText(k)))
-                                  .Where(t => t.text != null);
-
+                                  .Select(k => (k, text: SyntaxFacts.GetText(k)))
+                                  .Where(t => t.text != null)
+                                  .Cast<(SyntaxKind, string)>();
 
             var dynamicTokens = new[]
             {

diff --git a/src/Minsk.Tests/CodeAnalysis/Syntax/ParserTests.cs b/src/Minsk.Tests/CodeAnalysis/Syntax/ParserTests.cs
@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Diagnostics;
 using Minsk.CodeAnalysis.Syntax;
 using Xunit;
 
@@ -17,6 +18,9 @@ public void Parser_BinaryExpression_HonorsPrecedences(SyntaxKind op1, SyntaxKind
             var text = $"a {op1Text} b {op2Text} c";
             var expression = ParseExpression(text);
 
+            Debug.Assert(op1Text != null);
+            Debug.Assert(op2Text != null);
+
             if (op1Precedence >= op2Precedence)
             {
                 //     op2
@@ -74,6 +78,9 @@ public void Parser_UnaryExpression_HonorsPrecedences(SyntaxKind unaryKind, Synta
             var text = $"{unaryText} a {binaryText} b";
             var expression = ParseExpression(text);
 
+            Debug.Assert(unaryText != null);
+            Debug.Assert(binaryText != null);
+
             if (unaryPrecedence >= binaryPrecedence)
             {
                 //   binary

diff --git a/src/Minsk.Tests/Minsk.Tests.csproj b/src/Minsk.Tests/Minsk.Tests.csproj
@@ -4,8 +4,6 @@
     <IsPackable>false</IsPackable>
     <IsPublishable>false</IsPublishable>
     <TargetFramework>netcoreapp3.1</TargetFramework>
-    <!-- TODO: Remove when nullable annotations are added -->
-    <Nullable>Disable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/src/Minsk/CodeAnalysis/Binding/BoundBinaryExpression.cs b/src/Minsk/CodeAnalysis/Binding/BoundBinaryExpression.cs
@@ -10,7 +10,7 @@ public BoundBinaryExpression(BoundExpression left, BoundBinaryOperator op, Bound
             Left = left;
             Op = op;
             Right = right;
-            ConstantValue = ConstantFolding.ComputeConstant(left, op, right);
+            ConstantValue = ConstantFolding.Fold(left, op, right);
         }
 
         public override BoundNodeKind Kind => BoundNodeKind.BinaryExpression;

diff --git a/src/Minsk/CodeAnalysis/Binding/BoundUnaryExpression.cs b/src/Minsk/CodeAnalysis/Binding/BoundUnaryExpression.cs
@@ -9,7 +9,7 @@ public BoundUnaryExpression(BoundUnaryOperator op, BoundExpression operand)
         {
             Op = op;
             Operand = operand;
-            ConstantValue = ConstantFolding.ComputeConstant(op, operand);
+            ConstantValue = ConstantFolding.Fold(op, operand);
         }
 
         public override BoundNodeKind Kind => BoundNodeKind.UnaryExpression;

diff --git a/src/Minsk/CodeAnalysis/Binding/ConstantFolding.cs b/src/Minsk/CodeAnalysis/Binding/ConstantFolding.cs
@@ -5,7 +5,7 @@ namespace Minsk.CodeAnalysis.Binding
 {
     internal static class ConstantFolding
     {
-        public static BoundConstant? ComputeConstant(BoundUnaryOperator op, BoundExpression operand)
+        public static BoundConstant? Fold(BoundUnaryOperator op, BoundExpression operand)
         {
             if (operand.ConstantValue != null)
             {
@@ -27,7 +27,7 @@ internal static class ConstantFolding
             return null;
         }
 
-        public static BoundConstant? ComputeConstant(BoundExpression left, BoundBinaryOperator op, BoundExpression right)
+        public static BoundConstant? Fold(BoundExpression left, BoundBinaryOperator op, BoundExpression right)
         {
             var leftConstant = left.ConstantValue;
             var rightConstant = right.ConstantValue;

diff --git a/src/Minsk/CodeAnalysis/Compilation.cs b/src/Minsk/CodeAnalysis/Compilation.cs
@@ -128,6 +128,7 @@ public void EmitTree(FunctionSymbol symbol, TextWriter writer)
             body.WriteTo(writer);
         }
 
+        // TODO: References should be part of the compilation, not arguments for Emit
         public ImmutableArray<Diagnostic> Emit(string moduleName, string[] references, string outputPath)
         {
             var parseDiagnostics = SyntaxTrees.SelectMany(st => st.Diagnostics);

diff --git a/src/Minsk/CodeAnalysis/Diagnostic.cs b/src/Minsk/CodeAnalysis/Diagnostic.cs
@@ -4,29 +4,29 @@ namespace Minsk.CodeAnalysis
 {
     public sealed class Diagnostic
     {
-        private Diagnostic(TextLocation location, string message, bool isError)
+        private Diagnostic(bool isError, TextLocation location, string message)
         {
+            IsError = isError;
             Location = location;
             Message = message;
-            IsError = isError;
             IsWarning = !IsError;
         }
 
+        public bool IsError { get; }
         public TextLocation Location { get; }
         public string Message { get; }
-        public bool IsError { get; }
         public bool IsWarning { get; }
 
         public override string ToString() => Message;
 
         public static Diagnostic Error(TextLocation location, string message)
         {
-            return new Diagnostic(location, message, isError: true);
+            return new Diagnostic(isError: true, location, message);
         }
 
         public static Diagnostic Warning(TextLocation location, string message)
         {
-            return new Diagnostic(location, message, isError: false);
+            return new Diagnostic(isError: false, location, message);
         }
     }
 }