Warnings as errors + built-in analyzers (#55), user-facing changelog (#54)

.github/workflows/tooling-tests.yml

@@ -87,6 +87,9 @@ jobs:
       - name: Lint member ordering
         run: node tooling~/scripts/lint-member-ordering.mjs
 
+      - name: Lint multi-line comments
+        run: node tooling~/scripts/lint-multiline-comments.mjs
+
   package-content:
     # Clean-install guard for the UPM artifact (issue #22, PLAN.md T02
     # "package-content validators"): packs the package exactly like npm/UPM

.llm/context.md

@@ -151,6 +151,10 @@ frontmatter validity, index freshness, and pointer-file delegation; see
     Enforced by `npm --prefix tooling~ run lint:member-ordering` (pre-commit + CI; `:fix` is a
     permutation-only reorder that never crosses `#if` boundaries or type-load-initializer
     dependencies).
+21. Multi-line comments are block comments: two or more consecutive comment-only `//` lines
+    must be one `/*` ... `*/` block instead. Single `//` lines and `///` doc comments stay
+    legal. Enforced by `npm --prefix tooling~ run lint:multiline-comments` (pre-commit + CI;
+    `:fix` converts runs, refusing content that contains the block-comment close).
 
 ### Unity Package Rules
 
@@ -183,6 +187,17 @@ PR descriptions stay at or under ~20 lines, commit bodies at or under ~12, one l
 bullet, no nested bullets. Code comments stay minimal - only what the code cannot say.
 Details: [simple-writing](./skills/simple-writing/SKILL.md).
 
+### CHANGELOG (user-facing only)
+
+`CHANGELOG.md` records only changes a package consumer can observe: public API and
+serialized-data changes, behavior changes, fixes, install-size or console-output changes.
+Internal work (refactors, tooling, style enforcement, linters, measurement, CI lanes)
+stays out, and so do internal numbers (timings, allocation counts, test tallies) - those
+live in PR descriptions, issues, and `progress/` logs. If a user cannot observe the
+difference, it does not belong in the changelog. New `Unreleased` entries follow the
+policy stated in the file header; a PR that touches `CHANGELOG.md` keeps entries in the
+existing `Added/Changed/Fixed/Removed` buckets.
+
 ### LLM Attribution (GitHub)
 
 LLM-generated comments, issues, PR descriptions, and reviews start with

.pre-commit-config.yaml

@@ -47,6 +47,19 @@ repos:
           properties, fields, constructors, static methods, methods; each tier public >
           protected > internal > private) and nested types at the end of their containing type.
           Use `--fix` for a permutation-only reorder.
+      - id: multiline-comments-lint
+        name: Lint C# multi-line comments (block comments, not stacked '//'; PR #57 review)
+        entry: node tooling~/scripts/lint-multiline-comments.mjs
+        language: system
+        always_run: true
+        pass_filenames: false
+        stages:
+          - pre-commit
+          - pre-push
+        description: >
+          Two or more consecutive comment-only '//' lines must be one '/*'...'*/' block
+          comment instead. Single '//' lines and '///' doc comments stay legal. Use
+          `--fix` to convert runs to block comments.
       - id: llm-instructions-lint
         name: Lint .llm instructions (SKILL.md spec, index freshness, pointer delegation)
         entry: pwsh -NoProfile -File tooling~/scripts/lint-llm-instructions.ps1

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Changelog
 
-All notable changes to this project will be documented in this file.
+All notable, user-facing changes to this project will be documented in this file. Internal
+work (refactors, tooling, style enforcement, measurement infrastructure) stays out; if a
+user cannot observe the difference, it does not belong here.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
@@ -10,23 +12,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 - Deferred first-use command registration: `CommandShell.InitializeAutoRegisteredCommands` accepts a `deferRegistration` flag. The terminal applies its ignored/default command configuration when enabled and defers the discovery scan and delegate materialization to the first command request, the first read of `CommandShell.Commands`, or an explicit `CommandShell.EnsureAutoCommandsRegistered` call. `CommandShell.AutoCommandsRegistered` reports whether registration has been applied. Clearing auto commands cancels a pending registration. Note that `CommandShell.AutoRegisteredCommands` reflects only applied registration, so it stays empty until first use for deferred shells.
 - Source-generated command registration: the package now ships a Roslyn source generator that emits an internal `CommandCatalog` into every assembly declaring `[RegisterCommand]` methods. `CommandShell` binds those catalogs without walking assembly types; assemblies without a catalog (precompiled DLLs, analyzers unavailable) fall back to the previous reflection discovery with identical results. The public `CommandShell.RegisteredCommands` surface is unchanged.
