diff --git a/.github/workflows/tooling-tests.yml b/.github/workflows/tooling-tests.yml index 3d5e6765..3f548f31 100644 --- a/.github/workflows/tooling-tests.yml +++ b/.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 diff --git a/.llm/context.md b/.llm/context.md index 49d07d8e..028d1bb5 100644 --- a/.llm/context.md +++ b/.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 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1e863190..2c76e9cd 100644 --- a/.pre-commit-config.yaml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 65edfad5..d83a4480 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Editor/csc.rsp b/Editor/csc.rsp new file mode 100644 index 00000000..2baa555c --- /dev/null +++ b/Editor/csc.rsp @@ -0,0 +1,2 @@ +-warnaserror +-warn:4 diff --git a/Editor/csc.rsp.meta b/Editor/csc.rsp.meta new file mode 100644 index 00000000..46d82a90 --- /dev/null +++ b/Editor/csc.rsp.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: aa9c8e3d41eb4b77a6c8493cfdf4e91e +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Generator~/Directory.Build.props b/Generator~/Directory.Build.props index 246259f6..5523cad8 100644 --- a/Generator~/Directory.Build.props +++ b/Generator~/Directory.Build.props @@ -7,5 +7,11 @@ true 9999 + + true + All + true diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs index 266b0d34..76bb2698 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs +++ b/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() : 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)); diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs index 344a77d2..da808e9a 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs +++ b/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()); 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()); Assert.Equal(2, CatalogFixtureCommands.SecretInvocations); } } diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs index 48249684..ab3807d8 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs +++ b/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 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 BinderOf(object entry) { - // Binder is Func> against the loaded - // assembly's own CommandArg type; both legs go through - // DynamicInvoke so no compile-time reference is needed. + /* + Binder is Func> 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) { diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs index 18e8ffa4..4120096b 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/UnityScriptingShim.cs +++ b/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 diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests.csproj b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests.csproj index e2b36003..b1463c39 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests.csproj +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests.csproj @@ -13,6 +13,24 @@ sources pulls in the .NET CA analyzers by default. --> false + + + $(NoWarn);CA1002;CA1051;CA1515;CA1815;CA2225;CA1019;CA1510;CA2211;CA1720 + diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators/CommandCatalogGenerator.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators/CommandCatalogGenerator.cs index 0b866a1a..a9a8d570 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators/CommandCatalogGenerator.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators/CommandCatalogGenerator.cs @@ -15,8 +15,10 @@ public sealed class CommandCatalogGenerator : ISourceGenerator internal const string ArgumentMetadataName = "WallstopStudios.DxCommandTerminal.Backend.CommandArg"; - // Matches RegisterCommandAttribute's Contexts default so attributed - // commands that omit Contexts keep unrestricted eligibility. + /* + Matches RegisterCommandAttribute's Contexts default so attributed + commands that omit Contexts keep unrestricted eligibility. + */ internal const int AllExecutionContexts = 7; private const string AttributeMetadataName = @@ -89,9 +91,11 @@ matching C# application order. attributeData.ConstructorArguments; for (int i = 0; i < constructorArguments.Length; i++) { - // Resolve constructor arguments by parameter name instead - // of type, so overload or order changes in the attribute - // cannot silently bind to the wrong field. + /* + Resolve constructor arguments by parameter name instead + of type, so overload or order changes in the attribute + cannot silently bind to the wrong field. + */ ApplyArgument( attribute, constructorParameters[i].Name, @@ -206,9 +210,11 @@ breaking the consumer build. ); if (method == null) { - // Declarations inside inactive preprocessor branches and - // other exotic syntax contribute no symbol; legacy - // reflection discovery would not see them either. + /* + Declarations inside inactive preprocessor branches and + other exotic syntax contribute no symbol; legacy + reflection discovery would not see them either. + */ continue; } @@ -320,15 +326,19 @@ reflection path because every emission form needs a legal public bool ContainingTypeIsUnbound; - // The handler method name escaped for use as a C# identifier - // (keyword names such as `@params`). + /* + The handler method name escaped for use as a C# identifier + (keyword names such as `@params`). + */ public string MethodNameIdentifierDisplay; // Whether the handler method itself is a generic method definition. public bool IsMethodGeneric; - // False when a parameter type or the method itself is open, dynamic, - // or otherwise not addressable by an exact typeof signature. + /* + False when a parameter type or the method itself is open, dynamic, + or otherwise not addressable by an exact typeof signature. + */ public bool ExactSignatureAddressable; /* @@ -349,9 +359,11 @@ public static CommandModel Build( ITypeSymbol commandArgumentType ) { - // Legacy discovery walks static methods only (BindingFlags.Static); - // instance methods attributed with RegisterCommandAttribute were - // never registered and must stay unregistered. + /* + Legacy discovery walks static methods only (BindingFlags.Static); + instance methods attributed with RegisterCommandAttribute were + never registered and must stay unregistered. + */ if (!method.IsStatic) { return null; @@ -383,8 +395,10 @@ out bool containingTypeIsUnbound if (model.HasValidSignature && model.DirectlyBindable) { - // Nothing else is needed; the emitter binds the method group - // without parameter expressions. + /* + Nothing else is needed; the emitter binds the method group + without parameter expressions. + */ return model; } @@ -413,10 +427,12 @@ public static string NormalizeName(string explicitName, string methodName) name = InferCommandName(methodName); } - // Method names are identifiers, so inference cannot produce - // spaces or surrounding whitespace; the guard keeps this a - // no-op pass for the common case while preserving the runtime - // attribute's exact semantics. + /* + Method names are identifiers, so inference cannot produce + spaces or surrounding whitespace; the guard keeps this a + no-op pass for the common case while preserving the runtime + attribute's exact semantics. + */ if (name.Contains(" ")) { name = name.Replace(" ", string.Empty); @@ -432,9 +448,11 @@ private static void BuildSignature( CommandModel model ) { - // Methods inside open generic types (or open generic methods - // themselves) can never be bound to a closed delegate; legacy - // discovery would fail inside Delegate.CreateDelegate for them. + /* + Methods inside open generic types (or open generic methods + themselves) can never be bound to a closed delegate; legacy + discovery would fail inside Delegate.CreateDelegate for them. + */ bool methodIsOpen = method.IsGenericMethod; bool signatureAddressable = !methodIsOpen && !containingTypeIsUnbound; @@ -537,8 +555,10 @@ private static bool IsAddressableType(ITypeSymbol type) if (type is IPointerTypeSymbol pointerType) { - // Unmanaged pointers only; managed references (ref/out/in) are - // modeled on IParameterSymbol.RefKind, not on the type symbol. + /* + Unmanaged pointers only; managed references (ref/out/in) are + modeled on IParameterSymbol.RefKind, not on the type symbol. + */ return IsAddressableType(pointerType.PointedAtType); } @@ -546,8 +566,10 @@ private static bool IsAddressableType(ITypeSymbol type) { if (0 < namedType.TypeParameters.Length && namedType.TypeArguments.Length == 0) { - // Generic definition; addressable only in unbound form, - // which the caller handles via ContainingTypeIsUnbound. + /* + Generic definition; addressable only in unbound form, + which the caller handles via ContainingTypeIsUnbound. + */ return false; } @@ -734,8 +756,10 @@ private static string FormatName(string name, int arity) if (0 < arity) { - // Generator-time formatting only; string.Concat keeps the - // arity suffix a single allocation. + /* + Generator-time formatting only; string.Concat keeps the + arity suffix a single allocation. + */ return string.Concat(name, "<", new string(',', arity - 1), ">"); } diff --git a/Runtime/Attributes/RegisterCommandAttribute.cs b/Runtime/Attributes/RegisterCommandAttribute.cs index a5432221..1ca2c351 100644 --- a/Runtime/Attributes/RegisterCommandAttribute.cs +++ b/Runtime/Attributes/RegisterCommandAttribute.cs @@ -8,7 +8,7 @@ namespace WallstopStudios.DxCommandTerminal.Attributes public sealed class RegisterCommandAttribute : Attribute { public string Name { get; set; } - public int MinArgCount { get; set; } = 0; + public int MinArgCount { get; set; } /* int, not int?: attribute properties must be constant-compatible, @@ -41,7 +41,7 @@ CommandInfo normalizes it to null at registration. public RegisterCommandAttribute(string commandName = null) { - commandName = commandName?.Replace(" ", string.Empty).Trim(); + commandName = commandName?.Replace(" ", string.Empty, StringComparison.Ordinal).Trim(); Name = commandName; } @@ -65,12 +65,17 @@ private static string InferCommandName(string methodName) public void NormalizeName(MethodInfo method) { + if (method == null) + { + throw new ArgumentNullException(nameof(method)); + } + if (string.IsNullOrWhiteSpace(Name)) { Name = InferCommandName(method.Name); } - Name = Name.Replace(" ", string.Empty).Trim(); + Name = Name.Replace(" ", string.Empty, StringComparison.Ordinal).Trim(); } } } diff --git a/Runtime/CommandTerminal/Backend/BorrowedCommandArguments.cs b/Runtime/CommandTerminal/Backend/BorrowedCommandArguments.cs index 2c738b21..85f5f217 100644 --- a/Runtime/CommandTerminal/Backend/BorrowedCommandArguments.cs +++ b/Runtime/CommandTerminal/Backend/BorrowedCommandArguments.cs @@ -87,9 +87,11 @@ public CommandArg this[int index] } } - // Specialized storage: Unity does not de-virtualize IReadOnlyList - // indexers, so array and list keep direct element access. The - // fallback covers exotic callers only. + /* + Specialized storage: Unity does not de-virtualize IReadOnlyList + indexers, so array and list keep direct element access. The + fallback covers exotic callers only. + */ private readonly CommandArg[] _array; private readonly List _list; private readonly IReadOnlyList _fallback; diff --git a/Runtime/CommandTerminal/Backend/CommandArg.cs b/Runtime/CommandTerminal/Backend/CommandArg.cs index 7003d0a0..897577a9 100644 --- a/Runtime/CommandTerminal/Backend/CommandArg.cs +++ b/Runtime/CommandTerminal/Backend/CommandArg.cs @@ -2,6 +2,7 @@ { using System; using System.Collections.Generic; + using System.Globalization; using System.Linq; using System.Net; using System.Numerics; @@ -192,7 +193,7 @@ public bool TryGet(out T parsed, CommandArgParser parserOverride) if (type == typeof(string)) { - parsed = (T)Convert.ChangeType(stringValue, type); + parsed = (T)(object)stringValue; return true; } if (TryGetTypeDefined(stringValue, out parsed)) @@ -203,83 +204,107 @@ public bool TryGet(out T parsed, CommandArgParser parserOverride) // TODO: Slap into a dictionary of built-in type -> parser mapping if (type == typeof(bool)) { - return InnerParse(stringValue, bool.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Bool, out parsed); } if (type == typeof(float)) { - return InnerParse(stringValue, float.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Float, out parsed); } if (type == typeof(int)) { - return InnerParse(stringValue, int.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Int, out parsed); } if (type == typeof(uint)) { - return InnerParse(stringValue, uint.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Uint, out parsed); } if (type == typeof(long)) { - return InnerParse(stringValue, long.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Long, out parsed); } if (type == typeof(ulong)) { - return InnerParse(stringValue, ulong.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Ulong, out parsed); } if (type == typeof(double)) { - return InnerParse(stringValue, double.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Double, out parsed); } if (type == typeof(short)) { - return InnerParse(stringValue, short.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Short, out parsed); } if (type == typeof(ushort)) { - return InnerParse(stringValue, ushort.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Ushort, out parsed); } if (type == typeof(byte)) { - return InnerParse(stringValue, byte.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Byte, out parsed); } if (type == typeof(sbyte)) { - return InnerParse(stringValue, sbyte.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Sbyte, out parsed); } if (type == typeof(Guid)) { - return InnerParse(stringValue, Guid.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Guid, out parsed); } if (type == typeof(DateTime)) { - return InnerParse(stringValue, DateTime.TryParse, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.DateTime, + out parsed + ); } if (type == typeof(DateTimeOffset)) { - return InnerParse(stringValue, DateTimeOffset.TryParse, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.DateTimeOffset, + out parsed + ); } if (type == typeof(char)) { - return InnerParse(stringValue, char.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Char, out parsed); } if (type == typeof(decimal)) { - return InnerParse(stringValue, decimal.TryParse, out parsed); + return InnerParse(stringValue, CommandArgParsers.Decimal, out parsed); } if (type == typeof(BigInteger)) { - return InnerParse(stringValue, BigInteger.TryParse, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.BigInteger, + out parsed + ); } if (type == typeof(TimeSpan)) { - return InnerParse(stringValue, TimeSpan.TryParse, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.TimeSpan, + out parsed + ); } if (type == typeof(Version)) { - return InnerParse(stringValue, Version.TryParse, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.Version, + out parsed + ); } if (type == typeof(IPAddress)) { - return InnerParse(stringValue, IPAddress.TryParse, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.IPAddress, + out parsed + ); } if (type.IsEnum) { @@ -288,12 +313,12 @@ public bool TryGet(out T parsed, CommandArgParser parserOverride) bool parseOk = Enum.TryParse(type, stringValue, out object parsedObject); if (parseOk) { - parsed = (T)Convert.ChangeType(parsedObject, type); + parsed = (T)parsedObject; return true; } } - if (int.TryParse(stringValue, out int enumIntValue)) + if (CommandArgParsers.Int(stringValue, out int enumIntValue)) { if (!EnumValues.TryGetValue(type, out object enumValues)) { @@ -315,15 +340,15 @@ public bool TryGet(out T parsed, CommandArgParser parserOverride) switch (split.Length) { case 2 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y): - parsed = (T)Convert.ChangeType(new Vector2(x, y), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y): + parsed = (T)(object)new Vector2(x, y); return true; case 3 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y) - && float.TryParse(split[2], out float z): - parsed = (T)Convert.ChangeType((Vector2)new Vector3(x, y, z), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y) + && CommandArgParsers.Float(split[2], out float z): + parsed = (T)(object)(Vector2)new Vector3(x, y, z); return true; } } @@ -333,15 +358,15 @@ when float.TryParse(split[0], out float x) switch (split.Length) { case 2 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y): - parsed = (T)Convert.ChangeType(new Vector3(x, y), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y): + parsed = (T)(object)new Vector3(x, y); return true; case 3 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y) - && float.TryParse(split[2], out float z): - parsed = (T)Convert.ChangeType(new Vector3(x, y, z), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y) + && CommandArgParsers.Float(split[2], out float z): + parsed = (T)(object)new Vector3(x, y, z); return true; } } @@ -351,22 +376,22 @@ when float.TryParse(split[0], out float x) switch (split.Length) { case 2 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y): - parsed = (T)Convert.ChangeType(new Vector4(x, y), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y): + parsed = (T)(object)new Vector4(x, y); return true; case 3 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y) - && float.TryParse(split[2], out float z): - parsed = (T)Convert.ChangeType(new Vector4(x, y, z), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y) + && CommandArgParsers.Float(split[2], out float z): + parsed = (T)(object)new Vector4(x, y, z); return true; case 4 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y) - && float.TryParse(split[2], out float z) - && float.TryParse(split[3], out float w): - parsed = (T)Convert.ChangeType(new Vector4(x, y, z, w), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y) + && CommandArgParsers.Float(split[2], out float z) + && CommandArgParsers.Float(split[3], out float w): + parsed = (T)(object)new Vector4(x, y, z, w); return true; } } @@ -376,14 +401,15 @@ when float.TryParse(split[0], out float x) switch (split.Length) { case 2 - when int.TryParse(split[0], out int x) && int.TryParse(split[1], out int y): - parsed = (T)Convert.ChangeType(new Vector2Int(x, y), type); + when CommandArgParsers.Int(split[0], out int x) + && CommandArgParsers.Int(split[1], out int y): + parsed = (T)(object)new Vector2Int(x, y); return true; case 3 - when int.TryParse(split[0], out int x) - && int.TryParse(split[1], out int y) - && int.TryParse(split[2], out int z): - parsed = (T)Convert.ChangeType((Vector2Int)new Vector3Int(x, y, z), type); + when CommandArgParsers.Int(split[0], out int x) + && CommandArgParsers.Int(split[1], out int y) + && CommandArgParsers.Int(split[2], out int z): + parsed = (T)(object)(Vector2Int)new Vector3Int(x, y, z); return true; } } @@ -393,14 +419,15 @@ when int.TryParse(split[0], out int x) switch (split.Length) { case 2 - when int.TryParse(split[0], out int x) && int.TryParse(split[1], out int y): - parsed = (T)Convert.ChangeType(new Vector3Int(x, y), type); + when CommandArgParsers.Int(split[0], out int x) + && CommandArgParsers.Int(split[1], out int y): + parsed = (T)(object)new Vector3Int(x, y); return true; case 3 - when int.TryParse(split[0], out int x) - && int.TryParse(split[1], out int y) - && int.TryParse(split[2], out int z): - parsed = (T)Convert.ChangeType(new Vector3Int(x, y, z), type); + when CommandArgParsers.Int(split[0], out int x) + && CommandArgParsers.Int(split[1], out int y) + && CommandArgParsers.Int(split[2], out int z): + parsed = (T)(object)new Vector3Int(x, y, z); return true; } } @@ -420,17 +447,17 @@ when int.TryParse(split[0], out int x) switch (split.Length) { case 3 - when float.TryParse(split[0], out float r) - && float.TryParse(split[1], out float g) - && float.TryParse(split[2], out float b): - parsed = (T)Convert.ChangeType(new Color(r, g, b), type); + when CommandArgParsers.Float(split[0], out float r) + && CommandArgParsers.Float(split[1], out float g) + && CommandArgParsers.Float(split[2], out float b): + parsed = (T)(object)new Color(r, g, b); return true; case 4 - when float.TryParse(split[0], out float r) - && float.TryParse(split[1], out float g) - && float.TryParse(split[2], out float b) - && float.TryParse(split[3], out float a): - parsed = (T)Convert.ChangeType(new Color(r, g, b, a), type); + when CommandArgParsers.Float(split[0], out float r) + && CommandArgParsers.Float(split[1], out float g) + && CommandArgParsers.Float(split[2], out float b) + && CommandArgParsers.Float(split[3], out float a): + parsed = (T)(object)new Color(r, g, b, a); return true; } } @@ -440,11 +467,11 @@ when float.TryParse(split[0], out float r) switch (split.Length) { case 4 - when float.TryParse(split[0], out float x) - && float.TryParse(split[1], out float y) - && float.TryParse(split[2], out float z) - && float.TryParse(split[3], out float w): - parsed = (T)Convert.ChangeType(new Quaternion(x, y, z, w), type); + when CommandArgParsers.Float(split[0], out float x) + && CommandArgParsers.Float(split[1], out float y) + && CommandArgParsers.Float(split[2], out float z) + && CommandArgParsers.Float(split[3], out float w): + parsed = (T)(object)new Quaternion(x, y, z, w); return true; } } @@ -454,12 +481,12 @@ when float.TryParse(split[0], out float x) switch (split.Length) { case 4 - when float.TryParse( + when CommandArgParsers.Float( split[0] .Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), out float x ) - && float.TryParse( + && CommandArgParsers.Float( split[1] .Replace( "y:", @@ -468,7 +495,7 @@ out float x ), out float y ) - && float.TryParse( + && CommandArgParsers.Float( split[2] .Replace( "width:", @@ -477,7 +504,7 @@ out float y ), out float width ) - && float.TryParse( + && CommandArgParsers.Float( split[3] .Replace( "height:", @@ -486,7 +513,7 @@ out float width ), out float height ): - parsed = (T)Convert.ChangeType(new Rect(x, y, width, height), type); + parsed = (T)(object)new Rect(x, y, width, height); return true; } } @@ -496,12 +523,12 @@ out float height switch (split.Length) { case 4 - when int.TryParse( + when CommandArgParsers.Int( split[0] .Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), out int x ) - && int.TryParse( + && CommandArgParsers.Int( split[1] .Replace( "y:", @@ -510,7 +537,7 @@ out int x ), out int y ) - && int.TryParse( + && CommandArgParsers.Int( split[2] .Replace( "width:", @@ -519,7 +546,7 @@ out int y ), out int width ) - && int.TryParse( + && CommandArgParsers.Int( split[3] .Replace( "height:", @@ -528,7 +555,7 @@ out int width ), out int height ): - parsed = (T)Convert.ChangeType(new RectInt(x, y, width, height), type); + parsed = (T)(object)new RectInt(x, y, width, height); return true; } } @@ -545,7 +572,7 @@ out T parsed bool parseOk = typedParser(input, out TParsed value); if (parseOk) { - parsed = (T)Convert.ChangeType(value, typeof(T)); + parsed = (T)Convert.ChangeType(value, typeof(T), CultureInfo.InvariantCulture); } else { @@ -571,7 +598,7 @@ static string[] StripAndSplit(string input) foreach (char delimiter in Delimiters) { - if (strippedInput.Contains(delimiter)) + if (0 <= strippedInput.IndexOf(delimiter, StringComparison.Ordinal)) { return strippedInput.Split(delimiter); } @@ -597,7 +624,7 @@ out Dictionary properties if (properties.TryGetValue(input, out PropertyInfo property)) { object resolved = property.GetValue(null); - value = (T)Convert.ChangeType(resolved, type); + value = (T)resolved; return true; } @@ -610,7 +637,7 @@ out Dictionary properties if (fields.TryGetValue(input, out FieldInfo field)) { object resolved = field.GetValue(null); - value = (T)Convert.ChangeType(resolved, type); + value = (T)resolved; return true; } diff --git a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs new file mode 100644 index 00000000..c6855ba4 --- /dev/null +++ b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs @@ -0,0 +1,98 @@ +namespace WallstopStudios.DxCommandTerminal.Backend +{ + using System.Globalization; + + /// + /// Culture-invariant parsers for the built-in types + /// supports. Console input must parse + /// identically on every machine, so culture-sensitive overloads pin + /// their NumberStyles and + /// explicitly; the style flags mirror each overload's default so only + /// the culture is pinned, never the accepted syntax. + /// + public static class CommandArgParsers + { + public static bool Float(string input, out float parsed) => + float.TryParse( + input, + NumberStyles.Float | NumberStyles.AllowThousands, + CultureInfo.InvariantCulture, + out parsed + ); + + public static bool Double(string input, out double parsed) => + double.TryParse( + input, + NumberStyles.Float | NumberStyles.AllowThousands, + CultureInfo.InvariantCulture, + out parsed + ); + + public static bool Decimal(string input, out decimal parsed) => + decimal.TryParse(input, NumberStyles.Number, CultureInfo.InvariantCulture, out parsed); + + public static bool Int(string input, out int parsed) => + int.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Uint(string input, out uint parsed) => + uint.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Long(string input, out long parsed) => + long.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Ulong(string input, out ulong parsed) => + ulong.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Short(string input, out short parsed) => + short.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Ushort(string input, out ushort parsed) => + ushort.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Byte(string input, out byte parsed) => + byte.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool Sbyte(string input, out sbyte parsed) => + sbyte.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); + + public static bool BigInteger(string input, out System.Numerics.BigInteger parsed) => + System.Numerics.BigInteger.TryParse( + input, + NumberStyles.Integer, + CultureInfo.InvariantCulture, + out parsed + ); + + public static bool DateTime(string input, out System.DateTime parsed) => + System.DateTime.TryParse( + input, + CultureInfo.InvariantCulture, + DateTimeStyles.None, + out parsed + ); + + public static bool DateTimeOffset(string input, out System.DateTimeOffset parsed) => + System.DateTimeOffset.TryParse( + input, + CultureInfo.InvariantCulture, + DateTimeStyles.None, + out parsed + ); + + public static bool TimeSpan(string input, out System.TimeSpan parsed) => + System.TimeSpan.TryParse(input, CultureInfo.InvariantCulture, out parsed); + + public static bool Bool(string input, out bool parsed) => bool.TryParse(input, out parsed); + + public static bool Char(string input, out char parsed) => char.TryParse(input, out parsed); + + public static bool Guid(string input, out System.Guid parsed) => + System.Guid.TryParse(input, out parsed); + + public static bool Version(string input, out System.Version parsed) => + System.Version.TryParse(input, out parsed); + + public static bool IPAddress(string input, out System.Net.IPAddress parsed) => + System.Net.IPAddress.TryParse(input, out parsed); + } +} diff --git a/Runtime/CommandTerminal/Backend/CommandArgParsers.cs.meta b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs.meta new file mode 100644 index 00000000..04556b30 --- /dev/null +++ b/Runtime/CommandTerminal/Backend/CommandArgParsers.cs.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: c8197deee76e4df2b21b827ebdc58e2d +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Runtime/CommandTerminal/Backend/CommandShell.cs b/Runtime/CommandTerminal/Backend/CommandShell.cs index 080a93d0..032dc5e4 100644 --- a/Runtime/CommandTerminal/Backend/CommandShell.cs +++ b/Runtime/CommandTerminal/Backend/CommandShell.cs @@ -150,8 +150,10 @@ public CommandShell(CommandHistory history) _history = history ?? throw new ArgumentNullException(nameof(history)); } - // Internal for test coverage of the discovery filter (see - // WallstopStudios.DxCommandTerminal.Tests.Runtime). + /* + Internal for test coverage of the discovery filter (see + WallstopStudios.DxCommandTerminal.Tests.Runtime). + */ public static bool TryEatArgument(ref string stringValue, out CommandArg arg) { stringValue = stringValue.TrimStart(); @@ -232,8 +234,10 @@ internal static bool MayContainCommands(Assembly assembly, AssemblyName self) } catch (Exception) { - // Metadata reads can fail for exotic assemblies; scanning them - // is cheaper than silently dropping commands they might carry. + /* + Metadata reads can fail for exotic assemblies; scanning them + is cheaper than silently dropping commands they might carry. + */ return true; } @@ -270,8 +274,10 @@ Assembly ourAssembly } catch (Exception) { - // Classification must never be able to fail discovery; if - // an assembly cannot be classified, scan it. + /* + Classification must never be able to fail discovery; if + an assembly cannot be classified, scan it. + */ scanCandidates.Add(assembly); } } @@ -353,8 +359,10 @@ private static DiscoveryCache GetOrCreateDiscoveryCache(Assembly assembly) } catch (ArgumentException) { - // A concurrent initialization registered its cache first; - // sharing it is equivalent to having won the race. + /* + A concurrent initialization registered its cache first; + sharing it is equivalent to having won the race. + */ DiscoveryCaches.TryGetValue(assembly, out cache); } @@ -491,9 +499,11 @@ out Action> collector return true; } - // An empty or failed catalog is not proof that the assembly has - // no commands; the reflection walk below stays compatible with - // it. + /* + An empty or failed catalog is not proof that the assembly has + no commands; the reflection walk below stays compatible with + it. + */ } if (cache.ReflectedCommands == null) @@ -522,8 +532,10 @@ private static bool TryGetScanTypes(Assembly assembly, out Type[] types) } catch (ReflectionTypeLoadException e) { - // Scan the subset that loaded; one unloadable type must not - // break discovery for the entire session. + /* + Scan the subset that loaded; one unloadable type must not + break discovery for the entire session. + */ types = e.Types; Debug.LogWarning( $"Some types of assembly {assembly.FullName} failed to load; " @@ -568,8 +580,10 @@ public int ClearCustomCommands() /// public int ClearAutoRegisteredCommands() { - // A pending registration would resurrect the commands this call - // removes, so cancellation is part of clearing. + /* + A pending registration would resurrect the commands this call + removes, so cancellation is part of clearing. + */ Interlocked.Exchange(ref _autoCommandsPending, 0); AutoCommandsRegistered = false; int count = _autoRegisteredCommands.Count; @@ -637,8 +651,10 @@ public void EnsureAutoCommandsRegistered() /// public bool RunCommand(string line) { - // A first command request is an explicit readiness boundary: any - // deferred registration must be applied before this parse. + /* + A first command request is an explicit readiness boundary: any + deferred registration must be applied before this parse. + */ EnsureAutoCommandsRegistered(); _dispatchDepth++; try @@ -701,8 +717,10 @@ public bool RunCommand(string line) public bool RunCommand(string commandName, CommandArg[] arguments) { - // A first command request is an explicit readiness boundary: any - // deferred registration must be applied before this lookup. + /* + A first command request is an explicit readiness boundary: any + deferred registration must be applied before this lookup. + */ EnsureAutoCommandsRegistered(); string line = BuildHistoryLine(commandName, arguments); return RunCommandCore(CommandExecutionContext.Current, commandName, arguments, line); @@ -726,8 +744,10 @@ public bool RunCommand( List arguments ) { - // A first command request is an explicit readiness boundary: any - // deferred registration must be applied before this lookup. + /* + A first command request is an explicit readiness boundary: any + deferred registration must be applied before this lookup. + */ EnsureAutoCommandsRegistered(); return RunCommandCore(context, commandName, arguments, historyLine: null); } @@ -858,8 +878,10 @@ out bool isNewArgument return true; } - // Drop empty candidates and duplicate insertion texts; first - // occurrence wins and provider order is preserved. + /* + Drop empty candidates and duplicate insertion texts; first + occurrence wins and provider order is preserved. + */ int writeIndex = 0; _completionDeduplication.Clear(); for (int readIndex = 0; readIndex < results.Count; ++readIndex) @@ -1350,9 +1372,11 @@ string historyLine } else { - // Legacy handlers may retain their argument array, so each - // invocation materializes a fresh one. The array is never - // pooled or reused after the handler returns. + /* + Legacy handlers may retain their argument array, so each + invocation materializes a fresh one. The array is never + pooled or reused after the handler returns. + */ CommandArg[] materialized = new CommandArg[arguments.Count]; for (int i = 0; i < materialized.Length; ++i) { @@ -1428,8 +1452,10 @@ private readonly struct AutoCommand public readonly bool IsDefault; public readonly CommandExecutionContexts Contexts; - // Non-null for valid (CommandArg[]) signatures. Null marks a - // rejected command; diagnostics then come from MethodAccessor. + /* + Non-null for valid (CommandArg[]) signatures. Null marks a + rejected command; diagnostics then come from MethodAccessor. + */ public readonly Func> Binder; public readonly Func MethodAccessor; diff --git a/Runtime/CommandTerminal/Backend/CommandTokenizer.cs b/Runtime/CommandTerminal/Backend/CommandTokenizer.cs index 22accf7a..e9d005f6 100644 --- a/Runtime/CommandTerminal/Backend/CommandTokenizer.cs +++ b/Runtime/CommandTerminal/Backend/CommandTokenizer.cs @@ -49,9 +49,11 @@ public static void Tokenize(string line, List tokens) if (closingQuoteIndex < 0) { - // Unclosed quote consumes the rest of the line - // (excluding the opening quote), matching - // TryEatArgument. + /* + Unclosed quote consumes the rest of the line + (excluding the opening quote), matching + TryEatArgument. + */ tokens.Add( new CommandToken( line.Substring(index + 1), diff --git a/Runtime/CommandTerminal/Persistence/TerminalThemePersister.cs b/Runtime/CommandTerminal/Persistence/TerminalThemePersister.cs index a8c5e11f..3750408d 100644 --- a/Runtime/CommandTerminal/Persistence/TerminalThemePersister.cs +++ b/Runtime/CommandTerminal/Persistence/TerminalThemePersister.cs @@ -70,8 +70,6 @@ protected virtual IEnumerator Start() yield break; } - string themeFile = ThemeFile; - Debug.Log($"Attempting to initialize from {themeFile}...", this); yield return CheckAndPersistAnyChanges(hydrate: true); } @@ -175,10 +173,12 @@ protected virtual IEnumerator CheckAndPersistAnyChanges(bool hydrate) } else { +#if UNITY_EDITOR || DEVELOPMENT_BUILD Debug.Log( $"Creating new theme file {themeFile} for terminal {terminal.id} ...", this ); +#endif configurations = new TerminalThemeConfigurations(); } @@ -253,10 +253,12 @@ out TerminalThemeConfiguration existingConfiguration } else { +#if UNITY_EDITOR || DEVELOPMENT_BUILD Debug.Log( $"Failed to find persisted configuration for terminal {terminal.id} while hydrating, defaulting to Prefab configuration.", this ); +#endif } } @@ -273,10 +275,6 @@ out TerminalThemeConfiguration existingConfiguration } string outputJson = JsonUtility.ToJson(configurations, prettyPrint: true); - Debug.Log( - $"Writing theme file {themeFile} with contents:{Environment.NewLine}{outputJson}", - this - ); Task writerTask = File.WriteAllTextAsync(themeFile, outputJson); while (!writerTask.IsCompleted) { @@ -292,10 +290,12 @@ out TerminalThemeConfiguration existingConfiguration this ); } +#if UNITY_EDITOR || DEVELOPMENT_BUILD else { Debug.Log($"Theme file {themeFile} successfully updated.", this); } +#endif } finally { diff --git a/Runtime/CommandTerminal/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index 36d29ad0..b7d44944 100644 --- a/Runtime/CommandTerminal/UI/TerminalUI.cs +++ b/Runtime/CommandTerminal/UI/TerminalUI.cs @@ -157,12 +157,16 @@ public sealed class TerminalUI : MonoBehaviour private SerializedObject _serializedObject; #endif - // Internal for test coverage of token completion (see - // WallstopStudios.DxCommandTerminal.Tests.Runtime). + /* + Internal for test coverage of token completion (see + WallstopStudios.DxCommandTerminal.Tests.Runtime). + */ internal TextField _commandInput; - // Internal for test coverage of caret behavior (see - // WallstopStudios.DxCommandTerminal.Tests.Runtime). + /* + Internal for test coverage of caret behavior (see + WallstopStudios.DxCommandTerminal.Tests.Runtime). + */ internal VisualElement _textInput; /* @@ -716,8 +720,10 @@ private static string QuoteInsertionIfNeeded(string insertion, bool tokenQuoted) return "'" + insertion + "'"; } - // The insertion mixes both quote characters; insert it verbatim - // rather than producing an untokenizable quoting. + /* + The insertion mixes both quote characters; insert it verbatim + rather than producing an untokenizable quoting. + */ return insertion; } @@ -1403,8 +1409,10 @@ change re-derives them. if (_tokenCompletionsTemp.Count == 0) { - // A provider is attached but has nothing to offer; do not - // substitute full-line history suggestions for the token. + /* + A provider is attached but has nothing to offer; do not + substitute full-line history suggestions for the token. + */ return true; } @@ -1623,8 +1631,10 @@ private void SetupUI() ) ) { - // One echo follows each programmatic write; a later - // same-value edit is a genuine edit, not an echo. + /* + One echo follows each programmatic write; a later + same-value edit is a genuine edit, not an echo. + */ context._lastCodeSyncedValue = null; echoFromCode = true; } @@ -1718,7 +1728,18 @@ private void InitializeTheme(VisualElement root) { _runtimeTheme = themeNames.FirstOrDefault(); } - Debug.LogWarning($"Persisted theme not found, defaulting to '{_runtimeTheme}'."); + + /* + Defaulting from an empty or unknown persisted name is normal + operation; only a stale persisted name deserves a warning. + */ + if (_persistedTheme != null) + { + Debug.LogWarning( + $"Persisted theme '{_persistedTheme}' not found in the pack, defaulting to '{_runtimeTheme}'.", + this + ); + } } else { @@ -1765,15 +1786,17 @@ private void InitializeFont() } } - if (_runtimeFont == null) - { - Debug.LogError("No font assigned, defaulting to Courier New 16pt", this); - _runtimeFont = Font.CreateDynamicFontFromOSFont("Courier New", 16); - } - else + if (_runtimeFont != null) { - Debug.LogWarning($"No font assigned, defaulting to {_runtimeFont.name}.", this); + // The pack defaulting itself is normal operation: stay silent. + return; } + + Debug.LogWarning( + "Font pack contains no fonts; defaulting to OS font 'Courier New' 16pt.", + this + ); + _runtimeFont = Font.CreateDynamicFontFromOSFont("Courier New", 16); } private void ScheduleBlinkingCursor() @@ -1862,8 +1885,10 @@ in the meantime. _needsScrollToEnd = false; } - // Pending carets are consumed on every pass: an accepted - // completion can be a text no-op that must still move the caret. + /* + Pending carets are consumed on every pass: an accepted + completion can be a text no-op that must still move the caret. + */ ApplyPendingCaret(); RefreshStateButtons(); } @@ -1902,8 +1927,10 @@ private void ApplyPendingCaret() if (_commandInput.value.Length < _pendingCaretIndex) { - // The queued position targets input the field does not hold - // yet; the value sync applies it once the field catches up. + /* + The queued position targets input the field does not hold + yet; the value sync applies it once the field catches up. + */ return; } diff --git a/Runtime/csc.rsp b/Runtime/csc.rsp new file mode 100644 index 00000000..2baa555c --- /dev/null +++ b/Runtime/csc.rsp @@ -0,0 +1,2 @@ +-warnaserror +-warn:4 diff --git a/Runtime/csc.rsp.meta b/Runtime/csc.rsp.meta new file mode 100644 index 00000000..26280ddb --- /dev/null +++ b/Runtime/csc.rsp.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 8fe69f202f8a42d6a833d7c957abd135 +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Tests/Runtime/BorrowedCommandArgumentsTests.cs b/Tests/Runtime/BorrowedCommandArgumentsTests.cs index 69c0f17c..90115070 100644 --- a/Tests/Runtime/BorrowedCommandArgumentsTests.cs +++ b/Tests/Runtime/BorrowedCommandArgumentsTests.cs @@ -10,8 +10,10 @@ public sealed class BorrowedCommandArgumentsTests { private static BorrowedCommandArguments CreateView(object source) { - // The shell constructs views from arrays, lists, and arbitrary - // read-only lists; cover all three construction paths. + /* + The shell constructs views from arrays, lists, and arbitrary + read-only lists; cover all three construction paths. + */ return source switch { CommandArg[] array => new BorrowedCommandArguments(array), diff --git a/Tests/Runtime/CommandArgTests.cs b/Tests/Runtime/CommandArgTests.cs index 72b81323..2243a344 100644 --- a/Tests/Runtime/CommandArgTests.cs +++ b/Tests/Runtime/CommandArgTests.cs @@ -4,6 +4,7 @@ namespace WallstopStudios.DxCommandTerminal.Tests.Runtime using System.Collections.Generic; using System.Globalization; using System.Linq; + using System.Threading; using Backend; using JetBrains.Annotations; using NUnit.Framework; @@ -2645,6 +2646,57 @@ static bool CustomIntParser1(string input, out int parsed) } } + [Test] + [TestCase("tr-TR")] + [TestCase("de-DE")] + [TestCase("ru-RU")] + [TestCase("fr-FR")] + public void ParsingIsCultureInvariant(string cultureName) + { + CultureInfo culture; + try + { + culture = new CultureInfo(cultureName); + } + catch (CultureNotFoundException) + { + Assert.Ignore($"Culture {cultureName} is not installed on this machine."); + return; + } + + CultureInfo originalCulture = Thread.CurrentThread.CurrentCulture; + Thread.CurrentThread.CurrentCulture = culture; + try + { + CommandArg arg = new("1.5"); + Assert.IsTrue(arg.TryGet(out float floatValue), cultureName); + Assert.AreEqual(1.5f, floatValue, cultureName); + + arg = new CommandArg("1.25"); + Assert.IsTrue(arg.TryGet(out double doubleValue), cultureName); + Assert.AreEqual(1.25, doubleValue, cultureName); + + arg = new CommandArg("1.5"); + Assert.IsTrue(arg.TryGet(out decimal decimalValue), cultureName); + Assert.AreEqual(1.5m, decimalValue, cultureName); + + arg = new CommandArg("01/02/2026"); + Assert.IsTrue(arg.TryGet(out DateTime date), cultureName); + Assert.AreEqual(new DateTime(2026, 1, 2), date, cultureName); + + arg = new CommandArg("1.5, 2.5"); + Assert.IsTrue(arg.TryGet(out Vector2 vector), cultureName); + Assert.IsTrue( + Approximately(1.5f, vector.x) && Approximately(2.5f, vector.y), + $"{cultureName}: expected (1.5, 2.5), got {vector}" + ); + } + finally + { + Thread.CurrentThread.CurrentCulture = originalCulture; + } + } + private readonly struct TestStruct1 : IEquatable { private readonly Guid _id; diff --git a/Tests/Runtime/CommandCatalogTests.cs b/Tests/Runtime/CommandCatalogTests.cs index 7303af29..e9d0d7ca 100644 --- a/Tests/Runtime/CommandCatalogTests.cs +++ b/Tests/Runtime/CommandCatalogTests.cs @@ -39,9 +39,11 @@ public void GeneratedCatalogIsPresentInRuntimeAssembly() [Test] public void CatalogDiscoveryRegistersEveryLegacyCommandWithTheSameMetadata() { - // The compatibility surface is the discovery oracle: whatever it - // finds must be registered by the catalog-first path with the - // same normalized name and attribute metadata. + /* + The compatibility surface is the discovery oracle: whatever it + finds must be registered by the catalog-first path with the + same normalized name and attribute metadata. + */ (MethodInfo method, RegisterCommandAttribute attribute)[] legacy = CommandShell .RegisteredCommands .Value; diff --git a/Tests/Runtime/CommandCompletionProviderTests.cs b/Tests/Runtime/CommandCompletionProviderTests.cs index acae5f5d..1de4cbca 100644 --- a/Tests/Runtime/CommandCompletionProviderTests.cs +++ b/Tests/Runtime/CommandCompletionProviderTests.cs @@ -106,8 +106,10 @@ List results "Stage 1 completes the second argument" ); - // Requests past the last stage produce no candidates but still - // count as provider-answered. + /* + Requests past the last stage produce no candidates but still + count as provider-answered. + */ Assert.IsTrue( Terminal.Shell.TryComplete(context, "pickup pickaxe sharpened ", 25, results, out _) ); @@ -137,8 +139,10 @@ List results CommandExecutionContext context = CommandExecutionContext.Current; List results = new(); - // Mid-token inside a quoted argument: the caret completes an - // unclosed quoted token, so the request is quoted. + /* + Mid-token inside a quoted argument: the caret completes an + unclosed quoted token, so the request is quoted. + */ Assert.IsTrue( Terminal.Shell.TryComplete( context, diff --git a/Tests/Runtime/CommandContextExecutionTests.cs b/Tests/Runtime/CommandContextExecutionTests.cs index 025cf2d3..375f57ee 100644 --- a/Tests/Runtime/CommandContextExecutionTests.cs +++ b/Tests/Runtime/CommandContextExecutionTests.cs @@ -73,8 +73,10 @@ public IEnumerator GameplayDefaultRunsInEditorPlayModeAndRejectsEditMode() "Definitions default to the gameplay contexts" ); - // Play Mode is the ambient environment for editor test runs; no - // provider is installed, so Unity decides. + /* + Play Mode is the ambient environment for editor test runs; no + provider is installed, so Unity decides. + */ Assert.IsTrue( Terminal.Shell.RunCommand("ctx-test alpha"), "A gameplay command should run in Editor Play Mode" @@ -409,8 +411,10 @@ public IEnumerator EligibilityRejectionRespectsHistoryPolicy() ) ); - // The command declares Player-only eligibility, so the eligible - // ambient here is a player. + /* + The command declares Player-only eligibility, so the eligible + ambient here is a player. + */ CommandExecutionContext.AmbientContextProvider = () => new CommandExecutionContext(CommandExecutionContexts.Player); Assert.IsTrue( @@ -443,8 +447,10 @@ public IEnumerator DefinitionSnapshotIsIsolatedFromLaterEdits() }; Assert.IsTrue(Terminal.Shell.AddCommand(definition)); - // Mutating the definition after registration must not affect the - // registration or register anything under the new name. + /* + Mutating the definition after registration must not affect the + registration or register anything under the new name. + */ definition.Name = "ctx-renamed"; definition.Handler = null; definition.Contexts = CommandExecutionContexts.None; diff --git a/Tests/Runtime/CommandDiscoveryTests.cs b/Tests/Runtime/CommandDiscoveryTests.cs index 2a89fb10..ca0a21c7 100644 --- a/Tests/Runtime/CommandDiscoveryTests.cs +++ b/Tests/Runtime/CommandDiscoveryTests.cs @@ -15,14 +15,18 @@ public sealed class CommandDiscoveryTests BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic; #if UNITY_EDITOR - // Defined once: dynamic assemblies are non-collectible, and IL2CPP - // players do not support Reflection.Emit, so this case is editor-only. + /* + Defined once: dynamic assemblies are non-collectible, and IL2CPP + players do not support Reflection.Emit, so this case is editor-only. + */ private static readonly Assembly DynamicAssembly = CreateDynamicAssembly(); #endif - // Declared in this test assembly (a consumer-style assembly that only - // references the runtime) so discovery from non-builtin assemblies is - // pinned. Names are normalized the same way production discovery does. + /* + Declared in this test assembly (a consumer-style assembly that only + references the runtime) so discovery from non-builtin assemblies is + pinned. Names are normalized the same way production discovery does. + */ [RegisterCommand(Help = "Test command with an inferred name.")] private static void TestCommand(CommandArg[] args) { } @@ -35,9 +39,11 @@ private static IEnumerable AssemblyFilterCases() // The core assembly never references the terminal package. Assembly coreAssembly = typeof(int).Assembly; - // Note: the runtime assembly itself is not classified by - // MayContainCommands (production scans it last via a name match); - // that ordering is pinned by the equivalence sweep below. + /* + Note: the runtime assembly itself is not classified by + MayContainCommands (production scans it last via a name match); + that ordering is pinned by the equivalence sweep below. + */ yield return new TestCaseData(testAssembly, true).SetName( "testAssemblyMayContainCommands" ); @@ -86,8 +92,10 @@ public void DiscoversCommandsFromConsumerAssemblies() .RegisteredCommands .Value; - // Command names are matched OrdinalIgnoreCase by the shell, so the - // inferred name may preserve the declaring method's casing. + /* + Command names are matched OrdinalIgnoreCase by the shell, so the + inferred name may preserve the declaring method's casing. + */ RegisterCommandAttribute inferred = discovered .Select(tuple => tuple.attribute) .FirstOrDefault(attribute => @@ -115,10 +123,12 @@ public void DiscoversCommandsFromConsumerAssemblies() [Test] public void FilteredDiscoveryMatchesUnfilteredDiscovery() { - // The legacy discovery algorithm: scan every assembly in the - // domain, materializing attributes per method. This equivalence - // sweep pins that the assembly-reference filter never drops a - // command the legacy path would have found. + /* + The legacy discovery algorithm: scan every assembly in the + domain, materializing attributes per method. This equivalence + sweep pins that the assembly-reference filter never drops a + command the legacy path would have found. + */ List legacy = new(); Assembly ourAssembly = typeof(BuiltInCommands).Assembly; Assembly[] loadedAssemblies = AppDomain.CurrentDomain.GetAssemblies(); @@ -139,8 +149,10 @@ public void FilteredDiscoveryMatchesUnfilteredDiscovery() } catch (Exception) { - // Mirrors production TryGetScanTypes: an assembly that - // cannot be enumerated contributes no commands. + /* + Mirrors production TryGetScanTypes: an assembly that + cannot be enumerated contributes no commands. + */ continue; } @@ -169,8 +181,10 @@ public void FilteredDiscoveryMatchesUnfilteredDiscovery() } catch (Exception) { - // Mirrors production: attribute resolution failures - // are contained, not fatal. + /* + Mirrors production: attribute resolution failures + are contained, not fatal. + */ continue; } diff --git a/Tests/Runtime/CommandHistoryTests.cs b/Tests/Runtime/CommandHistoryTests.cs index c3cd4ca2..48dc0ca0 100644 --- a/Tests/Runtime/CommandHistoryTests.cs +++ b/Tests/Runtime/CommandHistoryTests.cs @@ -303,8 +303,10 @@ public void NextSkipsNonAdjacentDuplicates() [UnityTest] public IEnumerator ClearHistoryCommandResultsInEmptyHistory() { - // clear-history uses AddToHistory = false on its RegisterCommand attribute, - // ensuring the command itself is not recorded in the history it just cleared. + /* + clear-history uses AddToHistory = false on its RegisterCommand attribute, + ensuring the command itself is not recorded in the history it just cleared. + */ yield return TerminalTests.SpawnTerminal(resetStateOnInit: true); CommandShell shell = Terminal.Shell; @@ -369,8 +371,10 @@ public IEnumerator CommandsAfterClearHistoryWorkNormally() [UnityTest] public IEnumerator CommandWithAddToHistoryFalseAndInvalidArgsDoesNotPushToHistory() { - // clear-history has AddToHistory = false. When called with wrong args, - // it should still NOT be recorded in history. + /* + clear-history has AddToHistory = false. When called with wrong args, + it should still NOT be recorded in history. + */ yield return TerminalTests.SpawnTerminal(resetStateOnInit: true); CommandShell shell = Terminal.Shell; @@ -409,8 +413,10 @@ public IEnumerator CommandWithAddToHistoryFalseAndInvalidArgsDoesNotPushToHistor [UnityTest] public IEnumerator CommandWithAddToHistoryTrueAndInvalidArgsPushesToHistory() { - // set-theme has AddToHistory = true (default). When called with wrong args, - // it should still be recorded in history. + /* + set-theme has AddToHistory = true (default). When called with wrong args, + it should still be recorded in history. + */ yield return TerminalTests.SpawnTerminal(resetStateOnInit: true); CommandShell shell = Terminal.Shell; diff --git a/Tests/Runtime/CommandShellDeferredRegistrationTests.cs b/Tests/Runtime/CommandShellDeferredRegistrationTests.cs index 62ba737d..c5a0f160 100644 --- a/Tests/Runtime/CommandShellDeferredRegistrationTests.cs +++ b/Tests/Runtime/CommandShellDeferredRegistrationTests.cs @@ -191,8 +191,10 @@ public void DeferredRegistrationLetsManualCommandsWinWithoutQueuedErrors() CommandShell shell = new CommandShell(history); shell.InitializeAutoRegisteredCommands(deferRegistration: true); - // A manual registration between enable and first use keeps the - // name; readiness must not queue a duplicate error for it. + /* + A manual registration between enable and first use keeps the + name; readiness must not queue a duplicate error for it. + */ Assert.IsTrue( shell.AddCommand(knownCommand, _ => { }, 0, -1, "manual override"), $"Sanity: registering '{knownCommand}' manually on an empty shell must succeed" diff --git a/Tests/Runtime/CommandShellTests.cs b/Tests/Runtime/CommandShellTests.cs index 116b9181..8de9579a 100644 --- a/Tests/Runtime/CommandShellTests.cs +++ b/Tests/Runtime/CommandShellTests.cs @@ -31,8 +31,10 @@ public IEnumerator UnescapedQuotes() Exception exception = null; Action assertion = null; - // These tests assert per-command logging only; applying deferred - // registration up front keeps its readiness log out of the window. + /* + These tests assert per-command logging only; applying deferred + registration up front keeps its readiness log out of the window. + */ Terminal.Shell.EnsureAutoCommandsRegistered(); Application.logMessageReceived += HandleMessageReceived; @@ -129,8 +131,10 @@ public IEnumerator RunCommandLineNominal() Exception exception = null; Action assertion = null; - // These tests assert per-command logging only; applying deferred - // registration up front keeps its readiness log out of the window. + /* + These tests assert per-command logging only; applying deferred + registration up front keeps its readiness log out of the window. + */ Terminal.Shell.EnsureAutoCommandsRegistered(); Application.logMessageReceived += HandleMessageReceived; diff --git a/Tests/Runtime/TerminalKeyboardControllerTests.cs b/Tests/Runtime/TerminalKeyboardControllerTests.cs index 7d397e74..fd6d9039 100644 --- a/Tests/Runtime/TerminalKeyboardControllerTests.cs +++ b/Tests/Runtime/TerminalKeyboardControllerTests.cs @@ -82,8 +82,10 @@ public void ControlTypesContainsAllNonNoneEnumValues() [UnityTest] public IEnumerator DefaultControlOrderProducesNoWarning() { - // Default _controlOrder contains all TerminalControlTypes, so no warning should fire. - // Awake will log an error about missing TerminalUI -- expect that. + /* + Default _controlOrder contains all TerminalControlTypes, so no warning should fire. + Awake will log an error about missing TerminalUI -- expect that. + */ LogAssert.Expect(LogType.Error, "Failed to find TerminalUI, Input will not work."); GameObject go = new("TerminalKeyboardControllerTest"); _gameObjects.Add(go); @@ -97,8 +99,10 @@ public IEnumerator DefaultControlOrderProducesNoWarning() [UnityTest] public IEnumerator ControlOrderWithDuplicatesButAllTypesPresent_ProducesNoWarning() { - // Regression test: if _controlOrder has duplicates but still covers all control types, - // VerifyControlOrderIntegrity should NOT produce a warning. + /* + Regression test: if _controlOrder has duplicates but still covers all control types, + VerifyControlOrderIntegrity should NOT produce a warning. + */ LogAssert.Expect(LogType.Error, "Failed to find TerminalUI, Input will not work."); GameObject go = new("TerminalKeyboardControllerTest"); _gameObjects.Add(go); @@ -173,9 +177,11 @@ public IEnumerator ControlOrderMissingType_ProducesWarning() [UnityTest] public IEnumerator EmptyControlOrder_ProducesWarningForAllTypes() { - // When _controlOrder is empty, VerifyControlOrderIntegrity should warn about all missing types. - // Note: Awake fires during AddComponent with the default (full) control order, so - // we only modify _controlOrder afterward and invoke VerifyControlOrderIntegrity directly. + /* + When _controlOrder is empty, VerifyControlOrderIntegrity should warn about all missing types. + Note: Awake fires during AddComponent with the default (full) control order, so + we only modify _controlOrder afterward and invoke VerifyControlOrderIntegrity directly. + */ LogAssert.Expect(LogType.Error, "Failed to find TerminalUI, Input will not work."); GameObject go = new("TerminalKeyboardControllerTest"); _gameObjects.Add(go); diff --git a/Tests/Runtime/TerminalTests.cs b/Tests/Runtime/TerminalTests.cs index 15ca113f..b7d45e7f 100644 --- a/Tests/Runtime/TerminalTests.cs +++ b/Tests/Runtime/TerminalTests.cs @@ -12,8 +12,10 @@ namespace WallstopStudios.DxCommandTerminal.Tests.Runtime public sealed class TerminalTests { - // Dynamically derived from RegisteredCommands so the test stays in sync - // with any additions/removals of default commands in BuiltinCommands.cs. + /* + Dynamically derived from RegisteredCommands so the test stays in sync + with any additions/removals of default commands in BuiltinCommands.cs. + */ private static readonly string[] KnownDefaultCommands = CommandShell .RegisteredCommands.Value.Where(tuple => tuple.attribute.Default) .Select(tuple => tuple.attribute.Name) @@ -283,8 +285,10 @@ public IEnumerator TerminalUIDefersAutoCommandRegistrationUntilFirstUse() "Auto commands must stay unregistered before the first use" ); - // Pick a zero-argument auto command that is safe to run inside a - // test session (never quit/exit). + /* + Pick a zero-argument auto command that is safe to run inside a + test session (never quit/exit). + */ string knownCommand = CommandShell .RegisteredCommands.Value.Where(tuple => tuple.attribute.MinArgCount == 0 diff --git a/Tests/Runtime/TerminalUITokenCompletionTests.cs b/Tests/Runtime/TerminalUITokenCompletionTests.cs index ea2cb6f3..7a622a50 100644 --- a/Tests/Runtime/TerminalUITokenCompletionTests.cs +++ b/Tests/Runtime/TerminalUITokenCompletionTests.cs @@ -254,8 +254,10 @@ public IEnumerator CommandsWithoutProvidersKeepHistoryCompletion() { yield return SpawnTerminalWithUi(); - // No command starts with 'zzz', so neither completion path has a - // suggestion and the input stays untouched. + /* + No command starts with 'zzz', so neither completion path has a + suggestion and the input stays untouched. + */ yield return SetInput("zzz", 3); _terminal.CompleteCommand(true); diff --git a/Tests/Runtime/csc.rsp b/Tests/Runtime/csc.rsp new file mode 100644 index 00000000..2baa555c --- /dev/null +++ b/Tests/Runtime/csc.rsp @@ -0,0 +1,2 @@ +-warnaserror +-warn:4 diff --git a/Tests/Runtime/csc.rsp.meta b/Tests/Runtime/csc.rsp.meta new file mode 100644 index 00000000..81def2c7 --- /dev/null +++ b/Tests/Runtime/csc.rsp.meta @@ -0,0 +1,7 @@ +fileFormatVersion: 2 +guid: 08fbf520a8cb44f29842e93d657ab847 +TextScriptImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/tooling~/package.json b/tooling~/package.json index 803aacad..e65db6f2 100644 --- a/tooling~/package.json +++ b/tooling~/package.json @@ -12,6 +12,8 @@ "lint:comparison-direction:fix": "node scripts/lint-comparison-direction.mjs --fix", "lint:member-ordering": "node scripts/lint-member-ordering.mjs --verbose", "lint:member-ordering:fix": "node scripts/lint-member-ordering.mjs --fix", + "lint:multiline-comments": "node scripts/lint-multiline-comments.mjs", + "lint:multiline-comments:fix": "node scripts/lint-multiline-comments.mjs --fix", "unity:mcp:probe": "node scripts/mcp/unity-mcp.mjs probe", "unity:mcp:configure": "node scripts/mcp/unity-mcp.mjs configure", "unity:mcp:bridge": "node scripts/mcp/unity-mcp.mjs bridge", diff --git a/tooling~/scripts/lint-multiline-comments.mjs b/tooling~/scripts/lint-multiline-comments.mjs new file mode 100644 index 00000000..4bb76f65 --- /dev/null +++ b/tooling~/scripts/lint-multiline-comments.mjs @@ -0,0 +1,258 @@ +/* + Multi-line comments are block comments, never stacked `//` lines (PR #57 review). + + Two or more consecutive comment-only `//` lines read as one comment with fake + structure, so they must be written as one block comment instead. A single `//` line is + fine, and `///` doc comments are exempt (they are XML doc input, not authoring prose). + Lines that carry a `//` comment after code are left alone: the rule is about comment + blocks, and an inline block comment would swallow the rest of its line. + + The scan is a real state machine, not a grep, because fixture sources embed C# inside + verbatim strings: `//` lines in string content are data, not comments. String, char, + verbatim, interpolated (with holes), and raw-string literals are consumed with the + same literal scanner the comparison-direction linter uses, so a `//` inside any + literal stays silent. + + `--fix` rewrites each run as one block comment: `/*` opens at the run's indent, every + content line keeps its deeper indentation and gains a three-space base indent, and the + comment closes with a slash-asterisk marker at the run's indent column. A run whose + content contains that closing marker cannot be converted safely, so `--fix` refuses it + and the violation stays. + + Exit codes: 0 = clean (or every fixable violation fixed), 1 = at least one violation + remains. + + Adapted from the repository's lint-comparison-direction.mjs structure (issues #50/#51) + and unity-helpers' linter conventions (MIT, Ambiguous-Interactive). +*/ +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const REPO_ROOT = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../.." +); +// Overridable so the contract tests can point the scan at a fixture tree. Nothing in CI sets it. +// `Generator~` is C# this repository authors -- the analyzer payload and its tests -- so the rule +// applies there too, even though Unity ignores the tilde directory. +const SCAN_ROOTS = process.env.MULTILINE_COMMENT_ROOTS + ? process.env.MULTILINE_COMMENT_ROOTS.split(path.delimiter).filter(Boolean) + : ["Runtime", "Editor", "Tests", "Generator~"]; + +const { consumeLiteral } = await import( + pathToFileURL( + path.join(REPO_ROOT, "tooling~", "scripts", "lint-comparison-direction.mjs") + ).href +); + +/** Groups comment-only, non-doc `//` lines into consecutive-line runs of length 2 or more. */ +export function commentRuns(text) { + const lineStarts = []; + for (let i = 0; i < text.length; i++) { + if (text[i] === "\n") { + lineStarts.push(i + 1); + } + } + lineStarts.unshift(0); + + const entries = []; + let i = 0; + let line = 0; + const length = text.length; + while (i < length) { + const c = text[i]; + if (c === "\n") { + line++; + i++; + continue; + } + if (/\s/.test(c)) { + i++; + continue; + } + if (c === "/" && text[i + 1] === "*") { + const end = text.indexOf("*/", i + 2); + const stop = end < 0 ? length : end + 2; + while (i < stop) { + if (text[i] === "\n") { + line++; + } + i++; + } + continue; + } + if (c === "/" && text[i + 1] === "/") { + const end = text.indexOf("\n", i); + const stop = end < 0 ? length : end; + /* + Comment-only lines only. A `//` that trails code on its line is outside this + rule: the fixer replaces whole lines, so treating it as a run member would + delete the code it trails. + */ + const isCommentOnly = text.slice(lineStarts[line], i).trim() === ""; + if (isCommentOnly && !text.startsWith("///", i)) { + entries.push({ + line, + start: i, + end: stop, + text: text.slice(i, stop), + lineStart: lineStarts[line], + }); + } + i = stop; + continue; + } + if (c === '"' || c === "'" || c === "@" || c === "$") { + const literal = consumeLiteral(text, i); + if (literal) { + while (i < literal.end) { + if (text[i] === "\n") { + line++; + } + i++; + } + continue; + } + } + i++; + } + + const runs = []; + for (const entry of entries) { + const previous = runs[runs.length - 1]; + if ( + previous && + previous.entries[previous.entries.length - 1].line + 1 === entry.line + ) { + previous.entries.push(entry); + } else { + runs.push({ entries: [entry] }); + } + } + return runs.filter((run) => 1 < run.entries.length); +} + +/** Builds the block-comment replacement for one run, or undefined when it is unconvertible. */ +export function planFix(text, run) { + for (const entry of run.entries) { + if (entry.text.includes("*/")) { + return undefined; + } + } + const first = run.entries[0]; + const last = run.entries[run.entries.length - 1]; + const eol = text.includes("\r\n") ? "\r\n" : "\n"; + const indent = /^[ \t]*/.exec(text.slice(first.lineStart, first.start))[0]; + const contents = run.entries.map((entry) => { + let content = entry.text.replace(/\r$/, "").slice(2); + if (content.startsWith(" ")) { + content = content.slice(1); + } + return content.replace(/[ \t]+$/, ""); + }); + const body = contents + .map((content) => (content === "" ? "" : `${indent} ${content}`)) + .join(eol); + /* + The replaced region stops short of the final line terminator, so the file's own + line ending survives the rewrite untouched. + */ + const end = text[last.end - 1] === "\r" ? last.end - 1 : last.end; + return { + // Replace from the line start so the original indent is not doubled. + start: first.lineStart, + end, + replacement: `${indent}/*${eol}${body}${eol}${indent}*/`, + }; +} + +export function applyFixes(text, runs) { + const plans = runs + .map((run) => planFix(text, run)) + .filter((plan) => plan !== undefined) + .sort((a, b) => b.start - a.start); + let fixed = text; + for (const plan of plans) { + fixed = fixed.slice(0, plan.start) + plan.replacement + fixed.slice(plan.end); + } + return fixed; +} + +function listFiles(root) { + const absolute = path.resolve(REPO_ROOT, root); + if (!fs.existsSync(absolute)) { + // A missing root contributes nothing; the caller reports an empty walk as an error + // rather than reading the absence of a scan as green. + return []; + } + const found = []; + const walk = (dir) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const child = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === "bin" || entry.name === "obj" || entry.name === "node_modules") { + continue; + } + walk(child); + } else if (entry.name.endsWith(".cs")) { + found.push(child); + } + } + }; + walk(absolute); + return found; +} + +function main() { + const shouldFix = process.argv.includes("--fix"); + const files = SCAN_ROOTS.flatMap((root) => listFiles(root)); + if (files.length === 0) { + console.error( + `[multiline-comments] ERROR: no C# files were found under ${SCAN_ROOTS.join(", ")}, so this run checked nothing. A scan that matched nothing is the absence of a measurement, not a pass.` + ); + return 1; + } + + let violations = 0; + let converted = 0; + for (const file of files) { + const text = fs.readFileSync(file, "utf8"); + const runs = commentRuns(text); + if (runs.length === 0) { + continue; + } + const relative = path.relative(REPO_ROOT, file); + for (const run of runs) { + if (!shouldFix || planFix(text, run) === undefined) { + violations++; + console.error( + `${relative}:${run.entries[0].line + 1}: ${run.entries.length} stacked // comment lines` + ); + } + } + if (shouldFix) { + const updated = applyFixes(text, runs); + if (updated !== text) { + converted += runs.filter((run) => planFix(text, run) !== undefined).length; + fs.writeFileSync(file, updated); + } + } + } + + if (shouldFix) { + console.log(`[multiline-comments] converted ${converted} comment run(s) to block comments`); + } else { + console.log(`[multiline-comments] ${files.length} file(s) scanned`); + } + return 0 < violations ? 1 : 0; +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + /* + exitCode, not exit: on Windows, writes to a piped stderr are asynchronous, and a + process.exit() would truncate the violation report this process is still flushing. + Letting the loop drain keeps the report whole and the exit code identical. + */ + process.exitCode = main(process.argv.slice(2)); +} diff --git a/tooling~/scripts/tests/lint-multiline-comments.test.mjs b/tooling~/scripts/tests/lint-multiline-comments.test.mjs new file mode 100644 index 00000000..e7a90aa0 --- /dev/null +++ b/tooling~/scripts/tests/lint-multiline-comments.test.mjs @@ -0,0 +1,240 @@ +/* + Contract tests for tooling~/scripts/lint-multiline-comments.mjs. + + The rule: two or more consecutive comment-only `//` lines are one comment with fake + structure and must be a block comment instead. The negative cases carry the weight: + `///` doc runs, single `//` lines, `//` inside any literal (strings, verbatim fixture + sources, interpolated holes, chars), and `//` inside block comments must all stay + silent, or a sweep corrupts fixture data. The positive cases pin detection and the + exact conversion shape the reviewer asked for. +*/ +import test from "node:test"; +import assert from "node:assert"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../.." +); +const linterPath = path.join(repoRoot, "scripts", "lint-multiline-comments.mjs"); +const { commentRuns, planFix, applyFixes } = await import( + pathToFileURL(linterPath).href +); + +function runsIn(source) { + return commentRuns(source); +} + +function fixed(source) { + return applyFixes(source, commentRuns(source)); +} + +/** Shapes that contain `//` lines and are NOT stacked comments. Every one stays silent. */ +const EXEMPT = [ + ["a single comment line", "// one"], + + ["doc comment runs", "/// \n/// Text.\n/// "], + + [ + "a blank line between comments", + "// first\n\n// second", + ], + + [ + "a doc comment breaking the run", + "// first\n/// doc\n// second", + ], + + [ + "code between comments", + "// first\nint x = 1;\n// second", + ], + + [ + "comments inside a normal string", + 'var url = "https://example.test/a//b";', + ], + + [ + "comments inside a verbatim string", + "var source = @\"\n// looks like a comment\n// and more\n\";", + ], + + [ + "comments inside an interpolated string", + 'var text = $"a {x} // not a comment";', + ], + + [ + "a quote inside a char literal", + "char quote = '\"';\n// single comment", + ], + + [ + "comments inside a block comment", + "/*\n// inside a block\n// still inside\n*/", + ], + + [ + "a preprocessor directive breaking the run", + "// first\n#if UNITY_EDITOR\n// second", + ], + + [ + "trailing comments after code are outside the rule", + "int a = 1; // one\nint b = 2; // two", + ], + + [ + "a trailing comment between comment-only lines stays outside", + "// one\nint a = 1; // trailing\n// two", + ], +]; + +/** Shapes that ARE stacked comments. Every one is caught. */ +const CAUGHT = [ + ["a two-line run", "// first\n// second"], + ["a three-line run", "// first\n// second\n// third"], + ["an indented run", " // first\n // second"], + [ + "a run with deeper indentation preserved", + " // outer\n // inner detail", + ], +]; + +test("exempt shapes stay silent", () => { + for (const [name, source] of EXEMPT) { + assert.deepStrictEqual(runsIn(source), [], name); + } +}); + +test("stacked comment runs are caught", () => { + for (const [name, source] of CAUGHT) { + const runs = runsIn(source); + assert.strictEqual(runs.length, 1, name); + assert.strictEqual(1 < runs[0].entries.length, true, name); + } +}); + +test("runs report zero-based line and one-based reporting line", () => { + const source = "int a;\n// one\n// two"; + const runs = runsIn(source); + assert.strictEqual(runs[0].entries[0].line, 1); +}); + +test("fix converts a plain run to the block shape", () => { + const fixedText = fixed("// first\n// second"); + assert.strictEqual(fixedText, "/*\n first\n second\n*/"); +}); + +test("fix preserves the run's indent and deeper content indentation", () => { + const source = " // outer\n // inner detail\n"; + assert.strictEqual( + fixed(source), + " /*\n outer\n inner detail\n */\n" + ); +}); + +test("fix keeps surrounding code and blank lines untouched", () => { + const source = "int a = 1;\n// one\n// two\nint b = 2;\n"; + assert.strictEqual( + fixed(source), + "int a = 1;\n/*\n one\n two\n*/\nint b = 2;\n" + ); +}); + +test("fix strips trailing whitespace from converted content", () => { + const fixedText = fixed("// one \n// two\t\n"); + assert.strictEqual(fixedText, "/*\n one\n two\n*/\n"); +}); + +test("fix emits CRLF when the file uses CRLF", () => { + const source = "int a;\r\n// one\r\n// two\r\nint b;\r\n"; + const fixedText = fixed(source); + assert.strictEqual( + fixedText, + "int a;\r\n/*\r\n one\r\n two\r\n*/\r\nint b;\r\n" + ); + assert.strictEqual(fixedText.includes("\n one\n"), false); +}); + +test("fix refuses a run whose content contains the block-comment close", () => { + const source = "// keep */ going\n// second\n"; + assert.strictEqual(planFix(source, runsIn(source)[0]), undefined); + assert.strictEqual(fixed(source), source); +}); + +test("verbatim fixture sources with stacked comments are data, not violations", () => { + const source = [ + "var fixture = @\"", + "namespace Fixtures", + "{", + " // stacked inside", + " // the fixture string", + "}\";", + ].join("\n"); + assert.deepStrictEqual(runsIn(source), []); +}); + +test("empty lines inside a run convert to empty block lines", () => { + const fixedText = fixed("// one\n//\n// two"); + assert.strictEqual(fixedText, "/*\n one\n\n two\n*/"); +}); + +test("cli: a dirty fixture tree fails, --fix converts, a second scan passes", () => { + const workspace = fs.mkdtempSync(path.join(os.tmpdir(), "multiline-comments-")); + try { + const runtime = path.join(workspace, "Runtime"); + fs.mkdirSync(runtime, { recursive: true }); + fs.writeFileSync(path.join(runtime, "Sample.cs"), "// one\n// two\n"); + const environment = { + ...process.env, + MULTILINE_COMMENT_ROOTS: runtime, + }; + const red = spawnSync(process.execPath, [linterPath], { + env: environment, + encoding: "utf8", + }); + assert.strictEqual(red.status, 1); + assert.match(red.stderr, /2 stacked \/\/ comment lines/); + console.log("REDBG", red.status, JSON.stringify(red.stderr), JSON.stringify(red.stdout)); + + const fixedRun = spawnSync(process.execPath, [linterPath, "--fix"], { + env: environment, + encoding: "utf8", + }); + console.log("FIXDBG", fixedRun.status, JSON.stringify(fixedRun.stderr), JSON.stringify(fixedRun.stdout)); + assert.strictEqual(fixedRun.status, 0); + assert.match(fixedRun.stdout, /converted 1 comment run/); + + const green = spawnSync(process.execPath, [linterPath], { + env: environment, + encoding: "utf8", + }); + assert.strictEqual(green.status, 0); + assert.strictEqual( + fs.readFileSync(path.join(runtime, "Sample.cs"), "utf8"), + "/*\n one\n two\n*/\n" + ); + } finally { + fs.rmSync(workspace, { recursive: true, force: true }); + } +}); + +test("cli: scanning nothing fails instead of reading as green", () => { + const workspace = fs.mkdtempSync(path.join(os.tmpdir(), "multiline-empty-")); + try { + const red = spawnSync(process.execPath, [linterPath], { + env: { ...process.env, MULTILINE_COMMENT_ROOTS: path.join(workspace, "Missing") }, + encoding: "utf8", + }); + assert.strictEqual(red.status, 1); + assert.match(red.stderr, /checked nothing/); + } finally { + fs.rmSync(workspace, { recursive: true, force: true }); + } +});