no-mistakes(review): Warn on skipped docs generator only when selected; fix pin wording

Author: dmealing

CHANGELOG.md

@@ -160,7 +160,9 @@ generator catalog and the Atlas evaluation are 1.2 work.
     packaged generators emit for (`GeneratorApplicability.Packaged`): an owned `routes` whose
     `AppliesTo` you changed is documented as the packaged one behaves, unless you edit the
     owned builder.
-    `dotnet meta gen` skips a docs generator and says so, and `verify --codegen` leaves it out.
+    `dotnet meta gen` and `verify --codegen` leave a docs generator out. `gen` says so only
+    when a `--generators` selection names `api-docs`; an owned `Program.cs` that lists it runs
+    without a warning.
     New public API in `MetaObjects.Codegen`: `IDocsGenerator`, `DocsRunFacts`,
     `GeneratorApplicability`, `GenContext.Applicability` and `GenContext.Docs`,
     `CodegenCli.RunDocs`, and `ValueObjectNames.ResolveValueObject` (moved from

docs/features/own-your-codegen.md

@@ -1121,9 +1121,10 @@ packaged one behaves. To document your own surface, edit the owned builder
 class, which is why the copy depends on nothing but the engine.
 
 `Program.cs` lists `new ApiDocsGenerator()` beside the code generators. `dotnet meta gen` and
-`verify --codegen` skip it, and `gen` says so (`"api-docs" writes the api reference, so
-gen skipped it. Run it with dotnet meta docs.`): it writes pages under the docs directory,
-not code under `--out`.
+`verify --codegen` skip it: it writes pages under the docs directory, not code under `--out`.
+The skip is silent while the generator is only in that list. When a `--generators` selection
+names `api-docs`, `gen` says so (`"api-docs" writes the api reference, so gen skipped it.
+Run it with dotnet meta docs.`), with or without an owned project.
 
 ### Ejecting routes hands over the helper runtime too
 

server/csharp/MetaObjects.Cli.Tests/EjectCommandTests.cs

@@ -158,13 +158,37 @@ public void Ejecting_into_a_project_pinned_to_an_older_package_warns_and_names_t
         Assert.Contains("MetaObjects.Codegen 0.9.0", warning);
         Assert.Contains(EjectCommand.ToolVersion(), warning);
         Assert.Contains("will not compile", warning);
-        Assert.Contains($"line {pinLine} of codegen/Codegen.csproj", warning);
+        Assert.Contains($"set the version on line {pinLine} of codegen/Codegen.csproj to {EjectCommand.ToolVersion()}", warning);
         Assert.Contains(
             $"<PackageReference Include=\"MetaObjects.Codegen\" Version=\"{EjectCommand.ToolVersion()}\" />",
             result.Out[result.Out.ToList().IndexOf(warning) + 1]);
         Assert.Equal(csproj, File.ReadAllText(CsprojPath));
     }
 
+    // The line the warning names carries only the version in these shapes, so pasting a whole
+    // PackageReference element over it would nest one inside another, or drop what else the
+    // reference declares. The warning names the line and the version and prints no element.
+    [Theory]
+    [InlineData("<PackageReference Include=\"MetaObjects.Codegen\"\n                      Version=\"0.9.0\" />")]
+    [InlineData("<PackageReference Include=\"MetaObjects.Codegen\">\n      <Version>0.9.0</Version>\n    </PackageReference>")]
+    [InlineData("<PackageReference Include=\"MetaObjects.Codegen\" Version=\"0.9.0\" PrivateAssets=\"all\" />")]
+    public void An_older_pin_not_in_the_scaffolded_form_is_named_by_line_and_version_only(string reference)
+    {
+        ScaffoldPinnedAt(null);
+        var csproj =
+            "<Project Sdk=\"Microsoft.NET.Sdk\">\n  <ItemGroup>\n    " + reference + "\n  </ItemGroup>\n</Project>\n";
+        File.WriteAllText(CsprojPath, csproj);
+        var versionLine = Array.FindIndex(csproj.Split('\n'), l => l.Contains("0.9.0", StringComparison.Ordinal)) + 1;
+
+        var result = EjectCommand.Run(["api-docs"], _root, force: false);
+
+        Assert.Equal(0, result.ExitCode);
+        var warning = Assert.Single(result.Out, l => l.StartsWith("warning:", StringComparison.Ordinal));
+        Assert.Contains($"set the version on line {versionLine} of codegen/Codegen.csproj to {EjectCommand.ToolVersion()}.", warning);
+        Assert.DoesNotContain(result.Out, l => l.Contains("<PackageReference", StringComparison.Ordinal));
+        Assert.Equal(csproj, File.ReadAllText(CsprojPath));
+    }
+
     [Theory]
     [InlineData(null)]                      // the pin this tool wrote: equal
     [InlineData("99.0.0")]                  // newer