-- Context-aware execution (T08): new `CommandExecutionContexts`, `CommandExecutionContext`, `CommandDefinition`, `CommandHandler`, `BorrowedCommandArguments` types, registered through `CommandShell.AddCommand(CommandDefinition)`. Definitions default to `CommandExecutionContextSets.Gameplay`; Edit Mode execution is opt-in. `[RegisterCommand]` gains a `Contexts` property (default `CommandExecutionContextSets.All`, so existing attributed commands keep their availability). `CommandDefinition.MaxArgCount` is `int?` (`null` = unbounded).
+- Context-aware execution: new `CommandExecutionContexts`, `CommandExecutionContext`, `CommandDefinition`, `CommandHandler`, `BorrowedCommandArguments` types, registered through `CommandShell.AddCommand(CommandDefinition)`. Definitions default to `CommandExecutionContextSets.Gameplay`; Edit Mode execution is opt-in. `[RegisterCommand]` gains a `Contexts` property (default `CommandExecutionContextSets.All`, so existing attributed commands keep their availability). `CommandDefinition.MaxArgCount` is `int?` (`null` = unbounded).
 - Argument completion providers: `CommandShell.TryComplete` builds a `CommandCompletionContext` for the command under the caret and fills a caller-owned buffer. Custom replacement ranges use the validated `CommandCompletionReplacement` struct; provider exceptions are contained; results dedupe ordinally. Commands without a provider keep the previous history-based completion.
 - `TerminalUI` Tab now cycles provider token completions: only the active token is replaced, trailing text is preserved, space- or quote-containing insertions are quoted, and the caret lands after the insertion.
 - New shared `CommandTokenizer` for execution and completion, parity-pinned against `TryEatArgument` by a data-driven corpus.
 - `CommandDefinition` commands with `AddToHistory = false` dispatch without rebuilding the history line.
+- `CommandArgParsers`, a public static class exposing the culture-invariant parsers behind `CommandArg.TryGet`, one method per built-in type (`CommandArgParsers.Float`, `.Int`, `.DateTime`, ...), callable directly from command handlers and test code.
 
 ### Changed
 
 - Argument bounds use `int?` instead of the `-1 = unbounded` sentinel: `CommandInfo.maxArgCount` is now `int?` and `CommandShell.AddCommand` takes `int? maxArgs` (default `null`). Legacy callers passing `-1` keep compiling and mean unbounded. `[RegisterCommandAttribute].MaxArgCount` stays `int` (attribute properties cannot be nullable) and `CommandInfo` normalizes negatives to `null`.
-- `TerminalUI` no longer runs command discovery on its enable frame. Commands register at the first command request (typically the first command run or completion query), bounded by the same discovery path measured at 1.5-1.8 ms for 25 commands in the test project. The terminal re-applies its command configuration on every refresh, so auto commands cleared through `CommandShell.ClearAutoRegisteredCommands` return on the next first use after a terminal enable, instead of the previous enable-time registration.
+- `TerminalUI` no longer runs command discovery on its enable frame. Commands register at the first command request (typically the first command run or completion query). The terminal re-applies its command configuration on every refresh, so auto commands cleared through `CommandShell.ClearAutoRegisteredCommands` return on the next first use after a terminal enable, instead of the previous enable-time registration.
 - User commands now keep a name registered manually before first use: the colliding auto command is skipped with a console warning instead of queueing a duplicate `already defined` error for the first command request. This also applies at enable time when a user command shadows a built-in command.
 - Registering a static command whose signature is valid but not bindable (for example a generic method definition, a non-void handler, or a method inside an open generic type) now logs a contained error instead of aborting shell initialization, matching the catalog path's handling of rejected signatures.
 - An assembly holding commands inside private nested or file-local classes gets no generated catalog; the shell falls back to reflection for that assembly so no command is lost.
 - Nested command dispatches use per-depth parse scopes; legacy handlers still receive a fresh owned array per invocation, never pooled.
