fix: core audit round 3

src/Trax.Core.Testing/Fixtures/HygieneGuardFixtures.cs

@@ -31,21 +31,21 @@ public abstract class HygieneGuardFixture
     public void Tests_do_not_use_the_Ignore_attribute()
     {
         var result = HygieneGuards.NoIgnoreAttribute(Options);
-        Assert.That(result.Offenders, Is.Empty, result.FailureMessage);
+        GuardAssert.CheckedAndClean(result);
     }
 
     [Test]
     public void Tests_do_not_use_legacy_asserts()
     {
         var result = HygieneGuards.NoLegacyAsserts(Options);
-        Assert.That(result.Offenders, Is.Empty, result.FailureMessage);
+        GuardAssert.CheckedAndClean(result);
     }
 
     [Test]
     public void Tests_do_not_use_fixed_delays()
     {
         var result = HygieneGuards.NoFixedDelays(Options);
-        Assert.That(result.Offenders, Is.Empty, result.FailureMessage);
+        GuardAssert.CheckedAndClean(result);
     }
 }
 
@@ -63,13 +63,35 @@ public abstract class RepoConventionGuardFixture
     public void Directory_build_props_pins_the_expected_version()
     {
         var result = RepoConventionGuards.DirectoryBuildPropsVersion(Options);
-        Assert.That(result.Offenders, Is.Empty, result.FailureMessage);
+        GuardAssert.CheckedAndClean(result);
     }
 
     [Test]
     public void Cross_repo_package_references_are_centrally_managed()
     {
         var result = RepoConventionGuards.CrossRepoPackageVersions(Options);
+        GuardAssert.CheckedAndClean(result);
+    }
+}
+
+/// <summary>
+/// The assertion both fixtures make: the guard looked at something, and found nothing wrong.
+/// </summary>
+internal static class GuardAssert
+{
+    /// <summary>
+    /// A guard that inspected nothing proves nothing: a repo whose tests live somewhere other
+    /// than the configured roots would otherwise pass every check. Fails first on that, then on
+    /// any offender.
+    /// </summary>
+    public static void CheckedAndClean(GuardResult result)
+    {
+        Assert.That(
+            result.Inspected,
+            Is.GreaterThan(0),
+            "The guard inspected no files, so it checked nothing. Point the options' scan roots at "
+                + "the directories that hold this repo's sources and tests."
+        );
         Assert.That(result.Offenders, Is.Empty, result.FailureMessage);
     }
 }

src/Trax.Core.Testing/Guards/HygieneGuards.cs

@@ -10,9 +10,19 @@ namespace Trax.Core.Testing.Guards;
 public static class HygieneGuards
 {
     // Matches an [Ignore] attribute whether standalone ([Ignore] / [Ignore("...")]) or combined with
-    // others ([Test, Ignore(...)]), i.e. preceded by an open bracket or a comma.
+    // others ([Test, Ignore(...)]), i.e. preceded by an open bracket or a comma. The name may be
+    // qualified (NUnit.Framework.Ignore) or carry its suffix (IgnoreAttribute), and the list may
+    // span lines, so it is matched against the whole file rather than line by line. The lookahead
+    // leaves a following comma for the next attribute in the same list.
     private static readonly Regex IgnoreAttribute = new(
-        @"(?:\[|,)\s*Ignore(\s*\(|\s*\])",
+        @"(?:\[|,)\s*(?:[\w.]+\.)?Ignore(?:Attribute)?(?=\s*(?:\(|\]|,))",
+        RegexOptions.Compiled
+    );
+
+    // A per-case skip: [TestCase(1, Ignore = "...")] or IgnoreReason = on a case or fixture
+    // attribute. Same effect as [Ignore], one case at a time.
+    private static readonly Regex IgnoreNamedArgument = new(
+        @"\[[^\]]*?\b(?:TestCase|TestCaseSource|TestFixture|TestFixtureSource)(?:Attribute)?\b[^\]]*?\b(?:Ignore|IgnoreReason)\s*=(?!=)",
         RegexOptions.Compiled
     );
 
@@ -30,6 +40,9 @@ private static readonly (string Name, Regex Pattern)[] LegacyAssertPatterns =
         ("Assert.IsEmpty", new Regex(@"\bAssert\.IsEmpty\b", RegexOptions.Compiled)),
         ("Assert.IsNotEmpty", new Regex(@"\bAssert\.IsNotEmpty\b", RegexOptions.Compiled)),
         ("Assert.Contains", new Regex(@"\bAssert\.Contains\b", RegexOptions.Compiled)),
+        ("ClassicAssert", new Regex(@"\bClassicAssert\.\w+", RegexOptions.Compiled)),
+        ("CollectionAssert", new Regex(@"\bCollectionAssert\.\w+", RegexOptions.Compiled)),
+        ("StringAssert", new Regex(@"\bStringAssert\.\w+", RegexOptions.Compiled)),
     ];
 
     private static readonly Regex FixedDelay = new(
@@ -61,7 +74,12 @@ public static GuardResult NoIgnoreAttribute(ArchitectureGuardOptions options)
                 continue;
 
             var stripped = SourceText.StripCommentsAndStrings(File.ReadAllText(file));
-            foreach (var (line, _) in SourceText.MatchingLines(stripped, IgnoreAttribute))
+            var lines = new SortedSet<int>();
+            foreach (var pattern in new[] { IgnoreAttribute, IgnoreNamedArgument })
+            foreach (Match match in pattern.Matches(stripped))
+                lines.Add(stripped.Take(match.Index).Count(c => c == '\n') + 1);
+
+            foreach (var line in lines)
                 offenders.Add($"{rel}:{line}");
         }
 