server/csharp/MetaObjects.Cli.Tests/EjectEndToEndTests.cs

@@ -198,7 +198,7 @@ private void PointScaffoldAtThisCheckout(string repoRoot)
     /// An owned api-docs generator, run for real (ADR-0025 / ADR-0034 Amendment 3): eject `routes` and
     /// `api-docs`, add a page to the owned api-docs copy, and run the owned project's `docs` command. The
     /// reference is written by the OWNED api-docs copy, so the added page is on disk beside the model's;
-    /// and the same owned project's `gen` skips the docs generator and says so.
+    /// and the same owned project's `gen` skips the docs generator, saying so only when asked for it by name.
     /// </summary>
     [Fact]
     public void Owned_api_docs_writes_the_reference()
@@ -241,12 +241,20 @@ public void Owned_api_docs_writes_the_reference()
             "written by the owned api-docs",
             File.ReadAllText(Directory.GetFiles(docsOut, "OWNED.md", SearchOption.AllDirectories).Single()));
 
-        // The same owned project's gen leaves the docs generator out and says why.
-        var (genExit, genOutput) = RunCodegenProject(
-            "gen", MetadataDir, "--out", OutDir, "--namespace", "Acme.Generated", "--generators", "api-docs");
+        // The same owned project's gen leaves the docs generator out. With the generator only in the
+        // owned list, that is the everyday run and it has nothing to say about it.
+        var (genExit, genOutput) = RunCodegenProject("gen", MetadataDir, "--out", OutDir, "--namespace", "Acme.Generated");
         Assert.True(genExit == 0, $"gen failed (exit {genExit}):\n{genOutput}");
-        Assert.Contains("writes the api reference, so `gen` skipped it", genOutput);
-        Assert.Empty(Directory.Exists(OutDir) ? Directory.GetFiles(OutDir, "*.md", SearchOption.AllDirectories) : []);
+        Assert.DoesNotContain("writes the api reference", genOutput);
+        Assert.NotEmpty(Directory.GetFiles(OutDir, "*.cs", SearchOption.AllDirectories));
+        Assert.Empty(Directory.GetFiles(OutDir, "*.md", SearchOption.AllDirectories));
+
+        // Asked for by name, it is still left out, and gen says why.
+        var (namedExit, namedOutput) = RunCodegenProject(
+            "gen", MetadataDir, "--out", OutDir, "--namespace", "Acme.Generated", "--generators", "api-docs");
+        Assert.True(namedExit == 0, $"gen failed (exit {namedExit}):\n{namedOutput}");
+        Assert.Contains("writes the api reference, so `gen` skipped it", namedOutput);
+        Assert.Empty(Directory.GetFiles(OutDir, "*.md", SearchOption.AllDirectories));
     }
 
     /// <summary>

server/csharp/MetaObjects.Cli.Tests/GenListAndSelectionTests.cs

@@ -99,6 +99,21 @@ public void Previously_omitted_output_prompt_is_now_selectable_and_emits()
         Assert.True(File.Exists(Path.Combine(OutDir, "Alpha.responseFormat.cs")));
     }
 
+    [Fact]
+    public void Selecting_the_docs_generator_writes_no_pages_and_says_which_command_does()
+    {
+        // api-docs is in the catalog, so a selection can name it; its pages belong to `dotnet meta docs`.
+        var outcome = GenCommand.Run(
+            MetaDir, OutDir, "Acme.Generated",
+            emitAbstractShapes: false, generatorNames: ["entity", "api-docs"], templateRoot: null);
+
+        Assert.True(outcome.Ok, string.Join("; ", outcome.LoadErrors));
+        var warning = Assert.Single(outcome.Result!.Warnings, w => w.Contains("\"api-docs\"", StringComparison.Ordinal));
+        Assert.Contains("`dotnet meta docs`", warning);
+        Assert.NotEmpty(Directory.GetFiles(OutDir, "*.cs", SearchOption.AllDirectories));
+        Assert.Empty(Directory.GetFiles(OutDir, "*.md", SearchOption.AllDirectories));
+    }
+
     [Fact]
     public void Unknown_generator_name_surfaces_as_an_error_not_a_throw()
     {

server/csharp/MetaObjects.Cli/EjectCommand.cs

@@ -134,8 +134,9 @@ public static bool IsOlderVersion(string pinned, string required)
 
     /// <summary>The version <paramref name="csprojPath"/> pins <c>MetaObjects.Codegen</c> to and the
     /// 1-based line that carries it, or <c>null</c> when the file is not readable XML or has no such
-    /// PackageReference with a version.</summary>
-    private static (string Version, int Line)? PinnedCodegenVersion(string csprojPath)
+    /// PackageReference with a version. <c>Scaffolded</c> is true when that line is the whole
+    /// reference in the form eject scaffolds: one element, <c>Include</c> and <c>Version</c>, no children.</summary>
+    private static (string Version, int Line, bool Scaffolded)? PinnedCodegenVersion(string csprojPath)
     {
         XDocument doc;
         try { doc = XDocument.Load(csprojPath, LoadOptions.SetLineInfo); }
@@ -148,8 +149,10 @@ private static (string Version, int Line)? PinnedCodegenVersion(string csprojPat
             ?? reference?.Elements().FirstOrDefault(e => e.Name.LocalName == "Version");
         return version switch
         {
-            XAttribute a => (a.Value.Trim(), ((IXmlLineInfo)a).LineNumber),
-            XElement e => (e.Value.Trim(), ((IXmlLineInfo)e).LineNumber),
+            XAttribute a => (a.Value.Trim(), ((IXmlLineInfo)a).LineNumber,
+                ((IXmlLineInfo)a).LineNumber == ((IXmlLineInfo)reference!).LineNumber
+                && reference.Attributes().Count() == 2 && !reference.Nodes().Any()),
+            XElement e => (e.Value.Trim(), ((IXmlLineInfo)e).LineNumber, false),
             _ => null,
         };
     }
@@ -160,15 +163,14 @@ private static IReadOnlyList<string> OlderPinWarning(string csprojPath)
     {
         var tool = ToolVersion();
         if (PinnedCodegenVersion(csprojPath) is not { } pin || !IsOlderVersion(pin.Version, tool)) return [];
-        return
-        [
-            "",
+        var warning =
             $"warning: codegen/Codegen.csproj pins {PinnedPackage} {pin.Version}, older than the {tool} this " +
             $"eject wrote its sources from. Those sources are written against the {tool} package API, so the " +
             $"owned project will not compile until the pin is {tool} or later. Eject never edits that file: " +
-            $"change line {pin.Line} of codegen/Codegen.csproj to",
-            $"  <PackageReference Include=\"{PinnedPackage}\" Version=\"{tool}\" />",
-        ];
+            $"set the version on line {pin.Line} of codegen/Codegen.csproj to {tool}";
+        return pin.Scaffolded
+            ? ["", warning + ", so the line reads", $"  <PackageReference Include=\"{PinnedPackage}\" Version=\"{tool}\" />"]
+            : ["", warning + "."];
     }
 
     private static string ProgramCsContent(IEnumerable<string> classNames)

server/csharp/MetaObjects.Cli/GenCommand.cs

@@ -169,7 +169,7 @@ public static Outcome Run(
         // `codegen/Program.cs` calls too, with its owned generators composed into the same
         // selection (CodegenCli.ComposeGenerators) — one implementation, not two.
         var apiOutcome = MetaObjects.Codegen.CodegenCli.RunGen(
-            load, outDir, ns, emitAbstractShapes, generators, projectRoot, columnNaming, baseline);
+            load, outDir, ns, emitAbstractShapes, generators, projectRoot, columnNaming, baseline, names);
         return new Outcome(apiOutcome.LoadErrors, apiOutcome.Result);
     }
 

server/csharp/MetaObjects.Codegen.Tests/ApiDocsRunTests.cs

@@ -1,7 +1,7 @@
 // The docs run: `api-docs` as a generator (ADR-0025), what runs it, and what it documents.
 //
 // A project can own the api reference like any other generator. These cover the engine around it:
-// a docs generator is not a code generator (`gen` skips it and says so), the docs run takes a
+// a docs generator is not a code generator (`gen` skips it, and says so when asked for it by name), the docs run takes a
 // project's own copy in place of the packaged one, and what the reference documents is what the
 // PACKAGED generators emit for, whichever generators the run names.
 
@@ -94,9 +94,11 @@ public void Two_pages_at_one_path_fail_the_run_before_anything_is_written()
     }
 
     [Fact]
-    public void A_code_run_skips_a_docs_generator_and_says_so()
+    public void A_code_run_whose_selection_names_a_docs_generator_skips_it_and_says_so()
     {
-        var outcome = CodegenCli.RunGen(Load(), _out, "Acme.Generated", false, [new ApiDocsGenerator(), new EntityGenerator()]);
+        var outcome = CodegenCli.RunGen(
+            Load(), _out, "Acme.Generated", false, [new ApiDocsGenerator(), new EntityGenerator()],
+            selection: ["api-docs", "entity"]);
 
         Assert.True(outcome.Ok);
         Assert.Contains(outcome.Result!.Warnings, w => w == CodegenCli.DocsGeneratorSkipped("api-docs"));
@@ -105,6 +107,24 @@ public void A_code_run_skips_a_docs_generator_and_says_so()
         Assert.NotEmpty(outcome.Result.Files);
     }
 
+    // An owned project lists its docs generator beside its code generators, because that one list is
+    // where `docs` finds it. A code run that did not ask for it by name skips it and has nothing to say:
+    // a warning here would fire on every `gen` and could only be cleared by giving the generator up.
+    [Theory]
+    [InlineData(null)]
+    [InlineData("entity")]
+    public void A_code_run_that_only_finds_a_docs_generator_in_its_list_skips_it_silently(string? selected)
+    {
+        var outcome = CodegenCli.RunGen(
+            Load(), _out, "Acme.Generated", false, [new ApiDocsGenerator(), new EntityGenerator()],
+            selection: selected is null ? null : [selected]);
+
+        Assert.True(outcome.Ok);
+        Assert.DoesNotContain(outcome.Result!.Warnings, w => w.Contains("api reference", StringComparison.Ordinal));
+        Assert.DoesNotContain(outcome.Result.Files, f => f.Path.Contains("AGENT-API", StringComparison.Ordinal));
+        Assert.NotEmpty(outcome.Result.Files);
+    }
+
     [Fact]
     public void A_drift_check_leaves_a_docs_generator_out_too()
     {

server/csharp/MetaObjects.Codegen/CodegenCli.cs

@@ -200,10 +200,17 @@ public static GenOutcome RunGen(
     /// to the current directory for a caller that names no project (mirrors
     /// <c>GenConfig.ProjectRoot</c>'s own default).
     /// </param>
+    /// <param name="selection">
+    /// The <c>--generators</c> names the run was asked for, when it was asked by name. A docs
+    /// generator is never run here; one this selection names is reported as skipped
+    /// (<see cref="DocsGeneratorSkipped"/>), and one that is only in <paramref name="generators"/>
+    /// — an owned list, where the docs run looks for it — is left out without a word.
+    /// </param>
     public static GenOutcome RunGen(
         LoadResult load, string outDir, string ns, bool emitAbstractShapes,
         IReadOnlyList<IGenerator> generators, string? projectRoot = null,
-        ColumnNamingStrategy columnNaming = ColumnNamingStrategy.Literal, string baseline = "default")
+        ColumnNamingStrategy columnNaming = ColumnNamingStrategy.Literal, string baseline = "default",
+        IReadOnlyList<string>? selection = null)
     {
         var loadErrors = load.Errors.Select(e => e.Code.ToString()).ToList();
         if (loadErrors.Count > 0) return new GenOutcome(loadErrors, null);
@@ -228,9 +235,10 @@ public static GenOutcome RunGen(
             IncludeNames = generators.Any(g => g.Name == "names"),
         };
 
-        // A docs generator writes the api reference, not code: CodegenRunner leaves it out, and
-        // this run says so rather than writing pages into the generated-code tree.
-        var docsSkipped = generators.OfType<IDocsGenerator>().Select(g => g.Name).ToList();
+        // A docs generator writes the api reference, not code: CodegenRunner leaves it out. A run
+        // asked for one by name says so; an owned list carries one for the docs run, not for this.
+        var docsSkipped = generators.OfType<IDocsGenerator>().Select(g => g.Name)
+            .Where(name => selection?.Contains(name) == true).ToList();
 
         CodegenRunner.RunResult result;
         try
@@ -253,7 +261,7 @@ public static GenOutcome RunGen(
         return new GenOutcome(loadErrors, result);
     }
 
-    /// <summary>What a code run says about a docs generator it was handed and did not run.</summary>
+    /// <summary>What a code run says about a docs generator its selection named and it did not run.</summary>
     public static string DocsGeneratorSkipped(string name) =>
         $"\"{name}\" writes the api reference, so `gen` skipped it. Run it with `dotnet meta docs`.";
 
@@ -488,7 +496,7 @@ private static int RunGenArgs(string[] rest, IReadOnlyList<IGenerator> owned)
         }
 
         WarnUnknownAttrs(meta);
-        var outcome = RunGen(meta.Load(), a.OutDir, a.Namespace, a.EmitAbstractShapes, suite, projectRoot, columnNaming, a.Baseline);
+        var outcome = RunGen(meta.Load(), a.OutDir, a.Namespace, a.EmitAbstractShapes, suite, projectRoot, columnNaming, a.Baseline, a.Selection);
         if (!outcome.Ok)
         {
             ReportFailure(outcome, "codegen (owned) gen");

server/csharp/MetaObjects.Codegen/DocsGenerator.cs

@@ -4,7 +4,8 @@
 // from generators, but a docs generator must never be run by `gen`: it writes pages under the
 // docs directory, not code under the generated-code directory, and `gen`'s hash manifest and
 // overwrite policy are about code. A generator that takes part in the docs run says so by
-// implementing IDocsGenerator; `gen` and `verify --codegen` skip it and say they did.
+// implementing IDocsGenerator; `gen` and `verify --codegen` skip it, and `gen` says so when its
+// `--generators` selection named it.
 //
 // One list serves both commands. An owned `codegen/Program.cs` lists a docs generator beside the
 // code generators, and each command runs the half that is its own.