-- Comparison operators read left-to-right in ascending order (only `<`, `<=`, and `==`) and every C# type follows one member ordering: const, events, delegates, static properties, static fields, properties, fields, constructors, static methods, methods, each tier public > protected > internal > private, with nested types at the end of their containing type. Both rules are enforced by pre-commit hooks, a CI lane, and contract tests, and the codebase is swept to compliance.
+- The package compiles with warnings treated as errors: each package assembly ships a per-assembly compiler response file (`-warnaserror`), so a compiler warning inside these assemblies fails compilation for consumers too. A per-assembly response file replaces any project-level `csc.rsp` for these assemblies only.
+- The terminal no longer logs on its normal default-font and default-theme selection (previously every enable logged a `No font assigned` or `Persisted theme not found` warning). Warnings remain for a font pack containing no fonts, a stale persisted theme or font name, and misconfiguration; the missing-font fallback to an OS font is now a warning instead of an error. Theme persistence progress messages only appear in the Editor and in development builds.
 - The npm tarball no longer ships `Media/` (README screenshots and the demo GIF). The README references them through absolute repository URLs so rendering on GitHub and npm is unchanged. This cuts 7.5 MB from the tarball consumers download.
-- The command-catalog source generator emits with fewer allocations: a thread-cached, capacity-sized builder instead of per-line concatenation temporaries, and attribute matching that skips the display-string build for every non-matching attribute. The shipped analyzer payload stays byte-verified as the Release build of the generator sources.
+- Command argument parsing is culture-invariant: numeric, `DateTime`, `DateTimeOffset`, `TimeSpan`, and `BigInteger` arguments parse with `CultureInfo.InvariantCulture` regardless of the device locale, so `1.5` means one and a half on every machine. Locale-formatted input (for example `1,5` on a comma-decimal locale, or locale-formatted dates) is no longer accepted for these types.
+- `RegisterCommandAttribute.NormalizeName` validates its method argument and throws `ArgumentNullException` for null instead of failing later at registration.
 
 ### Fixed
 

Editor/csc.rsp

@@ -0,0 +1,2 @@
+-warnaserror
+-warn:4

Generator~/Directory.Build.props

@@ -7,5 +7,11 @@
     <PropertyGroup>
         <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
         <WarningLevel>9999</WarningLevel>
+        <!-- The SDK's built-in Roslyn analyzers (CA rules) run on every compilation, in the
+         most aggressive mode: every rule, and every analyzer diagnostic is an error. They ship
+         with the .NET SDK, so enforcement adds no dependency. -->
+        <EnableNETAnalyzers>true</EnableNETAnalyzers>
+        <AnalysisMode>All</AnalysisMode>
+        <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>
     </PropertyGroup>
 </Project>

Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs

@@ -425,7 +425,7 @@ public static void NumericContexts(CommandArg[] args)
         private static void AssertConditional(int defineCount, int expectedEntryCount)
         {
             string[] defines =
-                defineCount == 0 ? new string[0]
+                defineCount == 0 ? Array.Empty<string>()
                 : defineCount == 1 ? new[] { "UNITY_EDITOR" }
                 : new[] { "UNITY_EDITOR", "DEVELOPMENT_BUILD" };
 
@@ -500,8 +500,10 @@ public void ExecutesPublicAndPrivateBinders()
 
             Type argType = assembly.GetType("WallstopStudios.DxCommandTerminal.Backend.CommandArg");
 
-            // CommandArg is a value type, so its arrays cannot be cast to
-            // object[]; the arguments are boxed inside a one-element object[].
+            /*
+               CommandArg is a value type, so its arrays cannot be cast to
+               object[]; the arguments are boxed inside a one-element object[].
+            */
             Array publicArgs = Array.CreateInstance(argType, 1);
             catalog.BinderOf(catalog.Entries[0])(new object[] { publicArgs });
             Assert.Equal(1, publicInvocations.GetValue(null));
@@ -806,8 +808,10 @@ public void BlankInferredNamesAreEmittedAndRejectedLikeLegacy()
                     .output
             );
 
-            // The catalog is loadable: a blank inferred name must not poison
-            // the assembly's static initializer.
+            /*
+               The catalog is loadable: a blank inferred name must not poison
+               the assembly's static initializer.
+            */
             CatalogView catalog = CatalogView.Load(assembly);
             object entry = Assert.Single(catalog.Entries);
             Assert.Equal(string.Empty, catalog.NameOf(entry));

Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs

@@ -229,13 +229,15 @@ public void BindersExecutePublicAndPrivateCommands()
             publicEntry.Binder()(new CommandArg[] { new CommandArg("ignored") });
             Assert.Equal(1, CatalogFixtureCommands.PublicInvocations);
 
-            secretEntry.Binder()(new CommandArg[] { });
+            secretEntry.Binder()(Array.Empty<CommandArg>());
             Assert.Equal(1, CatalogFixtureCommands.SecretInvocations);
 
-            // Binders are cached per command; repeated collection and binding
-            // reuse the same delegate without re-resolving reflection.
+            /*
+               Binders are cached per command; repeated collection and binding
+               reuse the same delegate without re-resolving reflection.
+            */
             Assert.Equal(1, Collect().Count(entry => entry.Name == "Secret"));
-            secretEntry.Binder()(new CommandArg[] { });
+            secretEntry.Binder()(Array.Empty<CommandArg>());
             Assert.Equal(2, CatalogFixtureCommands.SecretInvocations);
         }
     }

Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs

@@ -33,6 +33,7 @@ internal static class TestCompilationFactory
             {
                 "Runtime/Attributes/RegisterCommandAttribute.cs",
                 "Runtime/CommandTerminal/Backend/CommandArg.cs",
+                "Runtime/CommandTerminal/Backend/CommandArgParsers.cs",
                 "Runtime/CommandTerminal/Backend/CommandExecutionContexts.cs",
                 "Runtime/CommandTerminal/Backend/CommandExecutionContextSets.cs",
                 "Runtime/CommandTerminal/Backend/CommandExecutionContextsExtensions.cs",
@@ -79,7 +80,8 @@ out ImmutableArray<Diagnostic> generatorDiagnostics
 
             CSharpCompilation output = (CSharpCompilation)outputCompilation;
             SyntaxTree generated = output.SyntaxTrees.FirstOrDefault(tree =>
-                tree.FilePath == GeneratedHintName || tree.FilePath.EndsWith(GeneratedHintName)
+                tree.FilePath == GeneratedHintName
+                || tree.FilePath.EndsWith(GeneratedHintName, StringComparison.Ordinal)
             );
             return (generated, output);
         }
@@ -99,9 +101,11 @@ public static CSharpCompilation CreateCompilation(
                 syntaxTrees.AddRange(ContractSources);
             }
 
-            // The fixture is parsed with the caller's preprocessor symbols so
-            // conditional compilation is exercised at parse time, exactly as
-            // it is in real compilations.
+            /*
+               The fixture is parsed with the caller's preprocessor symbols so
+               conditional compilation is exercised at parse time, exactly as
+               it is in real compilations.
+            */
             syntaxTrees.Add(
                 CSharpSyntaxTree.ParseText(
                     fixtureSource,
@@ -132,7 +136,8 @@ public static Assembly CompileAndLoad(CSharpCompilation compilation)
                 }
 
                 SyntaxTree generatedTree = compilation.SyntaxTrees.FirstOrDefault(tree =>
-                    tree.FilePath == GeneratedHintName || tree.ToString().Contains("auto-generated")
+                    tree.FilePath == GeneratedHintName
+                    || tree.ToString().Contains("auto-generated", StringComparison.Ordinal)
                 );
                 if (generatedTree != null)
                 {
@@ -224,10 +229,12 @@ public TestAssemblyLoadContext()
 
             protected override Assembly Load(AssemblyName assemblyName)
             {
-                // netstandard and the System.* facades forward to the shared
-                // framework; anything else is a genuine harness failure. The
-                // ALC requires the resolved simple name to match, so the
-                // netstandard facade is loaded from the runtime directory.
+                /*
+                   netstandard and the System.* facades forward to the shared
+                   framework; anything else is a genuine harness failure. The
+                   ALC requires the resolved simple name to match, so the
+                   netstandard facade is loaded from the runtime directory.
+                */
                 if (assemblyName.Name == "netstandard")
                 {
                     string runtimeDirectory = Path.GetDirectoryName(
@@ -319,9 +326,11 @@ public static CatalogView Load(Assembly assembly)
 
         public Func<object[], object> BinderOf(object entry)
         {
-            // Binder is Func<Action<CommandArg[]>> against the loaded
-            // assembly's own CommandArg type; both legs go through
-            // DynamicInvoke so no compile-time reference is needed.
+            /*
+               Binder is Func<Action<CommandArg[]>> against the loaded
+               assembly's own CommandArg type; both legs go through
+               DynamicInvoke so no compile-time reference is needed.
+            */
             Delegate binderFactory = (Delegate)GetValue(entry, "Binder");
             if (binderFactory == null)
             {

Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs

@@ -1,6 +1,8 @@
-// Minimal Unity scripting shim so Unity-free generator test compilations can include the
-// real Runtime sources that reference UnityEngine types (pattern adapted from unity-helpers,
-// MIT, Ambiguous-Interactive). Only the members CommandArg.cs actually touches are provided.
+/*
+   Minimal Unity scripting shim so Unity-free generator test compilations can include the
+   real Runtime sources that reference UnityEngine types (pattern adapted from unity-helpers,
+   MIT, Ambiguous-Interactive). Only the members CommandArg.cs actually touches are provided.
+*/
 namespace UnityEngine
 {
     public struct Vector2