src/Trax.Core.Testing/Guards/RepoConventionGuards.cs

@@ -111,10 +111,19 @@ include is null
                 var version =
                     reference.Attribute("Version")?.Value ?? reference.Element("Version")?.Value;
 
+                // VersionOverride sidesteps the central pin just as an inline Version does.
+                var versionOverride =
+                    reference.Attribute("VersionOverride")?.Value
+                    ?? reference.Element("VersionOverride")?.Value;
+
                 if (version is not null)
                     offenders.Add(
                         $"{rel} -> {include} carries inline Version=\"{version}\" (must be centrally managed)"
                     );
+                else if (versionOverride is not null)
+                    offenders.Add(
+                        $"{rel} -> {include} carries VersionOverride=\"{versionOverride}\" (must be centrally managed)"
+                    );
                 else if (!centralPins.Contains(include))
                     offenders.Add(
                         $"{rel} -> {include} has no <PackageVersion> pin in Directory.Packages.props"

src/Trax.Core.Testing/Guards/VocabularyGuards.cs

@@ -72,18 +72,24 @@ public static GuardResult TraxVocabularyIsUsed(
         var patterns = banned.ToDictionary(
             entry => entry,
             entry => new Regex(
-                // Attribute position: opening the list or following another attribute, then the
-                // name, then arguments or the closing bracket.
-                @"(?:\[|,)\s*(?:[\w.]+\.)?(?:"
+                // Attribute position: opening the list (with an optional target such as
+                // property:) or following another attribute, then the name with or without its
+                // Attribute suffix, then arguments, the closing bracket, or another attribute. The
+                // lookahead leaves that comma for the next attribute in the list.
+                @"(?:\[\s*(?:\w+\s*:\s*)?|,)\s*(?:[\w.]+\.)?(?:"
                     + string.Join("|", entry.Attributes.Select(Regex.Escape))
-                    + @")\s*(?:\(|\])",
+                    + @")(?:Attribute)?(?=\s*(?:\(|\]|,))",
                 RegexOptions.Compiled
             )
         );
 
         var offenders = new List<string>();
         var inspected = 0;
 
+        // A global using makes a library's attributes visible in files that never name it, so a
+        // library imported that way counts as present everywhere.
+        var globallyImported = GloballyImported(root, roots);
+
         foreach (var file in SourceFiles.CSharpUnder(root, roots))
         {
             var relative = Path.GetRelativePath(root, file).Replace('\\', '/');
@@ -98,7 +104,10 @@ public static GuardResult TraxVocabularyIsUsed(
 
             foreach (var (entry, pattern) in patterns)
             {
-                if (!text.Contains(entry.Library, StringComparison.Ordinal))
+                if (
+                    !globallyImported.Contains(entry.Library)
+                    && !text.Contains(entry.Library, StringComparison.Ordinal)
+                )
                     continue;
 
                 foreach (Match match in pattern.Matches(text))
@@ -123,4 +132,42 @@ public static GuardResult TraxVocabularyIsUsed(
                 + string.Join(", ", offenders)
         );
     }
+
+    private static readonly Regex GlobalUsing = new(
+        @"^\s*global\s+using\s+(?:static\s+)?(?:\w+\s*=\s*)?([\w.]+)",
+        RegexOptions.Compiled | RegexOptions.Multiline
+    );
+
+    private static readonly Regex ProjectUsing = new(
+        @"<Using\s+Include\s*=\s*""([\w.]+)""",
+        RegexOptions.Compiled
+    );
+
+    /// <summary>
+    /// Every namespace a global using brings in, from source files or a project's
+    /// <c>&lt;Using Include&gt;</c>, with each of its parent namespaces: importing
+    /// <c>HotChocolate.Authorization</c> is a use of <c>HotChocolate</c>.
+    /// </summary>
+    private static HashSet<string> GloballyImported(string root, string[] roots)
+    {
+        var names = new List<string>();
+
+        foreach (var file in SourceFiles.CSharpUnder(root, roots))
+        foreach (Match match in GlobalUsing.Matches(File.ReadAllText(file)))
+            names.Add(match.Groups[1].Value);
+
+        foreach (var project in SourceFiles.ProjectsUnder(root, roots))
+        foreach (Match match in ProjectUsing.Matches(File.ReadAllText(project)))
+            names.Add(match.Groups[1].Value);
+
+        var imported = new HashSet<string>(StringComparer.Ordinal);
+        foreach (var name in names)
+        {
+            var parts = name.Split('.');
+            for (var i = 1; i <= parts.Length; i++)
+                imported.Add(string.Join('.', parts.Take(i)));
+        }
+
+        return imported;
+    }
 }

src/Trax.Core.Testing/Trax.Core.Testing.csproj

@@ -15,4 +15,8 @@
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp" />
     <PackageReference Include="NUnit" />
   </ItemGroup>
+
+  <ItemGroup>
+    <InternalsVisibleTo Include="Trax.Core.Testing.Tests" />
+  </ItemGroup>
 </Project>

src/Trax.Core/Extensions/MonadTaskExtensions.cs

@@ -23,16 +23,28 @@ internal MonadTask(Task<Monad<TInput, TReturn>> source)
         Source = source;
     }
 
-    public TaskAwaiter<Monad<TInput, TReturn>> GetAwaiter() => Source.GetAwaiter();
+    public TaskAwaiter<Monad<TInput, TReturn>> GetAwaiter() => Joined().GetAwaiter();
 
     public ConfiguredTaskAwaitable<Monad<TInput, TReturn>> ConfigureAwait(
         bool continueOnCapturedContext
-    ) => Source.ConfigureAwait(continueOnCapturedContext);
+    ) => Joined().ConfigureAwait(continueOnCapturedContext);
 
-    public Task<Monad<TInput, TReturn>> AsTask() => Source;
+    public Task<Monad<TInput, TReturn>> AsTask() => Joined();
 
     public static implicit operator Task<Monad<TInput, TReturn>>(MonadTask<TInput, TReturn> mt) =>
-        mt.Source;
+        mt.Joined();
+
+    /// <summary>
+    /// The task, noting while a chain is read that the train waits for it, so a statement after
+    /// it starts from a finished chain rather than a second one running alongside.
+    /// </summary>
+    private Task<Monad<TInput, TReturn>> Joined()
+    {
+        if (Source.IsCompletedSuccessfully)
+            Source.Result.Recorder?.NoteJoined();
+
+        return Source;
+    }
 
     #region Chain
 

src/Trax.Core/Junction/Junction.cs

@@ -124,8 +124,9 @@ Train<TTrainIn, TTrainOut> train
     /// worker or a nested train, keeps that answer. The data attached here replaces whatever the
     /// exception carried before, so without this the class would be dropped on the way through.
     ///
-    /// <para>Only a <see cref="TrainException"/> carries a class in its message, because that is
-    /// the type Trax rebuilds a recorded failure as. Any other exception's message is its own
+    /// <para>Only an exception whose type is exactly <see cref="TrainException"/> carries a class
+    /// in its message, because that is the type Trax rebuilds a recorded failure as. A type
+    /// derived from it is treated like any other exception. Any other exception's message is its own
     /// text, and is never read as a record. A value outside <see cref="FailureClass"/> is carried
     /// as <see cref="FailureClass.Unclassified"/>, as the remote wire already does.</para>
     /// </remarks>
@@ -134,7 +135,10 @@ Train<TTrainIn, TTrainOut> train
         if (e.Data["TrainExceptionData"] is TrainExceptionData attached)
             return Defined(attached.FailureClass);
 
-        if (e is not TrainException || !e.Message.StartsWith('{'))
+        // Exactly TrainException: a consumer subclass carries its own text (often a remote
+        // system's response body), and reading a class out of it would let that text choose
+        // whether the failure is retried.
+        if (e.GetType() != typeof(TrainException) || !e.Message.StartsWith('{'))
             return null;
 
         try

src/Trax.Core/Monad/ChainStep.cs

@@ -64,4 +64,41 @@ internal void Record(ChainStepKind kind, Type? junction, Type? tIn, Type? tOut)
         _steps.Add(new ChainStep(kind, junction, tIn, tOut));
 
     internal void Refuse(string reason) => _refusals.Add(reason);
+
+    private string? _firstAsyncRootCall;
+
+    private bool _unlinkedRefused;
+
+    /// <summary>
+    /// Notes a chain call made on the train itself rather than on the result of an earlier call.
+    /// </summary>
+    /// <remarks>
+    /// Every such call starts from the train's monad, so two of them are two chains unless the
+    /// second is reached through the first. A synchronous call before the first junction step is
+    /// fine: it has finished by the time the next statement runs. After a junction step that the
+    /// body did not await, the next root call runs alongside that junction over the same Memory,
+    /// which no replay of the declaration can show, so it is refused. Awaiting the chain first
+    /// (<c>await Chain&lt;A&gt;(); Extract&lt;X, Y&gt;();</c>) keeps it one sequence.
+    /// </remarks>
+    internal void NoteRootCall(string call, bool startsAJunction)
+    {
+        if (_firstAsyncRootCall is not null && !_unlinkedRefused)
+        {
+            _unlinkedRefused = true;
+            Refuse(
+                $"Junctions() calls {call} as a separate statement after {_firstAsyncRootCall}, so "
+                    + "the two start separate chains that run at the same time. Link them into one "
+                    + "chain, for example Chain<A>().Chain<B>().Resolve()."
+            );
+        }
+
+        if (startsAJunction)
+            _firstAsyncRootCall ??= call;
+    }
+
+    /// <summary>
+    /// Notes that the body awaited the chain it started, so the next root call follows it
+    /// rather than running beside it.
+    /// </summary>
+    internal void NoteJoined() => _firstAsyncRootCall = null;
 }

