Supports Trimming by fallenwood · Pull Request #7 · fallenwood/minijinja-csharp

fallenwood · 2026-01-23T03:51:09Z

No description provided.

Copilot

Pull request overview

This PR adds support for trimming and native AOT compilation by removing reflection-based serialization in favor of explicit serialization through interfaces and source generation. This is a breaking change that requires users to migrate from anonymous objects to either dictionaries or types marked with [MiniJinjaContext].

Changes:

Introduced ITemplateSerializable interface and [MiniJinjaContext] attribute for AOT-compatible object serialization
Created a source generator that automatically implements ITemplateSerializable for types marked with [MiniJinjaContext]
Removed reflection and System.Text.Json dependencies, implementing custom JSON serialization for trimming compatibility
Updated all tests and examples to use dictionaries or the new attribute-based approach instead of anonymous objects
Enabled AOT and trimming analyzers in Directory.Build.props

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
minijinja/Value.cs	Added ITemplateSerializable interface, MiniJinjaContext/MiniJinjaProperty attributes, removed reflection and System.Text.Json, implemented custom JSON serialization
minijinja/Environment.cs	Updated ConvertContext to require ITemplateSerializable or dictionaries, removed reflection-based conversion
minijinja/Evaluator.cs	Removed type name from error message for trimming compatibility
minijinja/minijinja.csproj	Whitespace cleanup
minijinja.generator/MiniJinjaContextGenerator.cs	New source generator that generates ITemplateSerializable implementations for types marked with [MiniJinjaContext]
minijinja.generator/MiniJinja.SourceGenerator.csproj	New source generator project configuration
minijinja.generator/Directory.Build.props	Override parent build properties for source generator
tests/PropertyAttributeTests.cs	New tests for property naming strategies and custom attributes
tests/MiniJinjaContextTests.cs	New comprehensive tests for source generator with various scenarios
tests/*.cs (multiple files)	Converted anonymous object usage to dictionaries for AOT compatibility
tests/tests.csproj	Added source generator reference as analyzer
example/Program.cs	Added examples demonstrating both ITemplateSerializable and source generator approaches
example/example.csproj	Added source generator reference as analyzer
Directory.Build.props	Enabled AOT compatibility, trim analyzer, and single-file analyzer
Directory.Packages.props	Added Roslyn CodeAnalysis package versions
global.json	Changed SDK version from 10.0.102 to 10.0.101
.gitignore	Added output directory and .claude file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T03:57:28Z

+      "long" => "MiniJinja.Value.FromInt",
+      "float" => "MiniJinja.Value.FromDouble",
+      "double" => "MiniJinja.Value.FromDouble",
+      "string?" => "MiniJinja.Value.FromString",


The GetConversionMethod only handles "string?" explicitly, but doesn't handle other nullable value types like "int?", "bool?", "long?", "float?", or "double?". When these types appear in properties, they will fall through to the "IsValueType" check, which will return true for Nullable structs and use FromAny. While this will work, it's inconsistent with the explicit handling of primitive types and may be less efficient. Consider adding cases for common nullable value types.

Suggested change

"string?" => "MiniJinja.Value.FromString",

"string?" => "MiniJinja.Value.FromString",

"bool?" => "MiniJinja.Value.FromBool",

"int?" => "MiniJinja.Value.FromInt",

"long?" => "MiniJinja.Value.FromInt",

"float?" => "MiniJinja.Value.FromDouble",

"double?" => "MiniJinja.Value.FromDouble",

Copilot · 2026-01-23T03:57:28Z

+      "long" => "MiniJinja.Value.FromInt",
+      "float" => "MiniJinja.Value.FromDouble",
+      "double" => "MiniJinja.Value.FromDouble",
+      "string?" => "MiniJinja.Value.FromString",


The source generator calls FromString for both "string" and "string?" properties, but FromString has a non-nullable parameter signature. If a property with type string? has a null value, calling FromString will result in a NullReferenceException. Consider either making FromString accept string? and handle null by returning Value.None, or have the generator check for null before calling FromString.

Suggested change

"string?" => "MiniJinja.Value.FromString",

"string?" => "MiniJinja.Value.FromAny",

Copilot · 2026-01-23T03:57:28Z

    }

-    return result;
+    throw new TemplateError($"Context type {context.GetType().Name} must implement ITemplateSerializable, be a dictionary, or be a supported primitive type. For anonymous types, convert to a dictionary first.");


The error message mentions "or be a supported primitive type" but ConvertContext doesn't handle primitive types - it only accepts dictionaries or ITemplateSerializable implementations. This could be misleading. Consider removing "or be a supported primitive type" from the error message.

Suggested change

throw new TemplateError($"Context type {context.GetType().Name} must implement ITemplateSerializable, be a dictionary, or be a supported primitive type. For anonymous types, convert to a dictionary first.");

throw new TemplateError($"Context type {context.GetType().Name} must implement ITemplateSerializable or be a dictionary. For anonymous types, convert to a dictionary first.");

Copilot · 2026-01-23T03:57:29Z

+  private static void Execute((INamedTypeSymbol Symbol, TypeDeclarationSyntax Syntax) typeInfo, SourceProductionContext context) {
+    var typeSymbol = typeInfo.Symbol;
+
+    // Get the MiniJinjaContext attribute to read naming strategy
+    var contextAttribute = typeSymbol.GetAttributes()
+      .FirstOrDefault(a => a.AttributeClass?.ToDisplayString() == "MiniJinja.MiniJinjaContextAttribute");
+
+    var namingStrategy = 0; // Default to CamelCase
+    if (contextAttribute is not null) {
+      foreach (var namedArg in contextAttribute.NamedArguments) {
+        if (namedArg.Key == "KeyNamingStrategy" && namedArg.Value.Value is int strategyValue) {
+          namingStrategy = strategyValue;
+        }
+      }
+    }
+
+    // Get all public properties
+    var properties = typeSymbol.GetMembers()
+      .OfType<IPropertySymbol>()
+      .Where(p => p.DeclaredAccessibility == Accessibility.Public &&
+                  p.GetMethod is not null &&
+                  !p.IsStatic)
+      .ToList();
+
+    if (properties.Count == 0) {
+      return;
+    }
+
+    var namespaceName = typeSymbol.ContainingNamespace.IsGlobalNamespace
+      ? null
+      : typeSymbol.ContainingNamespace.ToDisplayString();
+
+    var typeName = typeSymbol.Name;
+    var typeKind = typeInfo.Syntax.Keyword.Text; // "class" or "struct"
+
+    var source = GenerateSource(namespaceName, typeName, typeKind, properties, namingStrategy);
+    context.AddSource($"{typeName}.g.cs", source);


The source generator doesn't validate that types marked with [MiniJinjaContext] are declared as partial. If a non-partial type is marked with the attribute, the generator will create a partial class implementation, which will cause a compiler error (CS0260: Missing partial modifier). Consider adding a diagnostic to provide a clearer error message when a non-partial type is marked with the attribute.

Copilot · 2026-01-23T03:57:29Z

+    <IsRoslynComponent>true</IsRoslynComponent>
+    <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
+    <IsPackable>true</IsPackable>
+    <ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally>


The source generator project has IsPackable set to true but lacks essential package metadata such as PackageId, Description, Authors, PackageProjectUrl, and PackageLicenseExpression. If this package is meant to be published to NuGet, these properties should be added. If it's meant to be packaged together with the main minijinja package, the packaging strategy should be clarified.

Suggested change

<ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally>

<ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally>

<PackageId>MiniJinja.SourceGenerator</PackageId>

<Description>Source generator for MiniJinja, providing compile-time support for MiniJinja templates.</Description>

<Authors>MiniJinja Contributors</Authors>

<PackageProjectUrl>https://github.com/mitsuhiko/minijinja</PackageProjectUrl>

<PackageLicenseExpression>MIT</PackageLicenseExpression>

Copilot · 2026-01-23T03:57:29Z

+      var propertyName = property.Name;
+      var keyName = customName ?? ConvertPropertyName(propertyName, namingStrategy);
+
+      sb.Append($"      [\"{keyName}\"] = ");
+
+      // Determine the appropriate Value.From* method based on the property type
+      var typeInfo = property.Type;
+      var conversionMethod = GetConversionMethod(typeInfo);
+
+      sb.AppendLine($"{conversionMethod}({propertyName}),");
+    }


The source generator doesn't detect or warn about property name collisions after applying the naming strategy. For example, if a class has both "UserID" and "UserId" properties with CamelCase strategy, both would become "userId" in the dictionary, causing the second property to overwrite the first. Consider adding a diagnostic to detect and report such collisions.

Copilot · 2026-01-23T03:57:29Z

+          sb.Append('"');
+          sb.Append(kv.Key);
+          sb.Append('"');


In the Map case of WriteJson, the dictionary keys are appended directly to the JSON output without escaping special characters. If a key contains characters like quotes, backslashes, or control characters, the generated JSON will be invalid. The key should be escaped using the same logic as string values.

Copilot · 2026-01-23T03:57:30Z

+        break;
+      default:
+        sb.Append('"');
+        sb.Append(ToString());


Redundant call to 'ToString' on a String object.

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 30 out of 31 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T05:53:18Z

      ListExpr list => this.EvaluateList(list),
      DictExpr dict => this.EvaluateDict(dict),
-      _ => throw new TemplateError($"Unknown expression type: {expr.GetType().Name}")
+      _ => throw new TemplateError("Unknown expression type")


The error message was changed to remove the expression type name, but this is inconsistent with error messages in other files (Value.cs line 617 and Environment.cs line 260) which still use GetType().Name. If the goal is to avoid GetType().Name for AOT/trimming compatibility, this should be applied consistently across the codebase. Alternatively, if GetType().Name is acceptable (which it appears to be based on the other files), this should include the expression type for better debugging.

Suggested change

_ => throw new TemplateError("Unknown expression type")

_ => throw new TemplateError($"Unknown expression type: {expr.GetType().Name}")

Copilot · 2026-01-23T05:53:18Z

+      "long" => "MiniJinja.Value.FromInt",
+      "float" => "MiniJinja.Value.FromDouble",
+      "double" => "MiniJinja.Value.FromDouble",
+      "string?" => "MiniJinja.Value.FromString",


The conversion method only explicitly handles string? but not other nullable value types (int?, bool?, double?, etc.). While these will fall through to MiniJinja.Value.FromAny (line 308), it would be more consistent and efficient to handle common nullable primitives explicitly. For example, int? could use FromInt, bool? could use FromBool, etc., since Value.FromAny should handle nullable primitives correctly.

Suggested change

"string?" => "MiniJinja.Value.FromString",

"string?" => "MiniJinja.Value.FromString",

"bool?" => "MiniJinja.Value.FromBool",

"int?" => "MiniJinja.Value.FromInt",

"long?" => "MiniJinja.Value.FromInt",

"float?" => "MiniJinja.Value.FromDouble",

"double?" => "MiniJinja.Value.FromDouble",

Copilot · 2026-01-23T05:53:19Z

+      default:
+        sb.Append('"');
+        sb.Append(ToString());
+        sb.Append('"');


In the default case, ToString() is written directly to the JSON output without escaping special characters. If ToString() returns a string containing quotes, backslashes, or control characters, the resulting JSON will be invalid. Consider applying the same escaping logic used for ValueKind.String (lines 407-424) to ensure valid JSON output.

Copilot · 2026-01-23T05:53:19Z

+      foreach (var namedArg in contextAttribute.NamedArguments.Where(na => na.Key == "KeyNamingStrategy")) {
+        if (namedArg.Value.Value is int strategyValue) {
+          namingStrategy = strategyValue;
+        }
+      }


This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

Copilot · 2026-01-23T05:53:19Z

+        break;
+      default:
+        sb.Append('"');
+        sb.Append(ToString());


Redundant call to 'ToString' on a String object.

fallenwood added 7 commits January 23, 2026 10:14

temp

e949da2

Fix build on linux

df9889e

Temp

d3e5ef2

Fix tests

b6fc04c

tmp

3bde543

Finalize

0aa9f8c

Finalize

afa041f

fallenwood requested a review from Copilot January 23, 2026 03:51

Copilot started reviewing on behalf of fallenwood January 23, 2026 03:51 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

fallenwood and others added 5 commits January 23, 2026 12:03

Update minijinja.generator/MiniJinjaContextGenerator.cs

e57b331

Co-authored-by: Copilot <[email protected]>

temp

6c4525a

Update project file to include analyzer and source generator DLL

ea71d6f

Fix example

20c9260

Resolve comments

a83cedf

fallenwood requested a review from Copilot January 23, 2026 05:48

Copilot started reviewing on behalf of fallenwood January 23, 2026 05:48 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

fallenwood merged commit 770fc87 into master Jan 23, 2026
7 checks passed

fallenwood deleted the trimming branch January 23, 2026 06:01

-      "string?" => "MiniJinja.Value.FromString",
+      "string?" => "MiniJinja.Value.FromString",
+      "bool?" => "MiniJinja.Value.FromBool",
+      "int?" => "MiniJinja.Value.FromInt",
+      "long?" => "MiniJinja.Value.FromInt",
+      "float?" => "MiniJinja.Value.FromDouble",
+      "double?" => "MiniJinja.Value.FromDouble",

	"string?" => "MiniJinja.Value.FromString",
	"string?" => "MiniJinja.Value.FromAny",

	throw new TemplateError($"Context type {context.GetType().Name} must implement ITemplateSerializable, be a dictionary, or be a supported primitive type. For anonymous types, convert to a dictionary first.");
	throw new TemplateError($"Context type {context.GetType().Name} must implement ITemplateSerializable or be a dictionary. For anonymous types, convert to a dictionary first.");

-    <ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally>
+    <ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally>
+    <PackageId>MiniJinja.SourceGenerator</PackageId>
+    <Description>Source generator for MiniJinja, providing compile-time support for MiniJinja templates.</Description>
+    <Authors>MiniJinja Contributors</Authors>
+    <PackageProjectUrl>https://github.com/mitsuhiko/minijinja</PackageProjectUrl>
+    <PackageLicenseExpression>MIT</PackageLicenseExpression>

	_ => throw new TemplateError("Unknown expression type")
	_ => throw new TemplateError($"Unknown expression type: {expr.GetType().Name}")

Conversation

fallenwood commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants