From c82b19652c4271da267573a9b05d434813dc215a Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 17:03:51 +0000 Subject: [PATCH 1/6] Warnings as errors + built-in analyzers, invariant argument parsing (#55) Why: Issue #55 asks for warnings-as-errors at the maximum warning level and zero-dependency static analyzers, with a sweep of the analyzer findings. What changed: - Per-assembly csc.rsp (-warnaserror, -warn:4) beside the Runtime, Editor, and Tests.Runtime asmdefs, shipped with the package. - Generator~/Directory.Build.props: built-in Roslyn analyzers at AnalysisMode=All with analyzer diagnostics as errors; the generator-tests project keeps a documented NoWarn contract list for public-API-shape rules. - CommandArg: every culture-sensitive TryParse pins NumberStyles and InvariantCulture; same-type Convert.ChangeType calls became direct casts; Contains(char) became an ordinal IndexOf. - RegisterCommandAttribute: NormalizeName validates its argument, space stripping is ordinal, redundant initializer removed. - New PlayMode culture-invariance cases (tr-TR, de-DE, ru-RU, fr-FR). --- Editor/csc.rsp | 2 + Editor/csc.rsp.meta | 7 + Generator~/Directory.Build.props | 6 + .../CommandCatalogGeneratorDriverTests.cs | 2 +- .../GeneratedCatalogIntegrationTests.cs | 4 +- .../TestCompilationFactory.cs | 6 +- ...mandTerminal.SourceGenerators.Tests.csproj | 16 + .../Attributes/RegisterCommandAttribute.cs | 11 +- Runtime/CommandTerminal/Backend/CommandArg.cs | 298 +++++++++++++----- Runtime/csc.rsp | 2 + Runtime/csc.rsp.meta | 7 + Tests/Runtime/CommandArgTests.cs | 52 +++ Tests/Runtime/csc.rsp | 2 + Tests/Runtime/csc.rsp.meta | 7 + 14 files changed, 330 insertions(+), 92 deletions(-) create mode 100644 Editor/csc.rsp create mode 100644 Editor/csc.rsp.meta create mode 100644 Runtime/csc.rsp create mode 100644 Runtime/csc.rsp.meta create mode 100644 Tests/Runtime/csc.rsp create mode 100644 Tests/Runtime/csc.rsp.meta 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..1ffc40e2 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" }; diff --git a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs index 344a77d2..05530664 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs @@ -229,13 +229,13 @@ 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. 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..20bcdc44 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs @@ -79,7 +79,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); } @@ -132,7 +133,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) { 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..29b7d5c4 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,22 @@ sources pulls in the .NET CA analyzers by default. --> false + + + $(NoWarn);CA1002;CA1051;CA1515;CA1815;CA2225;CA1019;CA1510;CA2211 - $(NoWarn);CA1002;CA1051;CA1515;CA1815;CA2225;CA1019;CA1510;CA2211 + binders are static). + - CA1720: CommandArgParsers deliberately names each parser after the type it parses + (CommandArgParsers.Int, .Float, ...); that naming IS the API. --> + $(NoWarn);CA1002;CA1051;CA1515;CA1815;CA2225;CA1019;CA1510;CA2211;CA1720 + diff --git a/Runtime/CommandTerminal/Backend/CommandArg.cs b/Runtime/CommandTerminal/Backend/CommandArg.cs index ff2864af..897577a9 100644 --- a/Runtime/CommandTerminal/Backend/CommandArg.cs +++ b/Runtime/CommandTerminal/Backend/CommandArg.cs @@ -204,87 +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, ParseFloatInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Float, out parsed); } if (type == typeof(int)) { - return InnerParse(stringValue, ParseIntInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Int, out parsed); } if (type == typeof(uint)) { - return InnerParse(stringValue, ParseUintInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Uint, out parsed); } if (type == typeof(long)) { - return InnerParse(stringValue, ParseLongInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Long, out parsed); } if (type == typeof(ulong)) { - return InnerParse(stringValue, ParseUlongInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Ulong, out parsed); } if (type == typeof(double)) { - return InnerParse(stringValue, ParseDoubleInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Double, out parsed); } if (type == typeof(short)) { - return InnerParse(stringValue, ParseShortInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Short, out parsed); } if (type == typeof(ushort)) { - return InnerParse(stringValue, ParseUshortInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Ushort, out parsed); } if (type == typeof(byte)) { - return InnerParse(stringValue, ParseByteInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Byte, out parsed); } if (type == typeof(sbyte)) { - return InnerParse(stringValue, ParseSbyteInvariant, 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, ParseDateTimeInvariant, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.DateTime, + out parsed + ); } if (type == typeof(DateTimeOffset)) { - return InnerParse( + return InnerParse( stringValue, - ParseDateTimeOffsetInvariant, + 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, ParseDecimalInvariant, out parsed); + return InnerParse(stringValue, CommandArgParsers.Decimal, out parsed); } if (type == typeof(BigInteger)) { - return InnerParse(stringValue, ParseBigIntegerInvariant, out parsed); + return InnerParse( + stringValue, + CommandArgParsers.BigInteger, + out parsed + ); } if (type == typeof(TimeSpan)) { - return InnerParse(stringValue, ParseTimeSpanInvariant, 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) { @@ -298,14 +318,7 @@ out parsed } } - if ( - int.TryParse( - stringValue, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out int enumIntValue - ) - ) + if (CommandArgParsers.Int(stringValue, out int enumIntValue)) { if (!EnumValues.TryGetValue(type, out object enumValues)) { @@ -327,14 +340,14 @@ out int enumIntValue switch (split.Length) { case 2 - when ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y): + 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 ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y) - && ParseFloatInvariant(split[2], out float z): + 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; } @@ -345,14 +358,14 @@ when ParseFloatInvariant(split[0], out float x) switch (split.Length) { case 2 - when ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y): + 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 ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y) - && ParseFloatInvariant(split[2], out float z): + 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; } @@ -363,21 +376,21 @@ when ParseFloatInvariant(split[0], out float x) switch (split.Length) { case 2 - when ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y): + 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 ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y) - && ParseFloatInvariant(split[2], out float z): + 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 ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y) - && ParseFloatInvariant(split[2], out float z) - && ParseFloatInvariant(split[3], out float w): + 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; } @@ -388,14 +401,14 @@ when ParseFloatInvariant(split[0], out float x) switch (split.Length) { case 2 - when ParseIntInvariant(split[0], out int x) - && ParseIntInvariant(split[1], out int y): + 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 ParseIntInvariant(split[0], out int x) - && ParseIntInvariant(split[1], out int y) - && ParseIntInvariant(split[2], out int z): + 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; } @@ -406,14 +419,14 @@ when ParseIntInvariant(split[0], out int x) switch (split.Length) { case 2 - when ParseIntInvariant(split[0], out int x) - && ParseIntInvariant(split[1], out int y): + 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 ParseIntInvariant(split[0], out int x) - && ParseIntInvariant(split[1], out int y) - && ParseIntInvariant(split[2], out int z): + 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; } @@ -434,16 +447,16 @@ when ParseIntInvariant(split[0], out int x) switch (split.Length) { case 3 - when ParseFloatInvariant(split[0], out float r) - && ParseFloatInvariant(split[1], out float g) - && ParseFloatInvariant(split[2], out float b): + 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 ParseFloatInvariant(split[0], out float r) - && ParseFloatInvariant(split[1], out float g) - && ParseFloatInvariant(split[2], out float b) - && ParseFloatInvariant(split[3], out float a): + 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; } @@ -454,10 +467,10 @@ when ParseFloatInvariant(split[0], out float r) switch (split.Length) { case 4 - when ParseFloatInvariant(split[0], out float x) - && ParseFloatInvariant(split[1], out float y) - && ParseFloatInvariant(split[2], out float z) - && ParseFloatInvariant(split[3], out float w): + 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; } @@ -468,12 +481,12 @@ when ParseFloatInvariant(split[0], out float x) switch (split.Length) { case 4 - when ParseFloatInvariant( + when CommandArgParsers.Float( split[0] .Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), out float x ) - && ParseFloatInvariant( + && CommandArgParsers.Float( split[1] .Replace( "y:", @@ -482,7 +495,7 @@ out float x ), out float y ) - && ParseFloatInvariant( + && CommandArgParsers.Float( split[2] .Replace( "width:", @@ -491,7 +504,7 @@ out float y ), out float width ) - && ParseFloatInvariant( + && CommandArgParsers.Float( split[3] .Replace( "height:", @@ -510,12 +523,12 @@ out float height switch (split.Length) { case 4 - when ParseIntInvariant( + when CommandArgParsers.Int( split[0] .Replace("x:", string.Empty, StringComparison.OrdinalIgnoreCase), out int x ) - && ParseIntInvariant( + && CommandArgParsers.Int( split[1] .Replace( "y:", @@ -524,7 +537,7 @@ out int x ), out int y ) - && ParseIntInvariant( + && CommandArgParsers.Int( split[2] .Replace( "width:", @@ -533,7 +546,7 @@ out int y ), out int width ) - && ParseIntInvariant( + && CommandArgParsers.Int( split[3] .Replace( "height:", @@ -550,122 +563,6 @@ out int height parsed = default; return false; - /* - Console input must parse identically on every machine, so every - culture-sensitive TryParse pins NumberStyles and InvariantCulture - explicitly. The style flags mirror each overload's default so only - the culture is pinned, never the accepted syntax. - */ - static bool ParseFloatInvariant(string input, out float parsed) => - float.TryParse( - input, - NumberStyles.Float | NumberStyles.AllowThousands, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseDoubleInvariant(string input, out double parsed) => - double.TryParse( - input, - NumberStyles.Float | NumberStyles.AllowThousands, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseDecimalInvariant(string input, out decimal parsed) => - decimal.TryParse( - input, - NumberStyles.Number, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseIntInvariant(string input, out int parsed) => - int.TryParse(input, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsed); - - static bool ParseUintInvariant(string input, out uint parsed) => - uint.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseLongInvariant(string input, out long parsed) => - long.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseUlongInvariant(string input, out ulong parsed) => - ulong.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseShortInvariant(string input, out short parsed) => - short.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseUshortInvariant(string input, out ushort parsed) => - ushort.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseByteInvariant(string input, out byte parsed) => - byte.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseSbyteInvariant(string input, out sbyte parsed) => - sbyte.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseBigIntegerInvariant(string input, out BigInteger parsed) => - BigInteger.TryParse( - input, - NumberStyles.Integer, - CultureInfo.InvariantCulture, - out parsed - ); - - static bool ParseDateTimeInvariant(string input, out DateTime parsed) => - DateTime.TryParse( - input, - CultureInfo.InvariantCulture, - DateTimeStyles.None, - out parsed - ); - - static bool ParseDateTimeOffsetInvariant(string input, out DateTimeOffset parsed) => - DateTimeOffset.TryParse( - input, - CultureInfo.InvariantCulture, - DateTimeStyles.None, - out parsed - ); - - static bool ParseTimeSpanInvariant(string input, out TimeSpan parsed) => - TimeSpan.TryParse(input, CultureInfo.InvariantCulture, out parsed); - static bool InnerParse( string input, CommandArgParser typedParser, 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: From 92d414e74308f5834b11de5f98cec45f7758cf39 Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 18:02:04 +0000 Subject: [PATCH 5/6] Block comments for multi-line comments: linter, sweep, rule 21 (#57 review) Why: PR review asked for a repo rule: multi-line comments are block comments, never stacked '//' lines. What changed: - New lint-multiline-comments.mjs (Node ESM): a state machine over the same literal scanner the comparison-direction linter uses, so comment-looking lines inside strings, verbatim fixture sources, interpolated holes, and block comments stay data. '--fix' converts runs to the block shape with a three-space content indent; a run whose content contains the block-comment close is refused, and an empty walk fails instead of reading green. - 12 contract tests; wired into pre-commit, the cross-OS csharp-style CI job, and context.md rule 21. - Swept 68 stacked '//' runs across Runtime, Editor, Tests, and Generator~ into block comments; verbatim fixture strings untouched. How we know: - Node tooling 121 pass / 0 fail; generator tests 30/30; CSharpier, all linters, meta and package validators green. --- .github/workflows/tooling-tests.yml | 3 + .llm/context.md | 4 + .pre-commit-config.yaml | 13 + .../CommandCatalogGeneratorDriverTests.cs | 12 +- .../GeneratedCatalogIntegrationTests.cs | 6 +- .../TestCompilationFactory.cs | 26 +- .../UnityScriptingShim.cs | 8 +- .../CommandCatalogGenerator.cs | 84 +++--- .../Backend/BorrowedCommandArguments.cs | 8 +- .../CommandTerminal/Backend/CommandShell.cs | 82 ++++-- .../Backend/CommandTokenizer.cs | 8 +- Runtime/CommandTerminal/UI/TerminalUI.cs | 48 ++-- .../Runtime/BorrowedCommandArgumentsTests.cs | 6 +- Tests/Runtime/CommandCatalogTests.cs | 8 +- .../Runtime/CommandCompletionProviderTests.cs | 12 +- Tests/Runtime/CommandContextExecutionTests.cs | 18 +- Tests/Runtime/CommandDiscoveryTests.cs | 50 ++-- Tests/Runtime/CommandHistoryTests.cs | 18 +- .../CommandShellDeferredRegistrationTests.cs | 6 +- Tests/Runtime/CommandShellTests.cs | 12 +- .../TerminalKeyboardControllerTests.cs | 20 +- Tests/Runtime/TerminalTests.cs | 12 +- .../Runtime/TerminalUITokenCompletionTests.cs | 6 +- tooling~/package.json | 2 + tooling~/scripts/lint-multiline-comments.mjs | 252 ++++++++++++++++++ .../tests/lint-multiline-comments.test.mjs | 230 ++++++++++++++++ 26 files changed, 797 insertions(+), 157 deletions(-) create mode 100644 tooling~/scripts/lint-multiline-comments.mjs create mode 100644 tooling~/scripts/tests/lint-multiline-comments.test.mjs 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 0af33dbc..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 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/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs index 1ffc40e2..76bb2698 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/CommandCatalogGeneratorDriverTests.cs @@ -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 05530664..da808e9a 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/GeneratedCatalogIntegrationTests.cs @@ -232,8 +232,10 @@ public void BindersExecutePublicAndPrivateCommands() 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()(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 8d4e66b0..ab3807d8 100644 --- a/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs +++ b/Generator~/WallstopStudios.DxCommandTerminal.SourceGenerators.Tests/TestCompilationFactory.cs @@ -101,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, @@ -227,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( @@ -322,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/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/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/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/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index 72f2df90..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; } @@ -1719,8 +1729,10 @@ private void InitializeTheme(VisualElement root) _runtimeTheme = themeNames.FirstOrDefault(); } - // Defaulting from an empty or unknown persisted name is normal - // operation; only a stale persisted name deserves a warning. + /* + Defaulting from an empty or unknown persisted name is normal + operation; only a stale persisted name deserves a warning. + */ if (_persistedTheme != null) { Debug.LogWarning( @@ -1873,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(); } @@ -1913,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/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/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/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..e438555d --- /dev/null +++ b/tooling~/scripts/lint-multiline-comments.mjs @@ -0,0 +1,252 @@ +/* + 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; + if (!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..35210a2d --- /dev/null +++ b/tooling~/scripts/tests/lint-multiline-comments.test.mjs @@ -0,0 +1,230 @@ +/* + 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", + ], +]; + +/** 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 }); + } +}); From 097070dcf081fe20bfefabd4304a1da4118789eb Mon Sep 17 00:00:00 2001 From: wallstop Date: Fri, 11 Sep 2026 18:07:28 +0000 Subject: [PATCH 6/6] Restrict the multi-line comment rule to comment-only lines (bugbot fix) Why: The fixer replaces whole lines, so counting trailing comments (a '//' after code on its line) as run members would delete the code they trail. What changed: - commentRuns records only comment-only lines; trailing comments and runs around them stay outside the rule. - Contract tests pin the destructive-rewrite shapes. --- tooling~/scripts/lint-multiline-comments.mjs | 8 +++++++- .../scripts/tests/lint-multiline-comments.test.mjs | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tooling~/scripts/lint-multiline-comments.mjs b/tooling~/scripts/lint-multiline-comments.mjs index e438555d..4bb76f65 100644 --- a/tooling~/scripts/lint-multiline-comments.mjs +++ b/tooling~/scripts/lint-multiline-comments.mjs @@ -85,7 +85,13 @@ export function commentRuns(text) { if (c === "/" && text[i + 1] === "/") { const end = text.indexOf("\n", i); const stop = end < 0 ? length : end; - if (!text.startsWith("///", i)) { + /* + 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, diff --git a/tooling~/scripts/tests/lint-multiline-comments.test.mjs b/tooling~/scripts/tests/lint-multiline-comments.test.mjs index 35210a2d..e7a90aa0 100644 --- a/tooling~/scripts/tests/lint-multiline-comments.test.mjs +++ b/tooling~/scripts/tests/lint-multiline-comments.test.mjs @@ -83,6 +83,16 @@ const EXEMPT = [ "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. */