src/Trax.Core/Monad/Monad.AddServices.cs

@@ -122,6 +122,12 @@ internal Monad<TInput, TReturn> AddServices(object?[] services, Type[] typeArray
 
                 if (services[i] is null)
                     Recorder.Refuse(NullServiceMessage(serviceType, whileRecording: true));
+                else if (services[i]!.GetType().IsValueType)
+                    // A struct passed as an interface is boxed, and the runtime refuses it.
+                    Recorder.Refuse(
+                        $"AddServices<{serviceType.Name}> received a value of the struct type "
+                            + $"{services[i]!.GetType().Name}; a service must be a class."
+                    );
 
                 // A value is stored under an interface it implements; the runtime refuses a
                 // class on every run.
@@ -141,8 +147,13 @@ internal Monad<TInput, TReturn> AddServices(object?[] services, Type[] typeArray
             if (services[i] is null)
                 throw new Exception(NullServiceMessage(typeArray[i], whileRecording: false));
 
-        foreach (var service in services.OfType<object>())
+        // Each service goes under the type argument it was passed as, in the same position:
+        // one object may be passed under several interfaces, and each is a separate slot the
+        // chain can find.
+        for (var i = 0; i < typeArray.Length; i++)
         {
+            var service = services[i]!;
+            var slot = typeArray[i];
             var serviceType = service.GetType();
 
             // Special handling for Moq mock objects
@@ -163,20 +174,16 @@ internal Monad<TInput, TReturn> AddServices(object?[] services, Type[] typeArray
                 return this;
             }
 
-            // Find the interface that matches the type parameter
-            var interfaces = serviceType.GetInterfaces();
-            var foundInterface = interfaces.FirstOrDefault(typeArray.Contains);
-
-            if (foundInterface is null)
+            if (!slot.IsInterface || !slot.IsInstanceOfType(service))
             {
                 Exception ??= new TrainException(
-                    $"Class ({serviceType}) does not have any interfaces."
+                    $"Class ({serviceType}) passed to AddServices as ({slot}) must be passed as an "
+                        + "interface it implements."
                 );
                 return this;
             }
 
-            // Store the service by its interface type
-            Memory[foundInterface] = service;
+            Memory[slot] = service;
         }
 
         return this;