Allow setting visibility and switching to static readonly props

kzu · kzu · commit d0a11a740ce0 · 2024-09-27T00:52:32.000-03:00
Setting the `ThisAssembly` class to public should be somewhat rare, but might be desirable in some cases. When the class is made public, there's a surprising "bug" in that updating the assembly providing the `ThisAssembly` class will *not* cause another assembly consuming its values to reflect the changes. This is because the compiler will efectively inline the constants, so effectively every call site has a copy of the value. This is somewhat unintuitive, but it's perfectly sensible when the use is for internal purposes. The new ThisAssemblyVisibility=public property allows both setting the generated class visibility to and switching (for constants) to static properties. For ThisAssembly.Strings and ThisAssembly.Resources, only the class visibility is changed. Fixes #64
diff --git a/src/ThisAssembly.Constants/CSharp.sbntxt b/src/ThisAssembly.Constants/CSharp.sbntxt
@@ -41,13 +41,13 @@
         {{- remarks -}}
         {{ obsolete }}
         {{~ if RawStrings && value.IsText ~}}
-	    public const string {{ value.Name | string.replace "-" "_" | string.replace " " "_" }} = 
+	    public {{ Modifier }} string {{ value.Name | string.replace "-" "_" | string.replace " " "_" }} ={{ Lambda }} 
 
 """
 {{ value.Value }}
 """;
         {{~ else ~}}
-	    public const {{ value.Type }} {{ value.Name | string.replace "-" "_" | string.replace " " "_" }} = 
+	    public {{ Modifier }} {{ value.Type }} {{ value.Name | string.replace "-" "_" | string.replace " " "_" }} ={{ Lambda }} 
             {{~ if value.IsText ~}}
             @"{{ value.Value }}";
             {{~ else ~}}
@@ -71,7 +71,7 @@ namespace {{ Namespace }};
 /// <summary>
 /// Provides access to the current assembly information.
 /// </summary>
-partial class ThisAssembly
+{{ Visibility }}partial class ThisAssembly
 {
     /// <summary>
     /// Provides access project-defined constants.
diff --git a/src/ThisAssembly.Constants/ConstantsGenerator.cs b/src/ThisAssembly.Constants/ConstantsGenerator.cs
@@ -58,7 +58,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
 
         // Read the ThisAssemblyNamespace property or default to null
         var right = context.AnalyzerConfigOptionsProvider
-            .Select((c, t) => c.GlobalOptions.TryGetValue("build_property.ThisAssemblyNamespace", out var ns) && !string.IsNullOrEmpty(ns) ? ns : null)
+            .Select((c, t) => (
+                c.GlobalOptions.TryGetValue("build_property.ThisAssemblyNamespace", out var ns) && !string.IsNullOrEmpty(ns) ? ns : null,
+                c.GlobalOptions.TryGetValue("build_property.ThisAssemblyVisibility", out var visibility) && !string.IsNullOrEmpty(visibility) ? visibility : null
+              ))
             .Combine(context.ParseOptionsProvider);
 
         var inputs = files.Combine(right);
@@ -69,9 +72,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
     }
 
     void GenerateConstant(SourceProductionContext spc,
-        (((string name, string value, string? type, string? comment, string root), (string? ns, ParseOptions parse)), StatusOptions options) args)
+        (((string name, string value, string? type, string? comment, string root), ((string? ns, string? visibility), ParseOptions parse)), StatusOptions options) args)
     {
-        var (((name, value, type, comment, root), (ns, parse)), options) = args;
+        var (((name, value, type, comment, root), ((ns, visibility), parse)), options) = args;
         var cs = (CSharpParseOptions)parse;
 
         if (!string.IsNullOrWhiteSpace(ns) &&
@@ -94,7 +97,7 @@ void GenerateConstant(SourceProductionContext spc,
         // For now, we only support C# though
         var file = parse.Language.Replace("#", "Sharp") + ".sbntxt";
         var template = Template.Parse(EmbeddedResource.GetContent(file), file);
-        var model = new Model(rootArea, ns);
+        var model = new Model(rootArea, ns, "public".Equals(visibility, StringComparison.OrdinalIgnoreCase));
         if ((int)cs.LanguageVersion >= 1100)
             model.RawStrings = true;
 
diff --git a/src/ThisAssembly.Constants/Model.cs b/src/ThisAssembly.Constants/Model.cs
@@ -9,13 +9,16 @@
 namespace ThisAssembly;
 
 [DebuggerDisplay("Values = {RootArea.Values.Count}")]
-record Model(Area RootArea, string? Namespace)
+record Model(Area RootArea, string? Namespace, bool IsPublic)
 {
     public bool RawStrings { get; set; } = false;
     public string Version => Assembly.GetExecutingAssembly().GetName().Version.ToString(3);
     public string Url => Devlooped.Sponsors.SponsorLink.Funding.HelpUrl;
     public string? Warn { get; set; }
     public string? Remarks { get; set; }
+    public string Visibility => IsPublic ? "public " : "";
+    public string Modifier => IsPublic ? "static" : "const";
+    public string Lambda => IsPublic ? ">" : "";
 }
 
 [DebuggerDisplay("Name = {Name}, NestedAreas = {NestedAreas.Count}, Values = {Values.Count}")]
diff --git a/src/ThisAssembly.Constants/ThisAssembly.Constants.targets b/src/ThisAssembly.Constants/ThisAssembly.Constants.targets
@@ -3,6 +3,7 @@
 
   <ItemGroup>
     <CompilerVisibleProperty Include="ThisAssemblyNamespace" />
+    <CompilerVisibleProperty Include="ThisAssemblyVisibility" />
 
     <CompilerVisibleItemMetadata Include="Constant" MetadataName="ItemType" />
     <CompilerVisibleItemMetadata Include="Constant" MetadataName="Comment" />
diff --git a/src/ThisAssembly.Resources/CSharp.sbntxt b/src/ThisAssembly.Resources/CSharp.sbntxt
@@ -52,7 +52,7 @@ namespace {{ Namespace }};
 /// <summary>
 /// Provides access to the current assembly information.
 /// </summary>
-partial class ThisAssembly
+{{ Visibility }}partial class ThisAssembly
 {
     /// <summary>
     /// Provides access to assembly resources.
diff --git a/src/ThisAssembly.Resources/Model.cs b/src/ThisAssembly.Resources/Model.cs
@@ -7,9 +7,10 @@
 namespace ThisAssembly;
 
 [DebuggerDisplay("Values = {RootArea.Values.Count}")]
-record Model(Area RootArea, string? Namespace)
+record Model(Area RootArea, string? Namespace, bool IsPublic)
 {
     public string Version => Assembly.GetExecutingAssembly().GetName().Version.ToString(3);
+    public string Visibility => IsPublic ? "public " : "";
 }
 
 [DebuggerDisplay("Name = {Name}")]
diff --git a/src/ThisAssembly.Resources/ResourcesGenerator.cs b/src/ThisAssembly.Resources/ResourcesGenerator.cs
@@ -47,7 +47,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
 
         // Read the ThisAssemblyNamespace property or default to null
         var right = context.AnalyzerConfigOptionsProvider
-            .Select((c, t) => c.GlobalOptions.TryGetValue("build_property.ThisAssemblyNamespace", out var ns) && !string.IsNullOrEmpty(ns) ? ns : null);
+            .Select((c, t) => (
+                c.GlobalOptions.TryGetValue("build_property.ThisAssemblyNamespace", out var ns) && !string.IsNullOrEmpty(ns) ? ns : null,
+                c.GlobalOptions.TryGetValue("build_property.ThisAssemblyVisibility", out var visibility) && !string.IsNullOrEmpty(visibility) ? visibility : null
+              ));
 
         context.RegisterSourceOutput(
             files.Combine(right),
@@ -56,9 +59,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
 
     static void GenerateSource(SourceProductionContext spc,
         ((ImmutableArray<(string resourceName, string? kind, string? comment)> files,
-            ImmutableArray<string> extensions), string? ns) args)
+            ImmutableArray<string> extensions), (string? ns, string? visibility)) args)
     {
-        var ((files, extensions), ns) = args;
+        var ((files, extensions), (ns, visibility)) = args;
 
         var file = "CSharp.sbntxt";
         var template = Template.Parse(EmbeddedResource.GetContent(file), file);
@@ -87,7 +90,7 @@ static void GenerateSource(SourceProductionContext spc,
                 .ToList();
 
             var root = Area.Load(basePath, resources);
-            var model = new Model(root, ns);
+            var model = new Model(root, ns, "public".Equals(visibility, StringComparison.OrdinalIgnoreCase));
 
             var output = template.Render(model, member => member.Name);
 
diff --git a/src/ThisAssembly.Resources/readme.md b/src/ThisAssembly.Resources/readme.md
@@ -43,7 +43,15 @@ treated as a text file:
 You can also add a `Comment` item metadata attribute, which will be used as the `<summary>` XML 
 doc for the generated member.
 
+## Customizing the generated code
+
+The following MSBuild properties can be used to customize the generated code:
+
+| Property                | Description                                                                                          |
+|-------------------------|------------------------------------------------------------------------------------------------------|
+| ThisAssemblyNamespace   | Sets the namespace of the generated `ThisAssembly` root class. If not set, it will be in the global namespace. |
+| ThisAssemblyVisibility  | Sets the visibility modifier of the generated `ThisAssembly` root class. If not set, it will be internal. |
+
 <!-- #resources -->
-<!-- include ../visibility.md -->
 <!-- include https://github.com/devlooped/sponsors/raw/main/footer.md -->
 <!-- exclude -->
diff --git a/src/ThisAssembly.Strings/CSharp.sbntxt b/src/ThisAssembly.Strings/CSharp.sbntxt
@@ -86,7 +86,7 @@ namespace {{ Namespace }};
 /// <summary>
 /// Provides access to the current assembly information.
 /// </summary>
-partial class ThisAssembly
+{{ Visibility }}partial class ThisAssembly
 {
     /// <summary>
     /// Provides access to the assembly strings.
diff --git a/src/ThisAssembly.Strings/Model.cs b/src/ThisAssembly.Strings/Model.cs
@@ -7,12 +7,13 @@
 using System.Xml.Linq;
 
 [DebuggerDisplay("ResourceName = {ResourceName}, Values = {RootArea.Values.Count}")]
-record Model(ResourceArea RootArea, string ResourceName, string? Namespace)
+record Model(ResourceArea RootArea, string ResourceName, string? Namespace, bool IsPublic)
 {
     public string? Version => Assembly.GetExecutingAssembly().GetName().Version?.ToString(3);
     public string Url => Devlooped.Sponsors.SponsorLink.Funding.HelpUrl;
     public string? Warn { get; set; }
     public string? Remarks { get; set; }
+    public string Visibility => IsPublic ? "public " : "";
 }
 
 static class ResourceFile
diff --git a/src/ThisAssembly.Strings/StringsGenerator.cs b/src/ThisAssembly.Strings/StringsGenerator.cs
@@ -20,14 +20,17 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
     {
         // Read the ThisAssemblyNamespace property or default to null
         var right = context.AnalyzerConfigOptionsProvider
-            .Select((c, t) => c.GlobalOptions.TryGetValue("build_property.ThisAssemblyNamespace", out var ns) && !string.IsNullOrEmpty(ns) ? ns : null)
+            .Select((c, t) => (
+                c.GlobalOptions.TryGetValue("build_property.ThisAssemblyNamespace", out var ns) && !string.IsNullOrEmpty(ns) ? ns : null,
+                c.GlobalOptions.TryGetValue("build_property.ThisAssemblyVisibility", out var visibility) && !string.IsNullOrEmpty(visibility) ? visibility : null
+              ))
             .Combine(context.CompilationProvider.Select((s, _) => s.Language));
 
         context.RegisterSourceOutput(
             right,
             (spc, args) =>
             {
-                var (ns, _) = args;
+                var ((ns, _), _) = args;
 
                 var strings = EmbeddedResource.GetContent($"ThisAssembly.Strings.sbntxt");
                 var template = Template.Parse(strings);
@@ -55,15 +58,15 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
     }
 
     static void GenerateSource(SourceProductionContext spc,
-        (((string fileName, SourceText? text, string resourceName), (string? ns, string language)), (ParseOptions parse, StatusOptions options)) arg)
+        (((string fileName, SourceText? text, string resourceName), ((string? ns, string? visibility), string language)), (ParseOptions parse, StatusOptions options)) arg)
     {
-        var (((fileName, resourceText, resourceName), (ns, language)), (parse, options)) = arg;
+        var (((fileName, resourceText, resourceName), ((ns, visibility), language)), (parse, options)) = arg;
 
         var file = language.Replace("#", "Sharp") + ".sbntxt";
         var template = Template.Parse(EmbeddedResource.GetContent(file), file);
 
         var rootArea = ResourceFile.LoadText(resourceText!.ToString(), "Strings");
-        var model = new Model(rootArea, resourceName, ns);
+        var model = new Model(rootArea, resourceName, ns, "public".Equals(visibility, StringComparison.OrdinalIgnoreCase));
         if (IsEditor)
         {
             var status = Diagnostics.GetOrSetStatus(options);
diff --git a/src/ThisAssembly.Strings/readme.md b/src/ThisAssembly.Strings/readme.md
@@ -68,7 +68,15 @@ partial class ThisAssembly
 }
 ```
 
+## Customizing the generated code
+
+The following MSBuild properties can be used to customize the generated code:
+
+| Property                | Description                                                                                          |
+|-------------------------|------------------------------------------------------------------------------------------------------|
+| ThisAssemblyNamespace   | Sets the namespace of the generated `ThisAssembly` root class. If not set, it will be in the global namespace. |
+| ThisAssemblyVisibility  | Sets the visibility modifier of the generated `ThisAssembly` root class. If not set, it will be internal. |
+
 <!-- #strings -->
-<!-- include ../visibility.md -->
 <!-- include https://github.com/devlooped/sponsors/raw/main/footer.md -->
 <!-- exclude -->
diff --git a/src/ThisAssembly.Tests/ThisAssembly.Tests.csproj b/src/ThisAssembly.Tests/ThisAssembly.Tests.csproj
@@ -4,6 +4,7 @@
     <IsPackable>false</IsPackable>
     <TargetFramework>net8.0</TargetFramework>
     <ThisAssemblyNamespace>ThisAssemblyTests</ThisAssemblyNamespace>
+    <ThisAssemblyVisibility>public</ThisAssemblyVisibility>
     <Multiline>
       A Description
       with a newline and
diff --git a/src/visibility.md b/src/visibility.md
@@ -3,14 +3,56 @@
 Set the `$(ThisAssemblyNamespace)` MSBuild property to set the namespace of the 
 generated `ThisAssembly` root class. Otherwise, it will be generated in the global namespace.
 
-All generated classes are partial and have no visibility modifier, so they can be extended 
-manually with another partial that can add members or modify their visibility to make them 
-public, if needed. The C# default for this case is for all classes to be internal.
+The generated root `ThisAssembly` class is partial and has no visibility modifier by default, 
+making it internal by default in C#.
 
+You can set the `$(ThisAssemblyVisibility)` MSBuild property to `public` to make it public. 
+This will also change all constants to be static readonly properties instead. 
+
+Default:
+```csharp
+partial class ThisAssembly
+{
+    public partial class Constants
+    {
+        public const string Hello = "World";
+    }
+}
+```
+
+In this case, the compiler will inline the constants directly into the consuming code at 
+the call site, which is optimal for performance for the common usage of constants.
+
+Public:
 ```csharp
-// makes the generated classes public
+public partial class ThisAssembly
+{
+    public partial class Constants
+    {
+        public static string Hello => "World";
+    }
+}
+```
+
+This makes it possible for consuming code to remain unchanged and not require 
+a recompile when the the values of `ThisAssembly` are changed in a referenced assembly.
+
+If you want to keep the properties as constants, you can instead extend the generated 
+code by defining a another partial that can modify its visibility as needed (or add 
+new members). 
+
+```csharp
+// makes the generated class public
 public partial ThisAssembly 
 {
-    public partial class Constants { }
+    // Nested classes are always public since the outer class 
+    // already limits their visibility
+    partial class Constants 
+    {
+        // add some custom constants
+        public const string MyConstant = "This isn't configurable via MSBuild";
+
+        // generated code will remain as constants
+    }
 }
 ```

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ namespace {{ Namespace }};`
`52`	`52`	`/// <summary>`
`53`	`53`	`/// Provides access to the current assembly information.`
`54`	`54`	`/// </summary>`
`55`		`-partial class ThisAssembly`
	`55`	`+{{ Visibility }}partial class ThisAssembly`
`56`	`56`	`{`
`57`	`57`	`/// <summary>`
`58`	`58`	`/// Provides access to assembly resources.`
Original file line number	Diff line number	Diff line change
`@@ -7,9 +7,10 @@`
`7`	`7`	`namespace ThisAssembly;`
`8`	`8`
`9`	`9`	`[DebuggerDisplay("Values = {RootArea.Values.Count}")]`
`10`		`-record Model(Area RootArea, string? Namespace)`
	`10`	`+record Model(Area RootArea, string? Namespace, bool IsPublic)`
`11`	`11`	`{`
`12`	`12`	`public string Version => Assembly.GetExecutingAssembly().GetName().Version.ToString(3);`
	`13`	`+ public string Visibility => IsPublic ? "public " : "";`
`13`	`14`	`}`
`14`	`15`
`15`	`16`	`[DebuggerDisplay("Name = {Name}")]`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ namespace {{ Namespace }};`
`86`	`86`	`/// <summary>`
`87`	`87`	`/// Provides access to the current assembly information.`
`88`	`88`	`/// </summary>`
`89`		`-partial class ThisAssembly`
	`89`	`+{{ Visibility }}partial class ThisAssembly`
`90`	`90`	`{`
`91`	`91`	`/// <summary>`
`92`	`92`	`/// Provides access to the assembly strings.`
Original file line number	Diff line number	Diff line change
`@@ -7,12 +7,13 @@`
`7`	`7`	`using System.Xml.Linq;`
`8`	`8`
`9`	`9`	`[DebuggerDisplay("ResourceName = {ResourceName}, Values = {RootArea.Values.Count}")]`
`10`		`-record Model(ResourceArea RootArea, string ResourceName, string? Namespace)`
	`10`	`+record Model(ResourceArea RootArea, string ResourceName, string? Namespace, bool IsPublic)`
`11`	`11`	`{`
`12`	`12`	`public string? Version => Assembly.GetExecutingAssembly().GetName().Version?.ToString(3);`
`13`	`13`	`public string Url => Devlooped.Sponsors.SponsorLink.Funding.HelpUrl;`
`14`	`14`	`public string? Warn { get; set; }`
`15`	`15`	`public string? Remarks { get; set; }`
	`16`	`+ public string Visibility => IsPublic ? "public " : "";`
`16`	`17`	`}`
`17`	`18`
`18`	`19`	`static class ResourceFile`